gfe-relnote: Add an IsAckFrameEmpty const method to received_packet_manager and make sure GetUpdatedAckFrame is only called to retrieve nonempty ACK frame. Refactoring only, not protected.
PiperOrigin-RevId: 296026015
Change-Id: I41976873d68db37b5c276befd3330c9cbda00108
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 0bfb6ea..0420637 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -67,7 +67,6 @@
if (connection_->SupportsMultiplePacketNumberSpaces()) {
connection_->SendAllPendingAcks();
} else {
- DCHECK(!connection_->GetUpdatedAckFrame().ack_frame->packets.Empty());
connection_->SendAck();
}
}
@@ -1515,6 +1514,9 @@
}
const QuicFrame QuicConnection::GetUpdatedAckFrame() {
+ DCHECK(!uber_received_packet_manager_.IsAckFrameEmpty(
+ QuicUtils::GetPacketNumberSpace(encryption_level_)))
+ << "Try to retrieve an empty ACK frame";
return uber_received_packet_manager_.GetUpdatedAckFrame(
QuicUtils::GetPacketNumberSpace(encryption_level_),
clock_->ApproximateNow());
@@ -2919,7 +2921,8 @@
ScopedPacketFlusher flusher(this);
// Always bundle an ACK with connection close for debugging purpose.
if (error != QUIC_PACKET_WRITE_ERROR &&
- !GetUpdatedAckFrame().ack_frame->packets.Empty()) {
+ !uber_received_packet_manager_.IsAckFrameEmpty(
+ QuicUtils::GetPacketNumberSpace(encryption_level_))) {
SendAck();
}
QuicConnectionCloseFrame* frame;
@@ -2955,13 +2958,12 @@
SetDefaultEncryptionLevel(level);
// Bundle an ACK of the corresponding packet number space for debugging
// purpose.
- if (error != QUIC_PACKET_WRITE_ERROR) {
- const QuicFrame ack_frame = GetUpdatedAckFrame();
- if (!ack_frame.ack_frame->packets.Empty()) {
- QuicFrames frames;
- frames.push_back(ack_frame);
- packet_creator_.FlushAckFrame(frames);
- }
+ if (error != QUIC_PACKET_WRITE_ERROR &&
+ !uber_received_packet_manager_.IsAckFrameEmpty(
+ QuicUtils::GetPacketNumberSpace(encryption_level_))) {
+ QuicFrames frames;
+ frames.push_back(GetUpdatedAckFrame());
+ packet_creator_.FlushAckFrame(frames);
}
auto* frame =
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc
index 4333323..6298dcd 100644
--- a/quic/core/quic_received_packet_manager.cc
+++ b/quic/core/quic_received_packet_manager.cc
@@ -346,4 +346,8 @@
return least_received_packet_number_;
}
+bool QuicReceivedPacketManager::IsAckFrameEmpty() const {
+ return ack_frame_.packets.Empty();
+}
+
} // namespace quic
diff --git a/quic/core/quic_received_packet_manager.h b/quic/core/quic_received_packet_manager.h
index 92a9e2f..b01390d 100644
--- a/quic/core/quic_received_packet_manager.h
+++ b/quic/core/quic_received_packet_manager.h
@@ -90,6 +90,9 @@
// received.
QuicPacketNumber PeerFirstSendingPacketNumber() const;
+ // Returns true if ack frame is empty.
+ bool IsAckFrameEmpty() const;
+
void set_connection_stats(QuicConnectionStats* stats) { stats_ = stats; }
// For logging purposes.
diff --git a/quic/core/uber_received_packet_manager.cc b/quic/core/uber_received_packet_manager.cc
index f62e1ea..723ac30 100644
--- a/quic/core/uber_received_packet_manager.cc
+++ b/quic/core/uber_received_packet_manager.cc
@@ -168,6 +168,14 @@
return ack_timeout;
}
+bool UberReceivedPacketManager::IsAckFrameEmpty(
+ PacketNumberSpace packet_number_space) const {
+ if (!supports_multiple_packet_number_spaces_) {
+ return received_packet_managers_[0].IsAckFrameEmpty();
+ }
+ return received_packet_managers_[packet_number_space].IsAckFrameEmpty();
+}
+
QuicPacketNumber UberReceivedPacketManager::peer_least_packet_awaiting_ack()
const {
DCHECK(!supports_multiple_packet_number_spaces_);
diff --git a/quic/core/uber_received_packet_manager.h b/quic/core/uber_received_packet_manager.h
index 14ba868..d76ce13 100644
--- a/quic/core/uber_received_packet_manager.h
+++ b/quic/core/uber_received_packet_manager.h
@@ -70,6 +70,9 @@
// Get the earliest ack_timeout of all packet number spaces.
QuicTime GetEarliestAckTimeout() const;
+ // Return true if ack frame of |packet_number_space| is empty.
+ bool IsAckFrameEmpty(PacketNumberSpace packet_number_space) const;
+
QuicPacketNumber peer_least_packet_awaiting_ack() const;
size_t min_received_before_ack_decimation() const;
diff --git a/quic/core/uber_received_packet_manager_test.cc b/quic/core/uber_received_packet_manager_test.cc
index b1e7140..a2fb640 100644
--- a/quic/core/uber_received_packet_manager_test.cc
+++ b/quic/core/uber_received_packet_manager_test.cc
@@ -134,7 +134,9 @@
};
TEST_F(UberReceivedPacketManagerTest, DontWaitForPacketsBefore) {
+ EXPECT_TRUE(manager_->IsAckFrameEmpty(APPLICATION_DATA));
RecordPacketReceipt(2);
+ EXPECT_FALSE(manager_->IsAckFrameEmpty(APPLICATION_DATA));
RecordPacketReceipt(7);
EXPECT_TRUE(manager_->IsAwaitingPacket(ENCRYPTION_FORWARD_SECURE,
QuicPacketNumber(3u)));
