Remove unused return value for QuicSession::OnStopSendingFrame(). gfe-relnote: protected by disabled v99 flag. PiperOrigin-RevId: 277115916 Change-Id: I15b4a9ce1b5316d723e097f6aa8d590b2955e4e0
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index bc97119..f618ddf 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -165,7 +165,7 @@ virtual void OnForwardProgressConfirmed() = 0; // Called when a STOP_SENDING frame has been received. - virtual bool OnStopSendingFrame(const QuicStopSendingFrame& frame) = 0; + virtual void OnStopSendingFrame(const QuicStopSendingFrame& frame) = 0; }; // Interface which gets callbacks from the QuicConnection at interesting
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index b9cef8d..09e5c74 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -199,11 +199,8 @@ GetMutableCryptoStream()->OnCryptoFrame(frame); } -bool QuicSession::OnStopSendingFrame(const QuicStopSendingFrame& frame) { - // We are not version 99. In theory, if not in version 99 then the framer - // could not call OnStopSending... This is just a check that is good when - // both a new protocol and a new implementation of that protocol are both - // being developed. +void QuicSession::OnStopSendingFrame(const QuicStopSendingFrame& frame) { + // STOP_SENDING is in IETF QUIC only. DCHECK(VersionHasIetfQuicFrames(transport_version())); QuicStreamId stream_id = frame.stream_id; @@ -215,21 +212,7 @@ connection()->CloseConnection( QUIC_INVALID_STREAM_ID, "Received STOP_SENDING for an invalid stream", ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); - return false; - } - - // Ignore STOP_SENDING for static streams. - // TODO(fkastenholz): IETF Quic does not have static streams and does not - // make exceptions for them with respect to processing things like - // STOP_SENDING. - if (QuicUtils::IsCryptoStreamId(transport_version(), stream_id)) { - QUIC_DVLOG(1) << ENDPOINT - << "Received STOP_SENDING for a static stream, id: " - << stream_id << " Closing connection"; - connection()->CloseConnection( - QUIC_INVALID_STREAM_ID, "Received STOP_SENDING for a static stream", - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); - return false; + return; } if (visitor_) { @@ -242,7 +225,7 @@ << ENDPOINT << "Received STOP_SENDING for closed or non-existent stream, id: " << stream_id << " Ignoring."; - return true; // Continue processing the packet. + return; } // If stream is non-existent, close the connection StreamMap::iterator it = stream_map_.find(stream_id); @@ -254,7 +237,7 @@ IETF_QUIC_PROTOCOL_VIOLATION, "Received STOP_SENDING for a non-existent stream", ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); - return false; + return; } // Get the QuicStream for this stream. Ignore the STOP_SENDING @@ -267,7 +250,7 @@ QUIC_BUG << ENDPOINT << "Received STOP_SENDING for NULL QuicStream, stream_id: " << stream_id << ". Ignoring."; - return true; + return; } if (stream->is_static()) { @@ -277,7 +260,7 @@ connection()->CloseConnection( QUIC_INVALID_STREAM_ID, "Received STOP_SENDING for a static stream", ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); - return false; + return; } stream->OnStopSending(frame.application_error_code); @@ -289,8 +272,6 @@ static_cast<quic::QuicRstStreamErrorCode>(frame.application_error_code), stream->stream_bytes_written(), /*close_write_side_only=*/true); - - return true; } void QuicSession::PendingStreamOnRstStream(const QuicRstStreamFrame& frame) {
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index fba1e63..6a6a4b2 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -126,7 +126,7 @@ void OnForwardProgressConfirmed() override; bool OnMaxStreamsFrame(const QuicMaxStreamsFrame& frame) override; bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame) override; - bool OnStopSendingFrame(const QuicStopSendingFrame& frame) override; + void OnStopSendingFrame(const QuicStopSendingFrame& frame) override; // QuicStreamFrameDataProducer WriteStreamDataResult WriteStreamData(QuicStreamId id,
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index f99c4a5..e13f0db 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -1603,7 +1603,8 @@ // stream and therefore fulfill all of the expects. QuicStopSendingFrame frame(kInvalidControlFrameId, stream->id(), QUIC_STREAM_CANCELLED); - EXPECT_TRUE(session_.OnStopSendingFrame(frame)); + EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0); + session_.OnStopSendingFrame(frame); } EXPECT_EQ(kByteOffset, session_.flow_controller()->bytes_consumed()); } @@ -2114,7 +2115,8 @@ // stream and therefore fulfill all of the expects. QuicStopSendingFrame frame(kInvalidControlFrameId, stream2->id(), QUIC_STREAM_CANCELLED); - EXPECT_TRUE(session_.OnStopSendingFrame(frame)); + EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0); + session_.OnStopSendingFrame(frame); } EXPECT_FALSE(QuicContainsKey(session_.zombie_streams(), stream2->id())); ASSERT_EQ(1u, session_.closed_streams()->size()); @@ -2621,7 +2623,7 @@ *connection_, CloseConnection(QUIC_INVALID_STREAM_ID, "Received STOP_SENDING for an invalid stream", _)); - EXPECT_FALSE(session_.OnStopSendingFrame(frame)); + session_.OnStopSendingFrame(frame); } // Second test, streams in the static stream map are not subject to @@ -2641,7 +2643,7 @@ EXPECT_CALL(*connection_, CloseConnection(QUIC_INVALID_STREAM_ID, "Received STOP_SENDING for a static stream", _)); - EXPECT_FALSE(session_.OnStopSendingFrame(frame)); + session_.OnStopSendingFrame(frame); } // Third test, if stream id specifies a closed stream: @@ -2661,7 +2663,7 @@ stream->CloseReadSide(); QuicStopSendingFrame frame(1, stream_id, 123); EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0); - EXPECT_TRUE(session_.OnStopSendingFrame(frame)); + session_.OnStopSendingFrame(frame); } // Fourth test, if stream id specifies a nonexistent stream, return false and @@ -2679,7 +2681,7 @@ CloseConnection(IETF_QUIC_PROTOCOL_VIOLATION, "Received STOP_SENDING for a non-existent stream", _)) .Times(1); - EXPECT_FALSE(session_.OnStopSendingFrame(frame)); + session_.OnStopSendingFrame(frame); } // For a valid stream, ensure that all works @@ -2703,7 +2705,8 @@ EXPECT_CALL( *connection_, OnStreamReset(stream_id, static_cast<QuicRstStreamErrorCode>(123))); - EXPECT_TRUE(session_.OnStopSendingFrame(frame)); + EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0); + session_.OnStopSendingFrame(frame); // When the STOP_SENDING is received, the node generates a RST_STREAM, // which closes the stream in the write direction. Ensure this. EXPECT_FALSE(QuicStreamPeer::read_side_closed(stream));
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index e39b416..1bcfa6a 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -412,7 +412,7 @@ MOCK_METHOD1(OnMaxStreamsFrame, bool(const QuicMaxStreamsFrame& frame)); MOCK_METHOD1(OnStreamsBlockedFrame, bool(const QuicStreamsBlockedFrame& frame)); - MOCK_METHOD1(OnStopSendingFrame, bool(const QuicStopSendingFrame& frame)); + MOCK_METHOD1(OnStopSendingFrame, void(const QuicStopSendingFrame& frame)); }; class MockQuicConnectionHelper : public QuicConnectionHelperInterface {
diff --git a/quic/test_tools/simulator/quic_endpoint.h b/quic/test_tools/simulator/quic_endpoint.h index 0b9b54a..d1291f8 100644 --- a/quic/test_tools/simulator/quic_endpoint.h +++ b/quic/test_tools/simulator/quic_endpoint.h
@@ -81,9 +81,7 @@ const QuicStreamsBlockedFrame& /*frame*/) override { return true; } - bool OnStopSendingFrame(const QuicStopSendingFrame& /*frame*/) override { - return true; - } + void OnStopSendingFrame(const QuicStopSendingFrame& /*frame*/) override {} // End QuicConnectionVisitorInterface implementation.