gfe-relnote: In QUIC, remove is_config_negotiated_ from QuicStreamIdManager. No functional change expected, not protected. PiperOrigin-RevId: 301691294 Change-Id: I422896ecea4648b6f53ddd64c76923117fccb5b9
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index a10f415..004e334 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -840,11 +840,19 @@ void QuicSession::SendMaxStreams(QuicStreamCount stream_count, bool unidirectional) { + if (!is_configured_) { + QUIC_BUG << "Try to send max streams before config negotiated."; + return; + } control_frame_manager_.WriteOrBufferMaxStreams(stream_count, unidirectional); } void QuicSession::SendStreamsBlocked(QuicStreamCount stream_count, bool unidirectional) { + if (!is_configured_) { + QUIC_BUG << "Try to send stream blocked before config negotiated."; + return; + } control_frame_manager_.WriteOrBufferStreamsBlocked(stream_count, unidirectional); } @@ -1137,14 +1145,6 @@ is_configured_ = true; connection()->OnConfigNegotiated(); - // Inform stream ID manager so that it can reevaluate any deferred - // STREAMS_BLOCKED or MAX_STREAMS frames against the config and either send - // the frames or discard them. - if (VersionHasIetfQuicFrames(connection_->transport_version())) { - QuicConnection::ScopedPacketFlusher flusher(connection()); - v99_streamid_manager_.OnConfigNegotiated(); - } - // Ask flow controllers to try again since the config could have unblocked us. if (connection_->version().AllowsLowFlowControlLimits()) { OnCanWrite();
diff --git a/quic/core/quic_stream_id_manager.cc b/quic/core/quic_stream_id_manager.cc index 946bc9a..0e11a55 100644 --- a/quic/core/quic_stream_id_manager.cc +++ b/quic/core/quic_stream_id_manager.cc
@@ -31,7 +31,6 @@ unidirectional_(unidirectional), perspective_(perspective), transport_version_(transport_version), - is_config_negotiated_(false), outgoing_max_streams_(max_allowed_outgoing_streams), next_outgoing_stream_id_(GetFirstOutgoingStreamId()), outgoing_stream_count_(0), @@ -134,14 +133,6 @@ } void QuicStreamIdManager::SendMaxStreamsFrame() { - if (!is_config_negotiated_) { - // The config has not yet been negotiated, so we can not send the - // MAX STREAMS frame yet. Record that we would have sent one and then - // return. A new frame will be generated once the configuration is - // received. - QUIC_BUG << "Attempt to send Max Streams Frame before config is negotiated"; - return; - } incoming_advertised_max_streams_ = incoming_actual_max_streams_; delegate_->SendMaxStreams(incoming_advertised_max_streams_, unidirectional_); } @@ -185,11 +176,6 @@ if (outgoing_stream_count_ < outgoing_max_streams_) { return true; } - if (!is_config_negotiated_) { - QUIC_BUG - << "Creating streams before Quic session is configured is prohibitied"; - return false; - } // Next stream ID would exceed the limit, need to inform the peer. delegate_->SendStreamsBlocked(outgoing_max_streams_, unidirectional_); QUIC_CODE_COUNT(quic_reached_outgoing_stream_id_limit); @@ -318,8 +304,4 @@ } } -void QuicStreamIdManager::OnConfigNegotiated() { - is_config_negotiated_ = true; -} - } // namespace quic
diff --git a/quic/core/quic_stream_id_manager.h b/quic/core/quic_stream_id_manager.h index 74d9c6f..af9d44a 100644 --- a/quic/core/quic_stream_id_manager.h +++ b/quic/core/quic_stream_id_manager.h
@@ -170,10 +170,6 @@ Perspective perspective() const; Perspective peer_perspective() const; - // Called when session has been configured. Causes the Stream ID manager to - // send out any pending MAX_STREAMS and STREAMS_BLOCKED frames. - void OnConfigNegotiated(); - private: friend class test::QuicSessionPeer; friend class test::QuicStreamIdManagerPeer; @@ -204,9 +200,6 @@ // Transport version used for this manager. const QuicTransportVersion transport_version_; - // True if the config has been negotiated_; - bool is_config_negotiated_; - // This is the number of streams that this node can initiate. // This limit is: // - Initiated to a value specified in the constructor
diff --git a/quic/core/quic_stream_id_manager_test.cc b/quic/core/quic_stream_id_manager_test.cc index 12368e9..d6a9cb9 100644 --- a/quic/core/quic_stream_id_manager_test.cc +++ b/quic/core/quic_stream_id_manager_test.cc
@@ -185,9 +185,6 @@ // the count most recently advertised in a MAX_STREAMS frame. This should cause // a MAX_STREAMS frame with the most recently advertised count to be sent. TEST_P(QuicStreamIdManagerTest, ProcessStreamsBlockedOk) { - // Set the config negotiated so that the MAX_STREAMS is transmitted. - stream_id_manager_.OnConfigNegotiated(); - QuicStreamCount stream_count = stream_id_manager_.incoming_initial_max_open_streams(); QuicStreamsBlockedFrame frame(0, stream_count - 1, IsUnidirectional()); @@ -283,9 +280,6 @@ QuicStreamCount advertised_stream_count = stream_id_manager_.incoming_advertised_max_streams(); - // Set the config negotiated to allow frame transmission. - stream_id_manager_.OnConfigNegotiated(); - QuicStreamsBlockedFrame frame; frame.unidirectional = IsUnidirectional(); @@ -346,8 +340,6 @@ EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidirectional())); stream_id_manager_.SetMaxOpenOutgoingStreams(kDefaultMaxStreamsPerConnection); - stream_id_manager_.OnConfigNegotiated(); - QuicStreamId stream_id = IsUnidirectional() ? QuicUtils::GetFirstUnidirectionalStreamId( @@ -391,9 +383,6 @@ } TEST_P(QuicStreamIdManagerTest, MaxStreamsWindow) { - // Set the config negotiated to allow frame transmission. - stream_id_manager_.OnConfigNegotiated(); - // Test that a MAX_STREAMS frame is generated when the peer has less than // |max_streams_window_| streams left that it can initiate. @@ -460,9 +449,6 @@ } TEST_P(QuicStreamIdManagerTest, StreamsBlockedEdgeConditions) { - // Set the config negotiated to allow frame transmission. - stream_id_manager_.OnConfigNegotiated(); - QuicStreamsBlockedFrame frame; frame.unidirectional = IsUnidirectional(); @@ -483,26 +469,6 @@ stream_id_manager_.OnStreamsBlockedFrame(frame); } -TEST_P(QuicStreamIdManagerTest, NoMaxStreamsFrameBeforeConfigured) { - // The config has not been negotiated so QUIC_BUG will be triggered. - EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0); - - QuicStreamsBlockedFrame frame(1u, 0u, IsUnidirectional()); - // Trigger sending of MAX_STREAMS. - EXPECT_QUIC_BUG( - stream_id_manager_.OnStreamsBlockedFrame(frame), - "Attempt to send Max Streams Frame before config is negotiated"); -} - -TEST_P(QuicStreamIdManagerTest, NoStreamCreationBeforeConfigured) { - // We should not see a STREAMS_BLOCKED frame because we're not configured.. - EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0); - - EXPECT_QUIC_BUG( - stream_id_manager_.CanOpenNextOutgoingStream(), - "Creating streams before Quic session is configured is prohibitied"); -} - TEST_P(QuicStreamIdManagerTest, CheckMaxAllowedOutgoingInitialization) { const size_t kIncomingStreamCount = 123; EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidirectional())); @@ -514,9 +480,6 @@ // available. This has a useful side effect of testing that when streams are // closed, the number of available stream ids increases. TEST_P(QuicStreamIdManagerTest, MaxStreamsSlidingWindow) { - // Simulate config being negotiated, causing the limits all to be initialized. - stream_id_manager_.OnConfigNegotiated(); - QuicStreamCount first_advert = stream_id_manager_.incoming_advertised_max_streams(); @@ -544,7 +507,6 @@ TEST_P(QuicStreamIdManagerTest, NewStreamDoesNotExceedLimit) { EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidirectional())); stream_id_manager_.SetMaxOpenOutgoingStreams(100); - stream_id_manager_.OnConfigNegotiated(); size_t stream_count = stream_id_manager_.outgoing_max_streams(); EXPECT_NE(0u, stream_count);
diff --git a/quic/core/uber_quic_stream_id_manager.h b/quic/core/uber_quic_stream_id_manager.h index 78c012b..52dc85f 100644 --- a/quic/core/uber_quic_stream_id_manager.h +++ b/quic/core/uber_quic_stream_id_manager.h
@@ -86,11 +86,6 @@ QuicStreamCount advertised_max_incoming_bidirectional_streams() const; QuicStreamCount advertised_max_incoming_unidirectional_streams() const; - void OnConfigNegotiated() { - bidirectional_stream_id_manager_.OnConfigNegotiated(); - unidirectional_stream_id_manager_.OnConfigNegotiated(); - } - private: friend class test::QuicSessionPeer; friend class test::UberQuicStreamIdManagerPeer;
diff --git a/quic/core/uber_quic_stream_id_manager_test.cc b/quic/core/uber_quic_stream_id_manager_test.cc index d7e9cc1..d88ebe1 100644 --- a/quic/core/uber_quic_stream_id_manager_test.cc +++ b/quic/core/uber_quic_stream_id_manager_test.cc
@@ -145,7 +145,6 @@ } TEST_P(UberQuicStreamIdManagerTest, SetMaxOpenOutgoingStreams) { - manager_.OnConfigNegotiated(); const size_t kNumMaxOutgoingStream = 123; // Set the uni- and bi- directional limits to different values to ensure // that they are managed separately. @@ -318,9 +317,6 @@ } TEST_P(UberQuicStreamIdManagerTest, OnStreamsBlockedFrame) { - // Allow MAX_STREAMS frame transmission - manager_.OnConfigNegotiated(); - QuicStreamCount stream_count = manager_.advertised_max_incoming_bidirectional_streams() - 1; @@ -353,7 +349,6 @@ } TEST_P(UberQuicStreamIdManagerTest, SetMaxOpenOutgoingStreamsPlusFrame) { - manager_.OnConfigNegotiated(); const size_t kNumMaxOutgoingStream = 123; // Set the uni- and bi- directional limits to different values to ensure // that they are managed separately.