Move handling of STOP_SENDING frame from QuicSession to QuicStream. Refactor only. not protected. PiperOrigin-RevId: 321656082 Change-Id: I430fc25ab69436c817d95c5ef6626e69ddb80dda
diff --git a/quic/core/http/quic_spdy_server_stream_base_test.cc b/quic/core/http/quic_spdy_server_stream_base_test.cc index b3cc147..a32de61 100644 --- a/quic/core/http/quic_spdy_server_stream_base_test.cc +++ b/quic/core/http/quic_spdy_server_stream_base_test.cc
@@ -50,7 +50,7 @@ TEST_F(QuicSpdyServerStreamBaseTest, SendQuicRstStreamNoErrorWithEarlyResponse) { stream_->StopReading(); - EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _)).Times(1); + EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _, _)).Times(1); QuicStreamPeer::SetFinSent(stream_); stream_->CloseWriteSide(); } @@ -59,22 +59,16 @@ DoNotSendQuicRstStreamNoErrorWithRstReceived) { EXPECT_FALSE(stream_->reading_stopped()); - EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _)).Times(0); + EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _, _)).Times(0); - if (!VersionHasIetfQuicFrames(session_.transport_version())) { - EXPECT_CALL(session_, SendRstStream(_, QUIC_RST_ACKNOWLEDGEMENT, _)) - .Times(1); - } else { - // Intercept & check that the call to the QuicConnection's OnStreamReast - // has correct stream ID and error code -- for V99/IETF Quic, it should - // have the STREAM_CANCELLED error code, not RST_ACK... Capture - // OnStreamReset (rather than SendRstStream) because the V99 path bypasses - // SendRstStream, calling SendRstStreamInner directly. Mocking - // SendRstStreamInner is problematic since the test relies on it to perform - // the closing operations and getting the stream in the correct state. - EXPECT_CALL(*(static_cast<MockQuicConnection*>(session_.connection())), - OnStreamReset(stream_->id(), QUIC_STREAM_CANCELLED)); - } + EXPECT_CALL( + session_, + SendRstStream(_, + VersionHasIetfQuicFrames(session_.transport_version()) + ? QUIC_STREAM_CANCELLED + : QUIC_RST_ACKNOWLEDGEMENT, + _, _)) + .Times(1); QuicRstStreamFrame rst_frame(kInvalidControlFrameId, stream_->id(), QUIC_STREAM_CANCELLED, 1234); stream_->OnStreamReset(rst_frame);
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc index 63df00e..983ae8c 100644 --- a/quic/core/http/quic_spdy_stream_test.cc +++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -505,7 +505,7 @@ spdy::SpdyStreamPrecedence(kV3HighestPriority)); EXPECT_CALL(*session_, - SendRstStream(stream_->id(), QUIC_HEADERS_TOO_LARGE, 0)); + SendRstStream(stream_->id(), QUIC_HEADERS_TOO_LARGE, 0, _)); stream_->OnStreamHeaderList(false, 1 << 20, headers); EXPECT_THAT(stream_->stream_error(), IsStreamError(QUIC_HEADERS_TOO_LARGE)); @@ -521,7 +521,7 @@ QuicStreamFrame frame(stream_->id(), false, 0, headers); EXPECT_CALL(*session_, - SendRstStream(stream_->id(), QUIC_HEADERS_TOO_LARGE, 0)); + SendRstStream(stream_->id(), QUIC_HEADERS_TOO_LARGE, 0, _)); auto qpack_decoder_stream = QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get()); @@ -657,8 +657,8 @@ ConnectionCloseSource source) { session_->ReallyOnConnectionClosed(frame, source); })); - EXPECT_CALL(*session_, SendRstStream(_, _, _)); - EXPECT_CALL(*session_, SendRstStream(_, _, _)); + EXPECT_CALL(*session_, SendRstStream(_, _, _, _)); + EXPECT_CALL(*session_, SendRstStream(_, _, _, _)); stream_->OnStreamFrame(frame); } @@ -2124,8 +2124,8 @@ ConnectionCloseSource source) { session_->ReallyOnConnectionClosed(frame, source); })); - EXPECT_CALL(*session_, SendRstStream(_, _, _)); - EXPECT_CALL(*session_, SendRstStream(_, _, _)); + EXPECT_CALL(*session_, SendRstStream(_, _, _, _)); + EXPECT_CALL(*session_, SendRstStream(_, _, _, _)); stream_->OnStreamFrame(frame); } @@ -2163,8 +2163,8 @@ session_->ReallyOnConnectionClosed(frame, source); })); } - EXPECT_CALL(*session_, SendRstStream(stream_->id(), _, _)); - EXPECT_CALL(*session_, SendRstStream(stream2_->id(), _, _)); + EXPECT_CALL(*session_, SendRstStream(stream_->id(), _, _, _)); + EXPECT_CALL(*session_, SendRstStream(stream2_->id(), _, _, _)); // Invalid headers: Required Insert Count is zero, but the header block // contains a dynamic table reference. @@ -2893,7 +2893,7 @@ WritevData(qpack_decoder_stream->id(), /* write_length = */ 1, /* offset = */ 1, _, _, _)); EXPECT_CALL(*session_, - SendRstStream(stream_->id(), QUIC_STREAM_CANCELLED, 0)); + SendRstStream(stream_->id(), QUIC_STREAM_CANCELLED, 0, _)); stream_->Reset(QUIC_STREAM_CANCELLED); }
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 0a9f872..5bfc517 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -256,22 +256,7 @@ return; } - if (!stream->OnStopSending(frame.application_error_code)) { - return; - } - - // TODO(renjietang): Consider moving those code into the stream. - if (connection()->connected()) { - MaybeSendRstStreamFrame( - stream->id(), - static_cast<quic::QuicRstStreamErrorCode>(frame.application_error_code), - stream->stream_bytes_written()); - connection_->OnStreamReset(stream->id(), - static_cast<quic::QuicRstStreamErrorCode>( - frame.application_error_code)); - } - stream->set_rst_sent(true); - stream->CloseWriteSide(); + stream->OnStopSending(frame.application_error_code); } void QuicSession::OnPacketDecrypted(EncryptionLevel level) { @@ -759,11 +744,14 @@ void QuicSession::SendRstStream(QuicStreamId id, QuicRstStreamErrorCode error, - QuicStreamOffset bytes_written) { + QuicStreamOffset bytes_written, + bool send_rst_only) { if (connection()->connected()) { QuicConnection::ScopedPacketFlusher flusher(connection()); MaybeSendRstStreamFrame(id, error, bytes_written); - MaybeSendStopSendingFrame(id, error); + if (!send_rst_only) { + MaybeSendStopSendingFrame(id, error); + } connection_->OnStreamReset(id, error); } @@ -783,7 +771,7 @@ return; } - SendRstStream(id, error, 0); + SendRstStream(id, error, 0, /*send_rst_only = */ false); } void QuicSession::MaybeSendRstStreamFrame(QuicStreamId id,
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index 88e4412..2d104a9 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -210,10 +210,12 @@ bool WriteControlFrame(const QuicFrame& frame, TransmissionType type) override; - // Called by stream to send RST_STREAM (and STOP_SENDING). + // Called by stream to send RST_STREAM (and STOP_SENDING in IETF QUIC). + // if |send_rst_only|, STOP_SENDING will not be sent for IETF QUIC. virtual void SendRstStream(QuicStreamId id, QuicRstStreamErrorCode error, - QuicStreamOffset bytes_written); + QuicStreamOffset bytes_written, + bool send_rst_only); // Called to send RST_STREAM (and STOP_SENDING) and close stream. If stream // |id| does not exist, just send RST_STREAM (and STOP_SENDING).
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index 63a2bb1..2d8cb6e 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -2961,21 +2961,24 @@ .Times(1) .WillOnce(Invoke(&ClearControlFrame)); EXPECT_CALL(*connection_, OnStreamReset(read_only, _)); - session_.SendRstStream(read_only, QUIC_STREAM_CANCELLED, 0); + session_.SendRstStream(read_only, QUIC_STREAM_CANCELLED, 0, + /*send_rst_only = */ false); QuicStreamId write_only = GetNthServerInitiatedUnidirectionalId(0); EXPECT_CALL(*connection_, SendControlFrame(_)) .Times(1) .WillOnce(Invoke(&ClearControlFrame)); EXPECT_CALL(*connection_, OnStreamReset(write_only, _)); - session_.SendRstStream(write_only, QUIC_STREAM_CANCELLED, 0); + session_.SendRstStream(write_only, QUIC_STREAM_CANCELLED, 0, + /*send_rst_only = */ false); QuicStreamId bidirectional = GetNthClientInitiatedBidirectionalId(0); EXPECT_CALL(*connection_, SendControlFrame(_)) .Times(2) .WillRepeatedly(Invoke(&ClearControlFrame)); EXPECT_CALL(*connection_, OnStreamReset(bidirectional, _)); - session_.SendRstStream(bidirectional, QUIC_STREAM_CANCELLED, 0); + session_.SendRstStream(bidirectional, QUIC_STREAM_CANCELLED, 0, + /*send_rst_only = */ false); } TEST_P(QuicSessionTestServer, DecryptionKeyAvailableBeforeEncryptionKey) {
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc index 6318ac1..9e695d4 100644 --- a/quic/core/quic_stream.cc +++ b/quic/core/quic_stream.cc
@@ -494,6 +494,12 @@ } stream_error_ = static_cast<QuicRstStreamErrorCode>(code); + + session()->SendRstStream(id(), + static_cast<quic::QuicRstStreamErrorCode>(code), + stream_bytes_written(), /*send_rst_only = */ true); + rst_sent_ = true; + CloseWriteSide(); return true; } @@ -579,7 +585,8 @@ void QuicStream::Reset(QuicRstStreamErrorCode error) { stream_error_ = error; - session()->SendRstStream(id(), error, stream_bytes_written()); + session()->SendRstStream(id(), error, stream_bytes_written(), + /*send_rst_only = */ false); rst_sent_ = true; if (read_side_closed_ && write_side_closed_ && !IsWaitingForAcks()) { session()->OnStreamDoneWaitingForAcks(id_); @@ -824,7 +831,7 @@ // RST_STREAM frame. QUIC_DLOG(INFO) << ENDPOINT << "Sending RST_STREAM in OnClose: " << id(); session_->SendRstStream(id(), QUIC_RST_ACKNOWLEDGEMENT, - stream_bytes_written()); + stream_bytes_written(), /*send_rst_only = */ false); session_->OnStreamDoneWaitingForAcks(id_); rst_sent_ = true; }
diff --git a/quic/core/quic_stream.h b/quic/core/quic_stream.h index 7a847f0..41484a3 100644 --- a/quic/core/quic_stream.h +++ b/quic/core/quic_stream.h
@@ -218,8 +218,6 @@ size_t busy_counter() const { return busy_counter_; } void set_busy_counter(size_t busy_counter) { busy_counter_ = busy_counter; } - void set_rst_sent(bool rst_sent) { rst_sent_ = rst_sent; } - void set_rst_received(bool rst_received) { rst_received_ = rst_received; } void set_stream_error(QuicRstStreamErrorCode error) { stream_error_ = error; }
diff --git a/quic/core/quic_stream_test.cc b/quic/core/quic_stream_test.cc index da709d4..1c76eb9 100644 --- a/quic/core/quic_stream_test.cc +++ b/quic/core/quic_stream_test.cc
@@ -109,7 +109,7 @@ // session_ now owns stream_. session_->ActivateStream(QuicWrapUnique(stream_)); // Ignore resetting when session_ is terminated. - EXPECT_CALL(*session_, SendRstStream(kTestStreamId, _, _)) + EXPECT_CALL(*session_, SendRstStream(kTestStreamId, _, _, _)) .Times(AnyNumber()); write_blocked_list_ = QuicSessionPeer::GetWriteBlockedStreams(session_.get()); @@ -456,7 +456,7 @@ EXPECT_FALSE(rst_sent()); // Now close the stream, and expect that we send a RST. - EXPECT_CALL(*session_, SendRstStream(_, _, _)); + EXPECT_CALL(*session_, SendRstStream(_, _, _, _)); stream_->CloseReadSide(); stream_->CloseWriteSide(); EXPECT_FALSE(session_->HasUnackedStreamData()); @@ -503,7 +503,7 @@ // Reset the stream. const int expected_resets = 1; - EXPECT_CALL(*session_, SendRstStream(_, _, _)).Times(expected_resets); + EXPECT_CALL(*session_, SendRstStream(_, _, _, _)).Times(expected_resets); stream_->Reset(QUIC_STREAM_CANCELLED); EXPECT_FALSE(fin_sent()); EXPECT_TRUE(rst_sent()); @@ -932,10 +932,11 @@ EXPECT_CALL(*connection_, SendControlFrame(_)) .Times(AtLeast(1)) .WillRepeatedly(Invoke(&ClearControlFrame)); - EXPECT_CALL(*session_, SendRstStream(stream_->id(), QUIC_STREAM_CANCELLED, 9)) + EXPECT_CALL(*session_, + SendRstStream(stream_->id(), QUIC_STREAM_CANCELLED, 9, _)) .WillOnce(InvokeWithoutArgs([this]() { session_->ReallySendRstStream(stream_->id(), QUIC_STREAM_CANCELLED, - stream_->stream_bytes_written()); + stream_->stream_bytes_written(), false); })); stream_->Reset(QUIC_STREAM_CANCELLED); @@ -968,7 +969,7 @@ QuicRstStreamFrame rst_frame(kInvalidControlFrameId, stream_->id(), QUIC_STREAM_CANCELLED, 9); EXPECT_CALL(*session_, - SendRstStream(stream_->id(), QUIC_RST_ACKNOWLEDGEMENT, 9)); + SendRstStream(stream_->id(), QUIC_RST_ACKNOWLEDGEMENT, 9, _)); stream_->OnStreamReset(rst_frame); EXPECT_EQ(1u, QuicStreamPeer::SendBuffer(stream_).size()); // Stream stops waiting for acks as it does not finish sending and rst is @@ -990,7 +991,7 @@ EXPECT_TRUE(session_->HasUnackedStreamData()); // RST_STREAM received. - EXPECT_CALL(*session_, SendRstStream(_, _, _)).Times(0); + EXPECT_CALL(*session_, SendRstStream(_, _, _, _)).Times(0); QuicRstStreamFrame rst_frame(kInvalidControlFrameId, stream_->id(), QUIC_STREAM_CANCELLED, 1234); stream_->OnStreamReset(rst_frame); @@ -1012,7 +1013,7 @@ EXPECT_TRUE(stream_->IsWaitingForAcks()); EXPECT_TRUE(session_->HasUnackedStreamData()); EXPECT_CALL(*session_, - SendRstStream(stream_->id(), QUIC_RST_ACKNOWLEDGEMENT, 9)); + SendRstStream(stream_->id(), QUIC_RST_ACKNOWLEDGEMENT, 9, _)); stream_->OnConnectionClosed(QUIC_INTERNAL_ERROR, ConnectionCloseSource::FROM_SELF); EXPECT_EQ(1u, QuicStreamPeer::SendBuffer(stream_).size()); @@ -1536,7 +1537,8 @@ connection_->AdvanceTime(QuicTime::Delta::FromSeconds(1)); // Verify stream gets reset because TTL expires. - EXPECT_CALL(*session_, SendRstStream(_, QUIC_STREAM_TTL_EXPIRED, _)).Times(1); + EXPECT_CALL(*session_, SendRstStream(_, QUIC_STREAM_TTL_EXPIRED, _, _)) + .Times(1); stream_->OnCanWrite(); } @@ -1554,7 +1556,8 @@ connection_->AdvanceTime(QuicTime::Delta::FromSeconds(1)); // Verify stream gets reset because TTL expires. - EXPECT_CALL(*session_, SendRstStream(_, QUIC_STREAM_TTL_EXPIRED, _)).Times(1); + EXPECT_CALL(*session_, SendRstStream(_, QUIC_STREAM_TTL_EXPIRED, _, _)) + .Times(1); stream_->RetransmitStreamData(0, 100, false, PTO_RETRANSMISSION); }
diff --git a/quic/qbone/qbone_stream_test.cc b/quic/qbone/qbone_stream_test.cc index 976847d..cfe9645 100644 --- a/quic/qbone/qbone_stream_test.cc +++ b/quic/qbone/qbone_stream_test.cc
@@ -64,7 +64,7 @@ // Called by QuicStream when they want to close stream. MOCK_METHOD(void, SendRstStream, - (QuicStreamId, QuicRstStreamErrorCode, QuicStreamOffset), + (QuicStreamId, QuicRstStreamErrorCode, QuicStreamOffset, bool), (override)); // Sets whether data is written to buffer, or else if this is write blocked. @@ -244,7 +244,7 @@ std::string packet = "0123456789"; int iterations = (QboneConstants::kMaxQbonePacketBytes / packet.size()) + 2; EXPECT_CALL(*session_, - SendRstStream(kStreamId, QUIC_BAD_APPLICATION_PAYLOAD, _)); + SendRstStream(kStreamId, QUIC_BAD_APPLICATION_PAYLOAD, _, _)); for (int i = 0; i < iterations; ++i) { QuicStreamFrame frame(kStreamId, i == (iterations - 1), i * packet.size(), packet);
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index 27799bb..24af0fc 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -806,7 +806,8 @@ SendRstStream, (QuicStreamId stream_id, QuicRstStreamErrorCode error, - QuicStreamOffset bytes_written), + QuicStreamOffset bytes_written, + bool send_rst_only), (override)); MOCK_METHOD(bool, ShouldKeepConnectionAlive, (), (const, override)); @@ -834,8 +835,9 @@ void ReallySendRstStream(QuicStreamId id, QuicRstStreamErrorCode error, - QuicStreamOffset bytes_written) { - QuicSession::SendRstStream(id, error, bytes_written); + QuicStreamOffset bytes_written, + bool send_rst_only) { + QuicSession::SendRstStream(id, error, bytes_written, send_rst_only); } private: @@ -924,7 +926,8 @@ SendRstStream, (QuicStreamId stream_id, QuicRstStreamErrorCode error, - QuicStreamOffset bytes_written), + QuicStreamOffset bytes_written, + bool send_rst_only), (override)); MOCK_METHOD(void, SendWindowUpdate,
diff --git a/quic/tools/quic_simple_server_stream_test.cc b/quic/tools/quic_simple_server_stream_test.cc index 5c84084..f5c1b53 100644 --- a/quic/tools/quic_simple_server_stream_test.cc +++ b/quic/tools/quic_simple_server_stream_test.cc
@@ -155,7 +155,8 @@ SendRstStream, (QuicStreamId stream_id, QuicRstStreamErrorCode error, - QuicStreamOffset bytes_written), + QuicStreamOffset bytes_written, + bool send_rst_only), (override)); // Matchers cannot be used on non-copyable types like SpdyHeaderBlock. void PromisePushResources( @@ -341,7 +342,7 @@ QuicStreamPeer::SetFinSent(stream_); stream_->CloseWriteSide(); - EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _)).Times(1); + EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _, _)).Times(1); stream_->StopReading(); } @@ -357,7 +358,7 @@ } EXPECT_CALL(session_, WritevData(_, kErrorLength, _, FIN, _, _)); - EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _)).Times(0); + EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _, _)).Times(0); stream_->OnStreamHeaderList(false, kFakeFrameLen, header_list_); std::unique_ptr<char[]> buffer; @@ -469,8 +470,8 @@ std::move(response_headers_), body); InSequence s; - EXPECT_CALL(session_, - SendRstStream(promised_stream->id(), QUIC_STREAM_CANCELLED, 0)); + EXPECT_CALL(session_, SendRstStream(promised_stream->id(), + QUIC_STREAM_CANCELLED, 0, _)); promised_stream->DoSendResponse(); } @@ -606,7 +607,7 @@ } TEST_P(QuicSimpleServerStreamTest, TestSendErrorResponse) { - EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _)).Times(0); + EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _, _)).Times(0); QuicStreamPeer::SetFinReceived(stream_); @@ -624,7 +625,7 @@ } TEST_P(QuicSimpleServerStreamTest, InvalidMultipleContentLength) { - EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _)).Times(0); + EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _, _)).Times(0); spdy::SpdyHeaderBlock request_headers; // \000 is a way to write the null byte when followed by a literal digit. @@ -643,7 +644,7 @@ } TEST_P(QuicSimpleServerStreamTest, InvalidLeadingNullContentLength) { - EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _)).Times(0); + EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _, _)).Times(0); spdy::SpdyHeaderBlock request_headers; // \000 is a way to write the null byte when followed by a literal digit. @@ -680,7 +681,7 @@ InSequence s; EXPECT_FALSE(stream_->reading_stopped()); - EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _)).Times(0); + EXPECT_CALL(session_, SendRstStream(_, QUIC_STREAM_NO_ERROR, _, _)).Times(0); if (VersionUsesHttp3(connection_->transport_version())) { // Unidirectional stream type and then a Stream Cancellation instruction is // sent on the QPACK decoder stream. Ignore these writes without any @@ -690,7 +691,8 @@ EXPECT_CALL(session_, WritevData(qpack_decoder_stream->id(), _, _, _, _, _)) .Times(AnyNumber()); } - EXPECT_CALL(session_, SendRstStream(_, QUIC_RST_ACKNOWLEDGEMENT, _)).Times(1); + EXPECT_CALL(session_, SendRstStream(_, QUIC_RST_ACKNOWLEDGEMENT, _, _)) + .Times(1); QuicRstStreamFrame rst_frame(kInvalidControlFrameId, stream_->id(), QUIC_STREAM_CANCELLED, 1234); stream_->OnStreamReset(rst_frame);