gfe-relnote: In QUIC, do not send STOP_WAITING if no_stop_waiting_frame_ is true. Protected by gfe2_reloadable_flag_quic_simplify_stop_waiting.

This change is from cl/214914333.

PiperOrigin-RevId: 249558708
Change-Id: I48d5045185980ecc0264ad6a9dff1e47f69a6d20

quic/core/quic_connection.cc

diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 73959f0..2260766 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -1205,7 +1205,13 @@
   // If the incoming ack's packets set expresses received packets: peer is still
   // acking packets which we never care about.
   // Send an ack to raise the high water mark.
-  PostProcessAfterAckFrame(GetLeastUnacked() > start,
+  bool send_stop_waiting = GetLeastUnacked() > start;
+  if (GetQuicReloadableFlag(quic_simplify_stop_waiting) &&
+      no_stop_waiting_frames_) {
+    QUIC_RELOADABLE_FLAG_COUNT(quic_simplify_stop_waiting);
+    send_stop_waiting = false;
+  }
+  PostProcessAfterAckFrame(send_stop_waiting,
                            ack_result == PACKETS_NEWLY_ACKED);
   processing_ack_frame_ = false;
 
@@ -3910,8 +3916,6 @@
   SetRetransmissionAlarm();
   MaybeSetPathDegradingAlarm(acked_new_packet);
 
-  // TODO(ianswett): Only increment stop_waiting_count_ if StopWaiting frames
-  // are sent.
   if (send_stop_waiting) {
     ++stop_waiting_count_;
   } else {

quic/core/quic_connection.h

diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 3d421f5..3126e0b 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1287,6 +1287,7 @@
   QuicPacketCount num_packets_received_since_last_ack_sent_;
   // Indicates how many consecutive times an ack has arrived which indicates
   // the peer needs to stop waiting for some packets.
+  // TODO(fayang): remove this when deprecating quic_simplify_stop_waiting.
   int stop_waiting_count_;
   // TODO(fayang): Remove ack_mode_, ack_decimation_delay_,
   // unlimited_ack_decimation_, fast_ack_after_quiescence_ when deprecating

quic/core/quic_connection_test.cc

diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 3e03e1f..ebfaeaa 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -883,11 +883,11 @@
          {AckResponse::kDefer, AckResponse::kImmediate}) {
       for (bool no_stop_waiting : {true, false}) {
         // After version 43, never use STOP_WAITING.
-        if (all_supported_versions[i].transport_version <= QUIC_VERSION_43 ||
-            no_stop_waiting) {
-          params.push_back(TestParams(all_supported_versions[i], ack_response,
-                                      no_stop_waiting));
-        }
+        params.push_back(TestParams(
+            all_supported_versions[i], ack_response,
+            all_supported_versions[i].transport_version <= QUIC_VERSION_43
+                ? no_stop_waiting
+                : true));
       }
     }
   }
@@ -2622,15 +2622,25 @@
   ProcessAckPacket(&frame2);
   EXPECT_CALL(*send_algorithm_,
               OnPacketSent(_, _, _, _, HAS_RETRANSMITTABLE_DATA));
-  connection_.SendStreamDataWithString(3, "foo", 9, NO_FIN);
+  connection_.SendStreamDataWithString(3, "foofoofoo", 9, NO_FIN);
   // Ack bundled.
   if (GetParam().no_stop_waiting) {
-    EXPECT_EQ(2u, writer_->frame_count());
+    if (GetQuicReloadableFlag(quic_simplify_stop_waiting)) {
+      // Do not ACK acks.
+      EXPECT_EQ(1u, writer_->frame_count());
+    } else {
+      EXPECT_EQ(2u, writer_->frame_count());
+    }
   } else {
     EXPECT_EQ(3u, writer_->frame_count());
   }
   EXPECT_EQ(1u, writer_->stream_frames().size());
-  EXPECT_FALSE(writer_->ack_frames().empty());
+  if (GetParam().no_stop_waiting &&
+      GetQuicReloadableFlag(quic_simplify_stop_waiting)) {
+    EXPECT_TRUE(writer_->ack_frames().empty());
+  } else {
+    EXPECT_FALSE(writer_->ack_frames().empty());
+  }
 
   // But an ack with no missing packets will not send an ack.
   AckPacket(original, &frame2);
@@ -6496,14 +6506,25 @@
   // Check that ack is bundled with outgoing data and the delayed ack
   // alarm is reset.
   if (GetParam().no_stop_waiting) {
-    EXPECT_EQ(2u, writer_->frame_count());
-    EXPECT_TRUE(writer_->stop_waiting_frames().empty());
+    if (GetQuicReloadableFlag(quic_simplify_stop_waiting)) {
+      // Do not ACK acks.
+      EXPECT_EQ(1u, writer_->frame_count());
+    } else {
+      EXPECT_EQ(2u, writer_->frame_count());
+      EXPECT_TRUE(writer_->stop_waiting_frames().empty());
+    }
   } else {
     EXPECT_EQ(3u, writer_->frame_count());
     EXPECT_FALSE(writer_->stop_waiting_frames().empty());
   }
-  EXPECT_FALSE(writer_->ack_frames().empty());
-  EXPECT_EQ(QuicPacketNumber(3u), LargestAcked(writer_->ack_frames().front()));
+  if (GetParam().no_stop_waiting &&
+      GetQuicReloadableFlag(quic_simplify_stop_waiting)) {
+    EXPECT_TRUE(writer_->ack_frames().empty());
+  } else {
+    EXPECT_FALSE(writer_->ack_frames().empty());
+    EXPECT_EQ(QuicPacketNumber(3u),
+              LargestAcked(writer_->ack_frames().front()));
+  }
   EXPECT_EQ(1u, writer_->stream_frames().size());
   EXPECT_FALSE(connection_.GetAckAlarm()->IsSet());
 }
@@ -8094,7 +8115,12 @@
   connection_.GetPingAlarm()->Fire();
   size_t padding_frame_count = writer_->padding_frames().size();
   if (GetParam().no_stop_waiting) {
-    EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count());
+    if (GetQuicReloadableFlag(quic_simplify_stop_waiting)) {
+      // Do not ACK acks.
+      EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count());
+    } else {
+      EXPECT_EQ(padding_frame_count + 2u, writer_->frame_count());
+    }
   } else {
     EXPECT_EQ(padding_frame_count + 3u, writer_->frame_count());
   }