Remove QuicStreamIdManager::DelegateInterface::OnStreamIdManagerError(). The errors can be replaced with false return value and be handled directly by QuicSession. gfe-relnote: no behavior change. not protected. PiperOrigin-RevId: 304243104 Change-Id: I68056d90fb0b9d502cdbd40fe2254dee6866ea8e
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 22252ae..5376bfd 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -825,13 +825,6 @@ control_frame_manager_.WriteOrBufferWindowUpdate(id, byte_offset); } -void QuicSession::OnStreamIdManagerError(QuicErrorCode error_code, - std::string error_details) { - connection_->CloseConnection( - error_code, error_details, - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); -} - void QuicSession::OnStreamError(QuicErrorCode error_code, std::string error_details) { connection_->CloseConnection( @@ -1630,7 +1623,15 @@ bool QuicSession::MaybeIncreaseLargestPeerStreamId( const QuicStreamId stream_id) { if (VersionHasIetfQuicFrames(transport_version())) { - return v99_streamid_manager_.MaybeIncreaseLargestPeerStreamId(stream_id); + std::string error_details; + if (v99_streamid_manager_.MaybeIncreaseLargestPeerStreamId( + stream_id, &error_details)) { + return true; + } + connection()->CloseConnection( + QUIC_INVALID_STREAM_ID, error_details, + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return false; } if (!stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)) { connection()->CloseConnection( @@ -2223,7 +2224,14 @@ } bool QuicSession::OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame) { - return v99_streamid_manager_.OnStreamsBlockedFrame(frame); + std::string error_details; + if (v99_streamid_manager_.OnStreamsBlockedFrame(frame, &error_details)) { + return true; + } + connection_->CloseConnection( + QUIC_STREAMS_BLOCKED_ERROR, error_details, + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return false; } size_t QuicSession::max_open_incoming_bidirectional_streams() const {
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index 567126a..ac99a3f 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -149,9 +149,6 @@ bool HasUnackedCryptoData() const override; bool HasUnackedStreamData() const override; - // QuicStreamIdManager::DelegateInterface methods: - void OnStreamIdManagerError(QuicErrorCode error_code, - std::string error_details) override; void SendMaxStreams(QuicStreamCount stream_count, bool unidirectional) override; // The default implementation does nothing. Subclasses should override if
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index 443af09..4da502d 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -815,9 +815,7 @@ *connection_, CloseConnection(QUIC_INVALID_STREAM_ID, "Stream id 798 would exceed stream count limit 50", - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET - - )) + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET)) .Times(1); EXPECT_EQ(nullptr, session_.GetOrCreateStream( GetNthClientInitiatedUnidirectionalId(199)));
diff --git a/quic/core/quic_stream_id_manager.cc b/quic/core/quic_stream_id_manager.cc index 9a52ff3..c6869a2 100644 --- a/quic/core/quic_stream_id_manager.cc +++ b/quic/core/quic_stream_id_manager.cc
@@ -51,15 +51,16 @@ // The peer sends a streams blocked frame when it can not open any more // streams because it has runs into the limit. bool QuicStreamIdManager::OnStreamsBlockedFrame( - const QuicStreamsBlockedFrame& frame) { + const QuicStreamsBlockedFrame& frame, + std::string* error_details) { // Ensure that the frame has the correct directionality. DCHECK_EQ(frame.unidirectional, unidirectional_); if (frame.stream_count > incoming_advertised_max_streams_) { // Peer thinks it can send more streams that we've told it. // This is a protocol error. - QUIC_CODE_COUNT(quic_streams_blocked_too_big); - delegate_->OnStreamIdManagerError(QUIC_STREAMS_BLOCKED_ERROR, - "Invalid stream count specified"); + *error_details = quiche::QuicheStrCat( + "StreamsBlockedFrame's stream count ", frame.stream_count, + " exceeds incoming max stream ", incoming_advertised_max_streams_); return false; } if (frame.stream_count < incoming_actual_max_streams_) { @@ -68,7 +69,6 @@ // frame in this case is not controlled by the window. SendMaxStreamsFrame(); } - QUIC_CODE_COUNT(quic_streams_blocked_id_correct); return true; } @@ -155,7 +155,8 @@ } bool QuicStreamIdManager::MaybeIncreaseLargestPeerStreamId( - const QuicStreamId stream_id) { + const QuicStreamId stream_id, + std::string* error_details) { // |stream_id| must be an incoming stream of the right directionality. DCHECK_NE(QuicUtils::IsBidirectionalStreamId(stream_id), unidirectional_); DCHECK_NE(QuicUtils::IsServerInitiatedStreamId(transport_version_, stream_id), @@ -186,11 +187,9 @@ << "Failed to create a new incoming stream with id:" << stream_id << ", reaching MAX_STREAMS limit: " << incoming_advertised_max_streams_ << "."; - delegate_->OnStreamIdManagerError( - QUIC_INVALID_STREAM_ID, - quiche::QuicheStrCat("Stream id ", stream_id, - " would exceed stream count limit ", - incoming_advertised_max_streams_)); + *error_details = quiche::QuicheStrCat("Stream id ", stream_id, + " would exceed stream count limit ", + incoming_advertised_max_streams_); return false; }
diff --git a/quic/core/quic_stream_id_manager.h b/quic/core/quic_stream_id_manager.h index b3079b2..fa4b1a0 100644 --- a/quic/core/quic_stream_id_manager.h +++ b/quic/core/quic_stream_id_manager.h
@@ -33,10 +33,6 @@ public: virtual ~DelegateInterface() = default; - // Closes the connection when an error is encountered. - virtual void OnStreamIdManagerError(QuicErrorCode error_code, - std::string error_details) = 0; - // Send a MAX_STREAMS frame. virtual void SendMaxStreams(QuicStreamCount stream_count, bool unidirectional) = 0; @@ -69,11 +65,10 @@ ", max_streams_window_: ", max_streams_window_, " }"); } - // Processes the STREAMS_BLOCKED frame, invoked from - // QuicSession::OnStreamsBlockedFrame. It has the same semantics as the - // QuicFramerVisitorInterface, returning true if the framer should continue - // processing the packet, false if not. - bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame); + // Processes the STREAMS_BLOCKED frame. If error is encountered, populates + // |error_details| and returns false. + bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame, + std::string* error_details); // Indicates whether the next outgoing stream ID can be allocated or not. bool CanOpenNextOutgoingStream() const; @@ -100,12 +95,13 @@ bool MaybeAllowNewOutgoingStreams(QuicStreamCount max_open_streams); // Checks if the incoming stream ID exceeds the MAX_STREAMS limit. If the - // limit is exceeded, closes the connection and returns false. Uses the + // limit is exceeded, populates |error_detials| and returns false. Uses the // actual maximium, not the most recently advertised value, in order to // enforce the Google-QUIC number of open streams behavior. // This method should be called exactly once for each incoming stream // creation. - bool MaybeIncreaseLargestPeerStreamId(const QuicStreamId stream_id); + bool MaybeIncreaseLargestPeerStreamId(const QuicStreamId stream_id, + std::string* error_details); // Returns true if |id| is still available. bool IsAvailableStream(QuicStreamId id) const;
diff --git a/quic/core/quic_stream_id_manager_test.cc b/quic/core/quic_stream_id_manager_test.cc index 73a4ff9..3207e59 100644 --- a/quic/core/quic_stream_id_manager_test.cc +++ b/quic/core/quic_stream_id_manager_test.cc
@@ -24,9 +24,6 @@ class MockDelegate : public QuicStreamIdManager::DelegateInterface { public: - MOCK_METHOD1(OnCanCreateNewOutgoingStream, void(bool unidirectional)); - MOCK_METHOD2(OnStreamIdManagerError, - void(QuicErrorCode error_code, std::string error_details)); MOCK_METHOD2(SendMaxStreams, void(QuicStreamCount stream_count, bool unidirectional)); }; @@ -153,7 +150,8 @@ stream_id_manager_.incoming_initial_max_open_streams(); QuicStreamsBlockedFrame frame(0, stream_count - 1, IsUnidirectional()); EXPECT_CALL(delegate_, SendMaxStreams(stream_count, IsUnidirectional())); - stream_id_manager_.OnStreamsBlockedFrame(frame); + std::string error_details; + EXPECT_TRUE(stream_id_manager_.OnStreamsBlockedFrame(frame, &error_details)); } // Check the case of the stream count in a STREAMS_BLOCKED frame is equal to the @@ -170,12 +168,15 @@ // the count most recently advertised in a MAX_STREAMS frame. Expect a // connection close with an error. TEST_P(QuicStreamIdManagerTest, ProcessStreamsBlockedTooBig) { - EXPECT_CALL(delegate_, OnStreamIdManagerError(QUIC_STREAMS_BLOCKED_ERROR, _)); EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0); QuicStreamCount stream_count = stream_id_manager_.incoming_initial_max_open_streams() + 1; QuicStreamsBlockedFrame frame(0, stream_count, IsUnidirectional()); - stream_id_manager_.OnStreamsBlockedFrame(frame); + std::string error_details; + EXPECT_FALSE(stream_id_manager_.OnStreamsBlockedFrame(frame, &error_details)); + EXPECT_EQ( + error_details, + "StreamsBlockedFrame's stream count 101 exceeds incoming max stream 100"); } // Same basic tests as above, but calls @@ -187,27 +188,28 @@ TEST_P(QuicStreamIdManagerTest, IsIncomingStreamIdValidBelowLimit) { QuicStreamId stream_id = GetNthIncomingStreamId( stream_id_manager_.incoming_actual_max_streams() - 2); - EXPECT_CALL(delegate_, OnStreamIdManagerError(_, _)).Times(0); - EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)); + EXPECT_TRUE( + stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id, nullptr)); } // Accept a stream with an ID that equals the limit. TEST_P(QuicStreamIdManagerTest, IsIncomingStreamIdValidAtLimit) { QuicStreamId stream_id = GetNthIncomingStreamId( stream_id_manager_.incoming_actual_max_streams() - 1); - EXPECT_CALL(delegate_, OnStreamIdManagerError(_, _)).Times(0); - EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)); + EXPECT_TRUE( + stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id, nullptr)); } // Close the connection if the id exceeds the limit. TEST_P(QuicStreamIdManagerTest, IsIncomingStreamIdInValidAboveLimit) { QuicStreamId stream_id = GetNthIncomingStreamId(stream_id_manager_.incoming_actual_max_streams()); - std::string error_details = quiche::QuicheStrCat( - "Stream id ", stream_id, " would exceed stream count limit 100"); - EXPECT_CALL(delegate_, - OnStreamIdManagerError(QUIC_INVALID_STREAM_ID, error_details)); - EXPECT_FALSE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)); + std::string error_details; + EXPECT_FALSE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId( + stream_id, &error_details)); + EXPECT_EQ(error_details, + quiche::QuicheStrCat("Stream id ", stream_id, + " would exceed stream count limit 100")); } TEST_P(QuicStreamIdManagerTest, OnStreamsBlockedFrame) { @@ -222,13 +224,16 @@ // 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_TRUE(stream_id_manager_.OnStreamsBlockedFrame(frame)); + std::string error_details; + EXPECT_TRUE(stream_id_manager_.OnStreamsBlockedFrame(frame, &error_details)); // If the peer is saying it's blocked on a stream count that is larger // than what we've advertised, the connection should get closed. frame.stream_count = advertised_stream_count + 1; - EXPECT_CALL(delegate_, OnStreamIdManagerError(QUIC_STREAMS_BLOCKED_ERROR, _)); - EXPECT_FALSE(stream_id_manager_.OnStreamsBlockedFrame(frame)); + EXPECT_FALSE(stream_id_manager_.OnStreamsBlockedFrame(frame, &error_details)); + EXPECT_EQ( + error_details, + "StreamsBlockedFrame's stream count 101 exceeds incoming max stream 100"); // If the peer is saying it's blocked on a count that is less than // our actual count, we send a MAX_STREAMS frame and update @@ -260,7 +265,7 @@ SendMaxStreams(stream_id_manager_.incoming_actual_max_streams(), IsUnidirectional())); - EXPECT_TRUE(stream_id_manager_.OnStreamsBlockedFrame(frame)); + EXPECT_TRUE(stream_id_manager_.OnStreamsBlockedFrame(frame, &error_details)); // Check that the saved frame is correct. EXPECT_EQ(stream_id_manager_.incoming_actual_max_streams(), stream_id_manager_.incoming_advertised_max_streams()); @@ -302,16 +307,20 @@ TEST_P(QuicStreamIdManagerTest, MaybeIncreaseLargestPeerStreamId) { QuicStreamId max_stream_id = GetNthIncomingStreamId( stream_id_manager_.incoming_actual_max_streams() - 1); - EXPECT_TRUE( - stream_id_manager_.MaybeIncreaseLargestPeerStreamId(max_stream_id)); + EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(max_stream_id, + nullptr)); QuicStreamId first_stream_id = GetNthIncomingStreamId(0); - EXPECT_TRUE( - stream_id_manager_.MaybeIncreaseLargestPeerStreamId(first_stream_id)); + EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId( + first_stream_id, nullptr)); // A bad stream ID results in a closed connection. - EXPECT_CALL(delegate_, OnStreamIdManagerError(QUIC_INVALID_STREAM_ID, _)); + std::string error_details; EXPECT_FALSE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId( - max_stream_id + kV99StreamIdIncrement)); + max_stream_id + kV99StreamIdIncrement, &error_details)); + EXPECT_EQ( + error_details, + quiche::QuicheStrCat("Stream id ", max_stream_id + kV99StreamIdIncrement, + " would exceed stream count limit 100")); } TEST_P(QuicStreamIdManagerTest, MaxStreamsWindow) { @@ -334,7 +343,8 @@ size_t old_available_incoming_streams = stream_id_manager_.available_incoming_streams(); while (stream_count) { - EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)); + EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id, + nullptr)); // This node should think that the peer believes it has one fewer // stream it can create. @@ -375,7 +385,8 @@ // SendMaxStreams(stream_id_manager_.incoming_actual_max_streams(), // IsUnidirectional())); EXPECT_CALL(delegate_, SendMaxStreams(_, IsUnidirectional())); - EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)); + EXPECT_TRUE( + stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id, nullptr)); stream_id_manager_.OnStreamClosed(stream_id); } @@ -388,14 +399,15 @@ EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0); stream_id_manager_.SetMaxOpenIncomingStreams(0); frame.stream_count = 0; - stream_id_manager_.OnStreamsBlockedFrame(frame); + std::string error_details; + EXPECT_TRUE(stream_id_manager_.OnStreamsBlockedFrame(frame, &error_details)); // 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())); stream_id_manager_.SetMaxOpenIncomingStreams(123); frame.stream_count = 0; - stream_id_manager_.OnStreamsBlockedFrame(frame); + EXPECT_TRUE(stream_id_manager_.OnStreamsBlockedFrame(frame, &error_details)); } // Test that a MAX_STREAMS frame is generated when half the stream ids become @@ -419,7 +431,8 @@ SendMaxStreams(first_advert + stream_id_manager_.max_streams_window(), IsUnidirectional())); while (i) { - EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(id)); + EXPECT_TRUE( + stream_id_manager_.MaybeIncreaseLargestPeerStreamId(id, nullptr)); stream_id_manager_.OnStreamClosed(id); i--; id += kV99StreamIdIncrement; @@ -445,8 +458,8 @@ } TEST_P(QuicStreamIdManagerTest, AvailableStreams) { - stream_id_manager_.MaybeIncreaseLargestPeerStreamId( - GetNthIncomingStreamId(3)); + stream_id_manager_.MaybeIncreaseLargestPeerStreamId(GetNthIncomingStreamId(3), + nullptr); EXPECT_TRUE(stream_id_manager_.IsAvailableStream(GetNthIncomingStreamId(1))); EXPECT_TRUE(stream_id_manager_.IsAvailableStream(GetNthIncomingStreamId(2))); @@ -460,13 +473,13 @@ TEST_P(QuicStreamIdManagerTest, ExtremeMaybeIncreaseLargestPeerStreamId) { QuicStreamId too_big_stream_id = GetNthIncomingStreamId( stream_id_manager_.incoming_actual_max_streams() + 20); - std::string error_details = quiche::QuicheStrCat( - "Stream id ", too_big_stream_id, " would exceed stream count limit 100"); - EXPECT_CALL(delegate_, - OnStreamIdManagerError(QUIC_INVALID_STREAM_ID, error_details)); - EXPECT_FALSE( - stream_id_manager_.MaybeIncreaseLargestPeerStreamId(too_big_stream_id)); + std::string error_details; + EXPECT_FALSE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId( + too_big_stream_id, &error_details)); + EXPECT_EQ(error_details, + quiche::QuicheStrCat("Stream id ", too_big_stream_id, + " would exceed stream count limit 100")); } } // namespace
diff --git a/quic/core/uber_quic_stream_id_manager.cc b/quic/core/uber_quic_stream_id_manager.cc index 7ef5688..daa5630 100644 --- a/quic/core/uber_quic_stream_id_manager.cc +++ b/quic/core/uber_quic_stream_id_manager.cc
@@ -67,12 +67,14 @@ } bool UberQuicStreamIdManager::MaybeIncreaseLargestPeerStreamId( - QuicStreamId id) { + QuicStreamId id, + std::string* error_details) { if (QuicUtils::IsBidirectionalStreamId(id)) { return bidirectional_stream_id_manager_.MaybeIncreaseLargestPeerStreamId( - id); + id, error_details); } - return unidirectional_stream_id_manager_.MaybeIncreaseLargestPeerStreamId(id); + return unidirectional_stream_id_manager_.MaybeIncreaseLargestPeerStreamId( + id, error_details); } void UberQuicStreamIdManager::OnStreamClosed(QuicStreamId id) { @@ -84,11 +86,14 @@ } bool UberQuicStreamIdManager::OnStreamsBlockedFrame( - const QuicStreamsBlockedFrame& frame) { + const QuicStreamsBlockedFrame& frame, + std::string* error_details) { if (frame.unidirectional) { - return unidirectional_stream_id_manager_.OnStreamsBlockedFrame(frame); + return unidirectional_stream_id_manager_.OnStreamsBlockedFrame( + frame, error_details); } - return bidirectional_stream_id_manager_.OnStreamsBlockedFrame(frame); + return bidirectional_stream_id_manager_.OnStreamsBlockedFrame(frame, + error_details); } bool UberQuicStreamIdManager::IsIncomingStream(QuicStreamId id) const {
diff --git a/quic/core/uber_quic_stream_id_manager.h b/quic/core/uber_quic_stream_id_manager.h index b505487..d3f374a 100644 --- a/quic/core/uber_quic_stream_id_manager.h +++ b/quic/core/uber_quic_stream_id_manager.h
@@ -56,13 +56,15 @@ QuicStreamId GetNextOutgoingUnidirectionalStreamId(); // Returns true if the incoming |id| is within the limit. - bool MaybeIncreaseLargestPeerStreamId(QuicStreamId id); + bool MaybeIncreaseLargestPeerStreamId(QuicStreamId id, + std::string* error_details); // Called when |id| is released. void OnStreamClosed(QuicStreamId id); // Called when a STREAMS_BLOCKED frame is received. - bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame); + bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame, + std::string* error_details); // Return true if |id| is peer initiated. bool IsIncomingStream(QuicStreamId id) const;
diff --git a/quic/core/uber_quic_stream_id_manager_test.cc b/quic/core/uber_quic_stream_id_manager_test.cc index 3522008..4fc1cca 100644 --- a/quic/core/uber_quic_stream_id_manager_test.cc +++ b/quic/core/uber_quic_stream_id_manager_test.cc
@@ -46,9 +46,6 @@ class MockDelegate : public QuicStreamIdManager::DelegateInterface { public: - MOCK_METHOD1(OnCanCreateNewOutgoingStream, void(bool unidirectional)); - MOCK_METHOD2(OnStreamIdManagerError, - void(QuicErrorCode error_code, std::string error_details)); MOCK_METHOD2(SendMaxStreams, void(QuicStreamCount stream_count, bool unidirectional)); }; @@ -191,21 +188,29 @@ size_t i; for (i = 0; i < kNumMaxIncomingStreams; i++) { EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId( - GetNthPeerInitiatedUnidirectionalStreamId(i))); + GetNthPeerInitiatedUnidirectionalStreamId(i), nullptr)); EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId( - GetNthPeerInitiatedBidirectionalStreamId(i))); + GetNthPeerInitiatedBidirectionalStreamId(i), nullptr)); } // Should be able to open the next bidirectional stream EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId( - GetNthPeerInitiatedBidirectionalStreamId(i))); + GetNthPeerInitiatedBidirectionalStreamId(i), nullptr)); // We should have exhausted the counts, the next streams should fail - EXPECT_CALL(delegate_, OnStreamIdManagerError(QUIC_INVALID_STREAM_ID, _)); + std::string error_details; EXPECT_FALSE(manager_.MaybeIncreaseLargestPeerStreamId( - GetNthPeerInitiatedUnidirectionalStreamId(i))); - EXPECT_CALL(delegate_, OnStreamIdManagerError(QUIC_INVALID_STREAM_ID, _)); + GetNthPeerInitiatedUnidirectionalStreamId(i), &error_details)); + EXPECT_EQ(error_details, + quiche::QuicheStrCat( + "Stream id ", GetNthPeerInitiatedUnidirectionalStreamId(i), + " would exceed stream count limit ", kNumMaxIncomingStreams)); EXPECT_FALSE(manager_.MaybeIncreaseLargestPeerStreamId( - GetNthPeerInitiatedBidirectionalStreamId(i + 1))); + GetNthPeerInitiatedBidirectionalStreamId(i + 1), &error_details)); + EXPECT_EQ( + error_details, + quiche::QuicheStrCat( + "Stream id ", GetNthPeerInitiatedBidirectionalStreamId(i + 1), + " would exceed stream count limit ", kNumMaxIncomingStreams + 1)); } TEST_P(UberQuicStreamIdManagerTest, GetNextOutgoingStreamId) { @@ -223,14 +228,14 @@ TEST_P(UberQuicStreamIdManagerTest, AvailableStreams) { EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId( - GetNthPeerInitiatedBidirectionalStreamId(3))); + GetNthPeerInitiatedBidirectionalStreamId(3), nullptr)); EXPECT_TRUE( manager_.IsAvailableStream(GetNthPeerInitiatedBidirectionalStreamId(1))); EXPECT_TRUE( manager_.IsAvailableStream(GetNthPeerInitiatedBidirectionalStreamId(2))); EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId( - GetNthPeerInitiatedUnidirectionalStreamId(3))); + GetNthPeerInitiatedUnidirectionalStreamId(3), nullptr)); EXPECT_TRUE( manager_.IsAvailableStream(GetNthPeerInitiatedUnidirectionalStreamId(1))); EXPECT_TRUE( @@ -238,32 +243,40 @@ } TEST_P(UberQuicStreamIdManagerTest, MaybeIncreaseLargestPeerStreamId) { - EXPECT_CALL(delegate_, OnStreamIdManagerError(_, _)).Times(0); - EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId(StreamCountToId( - manager_.max_incoming_bidirectional_streams(), - QuicUtils::InvertPerspective(perspective()), /* bidirectional=*/true))); - EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId(StreamCountToId( - manager_.max_incoming_bidirectional_streams(), - QuicUtils::InvertPerspective(perspective()), /* bidirectional=*/false))); + EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId( + StreamCountToId(manager_.max_incoming_bidirectional_streams(), + QuicUtils::InvertPerspective(perspective()), + /* bidirectional=*/true), + nullptr)); + EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId( + StreamCountToId(manager_.max_incoming_bidirectional_streams(), + QuicUtils::InvertPerspective(perspective()), + /* bidirectional=*/false), + nullptr)); - std::string error_details = + std::string expected_error_details = perspective() == Perspective::IS_SERVER ? "Stream id 400 would exceed stream count limit 100" : "Stream id 401 would exceed stream count limit 100"; + std::string error_details; - EXPECT_CALL(delegate_, - OnStreamIdManagerError(QUIC_INVALID_STREAM_ID, error_details)); - EXPECT_FALSE(manager_.MaybeIncreaseLargestPeerStreamId(StreamCountToId( - manager_.max_incoming_bidirectional_streams() + 1, - QuicUtils::InvertPerspective(perspective()), /* bidirectional=*/true))); - error_details = perspective() == Perspective::IS_SERVER - ? "Stream id 402 would exceed stream count limit 100" - : "Stream id 403 would exceed stream count limit 100"; - EXPECT_CALL(delegate_, - OnStreamIdManagerError(QUIC_INVALID_STREAM_ID, error_details)); - EXPECT_FALSE(manager_.MaybeIncreaseLargestPeerStreamId(StreamCountToId( - manager_.max_incoming_bidirectional_streams() + 1, - QuicUtils::InvertPerspective(perspective()), /* bidirectional=*/false))); + EXPECT_FALSE(manager_.MaybeIncreaseLargestPeerStreamId( + StreamCountToId(manager_.max_incoming_bidirectional_streams() + 1, + QuicUtils::InvertPerspective(perspective()), + /* bidirectional=*/true), + &error_details)); + EXPECT_EQ(expected_error_details, error_details); + expected_error_details = + perspective() == Perspective::IS_SERVER + ? "Stream id 402 would exceed stream count limit 100" + : "Stream id 403 would exceed stream count limit 100"; + + EXPECT_FALSE(manager_.MaybeIncreaseLargestPeerStreamId( + StreamCountToId(manager_.max_incoming_bidirectional_streams() + 1, + QuicUtils::InvertPerspective(perspective()), + /* bidirectional=*/false), + &error_details)); + EXPECT_EQ(expected_error_details, error_details); } TEST_P(UberQuicStreamIdManagerTest, OnStreamsBlockedFrame) { @@ -275,7 +288,7 @@ EXPECT_CALL(delegate_, SendMaxStreams(manager_.max_incoming_bidirectional_streams(), frame.unidirectional)); - manager_.OnStreamsBlockedFrame(frame); + EXPECT_TRUE(manager_.OnStreamsBlockedFrame(frame, nullptr)); stream_count = manager_.advertised_max_incoming_unidirectional_streams() - 1; frame.stream_count = stream_count; @@ -284,7 +297,7 @@ EXPECT_CALL(delegate_, SendMaxStreams(manager_.max_incoming_unidirectional_streams(), frame.unidirectional)); - manager_.OnStreamsBlockedFrame(frame); + EXPECT_TRUE(manager_.OnStreamsBlockedFrame(frame, nullptr)); } TEST_P(UberQuicStreamIdManagerTest, IsIncomingStream) {