Remove using_default_max_streams_ from QuicStreamIdManager because the manager now has initial stream limit 0. gfe-relnote: protected by disabled v99 flag. PiperOrigin-RevId: 291748001 Change-Id: Ide87e4b13fdac7541c0be5a7691228bdecdeb26c
diff --git a/quic/core/quic_stream_id_manager.cc b/quic/core/quic_stream_id_manager.cc index 14aa8e9..f49b188 100644 --- a/quic/core/quic_stream_id_manager.cc +++ b/quic/core/quic_stream_id_manager.cc
@@ -34,7 +34,6 @@ outgoing_max_streams_(max_allowed_outgoing_streams), next_outgoing_stream_id_(GetFirstOutgoingStreamId()), outgoing_stream_count_(0), - using_default_max_streams_(true), incoming_actual_max_streams_(max_allowed_incoming_streams), // Advertised max starts at actual because it's communicated in the // handshake. @@ -91,22 +90,7 @@ // maximum stream count from the peer. bool QuicStreamIdManager::SetMaxOpenOutgoingStreams( QuicStreamCount max_open_streams) { - if (using_default_max_streams_) { - // This is the first MAX_STREAMS/transport negotiation we've received. Treat - // this a bit differently than later ones. The difference is that - // outgoing_max_streams_ is currently an estimate. The MAX_STREAMS frame or - // transport negotiation is authoritative and can reduce - // outgoing_max_streams_ -- so long as outgoing_max_streams_ is not set to - // be less than the number of existing outgoing streams. If that happens, - // close the connection. - if (max_open_streams < outgoing_stream_count_) { - delegate_->OnError(QUIC_MAX_STREAMS_ERROR, - "Stream limit less than existing stream count"); - return false; - } - using_default_max_streams_ = false; - } else if (max_open_streams <= outgoing_max_streams_) { - // Is not the 1st MAX_STREAMS or negotiation. + if (max_open_streams <= outgoing_max_streams_) { // Only update the stream count if it would increase the limit. // If it decreases the limit, or doesn't change it, then do not update. // Note that this handles the case of receiving a count of 0 in the frame
diff --git a/quic/core/quic_stream_id_manager.h b/quic/core/quic_stream_id_manager.h index 92d9a06..9a23399 100644 --- a/quic/core/quic_stream_id_manager.h +++ b/quic/core/quic_stream_id_manager.h
@@ -72,7 +72,6 @@ ", outgoing_max_streams_: ", outgoing_max_streams_, ", next_outgoing_stream_id_: ", next_outgoing_stream_id_, ", outgoing_stream_count_: ", outgoing_stream_count_, - ", using_default_max_streams_: ", using_default_max_streams_, ", incoming_actual_max_streams_: ", incoming_actual_max_streams_, ", incoming_advertised_max_streams_: ", incoming_advertised_max_streams_, @@ -228,17 +227,6 @@ // outgoing_max_streams_. QuicStreamCount outgoing_stream_count_; - // Set to true while the default (from the constructor) outgoing stream limit - // is in use. It is set to false when either a MAX STREAMS frame is received - // or the transport negotiation completes and sets the stream limit (this is - // equivalent to a MAX_STREAMS frame). - // Necessary because outgoing_max_streams_ is a "best guess" - // until we receive an authoritative value from the peer. - // outgoing_max_streams_ is initialized in the constructor - // to some hard-coded value, which may or may not be consistent - // with what the peer wants. - bool using_default_max_streams_; - // FOR INCOMING STREAMS // The maximum number of streams that can be opened by the peer.
diff --git a/quic/core/quic_stream_id_manager_test.cc b/quic/core/quic_stream_id_manager_test.cc index 00193dd..76c9f85 100644 --- a/quic/core/quic_stream_id_manager_test.cc +++ b/quic/core/quic_stream_id_manager_test.cc
@@ -7,6 +7,7 @@ #include <string> #include <utility> +#include "net/third_party/quiche/src/quic/core/quic_constants.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" #include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" @@ -64,7 +65,7 @@ IsUnidirectional(), perspective(), transport_version(), - kDefaultMaxStreamsPerConnection, + 0, kDefaultMaxStreamsPerConnection) { DCHECK(VersionHasIetfQuicFrames(transport_version())); } @@ -97,8 +98,7 @@ ::testing::PrintToStringParamName()); TEST_P(QuicStreamIdManagerTest, Initialization) { - EXPECT_EQ(kDefaultMaxStreamsPerConnection, - stream_id_manager_.outgoing_max_streams()); + EXPECT_EQ(0u, stream_id_manager_.outgoing_max_streams()); EXPECT_EQ(kDefaultMaxStreamsPerConnection, stream_id_manager_.incoming_actual_max_streams()); @@ -236,32 +236,19 @@ } TEST_P(QuicStreamIdManagerTest, OnMaxStreamsFrame) { - // Get the current maximum allowed outgoing stream count. - QuicStreamCount initial_stream_count = - // need to know the number of request/response streams. - // This is the total number of outgoing streams (which includes both - // req/resp and statics). - stream_id_manager_.outgoing_max_streams(); - QuicMaxStreamsFrame frame; - - // Even though the stream count in the frame is < the initial maximum, - // it shouldn't be ignored since the initial max was set via - // the constructor (an educated guess) but the MAX STREAMS frame - // is authoritative. - frame.stream_count = initial_stream_count - 1; + frame.stream_count = 10; frame.unidirectional = IsUnidirectional(); EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidirectional())); EXPECT_TRUE(stream_id_manager_.OnMaxStreamsFrame(frame)); - EXPECT_EQ(initial_stream_count - 1u, - stream_id_manager_.outgoing_max_streams()); + EXPECT_EQ(10u, stream_id_manager_.outgoing_max_streams()); QuicStreamCount save_outgoing_max_streams = stream_id_manager_.outgoing_max_streams(); // Now that there has been one MAX STREAMS frame, we should not // accept a MAX_STREAMS that reduces the limit... - frame.stream_count = initial_stream_count - 2; + frame.stream_count = 8; frame.unidirectional = IsUnidirectional(); EXPECT_TRUE(stream_id_manager_.OnMaxStreamsFrame(frame)); // should not change from previous setting. @@ -269,12 +256,11 @@ stream_id_manager_.outgoing_max_streams()); // A stream count greater than the current limit should increase the limit. - frame.stream_count = initial_stream_count + 1; + frame.stream_count = 12; EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidirectional())); EXPECT_TRUE(stream_id_manager_.OnMaxStreamsFrame(frame)); - EXPECT_EQ(initial_stream_count + 1u, - stream_id_manager_.outgoing_max_streams()); + EXPECT_EQ(12u, stream_id_manager_.outgoing_max_streams()); } TEST_P(QuicStreamIdManagerTest, OnStreamsBlockedFrame) { @@ -343,7 +329,7 @@ size_t number_of_streams = kDefaultMaxStreamsPerConnection; EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidirectional())); - stream_id_manager_.SetMaxOpenOutgoingStreams(100); + stream_id_manager_.SetMaxOpenOutgoingStreams(kDefaultMaxStreamsPerConnection); stream_id_manager_.OnConfigNegotiated(); @@ -498,11 +484,6 @@ } TEST_P(QuicStreamIdManagerTest, HoldStreamsBlockedFrameXmit) { - // set outgoing limit to 0, will cause the CanOpenNext... to fail - // leading to a STREAMS_BLOCKED. - EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidirectional())); - stream_id_manager_.SetMaxOpenOutgoingStreams(0); - // We should not see a STREAMS_BLOCKED frame because we're not configured.. EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0); @@ -518,11 +499,6 @@ } TEST_P(QuicStreamIdManagerTest, HoldStreamsBlockedFrameNoXmit) { - // Set outgoing limit to 0, will cause the CanOpenNext... to fail - // leading to a STREAMS_BLOCKED. - EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidirectional())); - stream_id_manager_.SetMaxOpenOutgoingStreams(0); - // We should not see a STREAMS_BLOCKED frame because we're not configured.. EXPECT_CALL(delegate_, SendStreamsBlocked(_, IsUnidirectional())).Times(0);
diff --git a/quic/core/uber_quic_stream_id_manager_test.cc b/quic/core/uber_quic_stream_id_manager_test.cc index 037eb11..375e86e 100644 --- a/quic/core/uber_quic_stream_id_manager_test.cc +++ b/quic/core/uber_quic_stream_id_manager_test.cc
@@ -33,8 +33,8 @@ : manager_(perspective(), version(), &delegate_, - kDefaultMaxStreamsPerConnection, - kDefaultMaxStreamsPerConnection, + 0, + 0, kDefaultMaxStreamsPerConnection, kDefaultMaxStreamsPerConnection) {} @@ -182,6 +182,9 @@ } TEST_P(UberQuicStreamIdManagerTest, GetNextOutgoingStreamId) { + EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(_)).Times(2); + manager_.SetMaxOpenOutgoingBidirectionalStreams(10); + manager_.SetMaxOpenOutgoingUnidirectionalStreams(10); EXPECT_EQ(GetNthSelfInitiatedBidirectionalStreamId(0), manager_.GetNextOutgoingBidirectionalStreamId()); EXPECT_EQ(GetNthSelfInitiatedBidirectionalStreamId(1), @@ -247,7 +250,6 @@ QuicMaxStreamsFrame frame(kInvalidControlFrameId, max_outgoing_bidirectional_stream_count, /*unidirectional=*/false); - EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(frame.unidirectional)); EXPECT_TRUE(manager_.OnMaxStreamsFrame(frame)); EXPECT_EQ(max_outgoing_bidirectional_stream_count, manager_.max_outgoing_bidirectional_streams()); @@ -255,7 +257,6 @@ // Now try the unidirectioanl manager frame.stream_count = max_outgoing_unidirectional_stream_count; frame.unidirectional = true; - EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(frame.unidirectional)); EXPECT_TRUE(manager_.OnMaxStreamsFrame(frame)); EXPECT_EQ(max_outgoing_unidirectional_stream_count, manager_.max_outgoing_unidirectional_streams());