Fix missing QUIC_SESSION_PACKET_SENT
When cl/331651056 landed in the QUICHE shared code and changed
QuicConnectionDebugVisitor::OnPacketSent, it failed to update the
overrides in Chrome. This caused us to stop sending
QUIC_SESSION_PACKET_SENT events to netlog. This CL fixes that
and adds a test to ensure we don't regress.
[email protected]
Bug: 1141254
Change-Id: I2f6679ffa42779901cc9c088fbbff716f4e56a30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2492167
Auto-Submit: David Schinazi <[email protected]>
Reviewed-by: Renjie Tang <[email protected]>
Commit-Queue: David Schinazi <[email protected]>
Cr-Commit-Position: refs/heads/master@{#819949}
diff --git a/net/quic/quic_connection_logger.cc b/net/quic/quic_connection_logger.cc
index a065d98..193c705 100644
--- a/net/quic/quic_connection_logger.cc
+++ b/net/quic/quic_connection_logger.cc
@@ -183,40 +183,40 @@
}
void QuicConnectionLogger::OnPacketSent(
- const quic::SerializedPacket& serialized_packet,
+ quic::QuicPacketNumber packet_number,
+ quic::QuicPacketLength packet_length,
+ bool has_crypto_handshake,
quic::TransmissionType transmission_type,
+ quic::EncryptionLevel encryption_level,
+ const quic::QuicFrames& retransmittable_frames,
+ const quic::QuicFrames& nonretransmittable_frames,
quic::QuicTime sent_time) {
// 4.4.1.4. Minimum Packet Size
// The payload of a UDP datagram carrying the Initial packet MUST be
// expanded to at least 1200 octets
const quic::QuicPacketLength kMinClientInitialPacketLength = 1200;
- const quic::QuicPacketLength encrypted_length =
- serialized_packet.encrypted_length;
- switch (serialized_packet.encryption_level) {
+ switch (encryption_level) {
case quic::ENCRYPTION_INITIAL:
UMA_HISTOGRAM_CUSTOM_COUNTS("Net.QuicSession.SendPacketSize.Initial",
- encrypted_length, 1, kMaxOutgoingPacketSize,
- 50);
- if (encrypted_length < kMinClientInitialPacketLength) {
+ packet_length, 1, kMaxOutgoingPacketSize, 50);
+ if (packet_length < kMinClientInitialPacketLength) {
UMA_HISTOGRAM_CUSTOM_COUNTS(
"Net.QuicSession.TooSmallInitialSentPacket",
- kMinClientInitialPacketLength - encrypted_length, 1,
+ kMinClientInitialPacketLength - packet_length, 1,
kMinClientInitialPacketLength, 50);
}
break;
case quic::ENCRYPTION_HANDSHAKE:
UMA_HISTOGRAM_CUSTOM_COUNTS("Net.QuicSession.SendPacketSize.Hanshake",
- encrypted_length, 1, kMaxOutgoingPacketSize,
- 50);
+ packet_length, 1, kMaxOutgoingPacketSize, 50);
break;
case quic::ENCRYPTION_ZERO_RTT:
UMA_HISTOGRAM_CUSTOM_COUNTS("Net.QuicSession.SendPacketSize.0RTT",
- encrypted_length, 1, kMaxOutgoingPacketSize,
- 50);
+ packet_length, 1, kMaxOutgoingPacketSize, 50);
break;
case quic::ENCRYPTION_FORWARD_SECURE:
UMA_HISTOGRAM_CUSTOM_COUNTS(
- "Net.QuicSession.SendPacketSize.ForwardSecure", encrypted_length, 1,
+ "Net.QuicSession.SendPacketSize.ForwardSecure", packet_length, 1,
kMaxOutgoingPacketSize, 50);
break;
case quic::NUM_ENCRYPTION_LEVELS:
@@ -224,7 +224,22 @@
break;
}
- event_logger_.OnPacketSent(serialized_packet, transmission_type, sent_time);
+ event_logger_.OnPacketSent(packet_number, packet_length, has_crypto_handshake,
+ transmission_type, encryption_level,
+ retransmittable_frames, nonretransmittable_frames,
+ sent_time);
+}
+
+void QuicConnectionLogger::OnPacketSent(
+ const quic::SerializedPacket& serialized_packet,
+ quic::TransmissionType transmission_type,
+ quic::QuicTime sent_time) {
+ OnPacketSent(serialized_packet.packet_number,
+ serialized_packet.encrypted_length,
+ serialized_packet.has_crypto_handshake != quic::NOT_HANDSHAKE,
+ transmission_type, serialized_packet.encryption_level,
+ serialized_packet.retransmittable_frames,
+ serialized_packet.nonretransmittable_frames, sent_time);
}
void QuicConnectionLogger::OnPacketLoss(
diff --git a/net/quic/quic_connection_logger.h b/net/quic/quic_connection_logger.h
index a9def71..2909f04 100644
--- a/net/quic/quic_connection_logger.h
+++ b/net/quic/quic_connection_logger.h
@@ -46,10 +46,18 @@
void OnFrameAddedToPacket(const quic::QuicFrame& frame) override;
void OnStreamFrameCoalesced(const quic::QuicStreamFrame& frame) override;
- // QuicConnectionDebugVisitor Interface
+ // quic::QuicConnectionDebugVisitor Interface
void OnPacketSent(const quic::SerializedPacket& serialized_packet,
quic::TransmissionType transmission_type,
quic::QuicTime sent_time) override;
+ void OnPacketSent(quic::QuicPacketNumber packet_number,
+ quic::QuicPacketLength packet_length,
+ bool has_crypto_handshake,
+ quic::TransmissionType transmission_type,
+ quic::EncryptionLevel encryption_level,
+ const quic::QuicFrames& retransmittable_frames,
+ const quic::QuicFrames& nonretransmittable_frames,
+ quic::QuicTime sent_time) override;
void OnIncomingAck(quic::QuicPacketNumber ack_packet_number,
quic::EncryptionLevel ack_decrypted_level,
const quic::QuicAckFrame& frame,
diff --git a/net/quic/quic_event_logger.cc b/net/quic/quic_event_logger.cc
index bc03670..51f71762 100644
--- a/net/quic/quic_event_logger.cc
+++ b/net/quic/quic_event_logger.cc
@@ -24,20 +24,19 @@
return dict;
}
-base::Value NetLogQuicPacketSentParams(
- const quic::SerializedPacket& serialized_packet,
- quic::TransmissionType transmission_type,
- quic::QuicTime sent_time) {
+base::Value NetLogQuicPacketSentParams(quic::QuicPacketNumber packet_number,
+ quic::QuicPacketLength packet_length,
+ quic::TransmissionType transmission_type,
+ quic::EncryptionLevel encryption_level,
+ quic::QuicTime sent_time) {
base::Value dict(base::Value::Type::DICTIONARY);
dict.SetStringKey("transmission_type",
quic::TransmissionTypeToString(transmission_type));
- dict.SetKey("packet_number",
- NetLogNumberValue(serialized_packet.packet_number.ToUint64()));
- dict.SetIntKey("size", serialized_packet.encrypted_length);
+ dict.SetKey("packet_number", NetLogNumberValue(packet_number.ToUint64()));
+ dict.SetIntKey("size", packet_length);
dict.SetKey("sent_time_us", NetLogNumberValue(sent_time.ToDebuggingValue()));
- dict.SetStringKey(
- "encryption_level",
- quic::EncryptionLevelToString(serialized_packet.encryption_level));
+ dict.SetStringKey("encryption_level",
+ quic::EncryptionLevelToString(encryption_level));
return dict;
}
@@ -547,17 +546,35 @@
}
void QuicEventLogger::OnPacketSent(
- const quic::SerializedPacket& serialized_packet,
+ quic::QuicPacketNumber packet_number,
+ quic::QuicPacketLength packet_length,
+ bool /*has_crypto_handshake*/,
quic::TransmissionType transmission_type,
+ quic::EncryptionLevel encryption_level,
+ const quic::QuicFrames& /*retransmittable_frames*/,
+ const quic::QuicFrames& /*nonretransmittable_frames*/,
quic::QuicTime sent_time) {
if (!net_log_.IsCapturing())
return;
net_log_.AddEvent(NetLogEventType::QUIC_SESSION_PACKET_SENT, [&] {
- return NetLogQuicPacketSentParams(serialized_packet, transmission_type,
+ return NetLogQuicPacketSentParams(packet_number, packet_length,
+ transmission_type, encryption_level,
sent_time);
});
}
+void QuicEventLogger::OnPacketSent(
+ const quic::SerializedPacket& serialized_packet,
+ quic::TransmissionType transmission_type,
+ quic::QuicTime sent_time) {
+ OnPacketSent(serialized_packet.packet_number,
+ serialized_packet.encrypted_length,
+ serialized_packet.has_crypto_handshake != quic::NOT_HANDSHAKE,
+ transmission_type, serialized_packet.encryption_level,
+ serialized_packet.retransmittable_frames,
+ serialized_packet.nonretransmittable_frames, sent_time);
+}
+
void QuicEventLogger::OnIncomingAck(
quic::QuicPacketNumber ack_packet_number,
quic::EncryptionLevel /*ack_decrypted_level*/,
diff --git a/net/quic/quic_event_logger.h b/net/quic/quic_event_logger.h
index 8e8eb4d..6f4c4f8 100644
--- a/net/quic/quic_event_logger.h
+++ b/net/quic/quic_event_logger.h
@@ -28,10 +28,18 @@
void OnFrameAddedToPacket(const quic::QuicFrame& frame) override;
void OnStreamFrameCoalesced(const quic::QuicStreamFrame& frame) override;
- // QuicConnectionDebugVisitor Interface
+ // quic::QuicConnectionDebugVisitor Interface
void OnPacketSent(const quic::SerializedPacket& serialized_packet,
quic::TransmissionType transmission_type,
quic::QuicTime sent_time) override;
+ void OnPacketSent(quic::QuicPacketNumber packet_number,
+ quic::QuicPacketLength packet_length,
+ bool has_crypto_handshake,
+ quic::TransmissionType transmission_type,
+ quic::EncryptionLevel encryption_level,
+ const quic::QuicFrames& retransmittable_frames,
+ const quic::QuicFrames& nonretransmittable_frames,
+ quic::QuicTime sent_time) override;
void OnIncomingAck(quic::QuicPacketNumber ack_packet_number,
quic::EncryptionLevel ack_decrypted_level,
const quic::QuicAckFrame& frame,
diff --git a/net/quic/quic_network_transaction_unittest.cc b/net/quic/quic_network_transaction_unittest.cc
index 4d27e24..db93c09 100644
--- a/net/quic/quic_network_transaction_unittest.cc
+++ b/net/quic/quic_network_transaction_unittest.cc
@@ -1205,6 +1205,12 @@
NetLogEventPhase::NONE);
EXPECT_LT(0, pos);
+ // ... and also a TYPE_QUIC_SESSION_PACKET_SENT.
+ pos = ExpectLogContainsSomewhere(entries, 0,
+ NetLogEventType::QUIC_SESSION_PACKET_SENT,
+ NetLogEventPhase::NONE);
+ EXPECT_LT(0, pos);
+
// ... and also a TYPE_QUIC_SESSION_UNAUTHENTICATED_PACKET_HEADER_RECEIVED.
pos = ExpectLogContainsSomewhere(
entries, 0,
