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