Deprecate gfe2_reloadable_flag_quic_advance_ack_timeout_update.
PiperOrigin-RevId: 318076325
Change-Id: I06fe1c007d76fb56cf359d0ea7496bbbdae947df
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 5b9419f..8d274d0 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -3953,14 +3953,10 @@
client_->WaitForResponse();
EXPECT_EQ(kBarResponseBody, client_->response_body());
QuicConnectionStats client_stats = GetClientConnection()->GetStats();
- if (GetQuicReloadableFlag(quic_advance_ack_timeout_update)) {
- // Client sends CHLO in packet 1 and retransmitted in packet 2. Because of
- // the delay, server processes packet 2 and later drops packet 1. ACK is
- // bundled with SHLO, such that 1 can be detected loss by time threshold.
- EXPECT_LE(0u, client_stats.packets_lost);
- } else {
- EXPECT_EQ(0u, client_stats.packets_lost);
- }
+ // Client sends CHLO in packet 1 and retransmitted in packet 2. Because of
+ // the delay, server processes packet 2 and later drops packet 1. ACK is
+ // bundled with SHLO, such that 1 can be detected loss by time threshold.
+ EXPECT_LE(0u, client_stats.packets_lost);
EXPECT_TRUE(client_->client()->EarlyDataAccepted());
}
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 2c601bb..1272359 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -333,9 +333,6 @@
<< "QuicConnection: attempted to use server connection ID "
<< server_connection_id << " which is invalid with version "
<< QuicVersionToString(transport_version());
- if (advance_ack_timeout_update_) {
- QUIC_RELOADABLE_FLAG_COUNT(quic_advance_ack_timeout_update);
- }
framer_.set_visitor(this);
stats_.connection_creation_time = clock_->ApproximateNow();
// TODO(ianswett): Supply the NetworkChangeVisitor as a constructor argument
@@ -1153,14 +1150,9 @@
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
return false;
}
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- }
+ MaybeUpdateAckTimeout();
visitor_->OnStreamFrame(frame);
stats_.stream_bytes_received += frame.data_length;
- if (!advance_ack_timeout_update_) {
- should_last_packet_instigate_acks_ = true;
- }
consecutive_retransmittable_on_wire_ping_count_ = 0;
return connected_;
}
@@ -1175,13 +1167,8 @@
if (debug_visitor_ != nullptr) {
debug_visitor_->OnCryptoFrame(frame);
}
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- }
+ MaybeUpdateAckTimeout();
visitor_->OnCryptoFrame(frame);
- if (!advance_ack_timeout_update_) {
- should_last_packet_instigate_acks_ = true;
- }
return connected_;
}
@@ -1363,11 +1350,7 @@
if (debug_visitor_ != nullptr) {
debug_visitor_->OnPingFrame(frame);
}
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- } else {
- should_last_packet_instigate_acks_ = true;
- }
+ MaybeUpdateAckTimeout();
return true;
}
@@ -1409,13 +1392,8 @@
<< "RST_STREAM_FRAME received for stream: " << frame.stream_id
<< " with error: "
<< QuicRstStreamErrorCodeToString(frame.error_code);
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- }
+ MaybeUpdateAckTimeout();
visitor_->OnRstStream(frame);
- if (!advance_ack_timeout_update_) {
- should_last_packet_instigate_acks_ = true;
- }
return connected_;
}
@@ -1446,11 +1424,7 @@
// response.
received_path_challenge_payloads_.push_back(frame.data_buffer);
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- } else {
- should_last_packet_instigate_acks_ = true;
- }
+ MaybeUpdateAckTimeout();
return true;
}
@@ -1458,11 +1432,7 @@
if (debug_visitor_ != nullptr) {
debug_visitor_->OnPathResponseFrame(frame);
}
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- } else {
- should_last_packet_instigate_acks_ = true;
- }
+ MaybeUpdateAckTimeout();
if (!transmitted_connectivity_probe_payload_ ||
*transmitted_connectivity_probe_payload_ != frame.data_buffer) {
// Is not for the probe we sent, ignore it.
@@ -1549,13 +1519,8 @@
<< frame.last_good_stream_id
<< " and error: " << QuicErrorCodeToString(frame.error_code)
<< " and reason: " << frame.reason_phrase;
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- }
+ MaybeUpdateAckTimeout();
visitor_->OnGoAway(frame);
- if (!advance_ack_timeout_update_) {
- should_last_packet_instigate_acks_ = true;
- }
return connected_;
}
@@ -1570,13 +1535,8 @@
debug_visitor_->OnWindowUpdateFrame(frame, GetTimeOfLastReceivedPacket());
}
QUIC_DVLOG(1) << ENDPOINT << "WINDOW_UPDATE_FRAME received " << frame;
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- }
+ MaybeUpdateAckTimeout();
visitor_->OnWindowUpdateFrame(frame);
- if (!advance_ack_timeout_update_) {
- should_last_packet_instigate_acks_ = true;
- }
return connected_;
}
@@ -1613,14 +1573,9 @@
if (debug_visitor_ != nullptr) {
debug_visitor_->OnMessageFrame(frame);
}
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- }
+ MaybeUpdateAckTimeout();
visitor_->OnMessageReceived(
quiche::QuicheStringPiece(frame.data, frame.message_length));
- if (!advance_ack_timeout_update_) {
- should_last_packet_instigate_acks_ = true;
- }
return connected_;
}
@@ -1647,13 +1602,8 @@
if (debug_visitor_ != nullptr) {
debug_visitor_->OnHandshakeDoneFrame(frame);
}
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- }
+ MaybeUpdateAckTimeout();
visitor_->OnHandshakeDoneReceived();
- if (!advance_ack_timeout_update_) {
- should_last_packet_instigate_acks_ = true;
- }
return connected_;
}
@@ -1675,14 +1625,9 @@
}
QUIC_DLOG(INFO) << ENDPOINT
<< "BLOCKED_FRAME received for stream: " << frame.stream_id;
- if (advance_ack_timeout_update_) {
- MaybeUpdateAckTimeout();
- }
+ MaybeUpdateAckTimeout();
visitor_->OnBlockedFrame(frame);
stats_.blocked_frames_received++;
- if (!advance_ack_timeout_update_) {
- should_last_packet_instigate_acks_ = true;
- }
return connected_;
}
@@ -1754,7 +1699,7 @@
// For IETF QUIC, it is guaranteed that TLS will give connection the
// corresponding write key before read key. In other words, connection should
// never process a packet while an ACK for it cannot be encrypted.
- if (!advance_ack_timeout_update_ || !should_last_packet_instigate_acks_) {
+ if (!should_last_packet_instigate_acks_) {
uber_received_packet_manager_.MaybeUpdateAckTimeout(
should_last_packet_instigate_acks_, last_decrypted_packet_level_,
last_header_.packet_number, GetTimeOfLastReceivedPacket(),
@@ -4804,7 +4749,6 @@
}
void QuicConnection::MaybeUpdateAckTimeout() {
- DCHECK(advance_ack_timeout_update_);
if (should_last_packet_instigate_acks_) {
return;
}
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 2b1b8af..deb5ed2 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1712,9 +1712,6 @@
const bool extend_idle_time_on_decryptable_packets_ =
GetQuicReloadableFlag(quic_extend_idle_time_on_decryptable_packets);
- const bool advance_ack_timeout_update_ =
- GetQuicReloadableFlag(quic_advance_ack_timeout_update);
-
const bool update_ack_alarm_in_send_all_pending_acks_ =
GetQuicReloadableFlag(quic_update_ack_alarm_in_send_all_pending_acks);
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 864a824..600c28a 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -10976,15 +10976,9 @@
}));
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
ProcessDataPacket(1);
- if (GetQuicReloadableFlag(quic_advance_ack_timeout_update)) {
- // Verify ACK is bundled with WINDOW_UPDATE.
- EXPECT_FALSE(writer_->ack_frames().empty());
- EXPECT_FALSE(connection_.HasPendingAcks());
- } else {
- // ACK is pending.
- EXPECT_TRUE(writer_->ack_frames().empty());
- EXPECT_TRUE(connection_.HasPendingAcks());
- }
+ // Verify ACK is bundled with WINDOW_UPDATE.
+ EXPECT_FALSE(writer_->ack_frames().empty());
+ EXPECT_FALSE(connection_.HasPendingAcks());
}
TEST_P(QuicConnectionTest, AckAlarmFiresEarly) {
