In quic, do not truncate ack if there is ack queued in the creator. protected by gfe2_reloadable_flag_quic_donot_change_queued_ack.
PiperOrigin-RevId: 310024106
Change-Id: I9209e467a76e8a05d8f4aef0271cb67729ca3b86
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index fa4b3d8..7ec7955 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -3788,12 +3788,17 @@
void QuicConnection::PostProcessAfterAckFrame(bool send_stop_waiting,
bool acked_new_packet) {
if (no_stop_waiting_frames_) {
- uber_received_packet_manager_.DontWaitForPacketsBefore(
- last_decrypted_packet_level_,
- SupportsMultiplePacketNumberSpaces()
- ? sent_packet_manager_.GetLargestPacketPeerKnowsIsAcked(
- last_decrypted_packet_level_)
- : sent_packet_manager_.largest_packet_peer_knows_is_acked());
+ if (GetQuicReloadableFlag(quic_donot_change_queued_ack) &&
+ packet_creator_.has_ack()) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_donot_change_queued_ack);
+ } else {
+ uber_received_packet_manager_.DontWaitForPacketsBefore(
+ last_decrypted_packet_level_,
+ SupportsMultiplePacketNumberSpaces()
+ ? sent_packet_manager_.GetLargestPacketPeerKnowsIsAcked(
+ last_decrypted_packet_level_)
+ : sent_packet_manager_.largest_packet_peer_knows_is_acked());
+ }
}
// Always reset the retransmission alarm when an ack comes in, since we now
// have a better estimate of the current rtt than when it was set.
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 613358a..6f803c3 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -10495,6 +10495,48 @@
EXPECT_EQ(1u, writer_->ping_frames().size());
}
+// Regression test for b/155757133
+TEST_P(QuicConnectionTest, DonotChangeQueuedAcks) {
+ if (!connection_.SupportsMultiplePacketNumberSpaces()) {
+ return;
+ }
+ const size_t kMinRttMs = 40;
+ RttStats* rtt_stats = const_cast<RttStats*>(manager_->GetRttStats());
+ rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(kMinRttMs),
+ QuicTime::Delta::Zero(), QuicTime::Zero());
+ EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
+ EXPECT_CALL(*send_algorithm_, OnCongestionEvent(_, _, _, _, _));
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+
+ ProcessPacket(2);
+ ProcessPacket(3);
+ ProcessPacket(4);
+ // Process a packet containing stream frame followed by ACK of packets 1.
+ QuicFrames frames;
+ frames.push_back(QuicFrame(QuicStreamFrame(
+ QuicUtils::GetFirstBidirectionalStreamId(
+ connection_.version().transport_version, Perspective::IS_CLIENT),
+ false, 0u, quiche::QuicheStringPiece())));
+ QuicAckFrame ack_frame = InitAckFrame(1);
+ frames.push_back(QuicFrame(&ack_frame));
+ // Receiving stream frame causes something to send.
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).WillOnce(Invoke([this]() {
+ connection_.SendControlFrame(QuicFrame(new QuicWindowUpdateFrame(1, 0, 0)));
+ // Verify now the queued ACK contains packet number 2.
+ EXPECT_TRUE(QuicPacketCreatorPeer::QueuedFrames(
+ QuicConnectionPeer::GetPacketCreator(&connection_))[0]
+ .ack_frame->packets.Contains(QuicPacketNumber(2)));
+ }));
+ ProcessFramesPacketAtLevel(9, frames, ENCRYPTION_FORWARD_SECURE);
+ if (GetQuicReloadableFlag(quic_donot_change_queued_ack)) {
+ EXPECT_TRUE(writer_->ack_frames()[0].packets.Contains(QuicPacketNumber(2)));
+ } else {
+ // ACK frame changes mid packet serialiation!
+ EXPECT_FALSE(
+ writer_->ack_frames()[0].packets.Contains(QuicPacketNumber(2)));
+ }
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/test_tools/quic_packet_creator_peer.cc b/quic/test_tools/quic_packet_creator_peer.cc
index 8224012..1b5eb53 100644
--- a/quic/test_tools/quic_packet_creator_peer.cc
+++ b/quic/test_tools/quic_packet_creator_peer.cc
@@ -149,5 +149,10 @@
return creator->retry_token_;
}
+// static
+QuicFrames& QuicPacketCreatorPeer::QueuedFrames(QuicPacketCreator* creator) {
+ return creator->queued_frames_;
+}
+
} // namespace test
} // namespace quic
diff --git a/quic/test_tools/quic_packet_creator_peer.h b/quic/test_tools/quic_packet_creator_peer.h
index bdadc2b..60eb811 100644
--- a/quic/test_tools/quic_packet_creator_peer.h
+++ b/quic/test_tools/quic_packet_creator_peer.h
@@ -59,6 +59,7 @@
static EncryptionLevel GetEncryptionLevel(QuicPacketCreator* creator);
static QuicFramer* framer(QuicPacketCreator* creator);
static std::string GetRetryToken(QuicPacketCreator* creator);
+ static QuicFrames& QueuedFrames(QuicPacketCreator* creator);
};
} // namespace test
