Remove QuicSession::RegisterStaticStream() and merge it into QuicSession::ActivateStream(). As static_stream_map_ has been removed, static streams are getting less and less distinct from normal streams. And now after preventing v99_stream_id_manager from knowing about stream staticness, static streams are all the same as dynamic streams only to be different in is_static_ field. So I think it'd be a nice cleanup to have only one method. gfe-relnote: code refactoring. No behavior change. PiperOrigin-RevId: 267652775 Change-Id: I27137a225c69c0a41c644d473d59e7a4c3c64359
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index e626f2a..b789857 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -382,7 +382,7 @@ headers_stream->id()); headers_stream_ = headers_stream.get(); - RegisterStaticStream(std::move(headers_stream)); + ActivateStream(std::move(headers_stream)); } else { ConfigureMaxIncomingDynamicStreamsToSend( config()->GetMaxIncomingUnidirectionalStreamsToSend()); @@ -921,7 +921,7 @@ } auto receive_stream = QuicMakeUnique<QuicReceiveControlStream>(pending); receive_control_stream_ = receive_stream.get(); - RegisterStaticStream(std::move(receive_stream)); + ActivateStream(std::move(receive_stream)); receive_control_stream_->SetUnblocked(); QUIC_DVLOG(1) << "Receive Control stream is created"; return true; @@ -939,7 +939,7 @@ auto encoder_receive = QuicMakeUnique<QpackReceiveStream>( pending, qpack_decoder_->encoder_stream_receiver()); qpack_encoder_receive_stream_ = encoder_receive.get(); - RegisterStaticStream(std::move(encoder_receive)); + ActivateStream(std::move(encoder_receive)); qpack_encoder_receive_stream_->SetUnblocked(); QUIC_DVLOG(1) << "Receive QPACK Encoder stream is created"; return true; @@ -952,7 +952,7 @@ auto decoder_receive = QuicMakeUnique<QpackReceiveStream>( pending, qpack_encoder_->decoder_stream_receiver()); qpack_decoder_receive_stream_ = decoder_receive.get(); - RegisterStaticStream(std::move(decoder_receive)); + ActivateStream(std::move(decoder_receive)); qpack_decoder_receive_stream_->SetUnblocked(); QUIC_DVLOG(1) << "Receive Qpack Decoder stream is created"; return true; @@ -971,7 +971,7 @@ GetNextOutgoingUnidirectionalStreamId(), this, qpack_maximum_dynamic_table_capacity_, max_inbound_header_list_size_); send_control_stream_ = send_control.get(); - RegisterStaticStream(std::move(send_control)); + ActivateStream(std::move(send_control)); } if (!qpack_decoder_send_stream_ && @@ -979,7 +979,7 @@ auto decoder_send = QuicMakeUnique<QpackSendStream>( GetNextOutgoingUnidirectionalStreamId(), this, kQpackDecoderStream); qpack_decoder_send_stream_ = decoder_send.get(); - RegisterStaticStream(std::move(decoder_send)); + ActivateStream(std::move(decoder_send)); qpack_decoder_->set_qpack_stream_sender_delegate( qpack_decoder_send_stream_); } @@ -989,7 +989,7 @@ auto encoder_send = QuicMakeUnique<QpackSendStream>( GetNextOutgoingUnidirectionalStreamId(), this, kQpackEncoderStream); qpack_encoder_send_stream_ = encoder_send.get(); - RegisterStaticStream(std::move(encoder_send)); + ActivateStream(std::move(encoder_send)); qpack_encoder_->set_qpack_stream_sender_delegate( qpack_encoder_send_stream_); }
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index eb8ee80..b342e3d 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -122,24 +122,6 @@ QUIC_LOG_IF(WARNING, !zombie_streams_.empty()) << "Still have zombie streams"; } -void QuicSession::RegisterStaticStream(std::unique_ptr<QuicStream> stream) { - DCHECK(stream->is_static()); - QuicStreamId stream_id = stream->id(); - stream_map_[stream_id] = std::move(stream); - if (IsIncomingStream(stream_id)) { - ++num_incoming_static_streams_; - } else { - ++num_outgoing_static_streams_; - } - if (VersionHasIetfQuicFrames(transport_version()) && - !QuicUtils::IsBidirectionalStreamId(stream_id)) { - DCHECK_LE(num_incoming_static_streams_, - num_expected_unidirectional_static_streams_); - DCHECK_LE(num_outgoing_static_streams_, - num_expected_unidirectional_static_streams_); - } -} - void QuicSession::PendingStreamOnStreamFrame(const QuicStreamFrame& frame) { DCHECK(VersionHasStreamType(transport_version())); QuicStreamId stream_id = frame.stream_id; @@ -1197,14 +1179,25 @@ } void QuicSession::ActivateStream(std::unique_ptr<QuicStream> stream) { - DCHECK(!stream->is_static()); QuicStreamId stream_id = stream->id(); + bool is_static = stream->is_static(); QUIC_DVLOG(1) << ENDPOINT << "num_streams: " << stream_map_.size() << ". activating " << stream_id; DCHECK(!QuicContainsKey(stream_map_, stream_id)); stream_map_[stream_id] = std::move(stream); if (IsIncomingStream(stream_id)) { - ++num_dynamic_incoming_streams_; + is_static ? ++num_incoming_static_streams_ + : ++num_dynamic_incoming_streams_; + } else if (is_static) { + ++num_outgoing_static_streams_; + } + + if (VersionHasIetfQuicFrames(transport_version()) && + !QuicUtils::IsBidirectionalStreamId(stream_id) && is_static) { + DCHECK_LE(num_incoming_static_streams_, + num_expected_unidirectional_static_streams_); + DCHECK_LE(num_outgoing_static_streams_, + num_expected_unidirectional_static_streams_); } }
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index 1f03a40..96633d8 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -547,12 +547,6 @@ // ProcessPendingStream(). virtual bool UsesPendingStreams() const { return false; } - // Transfer ownership of |stream| to stream_map_, and register - // |stream| as static in stream id manager. |stream_already_counted| is true - // if |stream| is created from pending stream and is already known as an open - // stream. - void RegisterStaticStream(std::unique_ptr<QuicStream> stream); - StreamMap& stream_map() { return stream_map_; } const StreamMap& stream_map() const { return stream_map_; } @@ -581,7 +575,7 @@ // Returns true if the stream is a static stream. bool IsStaticStream(QuicStreamId id) const; - // Close connection when receive a frame for a locally-created nonexistant + // Close connection when receive a frame for a locally-created nonexistent // stream. // Prerequisite: IsClosedStream(stream_id) == false // Server session might need to override this method to allow server push
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index eafb0bd..4e419da 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -1388,8 +1388,7 @@ QuicUtils::GetHeadersStreamId(connection_->transport_version()); std::unique_ptr<TestStream> fake_headers_stream = QuicMakeUnique<TestStream>( headers_stream_id, &session_, /*is_static*/ true, BIDIRECTIONAL); - QuicSessionPeer::RegisterStaticStream(&session_, - std::move(fake_headers_stream)); + QuicSessionPeer::ActivateStream(&session_, std::move(fake_headers_stream)); // Send two bytes of payload. QuicStreamFrame data1(headers_stream_id, true, 0, QuicStringPiece("HT")); EXPECT_CALL(*connection_, @@ -1407,8 +1406,7 @@ QuicUtils::GetHeadersStreamId(connection_->transport_version()); std::unique_ptr<TestStream> fake_headers_stream = QuicMakeUnique<TestStream>( headers_stream_id, &session_, /*is_static*/ true, BIDIRECTIONAL); - QuicSessionPeer::RegisterStaticStream(&session_, - std::move(fake_headers_stream)); + QuicSessionPeer::ActivateStream(&session_, std::move(fake_headers_stream)); // Send two bytes of payload. QuicRstStreamFrame rst1(kInvalidControlFrameId, headers_stream_id, QUIC_ERROR_PROCESSING_STREAM, 0); @@ -2616,8 +2614,7 @@ QuicStreamId stream_id = 0; std::unique_ptr<TestStream> fake_static_stream = QuicMakeUnique<TestStream>( stream_id, &session_, /*is_static*/ true, BIDIRECTIONAL); - QuicSessionPeer::RegisterStaticStream(&session_, - std::move(fake_static_stream)); + QuicSessionPeer::ActivateStream(&session_, std::move(fake_static_stream)); // Check that a stream id in the static stream map is ignored. // Note that the notion of a static stream is Google-specific. QuicStopSendingFrame frame(1, stream_id, 123);
diff --git a/quic/qbone/qbone_client_session.cc b/quic/qbone/qbone_client_session.cc index 5c199e2..c07bdf6 100644 --- a/quic/qbone/qbone_client_session.cc +++ b/quic/qbone/qbone_client_session.cc
@@ -43,7 +43,7 @@ auto control_stream = QuicMakeUnique<QboneClientControlStream>(this, handler_); control_stream_ = control_stream.get(); - RegisterStaticStream(std::move(control_stream)); + ActivateStream(std::move(control_stream)); } int QboneClientSession::GetNumSentClientHellos() const {
diff --git a/quic/qbone/qbone_server_session.cc b/quic/qbone/qbone_server_session.cc index 37cdd4c..49f84eb 100644 --- a/quic/qbone/qbone_server_session.cc +++ b/quic/qbone/qbone_server_session.cc
@@ -59,7 +59,7 @@ auto control_stream = QuicMakeUnique<QboneServerControlStream>(this, handler_); control_stream_ = control_stream.get(); - RegisterStaticStream(std::move(control_stream)); + ActivateStream(std::move(control_stream)); } bool QboneServerSession::SendClientRequest(const QboneClientRequest& request) {
diff --git a/quic/test_tools/quic_session_peer.cc b/quic/test_tools/quic_session_peer.cc index db803af..22ccb28 100644 --- a/quic/test_tools/quic_session_peer.cc +++ b/quic/test_tools/quic_session_peer.cc
@@ -159,12 +159,6 @@ } // static -void QuicSessionPeer::RegisterStaticStream(QuicSession* session, - std::unique_ptr<QuicStream> stream) { - return session->RegisterStaticStream(std::move(stream)); -} - -// static bool QuicSessionPeer::IsStreamClosed(QuicSession* session, QuicStreamId id) { return session->IsClosedStream(id); }
diff --git a/quic/test_tools/quic_session_peer.h b/quic/test_tools/quic_session_peer.h index decc4b9..79338fc 100644 --- a/quic/test_tools/quic_session_peer.h +++ b/quic/test_tools/quic_session_peer.h
@@ -63,8 +63,6 @@ QuicSession* session); static void ActivateStream(QuicSession* session, std::unique_ptr<QuicStream> stream); - static void RegisterStaticStream(QuicSession* session, - std::unique_ptr<QuicStream> stream); // Discern the state of a stream. Exactly one of these should be true at a // time for any stream id > 0 (other than the special streams 1 and 3).
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index b3b6a68..90fa9b1 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -690,8 +690,6 @@ QuicSession::OnConnectionClosed(frame, source); } - using QuicSession::RegisterStaticStream; - // From QuicSession. MOCK_METHOD2(OnConnectionClosed, void(const QuicConnectionCloseFrame& frame,