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) {