Change `QuicUnackedPacketMap::NotifyFramesAcked` and `QuicUnackedPacketMap::MaybeAggregateAckedStreamFrame` to take `QuicTransmissionInfo*&` as a input-output parameter.
Both functions can potentially invalidate the existing `QuicTransmissionInfo` parameter, if they end up appending element to the unacked map. With this change, both functions will update the `QuicTransmissionInfo` before they return.
Protected by quic_reloadable_flag_quic_update_transmission_info_on_frame_acked.
PiperOrigin-RevId: 695497393
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h
index b4f0c7b..dbdcb4b 100755
--- a/quiche/common/quiche_feature_flags_list.h
+++ b/quiche/common/quiche_feature_flags_list.h
@@ -53,6 +53,7 @@
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_test_peer_addr_change_after_normalize, false, false, "If true, QuicConnection::ProcessValidatedPacket will use normalized address to test peer address changes.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_false, false, false, "A testonly reloadable flag that will always default to false.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_true, true, true, "A testonly reloadable flag that will always default to true.")
+QUICHE_FLAG(bool, quiche_reloadable_flag_quic_update_transmission_info_on_frame_acked, false, true, "If true, QuicUnackedPacketMap will update transmission info after session_notifier_->OnFrameAcked.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_use_alarm_multiplexer, false, false, "Manages all of the connection alarms via QuicAlarmMultiplexer.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_use_received_client_addresses_cache, true, true, "If true, use a LRU cache to record client addresses of packets received on server's original address.")
QUICHE_FLAG(bool, quiche_restart_flag_quic_support_ect1, false, false, "When true, allows sending of QUIC packets marked ECT(1). A different flag (TBD) will actually utilize this capability to send ECT(1).")
diff --git a/quiche/quic/core/frames/quic_frame.cc b/quiche/quic/core/frames/quic_frame.cc
index 6002ecd..f798703 100644
--- a/quiche/quic/core/frames/quic_frame.cc
+++ b/quiche/quic/core/frames/quic_frame.cc
@@ -177,6 +177,15 @@
}
}
+bool HasMessageFrame(const QuicFrames& frames) {
+ for (const QuicFrame& frame : frames) {
+ if (frame.type == MESSAGE_FRAME) {
+ return true;
+ }
+ }
+ return false;
+}
+
bool IsControlFrame(QuicFrameType type) {
switch (type) {
case RST_STREAM_FRAME:
diff --git a/quiche/quic/core/frames/quic_frame.h b/quiche/quic/core/frames/quic_frame.h
index dc99fc4..ffb9f77 100644
--- a/quiche/quic/core/frames/quic_frame.h
+++ b/quiche/quic/core/frames/quic_frame.h
@@ -146,6 +146,9 @@
QUICHE_EXPORT void RemoveFramesForStream(QuicFrames* frames,
QuicStreamId stream_id);
+// Returns true if |frames| contains at least one message frame.
+QUICHE_EXPORT bool HasMessageFrame(const QuicFrames& frames);
+
// Returns true if |type| is a retransmittable control frame.
QUICHE_EXPORT bool IsControlFrame(QuicFrameType type);
diff --git a/quiche/quic/core/frames/quic_frames_test.cc b/quiche/quic/core/frames/quic_frames_test.cc
index 37f45cc..7179d18 100644
--- a/quiche/quic/core/frames/quic_frames_test.cc
+++ b/quiche/quic/core/frames/quic_frames_test.cc
@@ -889,6 +889,19 @@
EXPECT_EQ(QuicPacketNumber(49u), queue.Max());
}
+TEST_F(QuicFramesTest, HasMessageFrame) {
+ QuicFrames frames;
+
+ frames.push_back(QuicFrame(QuicHandshakeDoneFrame()));
+ EXPECT_FALSE(HasMessageFrame(frames));
+
+ frames.push_back(
+ QuicFrame(new QuicMessageFrame(1, MemSliceFromString("message"))));
+ EXPECT_TRUE(HasMessageFrame(frames));
+
+ DeleteFrames(&frames);
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quiche/quic/core/quic_sent_packet_manager.cc b/quiche/quic/core/quic_sent_packet_manager.cc
index 5c4e897..c713350 100644
--- a/quiche/quic/core/quic_sent_packet_manager.cc
+++ b/quiche/quic/core/quic_sent_packet_manager.cc
@@ -573,10 +573,10 @@
}
void QuicSentPacketManager::MarkPacketHandled(QuicPacketNumber packet_number,
- QuicTransmissionInfo* info,
QuicTime ack_receive_time,
QuicTime::Delta ack_delay_time,
- QuicTime receive_timestamp) {
+ QuicTime receive_timestamp,
+ QuicTransmissionInfo*& info) {
if (info->has_ack_frequency) {
for (const auto& frame : info->retransmittable_frames) {
if (frame.type == ACK_FREQUENCY_FRAME) {
@@ -587,12 +587,17 @@
// Try to aggregate acked stream frames if acked packet is not a
// retransmission.
if (info->transmission_type == NOT_RETRANSMISSION) {
- unacked_packets_.MaybeAggregateAckedStreamFrame(*info, ack_delay_time,
- receive_timestamp);
+ unacked_packets_.MaybeAggregateAckedStreamFrame(
+ packet_number, ack_delay_time, receive_timestamp, info);
} else {
unacked_packets_.NotifyAggregatedStreamFrameAcked(ack_delay_time);
+ if (unacked_packets_.update_transmission_info_on_frame_acked()) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_update_transmission_info_on_frame_acked,
+ 1, 3);
+ info = unacked_packets_.GetMutableTransmissionInfo(packet_number);
+ }
const bool new_data_acked = unacked_packets_.NotifyFramesAcked(
- *info, ack_delay_time, receive_timestamp);
+ packet_number, ack_delay_time, receive_timestamp, info);
if (!new_data_acked && info->transmission_type != NOT_RETRANSMISSION) {
// Record as a spurious retransmission if this packet is a
// retransmission and no new data gets acked.
@@ -1451,9 +1456,9 @@
}
unacked_packets_.MaybeUpdateLargestAckedOfPacketNumberSpace(
packet_number_space, acked_packet.packet_number);
- MarkPacketHandled(acked_packet.packet_number, info, ack_receive_time,
+ MarkPacketHandled(acked_packet.packet_number, ack_receive_time,
last_ack_frame_.ack_delay_time,
- acked_packet.receive_timestamp);
+ acked_packet.receive_timestamp, info);
}
// Copy raw ECN counts to last_ack_frame_ so it is logged properly. Validated
// ECN counts are stored in valid_ecn_counts, and the congestion controller
diff --git a/quiche/quic/core/quic_sent_packet_manager.h b/quiche/quic/core/quic_sent_packet_manager.h
index faebd06..43176c3 100644
--- a/quiche/quic/core/quic_sent_packet_manager.h
+++ b/quiche/quic/core/quic_sent_packet_manager.h
@@ -545,10 +545,14 @@
// Removes the retransmittability and in flight properties from the packet at
// |info| due to receipt by the peer.
+ // Note: The function may cause the QuicTransmissionInfo for |packet_number|
+ // to move, in that case, |info| will be updated to point to the new
+ // QuicTransmissionInfo when the function returns.
void MarkPacketHandled(QuicPacketNumber packet_number,
- QuicTransmissionInfo* info, QuicTime ack_receive_time,
+ QuicTime ack_receive_time,
QuicTime::Delta ack_delay_time,
- QuicTime receive_timestamp);
+ QuicTime receive_timestamp,
+ QuicTransmissionInfo*& info);
// Request that |packet_number| be retransmitted after the other pending
// retransmissions. Does not add it to the retransmissions if it's already
diff --git a/quiche/quic/core/quic_unacked_packet_map.cc b/quiche/quic/core/quic_unacked_packet_map.cc
index 04d184b..d3e3ed3 100644
--- a/quiche/quic/core/quic_unacked_packet_map.cc
+++ b/quiche/quic/core/quic_unacked_packet_map.cc
@@ -9,13 +9,16 @@
#include <type_traits>
#include <utility>
+#include "absl/cleanup/cleanup.h"
#include "absl/container/inlined_vector.h"
+#include "quiche/quic/core/frames/quic_frame.h"
#include "quiche/quic/core/quic_connection_stats.h"
#include "quiche/quic/core/quic_packet_number.h"
#include "quiche/quic/core/quic_types.h"
#include "quiche/quic/core/quic_utils.h"
#include "quiche/quic/platform/api/quic_bug_tracker.h"
#include "quiche/quic/platform/api/quic_flag_utils.h"
+#include "quiche/common/simple_buffer_allocator.h"
namespace quic {
@@ -377,8 +380,10 @@
// Notify session that the data has been delivered (but do not notify
// send algorithm).
// TODO(b/148868195): use NotifyFramesNeutered.
- NotifyFramesAcked(*it, QuicTime::Delta::Zero(), QuicTime::Zero());
- QUICHE_DCHECK(!HasRetransmittableFrames(*it));
+ QuicTransmissionInfo* info = &(*it);
+ NotifyFramesAcked(packet_number, QuicTime::Delta::Zero(),
+ QuicTime::Zero(), info);
+ QUICHE_DCHECK(!HasRetransmittableFrames(*info));
}
}
QUICHE_DCHECK(!supports_multiple_packet_number_spaces_ ||
@@ -402,7 +407,9 @@
it->state = NEUTERED;
neutered_packets.push_back(packet_number);
// TODO(b/148868195): use NotifyFramesNeutered.
- NotifyFramesAcked(*it, QuicTime::Delta::Zero(), QuicTime::Zero());
+ QuicTransmissionInfo* info = &(*it);
+ NotifyFramesAcked(packet_number, QuicTime::Delta::Zero(),
+ QuicTime::Zero(), info);
}
}
QUICHE_DCHECK(!supports_multiple_packet_number_spaces() ||
@@ -482,14 +489,35 @@
session_notifier_ = session_notifier;
}
-bool QuicUnackedPacketMap::NotifyFramesAcked(const QuicTransmissionInfo& info,
+bool QuicUnackedPacketMap::NotifyFramesAcked(QuicPacketNumber packet_number,
QuicTime::Delta ack_delay,
- QuicTime receive_timestamp) {
+ QuicTime receive_timestamp,
+ QuicTransmissionInfo*& info) {
if (session_notifier_ == nullptr) {
return false;
}
bool new_data_acked = false;
- for (const QuicFrame& frame : info.retransmittable_frames) {
+ const QuicFrames* frames = &info->retransmittable_frames;
+ quiche::SimpleBufferAllocator allocator;
+ std::optional<QuicFrames> frames_copy;
+ const bool use_copied_frames = update_transmission_info_on_frame_acked_ &&
+ !HasMessageFrame(info->retransmittable_frames);
+
+ if (use_copied_frames) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_update_transmission_info_on_frame_acked,
+ 2, 3);
+ frames = &frames_copy.emplace(
+ CopyQuicFrames(&allocator, info->retransmittable_frames));
+ }
+
+ absl::Cleanup cleanup = [&]() {
+ if (use_copied_frames) {
+ DeleteFrames(&frames_copy.value());
+ info = GetMutableTransmissionInfo(packet_number);
+ }
+ };
+
+ for (const QuicFrame& frame : *frames) {
if (session_notifier_->OnFrameAcked(frame, ack_delay, receive_timestamp)) {
new_data_acked = true;
}
@@ -510,12 +538,32 @@
}
void QuicUnackedPacketMap::MaybeAggregateAckedStreamFrame(
- const QuicTransmissionInfo& info, QuicTime::Delta ack_delay,
- QuicTime receive_timestamp) {
+ QuicPacketNumber packet_number, QuicTime::Delta ack_delay,
+ QuicTime receive_timestamp, QuicTransmissionInfo*& info) {
if (session_notifier_ == nullptr) {
return;
}
- for (const auto& frame : info.retransmittable_frames) {
+ const QuicFrames* frames = &info->retransmittable_frames;
+ quiche::SimpleBufferAllocator allocator;
+ std::optional<QuicFrames> frames_copy;
+ const bool use_copied_frames = update_transmission_info_on_frame_acked_ &&
+ !HasMessageFrame(info->retransmittable_frames);
+
+ if (use_copied_frames) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_update_transmission_info_on_frame_acked,
+ 3, 3);
+ frames = &frames_copy.emplace(
+ CopyQuicFrames(&allocator, info->retransmittable_frames));
+ }
+
+ absl::Cleanup cleanup = [&]() {
+ if (use_copied_frames) {
+ DeleteFrames(&frames_copy.value());
+ info = GetMutableTransmissionInfo(packet_number);
+ }
+ };
+
+ for (const auto& frame : *frames) {
// Determine whether acked stream frame can be aggregated.
const bool can_aggregate =
frame.type == STREAM_FRAME &&
diff --git a/quiche/quic/core/quic_unacked_packet_map.h b/quiche/quic/core/quic_unacked_packet_map.h
index 74d74af..52c6103 100644
--- a/quiche/quic/core/quic_unacked_packet_map.h
+++ b/quiche/quic/core/quic_unacked_packet_map.h
@@ -51,10 +51,14 @@
// Returns true if the packet |packet_number| is unacked.
bool IsUnacked(QuicPacketNumber packet_number) const;
- // Notifies session_notifier that frames have been acked. Returns true if any
- // new data gets acked, returns false otherwise.
- bool NotifyFramesAcked(const QuicTransmissionInfo& info,
- QuicTime::Delta ack_delay, QuicTime receive_timestamp);
+ // Notifies session_notifier that frames in |packet_number| have been acked.
+ // Returns true if any new data gets acked, returns false otherwise.
+ // Note: The function may cause the QuicTransmissionInfo for |packet_number|
+ // to move, in that case, |info| will be updated to point to the new
+ // QuicTransmissionInfo when the function returns.
+ bool NotifyFramesAcked(QuicPacketNumber packet_number,
+ QuicTime::Delta ack_delay, QuicTime receive_timestamp,
+ QuicTransmissionInfo*& info);
// Notifies session_notifier that frames in |info| are considered as lost.
void NotifyFramesLost(const QuicTransmissionInfo& info,
@@ -186,13 +190,21 @@
// Try to aggregate acked contiguous stream frames. For noncontiguous stream
// frames or control frames, notify the session notifier they get acked
// immediately.
- void MaybeAggregateAckedStreamFrame(const QuicTransmissionInfo& info,
+ // Note: The function may cause the QuicTransmissionInfo for |packet_number|
+ // to move, in that case, |info| will be updated to point to the new
+ // QuicTransmissionInfo when the function returns.
+ void MaybeAggregateAckedStreamFrame(QuicPacketNumber packet_number,
QuicTime::Delta ack_delay,
- QuicTime receive_timestamp);
+ QuicTime receive_timestamp,
+ QuicTransmissionInfo*& info);
// Notify the session notifier of any stream data aggregated in
// aggregated_stream_frame_. No effect if the stream frame has an invalid
// stream id.
+ // Note: If the caller holds a reference to a QuicTransmissionInfo stored in
+ // unacked_packets_, the reference may be invalidated by this call, so after
+ // the call, caller should use GetTransmissionInfo() or
+ // GetMutableTransmissionInfo() to get a new reference.
void NotifyAggregatedStreamFrameAcked(QuicTime::Delta ack_delay);
// Returns packet number space that |packet_number| belongs to. Please use
@@ -259,6 +271,10 @@
", packets_in_flight: ", packets_in_flight_, "}");
}
+ bool update_transmission_info_on_frame_acked() const {
+ return update_transmission_info_on_frame_acked_;
+ }
+
private:
friend class test::QuicUnackedPacketMapPeer;
@@ -332,6 +348,9 @@
// Latched value of the quic_simple_inflight_time flag.
bool simple_inflight_time_;
+
+ const bool update_transmission_info_on_frame_acked_ =
+ GetQuicReloadableFlag(quic_update_transmission_info_on_frame_acked);
};
} // namespace quic
diff --git a/quiche/quic/core/quic_unacked_packet_map_test.cc b/quiche/quic/core/quic_unacked_packet_map_test.cc
index 0a3d8b8..ce2f413 100644
--- a/quiche/quic/core/quic_unacked_packet_map_test.cc
+++ b/quiche/quic/core/quic_unacked_packet_map_test.cc
@@ -4,7 +4,10 @@
#include "quiche/quic/core/quic_unacked_packet_map.h"
+#include <stdbool.h>
+
#include <cstddef>
+#include <cstdint>
#include <limits>
#include <vector>
@@ -18,6 +21,7 @@
#include "quiche/quic/test_tools/quic_unacked_packet_map_peer.h"
using testing::_;
+using testing::Invoke;
using testing::Return;
using testing::StrictMock;
@@ -50,15 +54,31 @@
SerializedPacket CreateRetransmittablePacketForStream(
uint64_t packet_number, QuicStreamId stream_id) {
+ return CreateRetransmittablePacketForStream(packet_number, stream_id,
+ /*fin=*/false, /*offset=*/0,
+ /*data_length=*/0);
+ }
+
+ SerializedPacket CreateRetransmittablePacketForStream(
+ uint64_t packet_number, QuicStreamId stream_id, bool fin,
+ QuicStreamOffset offset, QuicPacketLength data_length) {
SerializedPacket packet(QuicPacketNumber(packet_number),
PACKET_1BYTE_PACKET_NUMBER, nullptr, kDefaultLength,
false, false);
- QuicStreamFrame frame;
- frame.stream_id = stream_id;
+ QuicStreamFrame frame(stream_id, fin, offset, data_length);
packet.retransmittable_frames.push_back(QuicFrame(frame));
return packet;
}
+ SerializedPacket CreateRetransmittablePacket(
+ uint64_t packet_number, QuicFrames retransmittable_frames) {
+ SerializedPacket packet(QuicPacketNumber(packet_number),
+ PACKET_1BYTE_PACKET_NUMBER, nullptr, kDefaultLength,
+ false, false);
+ packet.retransmittable_frames = std::move(retransmittable_frames);
+ return packet;
+ }
+
SerializedPacket CreateNonRetransmittablePacket(uint64_t packet_number) {
return SerializedPacket(QuicPacketNumber(packet_number),
PACKET_1BYTE_PACKET_NUMBER, nullptr, kDefaultLength,
@@ -460,37 +480,50 @@
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
unacked_packets_.NotifyAggregatedStreamFrameAcked(QuicTime::Delta::Zero());
- QuicTransmissionInfo info1;
- QuicStreamFrame stream_frame1(3, false, 0, 100);
- info1.retransmittable_frames.push_back(QuicFrame(stream_frame1));
+ SerializedPacket packet1(
+ CreateRetransmittablePacketForStream(1, 3, false, 0, 100));
+ unacked_packets_.AddSentPacket(&packet1, NOT_RETRANSMISSION, now_, true, true,
+ ECN_NOT_ECT);
- QuicTransmissionInfo info2;
- QuicStreamFrame stream_frame2(3, false, 100, 100);
- info2.retransmittable_frames.push_back(QuicFrame(stream_frame2));
+ SerializedPacket packet2(
+ CreateRetransmittablePacketForStream(2, 3, false, 100, 100));
+ unacked_packets_.AddSentPacket(&packet2, NOT_RETRANSMISSION, now_, true, true,
+ ECN_NOT_ECT);
- QuicTransmissionInfo info3;
- QuicStreamFrame stream_frame3(3, false, 200, 100);
- info3.retransmittable_frames.push_back(QuicFrame(stream_frame3));
+ SerializedPacket packet3(
+ CreateRetransmittablePacketForStream(3, 3, false, 200, 100));
+ unacked_packets_.AddSentPacket(&packet3, NOT_RETRANSMISSION, now_, true, true,
+ ECN_NOT_ECT);
- QuicTransmissionInfo info4;
- QuicStreamFrame stream_frame4(3, true, 300, 0);
- info4.retransmittable_frames.push_back(QuicFrame(stream_frame4));
+ SerializedPacket packet4(
+ CreateRetransmittablePacketForStream(4, 3, true, 300, 0));
+ unacked_packets_.AddSentPacket(&packet4, NOT_RETRANSMISSION, now_, true, true,
+ ECN_NOT_ECT);
+
+ QuicTransmissionInfo* info1 =
+ unacked_packets_.GetMutableTransmissionInfo(QuicPacketNumber(1));
+ QuicTransmissionInfo* info2 =
+ unacked_packets_.GetMutableTransmissionInfo(QuicPacketNumber(2));
+ QuicTransmissionInfo* info3 =
+ unacked_packets_.GetMutableTransmissionInfo(QuicPacketNumber(3));
+ QuicTransmissionInfo* info4 =
+ unacked_packets_.GetMutableTransmissionInfo(QuicPacketNumber(4));
// Verify stream frames are aggregated.
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info1, QuicTime::Delta::Zero(), QuicTime::Zero());
+ QuicPacketNumber(1), QuicTime::Delta::Zero(), QuicTime::Zero(), info1);
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info2, QuicTime::Delta::Zero(), QuicTime::Zero());
+ QuicPacketNumber(2), QuicTime::Delta::Zero(), QuicTime::Zero(), info2);
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info3, QuicTime::Delta::Zero(), QuicTime::Zero());
+ QuicPacketNumber(3), QuicTime::Delta::Zero(), QuicTime::Zero(), info3);
// Verify aggregated stream frame gets acked since fin is acked.
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(1);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info4, QuicTime::Delta::Zero(), QuicTime::Zero());
+ QuicPacketNumber(4), QuicTime::Delta::Zero(), QuicTime::Zero(), info4);
}
// Regression test for b/112930090.
@@ -498,6 +531,7 @@
QuicByteCount kMaxAggregatedDataLength =
std::numeric_limits<decltype(QuicStreamFrame().data_length)>::max();
QuicStreamId stream_id = 2;
+ uint64_t next_packet_number = 1;
// acked_stream_length=512 covers the case where a frame will cause the
// aggregated frame length to be exactly 64K.
@@ -510,10 +544,13 @@
QuicByteCount aggregated_data_length = 0;
while (offset < 1e6) {
- QuicTransmissionInfo info;
- QuicStreamFrame stream_frame(stream_id, false, offset,
- acked_stream_length);
- info.retransmittable_frames.push_back(QuicFrame(stream_frame));
+ uint64_t packet_number = next_packet_number++;
+ SerializedPacket packet(CreateRetransmittablePacketForStream(
+ packet_number, stream_id, false, offset, acked_stream_length));
+ unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, now_, true,
+ true, ECN_NOT_ECT);
+ QuicTransmissionInfo* info = unacked_packets_.GetMutableTransmissionInfo(
+ QuicPacketNumber(packet_number));
const QuicStreamFrame& aggregated_stream_frame =
QuicUnackedPacketMapPeer::GetAggregatedStreamFrame(unacked_packets_);
@@ -522,7 +559,8 @@
// Verify the acked stream frame can be aggregated.
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info, QuicTime::Delta::Zero(), QuicTime::Zero());
+ QuicPacketNumber(packet_number), QuicTime::Delta::Zero(),
+ QuicTime::Zero(), info);
aggregated_data_length += acked_stream_length;
testing::Mock::VerifyAndClearExpectations(¬ifier_);
} else {
@@ -530,7 +568,8 @@
// data_length is overflow.
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(1);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info, QuicTime::Delta::Zero(), QuicTime::Zero());
+ QuicPacketNumber(packet_number), QuicTime::Delta::Zero(),
+ QuicTime::Zero(), info);
aggregated_data_length = acked_stream_length;
testing::Mock::VerifyAndClearExpectations(¬ifier_);
}
@@ -540,12 +579,17 @@
}
// Ack the last frame of the stream.
- QuicTransmissionInfo info;
- QuicStreamFrame stream_frame(stream_id, true, offset, acked_stream_length);
- info.retransmittable_frames.push_back(QuicFrame(stream_frame));
+ uint64_t packet_number = next_packet_number++;
+ SerializedPacket packet(CreateRetransmittablePacketForStream(
+ packet_number, stream_id, true, offset, acked_stream_length));
+ unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, now_, true,
+ true, ECN_NOT_ECT);
+ QuicTransmissionInfo* info = unacked_packets_.GetMutableTransmissionInfo(
+ QuicPacketNumber(packet_number));
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(1);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info, QuicTime::Delta::Zero(), QuicTime::Zero());
+ QuicPacketNumber(packet_number), QuicTime::Delta::Zero(),
+ QuicTime::Zero(), info);
testing::Mock::VerifyAndClearExpectations(¬ifier_);
}
}
@@ -556,30 +600,88 @@
QuicStreamFrame stream_frame1(3, false, 0, 100);
QuicStreamFrame stream_frame2(3, false, 100, 100);
QuicBlockedFrame blocked(2, 5, 0);
- QuicGoAwayFrame go_away(3, QUIC_PEER_GOING_AWAY, 5, "Going away.");
- QuicTransmissionInfo info1;
- info1.retransmittable_frames.push_back(QuicFrame(window_update));
- info1.retransmittable_frames.push_back(QuicFrame(stream_frame1));
- info1.retransmittable_frames.push_back(QuicFrame(stream_frame2));
+ SerializedPacket packet1(CreateRetransmittablePacket(
+ 1, {QuicFrame(window_update), QuicFrame(stream_frame1),
+ QuicFrame(stream_frame2)}));
+ unacked_packets_.AddSentPacket(&packet1, NOT_RETRANSMISSION, now_, true, true,
+ ECN_NOT_ECT);
+ QuicTransmissionInfo* info1 =
+ unacked_packets_.GetMutableTransmissionInfo(QuicPacketNumber(1));
- QuicTransmissionInfo info2;
- info2.retransmittable_frames.push_back(QuicFrame(blocked));
- info2.retransmittable_frames.push_back(QuicFrame(&go_away));
+ SerializedPacket packet2(CreateRetransmittablePacket(
+ 2,
+ {QuicFrame(blocked), QuicFrame(new QuicGoAwayFrame(
+ 3, QUIC_PEER_GOING_AWAY, 5, "Going away."))}));
+ unacked_packets_.AddSentPacket(&packet2, NOT_RETRANSMISSION, now_, true, true,
+ ECN_NOT_ECT);
+ QuicTransmissionInfo* info2 =
+ unacked_packets_.GetMutableTransmissionInfo(QuicPacketNumber(2));
// Verify 2 contiguous stream frames are aggregated.
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(1);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info1, QuicTime::Delta::Zero(), QuicTime::Zero());
+ QuicPacketNumber(1), QuicTime::Delta::Zero(), QuicTime::Zero(), info1);
// Verify aggregated stream frame gets acked.
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(3);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info2, QuicTime::Delta::Zero(), QuicTime::Zero());
+ QuicPacketNumber(2), QuicTime::Delta::Zero(), QuicTime::Zero(), info2);
EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
unacked_packets_.NotifyAggregatedStreamFrameAcked(QuicTime::Delta::Zero());
}
+TEST_P(QuicUnackedPacketMapTest, UpdateTransmissionInfoOnFrameAcked) {
+ if (!GetQuicReloadableFlag(quic_update_transmission_info_on_frame_acked)) {
+ return;
+ }
+ uint64_t next_packet_number = 1;
+ do {
+ uint64_t packet_number = next_packet_number++;
+ SerializedPacket packet(CreateRetransmittablePacket(
+ packet_number,
+ {QuicFrame(QuicPaddingFrame(static_cast<int>(packet_number) * 100))}));
+ unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, now_, true,
+ true, ECN_NOT_ECT);
+ } while (QuicUnackedPacketMapPeer::GetCapacity(unacked_packets_) >
+ QuicUnackedPacketMapPeer::GetSize(unacked_packets_));
+ QUIC_DLOG(INFO) << "unacked_packets_ at full capacity: "
+ << QuicUnackedPacketMapPeer::GetCapacity(unacked_packets_);
+
+ QuicPacketNumber largest_sent_packet_before_acked =
+ unacked_packets_.largest_sent_packet();
+ QuicTransmissionInfo* last_info = unacked_packets_.GetMutableTransmissionInfo(
+ largest_sent_packet_before_acked);
+ int last_padding_bytes =
+ last_info->retransmittable_frames[0].padding_frame.num_padding_bytes;
+
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _))
+ .WillOnce(Invoke([&](const QuicFrame& frame, QuicTime::Delta, QuicTime) {
+ EXPECT_EQ(frame.type, PADDING_FRAME);
+ EXPECT_EQ(frame.padding_frame.num_padding_bytes, last_padding_bytes);
+ // Append one more packet to the unacked packet map.
+ SerializedPacket packet(CreateRetransmittablePacket(
+ next_packet_number++, {QuicFrame(QuicBlockedFrame(2, 5, 0))}));
+ unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, now_, true,
+ true, ECN_NOT_ECT);
+ return true;
+ }));
+
+ QuicTransmissionInfo* last_info_updated = last_info;
+ unacked_packets_.NotifyFramesAcked(largest_sent_packet_before_acked,
+ QuicTime::Delta::Zero(), QuicTime::Zero(),
+ last_info_updated);
+ EXPECT_NE(last_info, last_info_updated);
+ EXPECT_EQ(unacked_packets_.GetMutableTransmissionInfo(
+ largest_sent_packet_before_acked),
+ last_info_updated);
+ ASSERT_EQ(last_info_updated->retransmittable_frames.size(), 1u);
+ EXPECT_EQ(last_info_updated->retransmittable_frames[0].type, PADDING_FRAME);
+ EXPECT_EQ(last_info_updated->retransmittable_frames[0]
+ .padding_frame.num_padding_bytes,
+ last_padding_bytes);
+}
+
TEST_P(QuicUnackedPacketMapTest, LargestSentPacketMultiplePacketNumberSpaces) {
unacked_packets_.EnableMultiplePacketNumberSpacesSupport();
EXPECT_FALSE(
diff --git a/quiche/quic/test_tools/quic_unacked_packet_map_peer.cc b/quiche/quic/test_tools/quic_unacked_packet_map_peer.cc
index 4df89b0..0375c04 100644
--- a/quiche/quic/test_tools/quic_unacked_packet_map_peer.cc
+++ b/quiche/quic/test_tools/quic_unacked_packet_map_peer.cc
@@ -7,23 +7,25 @@
namespace quic {
namespace test {
-// static
const QuicStreamFrame& QuicUnackedPacketMapPeer::GetAggregatedStreamFrame(
const QuicUnackedPacketMap& unacked_packets) {
return unacked_packets.aggregated_stream_frame_;
}
-// static
void QuicUnackedPacketMapPeer::SetPerspective(
QuicUnackedPacketMap* unacked_packets, Perspective perspective) {
*const_cast<Perspective*>(&unacked_packets->perspective_) = perspective;
}
-// static
size_t QuicUnackedPacketMapPeer::GetCapacity(
const QuicUnackedPacketMap& unacked_packets) {
return unacked_packets.unacked_packets_.capacity();
}
+size_t QuicUnackedPacketMapPeer::GetSize(
+ const QuicUnackedPacketMap& unacked_packets) {
+ return unacked_packets.unacked_packets_.size();
+}
+
} // namespace test
} // namespace quic
diff --git a/quiche/quic/test_tools/quic_unacked_packet_map_peer.h b/quiche/quic/test_tools/quic_unacked_packet_map_peer.h
index 5525506..ce4d87e 100644
--- a/quiche/quic/test_tools/quic_unacked_packet_map_peer.h
+++ b/quiche/quic/test_tools/quic_unacked_packet_map_peer.h
@@ -19,6 +19,8 @@
Perspective perspective);
static size_t GetCapacity(const QuicUnackedPacketMap& unacked_packets);
+
+ static size_t GetSize(const QuicUnackedPacketMap& unacked_packets);
};
} // namespace test
