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