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());
