Fix QUIC_BUG in FlushAckFrame to check whether frames is empty. Protected by FLAGS_quic_reloadable_flag_quic_single_ack_in_packet2. PiperOrigin-RevId: 349583801 Change-Id: I75958100168a1c77c8661d217224f6584fac284c

diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 9342bd6..3ea815b 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc

@@ -3927,8 +3927,8 @@
     bool send_ack = error != QUIC_PACKET_WRITE_ERROR &&
                     !uber_received_packet_manager_.IsAckFrameEmpty(
                         QuicUtils::GetPacketNumberSpace(encryption_level_));
-    if (GetQuicReloadableFlag(quic_single_ack_in_packet)) {
-      QUIC_RELOADABLE_FLAG_COUNT_N(quic_single_ack_in_packet, 1, 2);
+    if (GetQuicReloadableFlag(quic_single_ack_in_packet2)) {
+      QUIC_RELOADABLE_FLAG_COUNT_N(quic_single_ack_in_packet2, 1, 2);
       send_ack = !packet_creator_.has_ack() && send_ack;
     }
     if (send_ack) {
@@ -3974,8 +3974,8 @@
     bool send_ack = error != QUIC_PACKET_WRITE_ERROR &&
                     !uber_received_packet_manager_.IsAckFrameEmpty(
                         QuicUtils::GetPacketNumberSpace(encryption_level_));
-    if (GetQuicReloadableFlag(quic_single_ack_in_packet)) {
-      QUIC_RELOADABLE_FLAG_COUNT_N(quic_single_ack_in_packet, 2, 2);
+    if (GetQuicReloadableFlag(quic_single_ack_in_packet2)) {
+      QUIC_RELOADABLE_FLAG_COUNT_N(quic_single_ack_in_packet2, 2, 2);
       send_ack = !packet_creator_.has_ack() && send_ack;
     }
     if (send_ack) {

diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 1baf44a..dc434a5 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc

@@ -12750,7 +12750,7 @@
   ProcessFramesPacketWithAddresses(frames, kSelfAddress, kPeerAddress,
                                    ENCRYPTION_FORWARD_SECURE);
   ASSERT_FALSE(writer_->ack_frames().empty());
-  if (GetQuicReloadableFlag(quic_single_ack_in_packet)) {
+  if (GetQuicReloadableFlag(quic_single_ack_in_packet2)) {
     EXPECT_EQ(1u, writer_->ack_frames().size());
   } else {
     EXPECT_EQ(2u, writer_->ack_frames().size());
@@ -13070,6 +13070,33 @@
   EXPECT_FALSE(connection_.connected());
 }
 
+// Regresstion test for b/175685916
+TEST_P(QuicConnectionTest, TryToFlushAckWithAckQueued) {
+  if (!version().HasIetfQuicFrames()) {
+    return;
+  }
+  SetQuicReloadableFlag(quic_can_send_ack_frequency, true);
+  SetQuicReloadableFlag(quic_single_ack_in_packet2, true);
+  set_perspective(Perspective::IS_SERVER);
+
+  QuicConfig config;
+  QuicConfigPeer::SetReceivedMinAckDelayMs(&config, /*min_ack_delay_ms=*/1);
+  EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+  connection_.SetFromConfig(config);
+  connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+  connection_.OnHandshakeComplete();
+  QuicPacketCreatorPeer::SetPacketNumber(creator_, 200);
+
+  EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1);
+  ProcessDataPacketAtLevel(1, !kHasStopWaiting, ENCRYPTION_FORWARD_SECURE);
+  // Sending ACK_FREQUENCY bundles ACK. QuicConnectionPeer::SendPing
+  // will try to bundle ACK but there is no pending ACK.
+  EXPECT_CALL(visitor_, SendAckFrequency(_))
+      .WillOnce(Invoke(&notifier_,
+                       &SimpleSessionNotifier::WriteOrBufferAckFrequency));
+  QuicConnectionPeer::SendPing(&connection_);
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic

diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 1397140..27383de 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h

@@ -55,7 +55,7 @@
 QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_send_path_response, false)
 QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_send_timestamps, false)
 QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_send_version_negotiation_for_short_connection_ids, true)
-QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_single_ack_in_packet, false)
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_single_ack_in_packet2, false)
 QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_split_up_send_rst_2, true)
 QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_start_peer_migration_earlier, false)
 QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_testonly_default_false, false)

diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index 0ebfc30..7dbe37a 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc

@@ -1423,7 +1423,10 @@
 bool QuicPacketCreator::FlushAckFrame(const QuicFrames& frames) {
   QUIC_BUG_IF(!flusher_attached_) << "Packet flusher is not attached when "
                                      "generator tries to send ACK frame.";
-  QUIC_BUG_IF(GetQuicReloadableFlag(quic_single_ack_in_packet) && has_ack())
+  // MaybeBundleAckOpportunistically could be called nestedly when sending a
+  // control frame causing another control frame to be sent.
+  QUIC_BUG_IF(GetQuicReloadableFlag(quic_single_ack_in_packet2) &&
+              !frames.empty() && has_ack())
       << "Trying to flush " << frames << " when there is ACK queued";
   for (const auto& frame : frames) {
     DCHECK(frame.type == ACK_FRAME || frame.type == STOP_WAITING_FRAME);

diff --git a/quic/test_tools/simple_session_notifier.cc b/quic/test_tools/simple_session_notifier.cc
index ccb784f..1a75a4c 100644
--- a/quic/test_tools/simple_session_notifier.cc
+++ b/quic/test_tools/simple_session_notifier.cc

@@ -125,6 +125,24 @@
   WriteBufferedControlFrames();
 }
 
+void SimpleSessionNotifier::WriteOrBufferAckFrequency(
+    const QuicAckFrequencyFrame& ack_frequency_frame) {
+  QUIC_DVLOG(1) << "Writing ACK_FREQUENCY";
+  const bool had_buffered_data =
+      HasBufferedStreamData() || HasBufferedControlFrames();
+  QuicControlFrameId control_frame_id = ++last_control_frame_id_;
+  control_frames_.emplace_back((
+      QuicFrame(new QuicAckFrequencyFrame(control_frame_id,
+                                          /*sequence_number=*/control_frame_id,
+                                          ack_frequency_frame.packet_tolerance,
+                                          ack_frequency_frame.max_ack_delay))));
+  if (had_buffered_data) {
+    QUIC_DLOG(WARNING) << "Connection is write blocked";
+    return;
+  }
+  WriteBufferedControlFrames();
+}
+
 void SimpleSessionNotifier::NeuterUnencryptedData() {
   if (QuicVersionUsesCryptoFrames(connection_->transport_version())) {
     for (const auto& interval : crypto_bytes_transferred_[ENCRYPTION_INITIAL]) {

diff --git a/quic/test_tools/simple_session_notifier.h b/quic/test_tools/simple_session_notifier.h
index ac2f18a..5f7d4ae 100644
--- a/quic/test_tools/simple_session_notifier.h
+++ b/quic/test_tools/simple_session_notifier.h

@@ -35,6 +35,10 @@
   // Tries to write PING.
   void WriteOrBufferPing();
 
+  // Tries to write ACK_FREQUENCY.
+  void WriteOrBufferAckFrequency(
+      const QuicAckFrequencyFrame& ack_frequency_frame);
+
   // Tries to write CRYPTO data and returns the number of bytes written.
   size_t WriteCryptoData(EncryptionLevel level,
                          QuicByteCount data_length,