gfe-relnote: In QUIC, send PING when TLP fires and no packet is created. Protected by existing gfe2_reloadable_flag_quic_fix_rto_retransmission3. PiperOrigin-RevId: 266418710 Change-Id: I1979a67e966adee15a2926f1628ff5175ef5707d

diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index cc99744..257ef29 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc

@@ -2586,11 +2586,19 @@
 
   if (sent_packet_manager_.fix_rto_retransmission()) {
     if (packet_generator_.packet_number() == previous_created_packet_number &&
-        (retransmission_mode == QuicSentPacketManager::RTO_MODE ||
+        (retransmission_mode == QuicSentPacketManager::TLP_MODE ||
+         retransmission_mode == QuicSentPacketManager::RTO_MODE ||
          retransmission_mode == QuicSentPacketManager::PTO_MODE) &&
         !visitor_->WillingAndAbleToWrite()) {
       // Send PING if timer fires in RTO or PTO mode but there is no data to
       // send.
+      // When TLP fires, either new data or tail loss probe should be sent.
+      // There is corner case where TLP fires after RTO because packets get
+      // acked. Two packets are marked RTO_RETRANSMITTED, but the first packet
+      // is retransmitted as two packets because of packet number length
+      // increases (please see QuicConnectionTest.RtoPacketAsTwo).
+      QUIC_BUG_IF(retransmission_mode == QuicSentPacketManager::TLP_MODE &&
+                  stats_.rto_count == 0);
       DCHECK_LT(0u, sent_packet_manager_.pending_timer_transmission_count());
       visitor_->SendPing();
     }

diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 9f8127d..ec09686 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc

@@ -1283,8 +1283,10 @@
                                      StreamSendingState state,
                                      QuicPacketNumber* last_packet) {
     QuicByteCount packet_size;
+    // Save the last packet's size.
     EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
-        .WillOnce(SaveArg<3>(&packet_size));
+        .Times(AnyNumber())
+        .WillRepeatedly(SaveArg<3>(&packet_size));
     connection_.SendStreamDataWithString(id, data, offset, state);
     if (last_packet != nullptr) {
       *last_packet = creator_->packet_number();
@@ -9156,6 +9158,52 @@
             connection_close_frames[0].transport_close_frame_type);
 }
 
+// Regression test for b/137401387 and b/138962304.
+TEST_P(QuicConnectionTest, RtoPacketAsTwo) {
+  if (!QuicConnectionPeer::GetSentPacketManager(&connection_)
+           ->fix_rto_retransmission() ||
+      connection_.PtoEnabled()) {
+    return;
+  }
+  connection_.SetMaxTailLossProbes(1);
+  connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+  std::string stream_data(3000, 's');
+  // Send packets 1 - 66 and exhaust cwnd.
+  for (size_t i = 0; i < 22; ++i) {
+    // 3 packets for each stream, the first 2 are guaranteed to be full packets.
+    SendStreamDataToPeer(i + 2, stream_data, 0, FIN, nullptr);
+  }
+  CongestionBlockWrites();
+
+  // Fires TLP. Please note, this tail loss probe has 1 byte less stream data
+  // compared to packet 1 because packet number length increases.
+  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(67), _, _));
+  connection_.GetRetransmissionAlarm()->Fire();
+  // Fires RTO. Please note, although packets 2 and 3 *should* be RTOed, but
+  // packet 2 gets RTOed to two packets because packet number length increases.
+  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(68), _, _));
+  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(69), _, _));
+  connection_.GetRetransmissionAlarm()->Fire();
+
+  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
+  // Resets all streams except 2 and ack packets 1 and 2. Now, packet 3 is the
+  // only one containing retransmittable frames.
+  for (size_t i = 1; i < 22; ++i) {
+    notifier_.OnStreamReset(i + 2, QUIC_STREAM_CANCELLED);
+  }
+  EXPECT_CALL(*send_algorithm_, OnCongestionEvent(_, _, _, _, _));
+  QuicAckFrame frame =
+      InitAckFrame({{QuicPacketNumber(1), QuicPacketNumber(3)}});
+  ProcessAckPacket(1, &frame);
+  CongestionUnblockWrites();
+
+  // Fires TLP, verify a PING gets sent because packet 3 is marked
+  // RTO_RETRANSMITTED.
+  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(70), _, _));
+  EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { SendPing(); }));
+  connection_.GetRetransmissionAlarm()->Fire();
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic

diff --git a/quic/test_tools/simple_session_notifier.cc b/quic/test_tools/simple_session_notifier.cc
index 16c441c..72a23d7 100644
--- a/quic/test_tools/simple_session_notifier.cc
+++ b/quic/test_tools/simple_session_notifier.cc

@@ -175,6 +175,14 @@
   }
 }
 
+void SimpleSessionNotifier::OnStreamReset(QuicStreamId id,
+                                          QuicRstStreamErrorCode error) {
+  if (error != QUIC_STREAM_NO_ERROR) {
+    // Delete stream to avoid retransmissions.
+    stream_map_.erase(id);
+  }
+}
+
 bool SimpleSessionNotifier::WillingToWrite() const {
   QUIC_DVLOG(1) << "has_buffered_control_frames: " << HasBufferedControlFrames()
                 << " as_lost_control_frames: " << !lost_control_frames_.empty()

diff --git a/quic/test_tools/simple_session_notifier.h b/quic/test_tools/simple_session_notifier.h
index 7424053..b3d5d72 100644
--- a/quic/test_tools/simple_session_notifier.h
+++ b/quic/test_tools/simple_session_notifier.h

@@ -45,6 +45,9 @@
   // Called when connection_ becomes writable.
   void OnCanWrite();
 
+  // Called to reset stream.
+  void OnStreamReset(QuicStreamId id, QuicRstStreamErrorCode error);
+
   // Returns true if there are 1) unsent control frames and stream data, or 2)
   // lost control frames and stream data.
   bool WillingToWrite() const;