Deprecate --gfe2_reloadable_flag_quic_give_sent_packet_to_debug_visitor_after_sent.
Startblock:
after 2020-11-21 09:00 in Google/US-CAM
PiperOrigin-RevId: 343648304
Change-Id: Ibc53f46deea52a1357d81c2c4d79f8dba80b303a
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index b6ea5b1..90a1739 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -3052,12 +3052,6 @@
packet_send_time = packet_send_time + result.send_time_offset;
}
- if (!sent_packet_manager_.give_sent_packet_to_debug_visitor_after_sent() &&
- debug_visitor_ != nullptr) {
- // Pass the write result to the visitor.
- debug_visitor_->OnPacketSent(*packet, packet->transmission_type,
- packet_send_time);
- }
if (IsRetransmittable(*packet) == HAS_RETRANSMITTABLE_DATA &&
!is_termination_packet) {
// Start blackhole/path degrading detections if the sent packet is not
@@ -3100,10 +3094,7 @@
<< ENDPOINT
<< "Trying to start blackhole detection without no bytes in flight";
- if (sent_packet_manager_.give_sent_packet_to_debug_visitor_after_sent() &&
- debug_visitor_ != nullptr) {
- QUIC_RELOADABLE_FLAG_COUNT_N(
- quic_give_sent_packet_to_debug_visitor_after_sent, 1, 2);
+ if (debug_visitor_ != nullptr) {
if (sent_packet_manager_.unacked_packets().empty()) {
QUIC_BUG << "Unacked map is empty right after packet is sent";
} else {
@@ -4492,21 +4483,12 @@
return false;
}
- if (!sent_packet_manager_.give_sent_packet_to_debug_visitor_after_sent() &&
- debug_visitor_ != nullptr) {
- debug_visitor_->OnPacketSent(*packet, packet->transmission_type,
- packet_send_time);
- }
-
// Send in currrent path. Call OnPacketSent regardless of the write result.
sent_packet_manager_.OnPacketSent(packet.get(), packet_send_time,
packet->transmission_type,
NO_RETRANSMITTABLE_DATA, measure_rtt);
- if (sent_packet_manager_.give_sent_packet_to_debug_visitor_after_sent() &&
- debug_visitor_ != nullptr) {
- QUIC_RELOADABLE_FLAG_COUNT_N(
- quic_give_sent_packet_to_debug_visitor_after_sent, 2, 2);
+ if (debug_visitor_ != nullptr) {
if (sent_packet_manager_.unacked_packets().empty()) {
QUIC_BUG << "Unacked map is empty right after packet is sent";
} else {
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 599099d..599331d 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -225,12 +225,6 @@
~QuicConnectionDebugVisitor() override {}
// Called when a packet has been sent.
- // TODO(wub): Delete when deprecating
- // --quic_give_sent_packet_to_debug_visitor_after_sent.
- virtual void OnPacketSent(const SerializedPacket& /*serialized_packet*/,
- TransmissionType /*transmission_type*/,
- QuicTime /*sent_time*/) {}
-
virtual void OnPacketSent(QuicPacketNumber /*packet_number*/,
QuicPacketLength /*packet_length*/,
bool /*has_crypto_handshake*/,
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 93169ea..a329404 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -6768,20 +6768,10 @@
MockQuicConnectionDebugVisitor debug_visitor;
connection_.set_debug_visitor(&debug_visitor);
- if (connection_.sent_packet_manager()
- .give_sent_packet_to_debug_visitor_after_sent()) {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1);
- } else {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1);
- }
+ EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1);
connection_.SendStreamDataWithString(1, "foo", 0, NO_FIN);
- if (connection_.sent_packet_manager()
- .give_sent_packet_to_debug_visitor_after_sent()) {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1);
- } else {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1);
- }
+ EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1);
connection_.SendConnectivityProbingPacket(writer_.get(),
connection_.peer_address());
}
@@ -6908,12 +6898,7 @@
CongestionBlockWrites();
connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
- if (connection_.sent_packet_manager()
- .give_sent_packet_to_debug_visitor_after_sent()) {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1);
- } else {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1);
- }
+ EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1);
EXPECT_CALL(debug_visitor, OnPingSent()).Times(1);
connection_.SendControlFrame(QuicFrame(QuicPingFrame(1)));
EXPECT_FALSE(connection_.HasQueuedData());
@@ -6925,12 +6910,7 @@
connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
- if (connection_.sent_packet_manager()
- .give_sent_packet_to_debug_visitor_after_sent()) {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1);
- } else {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1);
- }
+ EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1);
EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent);
connection_.SendControlFrame(QuicFrame(new QuicBlockedFrame(1, 3)));
EXPECT_EQ(1u, connection_.GetStats().blocked_frames_sent);
@@ -6946,12 +6926,7 @@
QuicBlockedFrame blocked(1, 3);
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
- if (connection_.sent_packet_manager()
- .give_sent_packet_to_debug_visitor_after_sent()) {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(0);
- } else {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(0);
- }
+ EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(0);
EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent);
connection_.SendControlFrame(QuicFrame(&blocked));
EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent);
@@ -7682,45 +7657,26 @@
}
// Expect them retransmitted in cyclic order (foo, bar, test, foo, bar...).
QuicPacketCount sent_count = 0;
- if (connection_.sent_packet_manager()
- .give_sent_packet_to_debug_visitor_after_sent()) {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _))
- .WillRepeatedly(
- Invoke([this, &sent_count](QuicPacketNumber, QuicPacketLength, bool,
- TransmissionType, EncryptionLevel,
- const QuicFrames&, const QuicFrames&,
- QuicTime) {
- ASSERT_EQ(1u, writer_->stream_frames().size());
- if (connection_.version().CanSendCoalescedPackets()) {
- // There is a delay of sending coalesced packet, so (6, 0, 3, 6,
- // 0...).
- EXPECT_EQ(3 * ((sent_count + 2) % 3),
- writer_->stream_frames()[0]->offset);
- } else {
- // Identify the frames by stream offset (0, 3, 6, 0, 3...).
- EXPECT_EQ(3 * (sent_count % 3),
- writer_->stream_frames()[0]->offset);
- }
- sent_count++;
- }));
- } else {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _))
- .WillRepeatedly(Invoke([this, &sent_count](const SerializedPacket&,
- TransmissionType, QuicTime) {
- ASSERT_EQ(1u, writer_->stream_frames().size());
- if (connection_.version().CanSendCoalescedPackets()) {
- // There is a delay of sending coalesced packet, so (6, 0, 3, 6,
- // 0...).
- EXPECT_EQ(3 * ((sent_count + 2) % 3),
- writer_->stream_frames()[0]->offset);
- } else {
- // Identify the frames by stream offset (0, 3, 6, 0, 3...).
- EXPECT_EQ(3 * (sent_count % 3),
- writer_->stream_frames()[0]->offset);
- }
- sent_count++;
- }));
- }
+
+ EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _))
+ .WillRepeatedly(Invoke(
+ [this, &sent_count](QuicPacketNumber, QuicPacketLength, bool,
+ TransmissionType, EncryptionLevel,
+ const QuicFrames&, const QuicFrames&, QuicTime) {
+ ASSERT_EQ(1u, writer_->stream_frames().size());
+ if (connection_.version().CanSendCoalescedPackets()) {
+ // There is a delay of sending coalesced packet, so (6, 0, 3, 6,
+ // 0...).
+ EXPECT_EQ(3 * ((sent_count + 2) % 3),
+ writer_->stream_frames()[0]->offset);
+ } else {
+ // Identify the frames by stream offset (0, 3, 6, 0, 3...).
+ EXPECT_EQ(3 * (sent_count % 3),
+ writer_->stream_frames()[0]->offset);
+ }
+ sent_count++;
+ }));
+
EXPECT_CALL(*send_algorithm_, ShouldSendProbingPacket())
.WillRepeatedly(Return(true));
EXPECT_CALL(visitor_, SendProbingData()).WillRepeatedly([this] {
@@ -7744,12 +7700,7 @@
MockQuicConnectionDebugVisitor debug_visitor;
connection_.set_debug_visitor(&debug_visitor);
- if (connection_.sent_packet_manager()
- .give_sent_packet_to_debug_visitor_after_sent()) {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(0);
- } else {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(0);
- }
+ EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(0);
EXPECT_CALL(*send_algorithm_, ShouldSendProbingPacket())
.WillRepeatedly(Return(true));
EXPECT_CALL(visitor_, SendProbingData()).WillRepeatedly([this] {
@@ -9715,12 +9666,7 @@
}
MockQuicConnectionDebugVisitor debug_visitor;
connection_.set_debug_visitor(&debug_visitor);
- if (connection_.sent_packet_manager()
- .give_sent_packet_to_debug_visitor_after_sent()) {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(3);
- } else {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(3);
- }
+ EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(3);
EXPECT_CALL(debug_visitor, OnCoalescedPacketSent(_, _)).Times(1);
EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1);
{
@@ -9762,12 +9708,7 @@
MockQuicConnectionDebugVisitor debug_visitor;
connection_.set_debug_visitor(&debug_visitor);
- if (connection_.sent_packet_manager()
- .give_sent_packet_to_debug_visitor_after_sent()) {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1);
- } else {
- EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1);
- }
+ EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _, _, _, _, _)).Times(1);
// Our TestPacketWriter normally parses the sent packet using the version
// from the connection, so here we need to tell it to use the encapsulation
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 2fd5426..7ead17a 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -57,7 +57,6 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_pto_pending_timer_count, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_undecryptable_packets2, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_willing_and_able_to_write2, true)
-QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_give_sent_packet_to_debug_visitor_after_sent, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_goaway_with_max_stream_id, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_granular_qpack_error_codes, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_key_update_supported, true)
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index 7dad0b0..f6a6fa8 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -456,10 +456,6 @@
QuicTime GetEarliestPacketSentTimeForPto(
PacketNumberSpace* packet_number_space) const;
- bool give_sent_packet_to_debug_visitor_after_sent() const {
- return give_sent_packet_to_debug_visitor_after_sent_;
- }
-
private:
friend class test::QuicConnectionPeer;
friend class test::QuicSentPacketManagerPeer;
@@ -740,9 +736,6 @@
// The number of PTOs needed for path degrading alarm. If equals to 0, the
// traditional path degrading mechanism will be used.
int num_ptos_for_path_degrading_;
-
- const bool give_sent_packet_to_debug_visitor_after_sent_ =
- GetQuicReloadableFlag(quic_give_sent_packet_to_debug_visitor_after_sent);
};
} // namespace quic
diff --git a/quic/core/quic_trace_visitor.cc b/quic/core/quic_trace_visitor.cc
index 7ac7b7b..a419808 100644
--- a/quic/core/quic_trace_visitor.cc
+++ b/quic/core/quic_trace_visitor.cc
@@ -44,70 +44,6 @@
}
}
-void QuicTraceVisitor::OnPacketSent(const SerializedPacket& serialized_packet,
- TransmissionType /*transmission_type*/,
- QuicTime sent_time) {
- quic_trace::Event* event = trace_.add_events();
- event->set_event_type(quic_trace::PACKET_SENT);
- event->set_time_us(ConvertTimestampToRecordedFormat(sent_time));
- event->set_packet_number(serialized_packet.packet_number.ToUint64());
- event->set_packet_size(serialized_packet.encrypted_length);
- event->set_encryption_level(
- EncryptionLevelToProto(serialized_packet.encryption_level));
-
- for (const QuicFrame& frame : serialized_packet.retransmittable_frames) {
- switch (frame.type) {
- case STREAM_FRAME:
- case RST_STREAM_FRAME:
- case CONNECTION_CLOSE_FRAME:
- case WINDOW_UPDATE_FRAME:
- case BLOCKED_FRAME:
- case PING_FRAME:
- case HANDSHAKE_DONE_FRAME:
- case ACK_FREQUENCY_FRAME:
- PopulateFrameInfo(frame, event->add_frames());
- break;
-
- case PADDING_FRAME:
- case MTU_DISCOVERY_FRAME:
- case STOP_WAITING_FRAME:
- case ACK_FRAME:
- QUIC_BUG
- << "Frames of type are not retransmittable and are not supposed "
- "to be in retransmittable_frames";
- break;
-
- // New IETF frames, not used in current gQUIC version.
- case NEW_CONNECTION_ID_FRAME:
- case RETIRE_CONNECTION_ID_FRAME:
- case MAX_STREAMS_FRAME:
- case STREAMS_BLOCKED_FRAME:
- case PATH_RESPONSE_FRAME:
- case PATH_CHALLENGE_FRAME:
- case STOP_SENDING_FRAME:
- case MESSAGE_FRAME:
- case CRYPTO_FRAME:
- case NEW_TOKEN_FRAME:
- break;
-
- // Ignore gQUIC-specific frames.
- case GOAWAY_FRAME:
- break;
-
- case NUM_FRAME_TYPES:
- QUIC_BUG << "Unknown frame type encountered";
- break;
- }
- }
-
- // Output PCC DebugState on packet sent for analysis.
- if (connection_->sent_packet_manager()
- .GetSendAlgorithm()
- ->GetCongestionControlType() == kPCC) {
- PopulateTransportState(event->mutable_transport_state());
- }
-}
-
void QuicTraceVisitor::OnPacketSent(
QuicPacketNumber packet_number,
QuicPacketLength packet_length,
diff --git a/quic/core/quic_trace_visitor.h b/quic/core/quic_trace_visitor.h
index 889d90a..0e6a385 100644
--- a/quic/core/quic_trace_visitor.h
+++ b/quic/core/quic_trace_visitor.h
@@ -18,12 +18,6 @@
public:
explicit QuicTraceVisitor(const QuicConnection* connection);
- // TODO(wub): Delete when deprecating
- // --quic_give_sent_packet_to_debug_visitor_after_sent.
- void OnPacketSent(const SerializedPacket& serialized_packet,
- TransmissionType transmission_type,
- QuicTime sent_time) override;
-
void OnPacketSent(QuicPacketNumber packet_number,
QuicPacketLength packet_length,
bool has_crypto_handshake,
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index f764e0b..5c217c7 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -1457,13 +1457,6 @@
MockQuicConnectionDebugVisitor();
~MockQuicConnectionDebugVisitor() override;
- // TODO(wub): Delete when deprecating
- // --quic_give_sent_packet_to_debug_visitor_after_sent.
- MOCK_METHOD(void,
- OnPacketSent,
- (const SerializedPacket&, TransmissionType, QuicTime),
- (override));
-
MOCK_METHOD(void,
OnPacketSent,
(QuicPacketNumber,
