gfe-relnote: In QUIC, remove SendStreamsBlocked from QuicStreamIdManager::DelegateInterface . Refactoring only, no functional change expected, not protected.
PiperOrigin-RevId: 301895558
Change-Id: I18b8eee9403b01880537fb49b62d9102cb79e22e
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 004e334..32e01cb 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -847,16 +847,6 @@
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);
-}
-
void QuicSession::CloseStream(QuicStreamId stream_id) {
CloseStreamInner(stream_id, false);
}
@@ -1497,19 +1487,37 @@
}
bool QuicSession::CanOpenNextOutgoingBidirectionalStream() {
- if (VersionHasIetfQuicFrames(transport_version())) {
- return v99_streamid_manager_.CanOpenNextOutgoingBidirectionalStream();
+ if (!VersionHasIetfQuicFrames(transport_version())) {
+ return stream_id_manager_.CanOpenNextOutgoingStream(
+ GetNumOpenOutgoingStreams());
}
- return stream_id_manager_.CanOpenNextOutgoingStream(
- GetNumOpenOutgoingStreams());
+ if (v99_streamid_manager_.CanOpenNextOutgoingBidirectionalStream()) {
+ return true;
+ }
+ if (is_configured_) {
+ // Send STREAM_BLOCKED after config negotiated.
+ control_frame_manager_.WriteOrBufferStreamsBlocked(
+ v99_streamid_manager_.max_outgoing_bidirectional_streams(),
+ /*unidirectional=*/false);
+ }
+ return false;
}
bool QuicSession::CanOpenNextOutgoingUnidirectionalStream() {
- if (VersionHasIetfQuicFrames(transport_version())) {
- return v99_streamid_manager_.CanOpenNextOutgoingUnidirectionalStream();
+ if (!VersionHasIetfQuicFrames(transport_version())) {
+ return stream_id_manager_.CanOpenNextOutgoingStream(
+ GetNumOpenOutgoingStreams());
}
- return stream_id_manager_.CanOpenNextOutgoingStream(
- GetNumOpenOutgoingStreams());
+ if (v99_streamid_manager_.CanOpenNextOutgoingUnidirectionalStream()) {
+ return true;
+ }
+ if (is_configured_) {
+ // Send STREAM_BLOCKED after config negotiated.
+ control_frame_manager_.WriteOrBufferStreamsBlocked(
+ v99_streamid_manager_.max_outgoing_unidirectional_streams(),
+ /*unidirectional=*/true);
+ }
+ return false;
}
QuicStreamCount QuicSession::GetAdvertisedMaxIncomingBidirectionalStreams()
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index 8770dd9..37c00f7 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -154,8 +154,6 @@
std::string error_details) override;
void SendMaxStreams(QuicStreamCount stream_count,
bool unidirectional) override;
- void SendStreamsBlocked(QuicStreamCount stream_count,
- bool unidirectional) override;
// The default implementation does nothing. Subclasses should override if
// for example they queue up stream requests.
void OnCanCreateNewOutgoingStream(bool unidirectional) override;
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index 427f7a1..e3289e9 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -327,8 +327,11 @@
}
using QuicSession::ActivateStream;
+ using QuicSession::CanOpenNextOutgoingBidirectionalStream;
using QuicSession::CanOpenNextOutgoingUnidirectionalStream;
using QuicSession::closed_streams;
+ using QuicSession::GetNextOutgoingBidirectionalStreamId;
+ using QuicSession::GetNextOutgoingUnidirectionalStreamId;
using QuicSession::zombie_streams;
private:
@@ -1211,6 +1214,41 @@
EXPECT_TRUE(session_.WillingAndAbleToWrite());
}
+TEST_P(QuicSessionTestServer, SendStreamsBlocked) {
+ if (!VersionHasIetfQuicFrames(transport_version())) {
+ return;
+ }
+ for (size_t i = 0; i < kDefaultMaxStreamsPerConnection; ++i) {
+ ASSERT_TRUE(session_.CanOpenNextOutgoingBidirectionalStream());
+ session_.GetNextOutgoingBidirectionalStreamId();
+ }
+ // Next checking causes STREAMS_BLOCKED to be sent.
+ EXPECT_CALL(*connection_, SendControlFrame(_))
+ .WillOnce(Invoke([](const QuicFrame& frame) {
+ EXPECT_FALSE(frame.streams_blocked_frame.unidirectional);
+ EXPECT_EQ(kDefaultMaxStreamsPerConnection,
+ frame.streams_blocked_frame.stream_count);
+ ClearControlFrame(frame);
+ return true;
+ }));
+ EXPECT_FALSE(session_.CanOpenNextOutgoingBidirectionalStream());
+
+ for (size_t i = 0; i < kDefaultMaxStreamsPerConnection; ++i) {
+ ASSERT_TRUE(session_.CanOpenNextOutgoingUnidirectionalStream());
+ session_.GetNextOutgoingUnidirectionalStreamId();
+ }
+ // Next checking causes STREAM_BLOCKED to be sent.
+ EXPECT_CALL(*connection_, SendControlFrame(_))
+ .WillOnce(Invoke([](const QuicFrame& frame) {
+ EXPECT_TRUE(frame.streams_blocked_frame.unidirectional);
+ EXPECT_EQ(kDefaultMaxStreamsPerConnection,
+ frame.streams_blocked_frame.stream_count);
+ ClearControlFrame(frame);
+ return true;
+ }));
+ EXPECT_FALSE(session_.CanOpenNextOutgoingUnidirectionalStream());
+}
+
TEST_P(QuicSessionTestServer, BufferedHandshake) {
// This test is testing behavior of crypto stream flow control, but when
// CRYPTO frames are used, there is no flow control for the crypto handshake.
diff --git a/quic/core/quic_stream_id_manager.cc b/quic/core/quic_stream_id_manager.cc
index 05608e8..2a64118 100644
--- a/quic/core/quic_stream_id_manager.cc
+++ b/quic/core/quic_stream_id_manager.cc
@@ -171,15 +171,9 @@
return id;
}
-bool QuicStreamIdManager::CanOpenNextOutgoingStream() {
+bool QuicStreamIdManager::CanOpenNextOutgoingStream() const {
DCHECK(VersionHasIetfQuicFrames(transport_version_));
- if (outgoing_stream_count_ < outgoing_max_streams_) {
- return true;
- }
- // 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);
- return false;
+ return outgoing_stream_count_ < outgoing_max_streams_;
}
bool QuicStreamIdManager::MaybeIncreaseLargestPeerStreamId(
diff --git a/quic/core/quic_stream_id_manager.h b/quic/core/quic_stream_id_manager.h
index af9d44a..da94233 100644
--- a/quic/core/quic_stream_id_manager.h
+++ b/quic/core/quic_stream_id_manager.h
@@ -48,10 +48,6 @@
// Send a MAX_STREAMS frame.
virtual void SendMaxStreams(QuicStreamCount stream_count,
bool unidirectional) = 0;
-
- // Send a STREAMS_BLOCKED frame.
- virtual void SendStreamsBlocked(QuicStreamCount stream_count,
- bool unidirectional) = 0;
};
QuicStreamIdManager(DelegateInterface* delegate,
@@ -94,7 +90,7 @@
bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame);
// Indicates whether the next outgoing stream ID can be allocated or not.
- bool CanOpenNextOutgoingStream();
+ bool CanOpenNextOutgoingStream() const;
// Generate and send a MAX_STREAMS frame.
void SendMaxStreamsFrame();
diff --git a/quic/core/quic_stream_id_manager_test.cc b/quic/core/quic_stream_id_manager_test.cc
index d6a9cb9..e9a4948 100644
--- a/quic/core/quic_stream_id_manager_test.cc
+++ b/quic/core/quic_stream_id_manager_test.cc
@@ -29,8 +29,6 @@
void(QuicErrorCode error_code, std::string error_details));
MOCK_METHOD2(SendMaxStreams,
void(QuicStreamCount stream_count, bool unidirectional));
- MOCK_METHOD2(SendStreamsBlocked,
- void(QuicStreamCount stream_count, bool unidirectional));
};
struct TestParams {
@@ -208,7 +206,6 @@
TEST_P(QuicStreamIdManagerTest, ProcessStreamsBlockedTooBig) {
EXPECT_CALL(delegate_, OnStreamIdManagerError(QUIC_STREAMS_BLOCKED_ERROR, _));
EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0);
- EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0);
QuicStreamCount stream_count =
stream_id_manager_.incoming_initial_max_open_streams() + 1;
QuicStreamsBlockedFrame frame(0, stream_count, IsUnidirectional());
@@ -287,7 +284,6 @@
// If the peer is saying it's blocked on the stream count that
// we've advertised, it's a noop since the peer has the correct information.
frame.stream_count = advertised_stream_count;
- EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0);
EXPECT_TRUE(stream_id_manager_.OnStreamsBlockedFrame(frame));
// If the peer is saying it's blocked on a stream count that is larger
@@ -356,9 +352,7 @@
}
// If we try to check that the next outgoing stream id is available it should
- // A) fail and B) generate a STREAMS_BLOCKED frame.
- EXPECT_CALL(delegate_, SendStreamsBlocked(kDefaultMaxStreamsPerConnection,
- IsUnidirectional()));
+ // fail.
EXPECT_FALSE(stream_id_manager_.CanOpenNextOutgoingStream());
// If we try to get the next id (above the limit), it should cause a quic-bug.
@@ -395,7 +389,6 @@
// Should not get a control-frame transmission since the peer should have
// "plenty" of stream IDs to use.
- EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0);
EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0);
// Get the first incoming stream ID to try and allocate.
@@ -455,7 +448,6 @@
// Check that receipt of a STREAMS BLOCKED with stream-count = 0 does nothing
// when max_allowed_incoming_streams is 0.
EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0);
- EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0);
stream_id_manager_.SetMaxOpenIncomingStreams(0);
frame.stream_count = 0;
stream_id_manager_.OnStreamsBlockedFrame(frame);
@@ -463,7 +455,6 @@
// Check that receipt of a STREAMS BLOCKED with stream-count = 0 invokes a
// MAX STREAMS, count = 123, when the MaxOpen... is set to 123.
EXPECT_CALL(delegate_, SendMaxStreams(123u, IsUnidirectional()));
- EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0);
stream_id_manager_.SetMaxOpenIncomingStreams(123);
frame.stream_count = 0;
stream_id_manager_.OnStreamsBlockedFrame(frame);
@@ -519,9 +510,7 @@
EXPECT_EQ(stream_id_manager_.outgoing_stream_count(),
stream_id_manager_.outgoing_max_streams());
- // Create another, it should fail. Should also send a STREAMS_BLOCKED
- // control frame.
- EXPECT_CALL(delegate_, SendStreamsBlocked(_, IsUnidirectional()));
+ // Create another, it should fail.
EXPECT_FALSE(stream_id_manager_.CanOpenNextOutgoingStream());
}
diff --git a/quic/core/uber_quic_stream_id_manager.cc b/quic/core/uber_quic_stream_id_manager.cc
index 50e4d02..e85336f 100644
--- a/quic/core/uber_quic_stream_id_manager.cc
+++ b/quic/core/uber_quic_stream_id_manager.cc
@@ -48,11 +48,11 @@
unidirectional_stream_id_manager_.SetMaxOpenIncomingStreams(max_open_streams);
}
-bool UberQuicStreamIdManager::CanOpenNextOutgoingBidirectionalStream() {
+bool UberQuicStreamIdManager::CanOpenNextOutgoingBidirectionalStream() const {
return bidirectional_stream_id_manager_.CanOpenNextOutgoingStream();
}
-bool UberQuicStreamIdManager::CanOpenNextOutgoingUnidirectionalStream() {
+bool UberQuicStreamIdManager::CanOpenNextOutgoingUnidirectionalStream() const {
return unidirectional_stream_id_manager_.CanOpenNextOutgoingStream();
}
diff --git a/quic/core/uber_quic_stream_id_manager.h b/quic/core/uber_quic_stream_id_manager.h
index 52dc85f..bc76be3 100644
--- a/quic/core/uber_quic_stream_id_manager.h
+++ b/quic/core/uber_quic_stream_id_manager.h
@@ -39,10 +39,10 @@
QuicStreamCount max_open_streams);
// Returns true if next outgoing bidirectional stream ID can be allocated.
- bool CanOpenNextOutgoingBidirectionalStream();
+ bool CanOpenNextOutgoingBidirectionalStream() const;
// Returns true if next outgoing unidirectional stream ID can be allocated.
- bool CanOpenNextOutgoingUnidirectionalStream();
+ bool CanOpenNextOutgoingUnidirectionalStream() const;
// Returns the next outgoing bidirectional stream id.
QuicStreamId GetNextOutgoingBidirectionalStreamId();
diff --git a/quic/core/uber_quic_stream_id_manager_test.cc b/quic/core/uber_quic_stream_id_manager_test.cc
index d88ebe1..a8f86a6 100644
--- a/quic/core/uber_quic_stream_id_manager_test.cc
+++ b/quic/core/uber_quic_stream_id_manager_test.cc
@@ -51,8 +51,6 @@
void(QuicErrorCode error_code, std::string error_details));
MOCK_METHOD2(SendMaxStreams,
void(QuicStreamCount stream_count, bool unidirectional));
- MOCK_METHOD2(SendStreamsBlocked,
- void(QuicStreamCount stream_count, bool unidirectional));
};
class UberQuicStreamIdManagerTest : public QuicTestWithParam<TestParams> {
@@ -171,7 +169,6 @@
manager_.GetNextOutgoingUnidirectionalStreamId();
// Both should be exhausted...
- EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(2);
EXPECT_FALSE(manager_.CanOpenNextOutgoingUnidirectionalStream());
EXPECT_FALSE(manager_.CanOpenNextOutgoingBidirectionalStream());
}
@@ -375,7 +372,6 @@
manager_.GetNextOutgoingUnidirectionalStreamId();
// Both should be exhausted...
- EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(3);
EXPECT_FALSE(manager_.CanOpenNextOutgoingUnidirectionalStream());
EXPECT_FALSE(manager_.CanOpenNextOutgoingBidirectionalStream());
