gfe-relnote: (n/a) Remove a QUIC_BUG in quic::PacketNumberQueue::AddRange. Test only. Not protected.
Note this changes the semantics of AddRange, previously we do not allow adding ranges in the middle, now we do. It is safe because this function is only used by tests.
PiperOrigin-RevId: 278747688
Change-Id: Ic5349132cab019f893d68b568c60010a217bfd7c
diff --git a/quic/core/frames/quic_ack_frame.cc b/quic/core/frames/quic_ack_frame.cc
index 72e806c..2946c01 100644
--- a/quic/core/frames/quic_ack_frame.cc
+++ b/quic/core/frames/quic_ack_frame.cc
@@ -89,19 +89,7 @@
return;
}
- const auto new_interval = QuicInterval<QuicPacketNumber>(lower, higher);
-
- if (!packet_number_intervals_.Empty() &&
- packet_number_intervals_.SpanningInterval().Intersects(new_interval)) {
- // TODO(wub): Remove this QUIC_BUG, or the AddRange method entirely.
- // Ranges must be above or below all existing ranges.
- QUIC_BUG << "AddRange only supports adding packets above or below the "
- << "current min:" << Min() << " and max:" << Max()
- << ", but adding [" << lower << "," << higher << ")";
- return;
- }
-
- packet_number_intervals_.AddOptimizedForAppend(new_interval);
+ packet_number_intervals_.AddOptimizedForAppend(lower, higher);
}
bool PacketNumberQueue::RemoveUpTo(QuicPacketNumber higher) {
diff --git a/quic/core/frames/quic_ack_frame.h b/quic/core/frames/quic_ack_frame.h
index 18675a8..9003c76 100644
--- a/quic/core/frames/quic_ack_frame.h
+++ b/quic/core/frames/quic_ack_frame.h
@@ -36,8 +36,8 @@
// Adds |packet_number| to the set of packets in the queue.
void Add(QuicPacketNumber packet_number);
- // Adds packets between [lower, higher) to the set of packets in the queue. It
- // is undefined behavior to call this with |higher| < |lower|.
+ // Adds packets between [lower, higher) to the set of packets in the queue.
+ // No-op if |higher| < |lower|.
// NOTE(wub): Only used in tests as of Nov 2019.
void AddRange(QuicPacketNumber lower, QuicPacketNumber higher);
diff --git a/quic/core/frames/quic_frames_test.cc b/quic/core/frames/quic_frames_test.cc
index 63fe653..303fd0c 100644
--- a/quic/core/frames/quic_frames_test.cc
+++ b/quic/core/frames/quic_frames_test.cc
@@ -376,30 +376,27 @@
EXPECT_EQ(QuicPacketNumber(1u), ack_frame1.packets.Min());
EXPECT_EQ(QuicPacketNumber(99u), ack_frame1.packets.Max());
- std::vector<QuicInterval<QuicPacketNumber>> expected_intervals;
- expected_intervals.emplace_back(QuicInterval<QuicPacketNumber>(
- QuicPacketNumber(1), QuicPacketNumber(10)));
- expected_intervals.emplace_back(QuicInterval<QuicPacketNumber>(
- QuicPacketNumber(50), QuicPacketNumber(100)));
+ std::vector<QuicInterval<QuicPacketNumber>> expected_intervals{
+ {QuicPacketNumber(1), QuicPacketNumber(10)},
+ {QuicPacketNumber(50), QuicPacketNumber(100)},
+ };
const std::vector<QuicInterval<QuicPacketNumber>> actual_intervals(
ack_frame1.packets.begin(), ack_frame1.packets.end());
EXPECT_EQ(expected_intervals, actual_intervals);
- // Ensure adding a range within the existing ranges fails.
- EXPECT_QUIC_BUG(
- ack_frame1.packets.AddRange(QuicPacketNumber(20), QuicPacketNumber(30)),
- "");
+ // Add a range in the middle.
+ ack_frame1.packets.AddRange(QuicPacketNumber(20), QuicPacketNumber(30));
const std::vector<QuicInterval<QuicPacketNumber>> actual_intervals2(
ack_frame1.packets.begin(), ack_frame1.packets.end());
- std::vector<QuicInterval<QuicPacketNumber>> expected_intervals2;
- expected_intervals2.emplace_back(QuicInterval<QuicPacketNumber>(
- QuicPacketNumber(1), QuicPacketNumber(10)));
- expected_intervals2.emplace_back(QuicInterval<QuicPacketNumber>(
- QuicPacketNumber(50), QuicPacketNumber(100)));
+ std::vector<QuicInterval<QuicPacketNumber>> expected_intervals2{
+ {QuicPacketNumber(1), QuicPacketNumber(10)},
+ {QuicPacketNumber(20), QuicPacketNumber(30)},
+ {QuicPacketNumber(50), QuicPacketNumber(100)},
+ };
EXPECT_EQ(expected_intervals2.size(), ack_frame1.packets.NumIntervals());
EXPECT_EQ(expected_intervals2, actual_intervals2);
@@ -415,17 +412,13 @@
const std::vector<QuicInterval<QuicPacketNumber>> actual_intervals8(
ack_frame2.packets.begin(), ack_frame2.packets.end());
- std::vector<QuicInterval<QuicPacketNumber>> expected_intervals8;
- expected_intervals8.emplace_back(QuicInterval<QuicPacketNumber>(
- QuicPacketNumber(10), QuicPacketNumber(15)));
- expected_intervals8.emplace_back(QuicInterval<QuicPacketNumber>(
- QuicPacketNumber(20), QuicPacketNumber(25)));
- expected_intervals8.emplace_back(QuicInterval<QuicPacketNumber>(
- QuicPacketNumber(40), QuicPacketNumber(45)));
- expected_intervals8.emplace_back(QuicInterval<QuicPacketNumber>(
- QuicPacketNumber(60), QuicPacketNumber(65)));
- expected_intervals8.emplace_back(QuicInterval<QuicPacketNumber>(
- QuicPacketNumber(80), QuicPacketNumber(85)));
+ std::vector<QuicInterval<QuicPacketNumber>> expected_intervals8{
+ {QuicPacketNumber(10), QuicPacketNumber(15)},
+ {QuicPacketNumber(20), QuicPacketNumber(25)},
+ {QuicPacketNumber(40), QuicPacketNumber(45)},
+ {QuicPacketNumber(60), QuicPacketNumber(65)},
+ {QuicPacketNumber(80), QuicPacketNumber(85)},
+ };
EXPECT_EQ(expected_intervals8, actual_intervals8);
}
