Send pending ACKs after visitor_->OnCanWrite() in QuicConnection::OnCanWrite().

Previously, `QuicConnection::OnCanWrite()` called `SendAllPendingAcks()` before `visitor_->OnCanWrite()`. When a pending ACK's encryption level differed from the connection's current default encryption level (such as during a handshake or 0-RTT connection), `SendAllPendingAcks()` switched encryption levels via `ScopedEncryptionLevelContext`, immediately flushing a standalone ACK packet before `visitor_->OnCanWrite()` could write `CRYPTO` or `STREAM` frames at that encryption level.

When `send_acks_after_on_can_write_` is enabled (via `gfe2_reloadable_flag_quic_send_acks_after_on_can_write` and the `SAOW` connection option), `QuicConnection::OnCanWrite()` invokes `visitor_->OnCanWrite()` before `SendAllPendingAcks()` so that outgoing `CRYPTO` and `STREAM` packets opportunistically bundle pending ACKs at the same encryption level, and only remaining unbundled ACKs are sent afterward.

Overview of impact including Handshake Simulations: https://docs.google.com/document/d/1CiaBRfPAfKbL0SuEhrBQeZxDeXvdREseGe1AE89pR-I/edit?usp=sharing

Protected by quic_reloadable_flag_quic_send_acks_after_on_can_write.

PiperOrigin-RevId: 994903307
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h
index 7f950d8..dab5eb6 100755
--- a/quiche/common/quiche_feature_flags_list.h
+++ b/quiche/common/quiche_feature_flags_list.h
@@ -62,6 +62,7 @@
 QUICHE_FLAG(bool, quiche_reloadable_flag_quic_reject_empty_cid_in_ncid, false, true, "If true, QUIC framer will reject NEW_CONNECTION_ID frames with empty connection IDs.")
 QUICHE_FLAG(bool, quiche_reloadable_flag_quic_reject_unusable_ech_config, false, true, "When true, calls SSL_set_reject_unusable_ech_config in TlsClientHandshaker.")
 QUICHE_FLAG(bool, quiche_reloadable_flag_quic_require_handshake_confirmation, true, true, "If true, require handshake confirmation for QUIC connections, functionally disabling 0-rtt handshakes.")
+QUICHE_FLAG(bool, quiche_reloadable_flag_quic_send_acks_after_on_can_write, false, false, "When true, QuicConnection::OnCanWrite() calls visitor_->OnCanWrite() before SendAllPendingAcks() so outgoing data frames can opportunistically bundle pending ACKs first.")
 QUICHE_FLAG(bool, quiche_reloadable_flag_quic_test_peer_addr_change_after_normalize, false, false, "If true, QuicConnection::ProcessValidatedPacket will use normalized address to test peer address changes.")
 QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_false, false, false, "A testonly reloadable flag that will always default to false.")
 QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_true, true, true, "A testonly reloadable flag that will always default to true.")
diff --git a/quiche/quic/core/crypto/crypto_protocol.h b/quiche/quic/core/crypto/crypto_protocol.h
index c56eefc..c504c5d 100644
--- a/quiche/quic/core/crypto/crypto_protocol.h
+++ b/quiche/quic/core/crypto/crypto_protocol.h
@@ -515,6 +515,7 @@
 DEFINE_STATIC_QUIC_TAG(CHP1);  // 1-packet padding to CHLO.
 DEFINE_STATIC_QUIC_TAG(CHP2);  // 2-packet padding to CHLO.
 DEFINE_STATIC_QUIC_TAG(BCFR);  // Bundle CRYPTO frames when retransmitting.
+DEFINE_STATIC_QUIC_TAG(SAOW);  // Send ACKs after OnCanWrite.
 
 // Stats collection tags
 DEFINE_STATIC_QUIC_TAG(EPID);  // Endpoint identifier.
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc
index c29d91e..e805d37 100644
--- a/quiche/quic/core/http/end_to_end_test.cc
+++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -9329,6 +9329,15 @@
   server_thread_->Resume();
 }
 
+TEST_P(EndToEndTest, SendAcksAfterOnCanWrite) {
+  SetQuicReloadableFlag(quic_send_acks_after_on_can_write, true);
+  client_extra_copts_.push_back(kSAOW);
+  SetPacketLossPercentage(30);
+  ASSERT_TRUE(Initialize());
+  EXPECT_TRUE(client_->client()->WaitForHandshakeConfirmed());
+  SendSynchronousFooRequestAndCheckResponse();
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc
index c39b098..8018c7d 100644
--- a/quiche/quic/core/quic_connection.cc
+++ b/quiche/quic/core/quic_connection.cc
@@ -603,6 +603,12 @@
   if (config.HasClientSentConnectionOption(kDFER, perspective_)) {
     defer_send_in_response_to_packets_ = false;
   }
+  if (GetQuicReloadableFlag(quic_send_acks_after_on_can_write) &&
+      version().IsIetfQuic() &&
+      config.HasClientSentConnectionOption(kSAOW, perspective_)) {
+    send_acks_after_on_can_write_ = true;
+  }
+
   if (perspective_ == Perspective::IS_CLIENT &&
       config.HasClientSentConnectionOption(kCDFR, perspective_)) {
     defer_send_in_response_to_packets_ = true;
@@ -2988,23 +2994,50 @@
   ScopedPacketFlusher flusher(this);
 
   WriteQueuedPackets();
-  const QuicTime ack_timeout =
-      uber_received_packet_manager_.GetEarliestAckTimeout();
-  if (ack_timeout.IsInitialized() && ack_timeout <= clock_->ApproximateNow()) {
-    // Send an ACK now because either 1) we were write blocked when we last
-    // tried to send an ACK, or 2) both ack alarm and send alarm were set to
-    // go off together.
-    SendAllPendingAcks();
-  }
+  if (send_acks_after_on_can_write_) {
+    QUIC_RELOADABLE_FLAG_COUNT(quic_send_acks_after_on_can_write);
+    // Sending queued packets may have caused the socket to become write
+    // blocked, or the congestion manager to prohibit sending.
+    const bool can_write_retransmittable = CanWrite(HAS_RETRANSMITTABLE_DATA);
+    if (can_write_retransmittable) {
+      // Tell the session it can write.
+      visitor_->OnCanWrite();
+    }
+    if (!connected_) {
+      return;
+    }
+    const QuicTime ack_timeout =
+        uber_received_packet_manager_.GetEarliestAckTimeout();
+    if (ack_timeout.IsInitialized() &&
+        ack_timeout <= clock_->ApproximateNow()) {
+      // Send an ACK now because either 1) we were write blocked when we last
+      // tried to send an ACK, or 2) both ack alarm and send alarm were set to
+      // go off together.
+      SendAllPendingAcks();
+    }
+    if (!connected_ || !can_write_retransmittable) {
+      return;
+    }
+  } else {
+    const QuicTime ack_timeout =
+        uber_received_packet_manager_.GetEarliestAckTimeout();
+    if (ack_timeout.IsInitialized() &&
+        ack_timeout <= clock_->ApproximateNow()) {
+      // Send an ACK now because either 1) we were write blocked when we last
+      // tried to send an ACK, or 2) both ack alarm and send alarm were set to
+      // go off together.
+      SendAllPendingAcks();
+    }
 
-  // Sending queued packets may have caused the socket to become write blocked,
-  // or the congestion manager to prohibit sending.
-  if (!CanWrite(HAS_RETRANSMITTABLE_DATA)) {
-    return;
-  }
+    // Sending queued packets may have caused the socket to become write
+    // blocked, or the congestion manager to prohibit sending.
+    if (!CanWrite(HAS_RETRANSMITTABLE_DATA)) {
+      return;
+    }
 
-  // Tell the session it can write.
-  visitor_->OnCanWrite();
+    // Tell the session it can write.
+    visitor_->OnCanWrite();
+  }
 
   // After the visitor writes, it may have caused the socket to become write
   // blocked or the congestion manager to prohibit sending, so check again.
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h
index 442df61..1bbb824 100644
--- a/quiche/quic/core/quic_connection.h
+++ b/quiche/quic/core/quic_connection.h
@@ -2583,6 +2583,7 @@
   // True if spin bit is enabled for this connection.
   bool spin_bit_enabled_ : 1 = false;
   bool fix_mtu_discovery_ : 1 = GetQuicReloadableFlag(quic_fix_mtu_discovery);
+  bool send_acks_after_on_can_write_ : 1 = false;
 };
 
 }  // namespace quic
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index 72258ed..441e8ad 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -19143,6 +19143,83 @@
             kMaxNumOfActiveConnectionIds);
 }
 
+TEST_P(QuicConnectionTest,
+       SendAcksAfterOnCanWriteDisabledFlushesSeparateAckPacket) {
+  if (!connection_.SupportsMultiplePacketNumberSpaces()) {
+    return;
+  }
+  SetQuicReloadableFlag(quic_send_acks_after_on_can_write, false);
+  QuicConfig config;
+  config.SetConnectionOptionsToSend({kSAOW});
+  EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+  EXPECT_CALL(*send_algorithm_, EnableECT1()).WillOnce(Return(false));
+  EXPECT_CALL(*send_algorithm_, EnableECT0()).WillOnce(Return(false));
+  connection_.SetFromConfig(config);
+
+  connection_.SetEncrypter(ENCRYPTION_HANDSHAKE,
+                           std::make_unique<TaggingEncrypter>(0x02));
+  connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE);
+
+  EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(1);
+  ProcessCryptoPacketAtLevel(1, ENCRYPTION_INITIAL);
+  EXPECT_EQ(0u, writer_->packets_write_attempts());
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(100));
+
+  std::string crypto_data(1000, 'a');
+  EXPECT_CALL(visitor_, OnCanWrite()).WillOnce([&]() {
+    QuicConnection::ScopedEncryptionLevelContext context(&connection_,
+                                                         ENCRYPTION_INITIAL);
+    connection_.SendCryptoDataWithString(crypto_data, 0, ENCRYPTION_INITIAL);
+  });
+  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
+  connection_.OnCanWrite();
+
+  // When send_acks_after_on_can_write is false, SendAllPendingAcks() runs
+  // before visitor_->OnCanWrite() and flushes a standalone INITIAL ACK packet
+  // when switching back to ENCRYPTION_HANDSHAKE, resulting in 2 packets.
+  EXPECT_EQ(2u, writer_->packets_write_attempts());
+}
+
+TEST_P(QuicConnectionTest, SendAcksAfterOnCanWriteBundlesAckWithCryptoData) {
+  if (!connection_.SupportsMultiplePacketNumberSpaces()) {
+    return;
+  }
+  SetQuicReloadableFlag(quic_send_acks_after_on_can_write, true);
+  QuicConfig config;
+  config.SetConnectionOptionsToSend({kSAOW});
+  EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+  EXPECT_CALL(*send_algorithm_, EnableECT1()).WillOnce(Return(false));
+  EXPECT_CALL(*send_algorithm_, EnableECT0()).WillOnce(Return(false));
+  connection_.SetFromConfig(config);
+
+  connection_.SetEncrypter(ENCRYPTION_HANDSHAKE,
+                           std::make_unique<TaggingEncrypter>(0x02));
+  connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE);
+
+  // Receive an INITIAL packet so an INITIAL ACK alarm is scheduled, then
+  // advance the clock so the ACK timeout is due when OnCanWrite() runs.
+  EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(1);
+  ProcessCryptoPacketAtLevel(1, ENCRYPTION_INITIAL);
+  EXPECT_EQ(0u, writer_->packets_write_attempts());
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(100));
+
+  std::string crypto_data(1000, 'a');
+  EXPECT_CALL(visitor_, OnCanWrite()).WillOnce([&]() {
+    QuicConnection::ScopedEncryptionLevelContext context(&connection_,
+                                                         ENCRYPTION_INITIAL);
+    connection_.SendCryptoDataWithString(crypto_data, 0, ENCRYPTION_INITIAL);
+  });
+  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+  connection_.OnCanWrite();
+
+  // Because visitor_->OnCanWrite() ran before SendAllPendingAcks(), the pending
+  // INITIAL ACK was bundled into the single INITIAL CRYPTO packet rather than
+  // flushed as a separate INITIAL ACK packet before visitor_->OnCanWrite().
+  EXPECT_EQ(1u, writer_->packets_write_attempts());
+  EXPECT_EQ(1u, writer_->ack_frames().size());
+  EXPECT_GE(writer_->crypto_frames().size(), 1u);
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic