Remove QuicSession::SendRstStreamInner() because the |close_write_side_only| code paths share so little with each other.
gfe-relnote: protected by gfe2_reloadable_flag_quic_delete_send_rst_stream_inner
PiperOrigin-RevId: 282598706
Change-Id: I2a9df3f32f9ce8670d0f829238e0d8aa6f93dbd0
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 2870b37..9b53845 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -248,11 +248,6 @@
return;
}
- // Get the QuicStream for this stream. Ignore the STOP_SENDING
- // if the QuicStream pointer is NULL
- // QUESTION(fkastenholz): IS THIS THE RIGHT THING TO DO? (that is, this would
- // happen IFF there was an entry in the map, but the pointer is null. sounds
- // more like a deep programming error rather than a simple protocol problem).
QuicStream* stream = it->second.get();
if (stream == nullptr) {
QUIC_BUG << ENDPOINT
@@ -275,11 +270,15 @@
stream->set_stream_error(
static_cast<QuicRstStreamErrorCode>(frame.application_error_code));
- SendRstStreamInner(
- stream->id(),
- static_cast<quic::QuicRstStreamErrorCode>(frame.application_error_code),
- stream->stream_bytes_written(),
- /*close_write_side_only=*/true);
+ 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();
}
@@ -701,7 +700,24 @@
void QuicSession::SendRstStream(QuicStreamId id,
QuicRstStreamErrorCode error,
QuicStreamOffset bytes_written) {
- SendRstStreamInner(id, error, bytes_written, /*close_write_side_only=*/false);
+ if (!GetQuicReloadableFlag(quic_delete_send_rst_stream_inner)) {
+ SendRstStreamInner(id, error, bytes_written, false);
+ return;
+ }
+ QUIC_RELOADABLE_FLAG_COUNT(quic_delete_send_rst_stream_inner);
+ if (connection()->connected()) {
+ QuicConnection::ScopedPacketFlusher flusher(connection());
+ MaybeSendRstStreamFrame(id, error, bytes_written);
+ MaybeSendStopSendingFrame(id, error);
+
+ connection_->OnStreamReset(id, error);
+ }
+
+ if (error != QUIC_STREAM_NO_ERROR && QuicContainsKey(zombie_streams_, id)) {
+ OnStreamDoneWaitingForAcks(id);
+ return;
+ }
+ CloseStreamInner(id, true);
}
void QuicSession::SendRstStreamInner(QuicStreamId id,
@@ -740,6 +756,27 @@
}
}
+void QuicSession::MaybeSendRstStreamFrame(QuicStreamId id,
+ QuicRstStreamErrorCode error,
+ QuicStreamOffset bytes_written) {
+ DCHECK(connection()->connected());
+ if (!VersionHasIetfQuicFrames(transport_version()) ||
+ QuicUtils::GetStreamType(id, perspective(), IsIncomingStream(id)) !=
+ READ_UNIDIRECTIONAL) {
+ control_frame_manager_.WriteOrBufferRstStream(id, error, bytes_written);
+ }
+}
+
+void QuicSession::MaybeSendStopSendingFrame(QuicStreamId id,
+ QuicRstStreamErrorCode error) {
+ DCHECK(connection()->connected());
+ if (VersionHasIetfQuicFrames(transport_version()) &&
+ QuicUtils::GetStreamType(id, perspective(), IsIncomingStream(id)) !=
+ WRITE_UNIDIRECTIONAL) {
+ control_frame_manager_.WriteOrBufferStopSending(error, id);
+ }
+}
+
void QuicSession::SendGoAway(QuicErrorCode error_code,
const std::string& reason) {
// GOAWAY frame is not supported in v99.
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index 2d0f252..8ceba0f 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -671,6 +671,14 @@
// stream.
void PendingStreamOnRstStream(const QuicRstStreamFrame& frame);
+ // Does actual work of sending RESET_STREAM, if the stream type allows.
+ void MaybeSendRstStreamFrame(QuicStreamId id,
+ QuicRstStreamErrorCode error,
+ QuicStreamOffset bytes_written);
+
+ // Sends a STOP_SENDING frame if the stream type allows.
+ void MaybeSendStopSendingFrame(QuicStreamId id, QuicRstStreamErrorCode error);
+
// Keep track of highest received byte offset of locally closed streams, while
// waiting for a definitive final highest offset from the peer.
std::map<QuicStreamId, QuicStreamOffset>
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index d3df0a7..6334f66 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -2768,33 +2768,26 @@
}
QuicStreamId read_only = GetNthClientInitiatedUnidirectionalId(0);
- EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0);
- EXPECT_CALL(*connection_, OnStreamReset(read_only, _));
- session_.SendRstStreamInner(read_only, QUIC_STREAM_CANCELLED, 0,
- /*close_write_side_only = */ true);
EXPECT_CALL(*connection_, SendControlFrame(_))
.Times(1)
.WillOnce(Invoke(&ClearControlFrame));
EXPECT_CALL(*connection_, OnStreamReset(read_only, _));
- session_.SendRstStreamInner(read_only, QUIC_STREAM_CANCELLED, 0,
- /*close_write_side_only = */ false);
+ session_.SendRstStream(read_only, QUIC_STREAM_CANCELLED, 0);
QuicStreamId write_only = GetNthServerInitiatedUnidirectionalId(0);
EXPECT_CALL(*connection_, SendControlFrame(_))
.Times(1)
.WillOnce(Invoke(&ClearControlFrame));
EXPECT_CALL(*connection_, OnStreamReset(write_only, _));
- session_.SendRstStreamInner(write_only, QUIC_STREAM_CANCELLED, 0,
- /*close_write_side_only = */ false);
+ session_.SendRstStream(write_only, QUIC_STREAM_CANCELLED, 0);
QuicStreamId bidirectional = GetNthClientInitiatedBidirectionalId(0);
EXPECT_CALL(*connection_, SendControlFrame(_))
.Times(2)
.WillRepeatedly(Invoke(&ClearControlFrame));
EXPECT_CALL(*connection_, OnStreamReset(bidirectional, _));
- session_.SendRstStreamInner(bidirectional, QUIC_STREAM_CANCELLED, 0,
- /*close_write_side_only = */ false);
+ session_.SendRstStream(bidirectional, QUIC_STREAM_CANCELLED, 0);
}
// A client test class that can be used when the automatic configuration is not
diff --git a/quic/core/quic_stream_test.cc b/quic/core/quic_stream_test.cc
index ba21608..673c7a7 100644
--- a/quic/core/quic_stream_test.cc
+++ b/quic/core/quic_stream_test.cc
@@ -912,10 +912,8 @@
.WillRepeatedly(Invoke(&ClearControlFrame));
EXPECT_CALL(*session_, SendRstStream(stream_->id(), QUIC_STREAM_CANCELLED, 9))
.WillOnce(InvokeWithoutArgs([this]() {
- return QuicSessionPeer::SendRstStreamInner(
- session_.get(), stream_->id(), QUIC_STREAM_CANCELLED,
- stream_->stream_bytes_written(),
- /*close_write_side_only=*/false);
+ session_->ReallySendRstStream(stream_->id(), QUIC_STREAM_CANCELLED,
+ stream_->stream_bytes_written());
}));
stream_->Reset(QUIC_STREAM_CANCELLED);
@@ -1574,25 +1572,6 @@
}
}
-// SendOnlyRstStream must only send a RESET_STREAM (no bundled STOP_SENDING).
-TEST_P(QuicStreamTest, SendOnlyRstStream) {
- Initialize();
- if (!VersionHasIetfQuicFrames(connection_->transport_version())) {
- return;
- }
-
- EXPECT_CALL(*connection_,
- OnStreamReset(stream_->id(), QUIC_BAD_APPLICATION_PAYLOAD));
- EXPECT_CALL(*connection_, SendControlFrame(_))
- .Times(1)
- .WillOnce(Invoke(this, &QuicStreamTest::ClearResetStreamFrame));
-
- QuicSessionPeer::SendRstStreamInner(session_.get(), stream_->id(),
- QUIC_BAD_APPLICATION_PAYLOAD,
- stream_->stream_bytes_written(),
- /*close_write_side_only=*/true);
-}
-
TEST_P(QuicStreamTest, WindowUpdateForReadOnlyStream) {
Initialize();
diff --git a/quic/test_tools/quic_session_peer.cc b/quic/test_tools/quic_session_peer.cc
index 001f1ca..f9e785d 100644
--- a/quic/test_tools/quic_session_peer.cc
+++ b/quic/test_tools/quic_session_peer.cc
@@ -226,15 +226,6 @@
}
// static
-void QuicSessionPeer::SendRstStreamInner(QuicSession* session,
- QuicStreamId id,
- QuicRstStreamErrorCode error,
- QuicStreamOffset bytes_written,
- bool close_write_side_only) {
- session->SendRstStreamInner(id, error, bytes_written, close_write_side_only);
-}
-
-// static
PendingStream* QuicSessionPeer::GetPendingStream(QuicSession* session,
QuicStreamId stream_id) {
auto it = session->pending_stream_map_.find(stream_id);
diff --git a/quic/test_tools/quic_session_peer.h b/quic/test_tools/quic_session_peer.h
index eed3bdd..446cd67 100644
--- a/quic/test_tools/quic_session_peer.h
+++ b/quic/test_tools/quic_session_peer.h
@@ -79,11 +79,6 @@
QuicSession* session);
static QuicStreamIdManager* v99_unidirectional_stream_id_manager(
QuicSession* session);
- static void SendRstStreamInner(QuicSession* session,
- QuicStreamId id,
- QuicRstStreamErrorCode error,
- QuicStreamOffset bytes_written,
- bool close_write_side_only);
static PendingStream* GetPendingStream(QuicSession* session,
QuicStreamId stream_id);
static void set_is_configured(QuicSession* session, bool value);
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index c3c29fa..89dc2dc 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -668,6 +668,12 @@
QuicStreamOffset offset,
StreamSendingState state);
+ void ReallySendRstStream(QuicStreamId id,
+ QuicRstStreamErrorCode error,
+ QuicStreamOffset bytes_written) {
+ QuicSession::SendRstStream(id, error, bytes_written);
+ }
+
private:
std::unique_ptr<QuicCryptoStream> crypto_stream_;
};
