gfe-relnote: Pass receive_timestamp to OnMessageAcked().
When receive timestamps are turned on and message frames are enabled, this will
make receive timestamps available to the session whenever a message is acked.
This will be used for RTP over QUIC, where ack timestamps are required for RTP's
congestion controller and media adaptation. (The current implementation uses
QUIC like a tunnel and disables its congestion control.)
Not flag-protected: no change in behavior, just a change in what is visible
where.
PiperOrigin-RevId: 250703980
Change-Id: I01bcee29d5dfb808771c54e4fd14ebbfa8750375
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index 60e7430..b6e59a5 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -1406,17 +1406,17 @@
session_->OnStreamFrameRetransmitted(frame1);
EXPECT_CALL(*ack_listener1, OnPacketAcked(7, _));
- EXPECT_TRUE(
- session_->OnFrameAcked(QuicFrame(frame1), QuicTime::Delta::Zero()));
+ EXPECT_TRUE(session_->OnFrameAcked(QuicFrame(frame1), QuicTime::Delta::Zero(),
+ QuicTime::Zero()));
EXPECT_CALL(*ack_listener1, OnPacketAcked(5, _));
- EXPECT_TRUE(
- session_->OnFrameAcked(QuicFrame(frame2), QuicTime::Delta::Zero()));
+ EXPECT_TRUE(session_->OnFrameAcked(QuicFrame(frame2), QuicTime::Delta::Zero(),
+ QuicTime::Zero()));
EXPECT_CALL(*ack_listener2, OnPacketAcked(7, _));
- EXPECT_TRUE(
- session_->OnFrameAcked(QuicFrame(frame3), QuicTime::Delta::Zero()));
+ EXPECT_TRUE(session_->OnFrameAcked(QuicFrame(frame3), QuicTime::Delta::Zero(),
+ QuicTime::Zero()));
EXPECT_CALL(*ack_listener2, OnPacketAcked(5, _));
- EXPECT_TRUE(
- session_->OnFrameAcked(QuicFrame(frame4), QuicTime::Delta::Zero()));
+ EXPECT_TRUE(session_->OnFrameAcked(QuicFrame(frame4), QuicTime::Delta::Zero(),
+ QuicTime::Zero()));
}
TEST_P(QuicSpdyStreamTest, StreamBecomesZombieWithWriteThatCloses) {
@@ -1631,20 +1631,20 @@
EXPECT_CALL(*mock_ack_listener, OnPacketAcked(body.length(), _));
QuicStreamFrame frame(stream_->id(), false, 0, header + body);
- EXPECT_TRUE(
- session_->OnFrameAcked(QuicFrame(frame), QuicTime::Delta::Zero()));
+ EXPECT_TRUE(session_->OnFrameAcked(QuicFrame(frame), QuicTime::Delta::Zero(),
+ QuicTime::Zero()));
EXPECT_CALL(*mock_ack_listener, OnPacketAcked(0, _));
QuicStreamFrame frame2(stream_->id(), false, (header + body).length(),
header2);
- EXPECT_TRUE(
- session_->OnFrameAcked(QuicFrame(frame2), QuicTime::Delta::Zero()));
+ EXPECT_TRUE(session_->OnFrameAcked(QuicFrame(frame2), QuicTime::Delta::Zero(),
+ QuicTime::Zero()));
EXPECT_CALL(*mock_ack_listener, OnPacketAcked(body2.length(), _));
QuicStreamFrame frame3(stream_->id(), true,
(header + body).length() + header2.length(), body2);
- EXPECT_TRUE(
- session_->OnFrameAcked(QuicFrame(frame3), QuicTime::Delta::Zero()));
+ EXPECT_TRUE(session_->OnFrameAcked(QuicFrame(frame3), QuicTime::Delta::Zero(),
+ QuicTime::Zero()));
EXPECT_TRUE(
QuicSpdyStreamPeer::unacked_frame_headers_offsets(stream_).Empty());
@@ -1688,8 +1688,8 @@
OnPacketAcked(body.length() + body2.length(), _));
QuicStreamFrame frame(stream_->id(), true, 0,
header + body + header2 + body2);
- EXPECT_TRUE(
- session_->OnFrameAcked(QuicFrame(frame), QuicTime::Delta::Zero()));
+ EXPECT_TRUE(session_->OnFrameAcked(QuicFrame(frame), QuicTime::Delta::Zero(),
+ QuicTime::Zero()));
EXPECT_TRUE(
QuicSpdyStreamPeer::unacked_frame_headers_offsets(stream_).Empty());
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index 527f820..204c020 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -610,7 +610,8 @@
void QuicSentPacketManager::MarkPacketHandled(QuicPacketNumber packet_number,
QuicTransmissionInfo* info,
- QuicTime::Delta ack_delay_time) {
+ QuicTime::Delta ack_delay_time,
+ QuicTime receive_timestamp) {
QuicPacketNumber newest_transmission =
GetNewestRetransmission(packet_number, *info);
// Remove the most recent packet, if it is pending retransmission.
@@ -622,13 +623,14 @@
const bool fast_path = session_decides_what_to_write() &&
info->transmission_type == NOT_RETRANSMISSION;
if (fast_path) {
- unacked_packets_.MaybeAggregateAckedStreamFrame(*info, ack_delay_time);
+ unacked_packets_.MaybeAggregateAckedStreamFrame(*info, ack_delay_time,
+ receive_timestamp);
} else {
if (session_decides_what_to_write()) {
unacked_packets_.NotifyAggregatedStreamFrameAcked(ack_delay_time);
}
- const bool new_data_acked =
- unacked_packets_.NotifyFramesAcked(*info, ack_delay_time);
+ const bool new_data_acked = unacked_packets_.NotifyFramesAcked(
+ *info, ack_delay_time, receive_timestamp);
if (session_decides_what_to_write() && !new_data_acked &&
info->transmission_type != NOT_RETRANSMISSION) {
// Record as a spurious retransmission if this packet is a
@@ -651,8 +653,8 @@
// only handle nullptr encrypted packets in a special way.
const QuicTransmissionInfo& newest_transmission_info =
unacked_packets_.GetTransmissionInfo(newest_transmission);
- unacked_packets_.NotifyFramesAcked(newest_transmission_info,
- ack_delay_time);
+ unacked_packets_.NotifyFramesAcked(newest_transmission_info, ack_delay_time,
+ receive_timestamp);
if (HasCryptoHandshake(newest_transmission_info)) {
unacked_packets_.RemoveFromInFlight(newest_transmission);
}
@@ -1241,7 +1243,8 @@
packet_number_space, acked_packet.packet_number);
}
MarkPacketHandled(acked_packet.packet_number, info,
- last_ack_frame_.ack_delay_time);
+ last_ack_frame_.ack_delay_time,
+ acked_packet.receive_timestamp);
}
const bool acked_new_packet = !packets_acked_.empty();
PostProcessNewlyAckedPackets(last_ack_frame_, ack_receive_time, rtt_updated_,
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index 3df874d..787bf8d 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -481,7 +481,8 @@
// |info| due to receipt by the peer.
void MarkPacketHandled(QuicPacketNumber packet_number,
QuicTransmissionInfo* info,
- QuicTime::Delta ack_delay_time);
+ QuicTime::Delta ack_delay_time,
+ QuicTime receive_timestamp);
// 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/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc
index c508cca..a0746fe 100644
--- a/quic/core/quic_sent_packet_manager_test.cc
+++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -110,7 +110,7 @@
EXPECT_CALL(notifier_, HasUnackedCryptoData())
.WillRepeatedly(Return(false));
EXPECT_CALL(notifier_, OnStreamFrameRetransmitted(_)).Times(AnyNumber());
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).WillRepeatedly(Return(true));
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).WillRepeatedly(Return(true));
}
~QuicSentPacketManagerTest() override {}
@@ -469,7 +469,7 @@
VerifyRetransmittablePackets(nullptr, 0);
if (manager_.session_decides_what_to_write()) {
// Ack 2 causes 2 be considered as spurious retransmission.
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).WillOnce(Return(false));
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).WillOnce(Return(false));
ExpectAck(2);
manager_.OnAckFrameStart(QuicPacketNumber(2), QuicTime::Delta::Infinite(),
clock_.Now());
@@ -623,7 +623,7 @@
SendDataPacket(4);
if (manager_.session_decides_what_to_write()) {
// No new data gets acked in packet 3.
- EXPECT_CALL(notifier_, OnFrameAcked(_, _))
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _))
.WillOnce(Return(false))
.WillRepeatedly(Return(true));
}
@@ -727,7 +727,7 @@
// data gets acked.
ExpectAck(5);
EXPECT_CALL(*loss_algorithm, DetectLosses(_, _, _, _, _, _));
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).WillOnce(Return(false));
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).WillOnce(Return(false));
manager_.OnAckFrameStart(QuicPacketNumber(5), QuicTime::Delta::Infinite(),
clock_.Now());
manager_.OnAckRange(QuicPacketNumber(3), QuicPacketNumber(6));
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 89e08ef..9707c34 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -1582,9 +1582,10 @@
}
bool QuicSession::OnFrameAcked(const QuicFrame& frame,
- QuicTime::Delta ack_delay_time) {
+ QuicTime::Delta ack_delay_time,
+ QuicTime receive_timestamp) {
if (frame.type == MESSAGE_FRAME) {
- OnMessageAcked(frame.message_frame->message_id);
+ OnMessageAcked(frame.message_frame->message_id, receive_timestamp);
return true;
}
if (frame.type == CRYPTO_FRAME) {
@@ -1839,7 +1840,8 @@
return {result, 0};
}
-void QuicSession::OnMessageAcked(QuicMessageId message_id) {
+void QuicSession::OnMessageAcked(QuicMessageId message_id,
+ QuicTime receive_timestamp) {
QUIC_DVLOG(1) << ENDPOINT << "message " << message_id << " gets acked.";
}
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index 5988369..32dd680 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -142,7 +142,8 @@
// SessionNotifierInterface methods:
bool OnFrameAcked(const QuicFrame& frame,
- QuicTime::Delta ack_delay_time) override;
+ QuicTime::Delta ack_delay_time,
+ QuicTime receive_timestamp) override;
void OnStreamFrameRetransmitted(const QuicStreamFrame& frame) override;
void OnFrameLost(const QuicFrame& frame) override;
void RetransmitFrames(const QuicFrames& frames,
@@ -189,7 +190,8 @@
MessageResult SendMessage(QuicMemSliceSpan message);
// Called when message with |message_id| gets acked.
- virtual void OnMessageAcked(QuicMessageId message_id);
+ virtual void OnMessageAcked(QuicMessageId message_id,
+ QuicTime receive_timestamp);
// Called when message with |message_id| is considered as lost.
virtual void OnMessageLost(QuicMessageId message_id);
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index 4e3228e..c8a0a02 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -2209,7 +2209,7 @@
EXPECT_FALSE(session_.IsFrameOutstanding(QuicFrame(&frame2)));
// message 1 gets acked.
- session_.OnMessageAcked(1);
+ session_.OnMessageAcked(1, QuicTime::Zero());
EXPECT_FALSE(session_.IsFrameOutstanding(QuicFrame(&frame)));
}
diff --git a/quic/core/quic_unacked_packet_map.cc b/quic/core/quic_unacked_packet_map.cc
index 1a0b1af..5b3de44 100644
--- a/quic/core/quic_unacked_packet_map.cc
+++ b/quic/core/quic_unacked_packet_map.cc
@@ -407,13 +407,14 @@
}
bool QuicUnackedPacketMap::NotifyFramesAcked(const QuicTransmissionInfo& info,
- QuicTime::Delta ack_delay) {
+ QuicTime::Delta ack_delay,
+ QuicTime receive_timestamp) {
if (session_notifier_ == nullptr) {
return false;
}
bool new_data_acked = false;
for (const QuicFrame& frame : info.retransmittable_frames) {
- if (session_notifier_->OnFrameAcked(frame, ack_delay)) {
+ if (session_notifier_->OnFrameAcked(frame, ack_delay, receive_timestamp)) {
new_data_acked = true;
}
}
@@ -436,7 +437,8 @@
void QuicUnackedPacketMap::MaybeAggregateAckedStreamFrame(
const QuicTransmissionInfo& info,
- QuicTime::Delta ack_delay) {
+ QuicTime::Delta ack_delay,
+ QuicTime receive_timestamp) {
if (session_notifier_ == nullptr) {
return;
}
@@ -468,7 +470,7 @@
NotifyAggregatedStreamFrameAcked(ack_delay);
if (frame.type != STREAM_FRAME || frame.stream_frame.fin) {
- session_notifier_->OnFrameAcked(frame, ack_delay);
+ session_notifier_->OnFrameAcked(frame, ack_delay, receive_timestamp);
continue;
}
@@ -488,8 +490,11 @@
// Aggregated stream frame is empty.
return;
}
+ // Note: there is no receive_timestamp for an aggregated stream frame. The
+ // frames that are aggregated may not have been received at the same time.
session_notifier_->OnFrameAcked(QuicFrame(aggregated_stream_frame_),
- ack_delay);
+ ack_delay,
+ /*receive_timestamp=*/QuicTime::Zero());
// Clear aggregated stream frame.
aggregated_stream_frame_.stream_id = -1;
}
diff --git a/quic/core/quic_unacked_packet_map.h b/quic/core/quic_unacked_packet_map.h
index 1a37a1f..17589dd 100644
--- a/quic/core/quic_unacked_packet_map.h
+++ b/quic/core/quic_unacked_packet_map.h
@@ -50,7 +50,8 @@
// 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::Delta ack_delay,
+ QuicTime receive_timestamp);
// Notifies session_notifier that frames in |info| are considered as lost.
void NotifyFramesLost(const QuicTransmissionInfo& info,
@@ -179,7 +180,8 @@
// frames or control frames, notify the session notifier they get acked
// immediately.
void MaybeAggregateAckedStreamFrame(const QuicTransmissionInfo& info,
- QuicTime::Delta ack_delay);
+ QuicTime::Delta ack_delay,
+ QuicTime receive_timestamp);
// Notify the session notifier of any stream data aggregated in
// aggregated_stream_frame_. No effect if the stream frame has an invalid
diff --git a/quic/core/quic_unacked_packet_map_test.cc b/quic/core/quic_unacked_packet_map_test.cc
index 331a0e8..ff26542 100644
--- a/quic/core/quic_unacked_packet_map_test.cc
+++ b/quic/core/quic_unacked_packet_map_test.cc
@@ -551,7 +551,7 @@
TEST_P(QuicUnackedPacketMapTest, AggregateContiguousAckedStreamFrames) {
testing::InSequence s;
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(0);
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
unacked_packets_.NotifyAggregatedStreamFrameAcked(QuicTime::Delta::Zero());
QuicTransmissionInfo info1;
@@ -571,20 +571,20 @@
info4.retransmittable_frames.push_back(QuicFrame(stream_frame4));
// Verify stream frames are aggregated.
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(0);
- unacked_packets_.MaybeAggregateAckedStreamFrame(info1,
- QuicTime::Delta::Zero());
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(0);
- unacked_packets_.MaybeAggregateAckedStreamFrame(info2,
- QuicTime::Delta::Zero());
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(0);
- unacked_packets_.MaybeAggregateAckedStreamFrame(info3,
- QuicTime::Delta::Zero());
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
+ unacked_packets_.MaybeAggregateAckedStreamFrame(
+ info1, QuicTime::Delta::Zero(), QuicTime::Zero());
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
+ unacked_packets_.MaybeAggregateAckedStreamFrame(
+ info2, QuicTime::Delta::Zero(), QuicTime::Zero());
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
+ unacked_packets_.MaybeAggregateAckedStreamFrame(
+ info3, QuicTime::Delta::Zero(), QuicTime::Zero());
// Verify aggregated stream frame gets acked since fin is acked.
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(1);
- unacked_packets_.MaybeAggregateAckedStreamFrame(info4,
- QuicTime::Delta::Zero());
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(1);
+ unacked_packets_.MaybeAggregateAckedStreamFrame(
+ info4, QuicTime::Delta::Zero(), QuicTime::Zero());
}
// Regression test for b/112930090.
@@ -614,17 +614,17 @@
if (aggregated_stream_frame.data_length + acked_stream_length <=
kMaxAggregatedDataLength) {
// Verify the acked stream frame can be aggregated.
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(0);
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info, QuicTime::Delta::Zero());
+ info, QuicTime::Delta::Zero(), QuicTime::Zero());
aggregated_data_length += acked_stream_length;
testing::Mock::VerifyAndClearExpectations(¬ifier_);
} else {
// Verify the acked stream frame cannot be aggregated because
// data_length is overflow.
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(1);
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(1);
unacked_packets_.MaybeAggregateAckedStreamFrame(
- info, QuicTime::Delta::Zero());
+ info, QuicTime::Delta::Zero(), QuicTime::Zero());
aggregated_data_length = acked_stream_length;
testing::Mock::VerifyAndClearExpectations(¬ifier_);
}
@@ -637,9 +637,9 @@
QuicTransmissionInfo info;
QuicStreamFrame stream_frame(stream_id, true, offset, acked_stream_length);
info.retransmittable_frames.push_back(QuicFrame(stream_frame));
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(1);
- unacked_packets_.MaybeAggregateAckedStreamFrame(info,
- QuicTime::Delta::Zero());
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(1);
+ unacked_packets_.MaybeAggregateAckedStreamFrame(
+ info, QuicTime::Delta::Zero(), QuicTime::Zero());
testing::Mock::VerifyAndClearExpectations(¬ifier_);
}
}
@@ -662,15 +662,15 @@
info2.retransmittable_frames.push_back(QuicFrame(&go_away));
// Verify 2 contiguous stream frames are aggregated.
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(1);
- unacked_packets_.MaybeAggregateAckedStreamFrame(info1,
- QuicTime::Delta::Zero());
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(1);
+ unacked_packets_.MaybeAggregateAckedStreamFrame(
+ info1, QuicTime::Delta::Zero(), QuicTime::Zero());
// Verify aggregated stream frame gets acked.
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(3);
- unacked_packets_.MaybeAggregateAckedStreamFrame(info2,
- QuicTime::Delta::Zero());
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(3);
+ unacked_packets_.MaybeAggregateAckedStreamFrame(
+ info2, QuicTime::Delta::Zero(), QuicTime::Zero());
- EXPECT_CALL(notifier_, OnFrameAcked(_, _)).Times(0);
+ EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).Times(0);
unacked_packets_.NotifyAggregatedStreamFrameAcked(QuicTime::Delta::Zero());
}
diff --git a/quic/core/session_notifier_interface.h b/quic/core/session_notifier_interface.h
index 83fc0f1..a601c3e 100644
--- a/quic/core/session_notifier_interface.h
+++ b/quic/core/session_notifier_interface.h
@@ -19,7 +19,8 @@
// Called when |frame| is acked. Returns true if any new data gets acked,
// returns false otherwise.
virtual bool OnFrameAcked(const QuicFrame& frame,
- QuicTime::Delta ack_delay_time) = 0;
+ QuicTime::Delta ack_delay_time,
+ QuicTime receive_timestamp) = 0;
// Called when |frame| is retransmitted.
virtual void OnStreamFrameRetransmitted(const QuicStreamFrame& frame) = 0;
