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