Deprecate --gfe2_reloadable_flag_quic_goaway_with_max_stream_id.
Also remove QuicSpdySession::SendHttp3Shutdown(). Callers should use QuicSpdySession::SendHttp3GoAway() instead, which behaves identically.
PiperOrigin-RevId: 359798040
Change-Id: I53495a19141ec02438aad92a93b2c8c205ceaf53
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 8418bb2..81c60b2 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -508,14 +508,9 @@
ietf_server_push_enabled_(
GetQuicFlag(FLAGS_quic_enable_http3_server_push)),
http3_max_push_id_sent_(false),
- goaway_with_max_stream_id_(
- GetQuicReloadableFlag(quic_goaway_with_max_stream_id)),
next_available_datagram_flow_id_(perspective() == Perspective::IS_SERVER
? kFirstDatagramFlowIdServer
: kFirstDatagramFlowIdClient) {
- if (goaway_with_max_stream_id_) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_goaway_with_max_stream_id, 1, 2);
- }
h2_deframer_.set_visitor(spdy_framer_visitor_.get());
h2_deframer_.set_debug_visitor(spdy_framer_visitor_.get());
spdy_framer_.set_debug_visitor(spdy_framer_visitor_.get());
@@ -822,40 +817,18 @@
}
QuicStreamId stream_id;
- if (goaway_with_max_stream_id_) {
- stream_id = QuicUtils::GetMaxClientInitiatedBidirectionalStreamId(
- transport_version());
- if (last_sent_http3_goaway_id_.has_value()) {
- if (last_sent_http3_goaway_id_.value() == stream_id) {
- // Do not send GOAWAY twice.
- return;
- }
- if (last_sent_http3_goaway_id_.value() < stream_id) {
- // A previous GOAWAY frame was sent with smaller stream ID. This is not
- // possible, because the only time a GOAWAY frame with non-maximal
- // stream ID is sent is right before closing connection.
- QUIC_BUG << "GOAWAY frame with smaller ID already sent.";
- return;
- }
+ stream_id = QuicUtils::GetMaxClientInitiatedBidirectionalStreamId(
+ transport_version());
+ if (last_sent_http3_goaway_id_.has_value()) {
+ if (last_sent_http3_goaway_id_.value() == stream_id) {
+ // Do not send GOAWAY twice.
+ return;
}
- } else {
- stream_id = GetLargestPeerCreatedStreamId(/*unidirectional = */ false);
-
- if (stream_id == QuicUtils::GetInvalidStreamId(transport_version())) {
- // No client-initiated bidirectional streams received yet.
- // Send 0 to let client know that all requests can be retried.
- stream_id = 0;
- } else {
- // Tell client that streams starting with the next after the largest
- // received one can be retried.
- stream_id += QuicUtils::StreamIdDelta(transport_version());
- }
- if (last_sent_http3_goaway_id_.has_value() &&
- last_sent_http3_goaway_id_.value() <= stream_id) {
- // MUST not send GOAWAY with identifier larger than previously sent.
- // Do not bother sending one with same identifier as before, since
- // GOAWAY frames on the control stream are guaranteed to be processed in
- // order.
+ if (last_sent_http3_goaway_id_.value() < stream_id) {
+ // A previous GOAWAY frame was sent with smaller stream ID. This is not
+ // possible, because the only time a GOAWAY frame with non-maximal
+ // stream ID is sent is right before closing connection.
+ QUIC_BUG << "GOAWAY frame with smaller ID already sent.";
return;
}
}
@@ -864,30 +837,6 @@
last_sent_http3_goaway_id_ = stream_id;
}
-void QuicSpdySession::SendHttp3Shutdown() {
- if (goaway_with_max_stream_id_) {
- SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Server shutdown");
- return;
- }
-
- QUICHE_DCHECK_EQ(perspective(), Perspective::IS_SERVER);
- QUICHE_DCHECK(VersionUsesHttp3(transport_version()));
- QuicStreamCount advertised_max_incoming_bidirectional_streams =
- GetAdvertisedMaxIncomingBidirectionalStreams();
- const QuicStreamId stream_id =
- QuicUtils::GetFirstBidirectionalStreamId(transport_version(),
- Perspective::IS_CLIENT) +
- QuicUtils::StreamIdDelta(transport_version()) *
- advertised_max_incoming_bidirectional_streams;
- if (last_sent_http3_goaway_id_.has_value() &&
- last_sent_http3_goaway_id_.value() < stream_id) {
- send_control_stream_->SendGoAway(last_sent_http3_goaway_id_.value());
- return;
- }
- send_control_stream_->SendGoAway(stream_id);
- last_sent_http3_goaway_id_ = stream_id;
-}
-
void QuicSpdySession::WritePushPromise(QuicStreamId original_stream_id,
QuicStreamId promised_stream_id,
SpdyHeaderBlock headers) {
@@ -1555,13 +1504,12 @@
}
if (last_sent_http3_goaway_id_.has_value() &&
last_sent_http3_goaway_id_.value() <= stream_id) {
- if (goaway_with_max_stream_id_) {
- // A previous GOAWAY frame was sent with smaller stream ID. This is not
- // possible, because this is the only method sending a GOAWAY frame with
- // non-maximal stream ID, and this must only be called once, right
- // before closing connection.
- QUIC_BUG << "GOAWAY frame with smaller ID already sent.";
- }
+ // A previous GOAWAY frame was sent with smaller stream ID. This is not
+ // possible, because this is the only method sending a GOAWAY frame with
+ // non-maximal stream ID, and this must only be called once, right
+ // before closing connection.
+ QUIC_BUG << "GOAWAY frame with smaller ID already sent.";
+
// MUST not send GOAWAY with identifier larger than previously sent.
// Do not bother sending one with same identifier as before, since GOAWAY
// frames on the control stream are guaranteed to be processed in order.
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h
index 7e13afa..cff9e63 100644
--- a/quic/core/http/quic_spdy_session.h
+++ b/quic/core/http/quic_spdy_session.h
@@ -243,10 +243,6 @@
// before encryption gets established.
void SendHttp3GoAway(QuicErrorCode error_code, const std::string& reason);
- // Same as SendHttp3GoAway(). TODO(bnc): remove when
- // gfe2_reloadable_flag_quic_goaway_with_max_stream_id flag is deprecated.
- void SendHttp3Shutdown();
-
// Write |headers| for |promised_stream_id| on |original_stream_id| in a
// PUSH_PROMISE frame to peer.
virtual void WritePushPromise(QuicStreamId original_stream_id,
@@ -627,9 +623,6 @@
// recent MAX_PUSH_ID frame. Once true, never goes back to false.
bool http3_max_push_id_sent_;
- // Latched value of reloadable flag quic_goaway_with_max_stream_id.
- const bool goaway_with_max_stream_id_;
-
// Value of the smallest unused HTTP/3 datagram flow ID that this endpoint's
// datagram flow ID allocation service will use next.
QuicDatagramFlowId next_available_datagram_flow_id_;
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 848f215..b266633 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1132,15 +1132,8 @@
EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _))
.WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- if (GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
- // Send max stream id (currently 32 bits).
- EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 0xfffffffc));
- } else {
- // No client-initiated stream has been received, therefore a GOAWAY frame
- // with stream ID = 0 is sent to notify the client that all requests can be
- // retried on a different connection.
- EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 0));
- }
+ // Send max stream id (currently 32 bits).
+ EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 0xfffffffc));
session_.SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Goaway");
EXPECT_TRUE(session_.goaway_sent());
@@ -1183,15 +1176,8 @@
EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _))
.WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- if (GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
- // Send max stream id (currently 32 bits).
- EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 0xfffffffc));
- } else {
- // The first stream, of kTestStreamId = 0, could already have been
- // processed. A GOAWAY frame is sent to notify the client that requests
- // starting with stream ID = 4 can be retried on a different connection.
- EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 4));
- }
+ // Send max stream id (currently 32 bits).
+ EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 0xfffffffc));
session_.SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Goaway");
EXPECT_TRUE(session_.goaway_sent());
@@ -1200,59 +1186,6 @@
session_.SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Goaway");
}
-TEST_P(QuicSpdySessionTestServer, SendHttp3Shutdown) {
- if (GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
- return;
- }
-
- if (!VersionUsesHttp3(transport_version())) {
- return;
- }
-
- CompleteHandshake();
- StrictMock<MockHttp3DebugVisitor> debug_visitor;
- session_.set_debug_visitor(&debug_visitor);
-
- EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _))
- .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(_));
- session_.SendHttp3Shutdown();
- EXPECT_TRUE(session_.goaway_sent());
-
- const QuicStreamId kTestStreamId =
- GetNthClientInitiatedBidirectionalStreamId(transport_version(), 0);
- EXPECT_CALL(*connection_, OnStreamReset(kTestStreamId, _)).Times(0);
- EXPECT_TRUE(session_.GetOrCreateStream(kTestStreamId));
-}
-
-TEST_P(QuicSpdySessionTestServer, SendHttp3GoAwayAfterShutdownNotice) {
- if (GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
- return;
- }
-
- if (!VersionUsesHttp3(transport_version())) {
- return;
- }
-
- CompleteHandshake();
- StrictMock<MockHttp3DebugVisitor> debug_visitor;
- session_.set_debug_visitor(&debug_visitor);
-
- EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _))
- .Times(2)
- .WillRepeatedly(Return(WriteResult(WRITE_STATUS_OK, 0)));
- EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(_)).Times(2);
-
- session_.SendHttp3Shutdown();
- EXPECT_TRUE(session_.goaway_sent());
- session_.SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Goaway");
-
- const QuicStreamId kTestStreamId =
- GetNthClientInitiatedBidirectionalStreamId(transport_version(), 0);
- EXPECT_CALL(*connection_, OnStreamReset(kTestStreamId, _)).Times(0);
- EXPECT_TRUE(session_.GetOrCreateStream(kTestStreamId));
-}
-
TEST_P(QuicSpdySessionTestServer, DoNotSendGoAwayTwice) {
CompleteHandshake();
if (VersionHasIetfQuicFrames(transport_version())) {
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 82fe74b..40f8502 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -45,7 +45,6 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_encrypted_goaway, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_on_stream_reset, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_willing_and_able_to_write2, true)
-QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_goaway_with_max_stream_id, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_h3_datagram, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_parse_accept_ch_frame, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_pass_path_response_to_validator, false)
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 8cd0a57..e3b0408 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -911,19 +911,13 @@
return;
}
transport_goaway_sent_ = true;
- if (GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
- QUICHE_DCHECK_EQ(perspective(), Perspective::IS_SERVER);
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_goaway_with_max_stream_id, 2, 2);
- control_frame_manager_.WriteOrBufferGoAway(
- error_code,
- QuicUtils::GetMaxClientInitiatedBidirectionalStreamId(
- transport_version()),
- reason);
- } else {
- control_frame_manager_.WriteOrBufferGoAway(
- error_code, stream_id_manager_.largest_peer_created_stream_id(),
- reason);
- }
+
+ QUICHE_DCHECK_EQ(perspective(), Perspective::IS_SERVER);
+ control_frame_manager_.WriteOrBufferGoAway(
+ error_code,
+ QuicUtils::GetMaxClientInitiatedBidirectionalStreamId(
+ transport_version()),
+ reason);
}
void QuicSession::SendBlocked(QuicStreamId id) {
