QUICHE: always print ACK ranges as [a...b], except if the range is a single packet.
Even a moderately short list is easier to read when it's a range instead of each individual packet listed.
PiperOrigin-RevId: 676638766
diff --git a/quiche/quic/core/frames/quic_ack_frame.cc b/quiche/quic/core/frames/quic_ack_frame.cc
index ae6a2c9..b2f7a58 100644
--- a/quiche/quic/core/frames/quic_ack_frame.cc
+++ b/quiche/quic/core/frames/quic_ack_frame.cc
@@ -14,12 +14,6 @@
namespace quic {
-namespace {
-
-const QuicPacketCount kMaxPrintRange = 128;
-
-} // namespace
-
bool IsAwaitingPacket(const QuicAckFrame& ack_frame,
QuicPacketNumber packet_number,
QuicPacketNumber peer_least_packet_awaiting_ack) {
@@ -161,28 +155,15 @@
return packet_number_intervals_.rbegin()->Length();
}
-// Largest min...max range for packet numbers where we print the numbers
-// explicitly. If bigger than this, we print as a range [a,d] rather
-// than [a b c d]
-
std::ostream& operator<<(std::ostream& os, const PacketNumberQueue& q) {
for (const QuicInterval<QuicPacketNumber>& interval : q) {
- // Print as a range if there is a pathological condition.
- if ((interval.min() >= interval.max()) ||
- (interval.max() - interval.min() > kMaxPrintRange)) {
- // If min>max, it's really a bug, so QUIC_BUG it to
- // catch it in development.
- QUIC_BUG_IF(quic_bug_12614_2, interval.min() >= interval.max())
- << "Ack Range minimum (" << interval.min() << "Not less than max ("
- << interval.max() << ")";
- // print range as min...max rather than full list.
- // in the event of a bug, the list could be very big.
- os << interval.min() << "..." << (interval.max() - 1) << " ";
+ QUIC_BUG_IF(quic_bug_12614_2, interval.min() >= interval.max())
+ << "Ack Range minimum (" << interval.min() << "Not less than max ("
+ << interval.max() << ")";
+ if (interval.min() == interval.max() - 1) {
+ os << interval.min() << " ";
} else {
- for (QuicPacketNumber packet_number = interval.min();
- packet_number < interval.max(); ++packet_number) {
- os << packet_number << " ";
- }
+ os << interval.min() << "..." << (interval.max() - 1) << " ";
}
}
return os;
diff --git a/quiche/quic/core/frames/quic_frames_test.cc b/quiche/quic/core/frames/quic_frames_test.cc
index e41ff7a..37f45cc 100644
--- a/quiche/quic/core/frames/quic_frames_test.cc
+++ b/quiche/quic/core/frames/quic_frames_test.cc
@@ -42,13 +42,55 @@
std::ostringstream stream;
stream << frame;
EXPECT_EQ(
- "{ largest_acked: 5, ack_delay_time: 3, packets: [ 4 5 ], "
+ "{ largest_acked: 5, ack_delay_time: 3, packets: [ 4...5 ], "
"received_packets: [ 6 at 7 ], ecn_counters_populated: 0 }\n",
stream.str());
QuicFrame quic_frame(&frame);
EXPECT_FALSE(IsControlFrame(quic_frame.type));
}
+TEST_F(QuicFramesTest, AckFrameToStringMultipleIntervals) {
+ QuicAckFrame frame;
+ frame.largest_acked = QuicPacketNumber(6);
+ frame.ack_delay_time = QuicTime::Delta::FromMicroseconds(3);
+ frame.packets.Add(QuicPacketNumber(1));
+ frame.packets.Add(QuicPacketNumber(2));
+ frame.packets.Add(QuicPacketNumber(3));
+ frame.packets.Add(QuicPacketNumber(5));
+ frame.packets.Add(QuicPacketNumber(6));
+ frame.received_packet_times = {
+ {QuicPacketNumber(7),
+ QuicTime::Zero() + QuicTime::Delta::FromMicroseconds(7)}};
+ std::ostringstream stream;
+ stream << frame;
+ EXPECT_EQ(
+ "{ largest_acked: 6, ack_delay_time: 3, packets: [ 1...3 5...6 ], "
+ "received_packets: [ 7 at 7 ], ecn_counters_populated: 0 }\n",
+ stream.str());
+ QuicFrame quic_frame(&frame);
+ EXPECT_FALSE(IsControlFrame(quic_frame.type));
+}
+
+TEST_F(QuicFramesTest, AckFrameToStringMultipleIntervalsSinglePacketRange) {
+ QuicAckFrame frame;
+ frame.largest_acked = QuicPacketNumber(6);
+ frame.ack_delay_time = QuicTime::Delta::FromMicroseconds(3);
+ frame.packets.Add(QuicPacketNumber(1));
+ frame.packets.Add(QuicPacketNumber(5));
+ frame.packets.Add(QuicPacketNumber(6));
+ frame.received_packet_times = {
+ {QuicPacketNumber(7),
+ QuicTime::Zero() + QuicTime::Delta::FromMicroseconds(7)}};
+ std::ostringstream stream;
+ stream << frame;
+ EXPECT_EQ(
+ "{ largest_acked: 6, ack_delay_time: 3, packets: [ 1 5...6 ], "
+ "received_packets: [ 7 at 7 ], ecn_counters_populated: 0 }\n",
+ stream.str());
+ QuicFrame quic_frame(&frame);
+ EXPECT_FALSE(IsControlFrame(quic_frame.type));
+}
+
TEST_F(QuicFramesTest, BigAckFrameToString) {
QuicAckFrame frame;
frame.largest_acked = QuicPacketNumber(500);
