Deprecate gfe2_reloadable_flag_quic_update_ack_timeout_on_receipt_time.
PiperOrigin-RevId: 449225386
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index 2154d94..cc7e38e 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -11182,40 +11182,13 @@
std::make_unique<TaggingEncrypter>(0x01));
connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE);
// Verify HANDSHAKE packet gets processed.
- if (GetQuicReloadableFlag(quic_update_ack_timeout_on_receipt_time)) {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
- } else {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
- }
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
connection_.GetProcessUndecryptablePacketsAlarm()->Fire();
- if (GetQuicReloadableFlag(quic_update_ack_timeout_on_receipt_time)) {
- // Verify immediate ACK has been sent out when flush went out of scope.
- ASSERT_FALSE(connection_.HasPendingAcks());
- } else {
- ASSERT_TRUE(connection_.HasPendingAcks());
- // Send ACKs.
- clock_.AdvanceTime(connection_.GetAckAlarm()->deadline() - clock_.Now());
- connection_.GetAckAlarm()->Fire();
- }
+ // Verify immediate ACK has been sent out when flush went out of scope.
+ ASSERT_FALSE(connection_.HasPendingAcks());
ASSERT_FALSE(writer_->ack_frames().empty());
- if (GetQuicReloadableFlag(quic_update_ack_timeout_on_receipt_time)) {
- // Verify the ack_delay_time in the sent HANDSHAKE ACK frame is 100ms.
- EXPECT_EQ(QuicTime::Delta::FromMilliseconds(100),
- writer_->ack_frames()[0].ack_delay_time);
- ASSERT_TRUE(writer_->coalesced_packet() == nullptr);
- return;
- }
- // Verify the ack_delay_time in the INITIAL ACK frame is 1ms.
- EXPECT_EQ(QuicTime::Delta::FromMilliseconds(1),
- writer_->ack_frames()[0].ack_delay_time);
- // Process the coalesced HANDSHAKE packet.
- ASSERT_TRUE(writer_->coalesced_packet() != nullptr);
- auto packet = writer_->coalesced_packet()->Clone();
- writer_->framer()->ProcessPacket(*packet);
- ASSERT_FALSE(writer_->ack_frames().empty());
- // Verify the ack_delay_time in the HANDSHAKE ACK frame includes the
- // buffering time.
- EXPECT_EQ(QuicTime::Delta::FromMilliseconds(101),
+ // Verify the ack_delay_time in the sent HANDSHAKE ACK frame is 100ms.
+ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(100),
writer_->ack_frames()[0].ack_delay_time);
ASSERT_TRUE(writer_->coalesced_packet() == nullptr);
}
@@ -15295,27 +15268,15 @@
connection_.GetProcessUndecryptablePacketsAlarm()->Fire();
ASSERT_TRUE(connection_.HasPendingAcks());
- if (GetQuicReloadableFlag(quic_update_ack_timeout_on_receipt_time)) {
- EXPECT_EQ(packet_receipt_time + DefaultDelayedAckTime(),
- connection_.GetAckAlarm()->deadline());
- clock_.AdvanceTime(packet_receipt_time + DefaultDelayedAckTime() -
- clock_.Now());
- } else {
- EXPECT_EQ(clock_.Now() + DefaultDelayedAckTime(),
- connection_.GetAckAlarm()->deadline());
- clock_.AdvanceTime(DefaultDelayedAckTime());
- }
+ EXPECT_EQ(packet_receipt_time + DefaultDelayedAckTime(),
+ connection_.GetAckAlarm()->deadline());
+ clock_.AdvanceTime(packet_receipt_time + DefaultDelayedAckTime() -
+ clock_.Now());
// Fire ACK alarm.
connection_.GetAckAlarm()->Fire();
ASSERT_EQ(1u, writer_->ack_frames().size());
- if (GetQuicReloadableFlag(quic_update_ack_timeout_on_receipt_time)) {
- // Verify ACK delay time does not include queuing delay.
- EXPECT_EQ(DefaultDelayedAckTime(), writer_->ack_frames()[0].ack_delay_time);
- } else {
- // Verify ACK delay time = queuing delay + ack delay
- EXPECT_EQ(DefaultDelayedAckTime() + kQueuingDelay,
- writer_->ack_frames()[0].ack_delay_time);
- }
+ // Verify ACK delay time does not include queuing delay.
+ EXPECT_EQ(DefaultDelayedAckTime(), writer_->ack_frames()[0].ack_delay_time);
}
TEST_P(QuicConnectionTest, CoalesceOneRTTPacketWithInitialAndHandshakePackets) {
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index b515e22..96db0f0 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -79,8 +79,6 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_conservative_bursts, false)
// If true, stop resetting ideal_next_packet_send_time_ in pacing sender.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_donot_reset_ideal_next_packet_send_time, false)
-// If true, update ACK timeout based on packet receipt time rather than now.
-QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_update_ack_timeout_on_receipt_time, true)
// If true, use BBRv2 as the default congestion controller. Takes precedence over --quic_default_to_bbr.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_default_to_bbr_v2, false)
// If true, use PING manager to manage the PING alarm.
diff --git a/quiche/quic/core/quic_received_packet_manager.cc b/quiche/quic/core/quic_received_packet_manager.cc
index 213f6ba..bb7b87d 100644
--- a/quiche/quic/core/quic_received_packet_manager.cc
+++ b/quiche/quic/core/quic_received_packet_manager.cc
@@ -267,24 +267,9 @@
return;
}
- QuicTime ack_timeout_base = now;
- const bool quic_update_ack_timeout_on_receipt_time =
- GetQuicReloadableFlag(quic_update_ack_timeout_on_receipt_time);
- if (quic_update_ack_timeout_on_receipt_time) {
- if (last_packet_receipt_time <= now) {
- QUIC_CODE_COUNT(quic_update_ack_timeout_on_receipt_time);
- ack_timeout_base = last_packet_receipt_time;
- } else {
- QUIC_CODE_COUNT(quic_update_ack_timeout_on_now);
- ack_timeout_base = now;
- }
- }
- QuicTime updated_ack_time =
- ack_timeout_base +
- GetMaxAckDelay(last_received_packet_number, *rtt_stats);
- if (quic_update_ack_timeout_on_receipt_time) {
- updated_ack_time = std::max(now, updated_ack_time);
- }
+ const QuicTime updated_ack_time = std::max(
+ now, std::min(last_packet_receipt_time, now) +
+ GetMaxAckDelay(last_received_packet_number, *rtt_stats));
if (!ack_timeout_.IsInitialized() || ack_timeout_ > updated_ack_time) {
ack_timeout_ = updated_ack_time;
}
diff --git a/quiche/quic/core/quic_received_packet_manager_test.cc b/quiche/quic/core/quic_received_packet_manager_test.cc
index 65cfa86..c453707 100644
--- a/quiche/quic/core/quic_received_packet_manager_test.cc
+++ b/quiche/quic/core/quic_received_packet_manager_test.cc
@@ -650,13 +650,8 @@
kInstigateAck, QuicPacketNumber(3),
/*last_packet_receipt_time=*/packet_receipt_time3,
clock_.ApproximateNow(), &rtt_stats_);
- if (GetQuicReloadableFlag(quic_update_ack_timeout_on_receipt_time)) {
- // Make sure ACK timeout is based on receipt time.
- CheckAckTimeout(packet_receipt_time3 + kDelayedAckTime);
- } else {
- // Make sure ACK timeout is based on now.
- CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
- }
+ // Make sure ACK timeout is based on receipt time.
+ CheckAckTimeout(packet_receipt_time3 + kDelayedAckTime);
RecordPacketReceipt(4, clock_.ApproximateNow());
MaybeUpdateAckTimeout(kInstigateAck, 4);
@@ -677,13 +672,8 @@
kInstigateAck, QuicPacketNumber(3),
/*last_packet_receipt_time=*/packet_receipt_time3,
clock_.ApproximateNow(), &rtt_stats_);
- if (GetQuicReloadableFlag(quic_update_ack_timeout_on_receipt_time)) {
- // Given 100ms > ack delay, verify immediate ACK.
- CheckAckTimeout(clock_.ApproximateNow());
- } else {
- // Make sure ACK timeout is based on now.
- CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
- }
+ // Given 100ms > ack delay, verify immediate ACK.
+ CheckAckTimeout(clock_.ApproximateNow());
}
} // namespace
diff --git a/quiche/quic/core/uber_received_packet_manager_test.cc b/quiche/quic/core/uber_received_packet_manager_test.cc
index 5474421..ad62d8f 100644
--- a/quiche/quic/core/uber_received_packet_manager_test.cc
+++ b/quiche/quic/core/uber_received_packet_manager_test.cc
@@ -558,15 +558,9 @@
CheckAckTimeout(clock_.ApproximateNow());
EXPECT_TRUE(HasPendingAck());
- if (GetQuicReloadableFlag(quic_update_ack_timeout_on_receipt_time)) {
- // Verify ACK delay is based on packet receipt time.
- CheckAckTimeout(clock_.ApproximateNow() -
- QuicTime::Delta::FromMilliseconds(11) + kDelayedAckTime);
- } else {
- // Delayed ack is scheduled.
- CheckAckTimeout(clock_.ApproximateNow() -
- QuicTime::Delta::FromMilliseconds(1) + kDelayedAckTime);
- }
+ // Verify ACK delay is based on packet receipt time.
+ CheckAckTimeout(clock_.ApproximateNow() -
+ QuicTime::Delta::FromMilliseconds(11) + kDelayedAckTime);
}
} // namespace
