Change QuicConnection's ScopedPacketBundler to use SEND_QUEUED_ACK instead of NO_ACK, to ensure a q…

net/quic/quic_connection.cc

Index: net/quic/quic_connection.cc
diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc
index 74bc9e3aaad9c97b7f8ca048a65b9cf814861f29..3e807a35281453173730faff0259dea8612d4b5d 100644
--- a/net/quic/quic_connection.cc
+++ b/net/quic/quic_connection.cc
@@ -1105,7 +1105,7 @@ QuicConsumedData QuicConnection::SendStreamData(
   // also if there is possibility of revival. Only bundle an ack if there's no
   // processing left that may cause received_info_ to change.
   ScopedRetransmissionScheduler alarm_delayer(this);
-  ScopedPacketBundler ack_bundler(this, BUNDLE_PENDING_ACK);
+  ScopedPacketBundler ack_bundler(this, SEND_ACK_IF_PENDING);
   return packet_generator_.ConsumeData(id, iov, offset, fin, listener);
 }
 
@@ -1113,7 +1113,7 @@ void QuicConnection::SendRstStream(QuicStreamId id,
                                    QuicRstStreamErrorCode error,
                                    QuicStreamOffset bytes_written) {
   // Opportunistically bundle an ack with this outgoing packet.
-  ScopedPacketBundler ack_bundler(this, BUNDLE_PENDING_ACK);
+  ScopedPacketBundler ack_bundler(this, SEND_ACK_IF_PENDING);
   packet_generator_.AddControlFrame(QuicFrame(new QuicRstStreamFrame(
       id, AdjustErrorForVersion(error, version()), bytes_written)));
 
@@ -1147,20 +1147,20 @@ void QuicConnection::SendRstStream(QuicStreamId id,
 void QuicConnection::SendWindowUpdate(QuicStreamId id,
                                       QuicStreamOffset byte_offset) {
   // Opportunistically bundle an ack with this outgoing packet.
-  ScopedPacketBundler ack_bundler(this, BUNDLE_PENDING_ACK);
+  ScopedPacketBundler ack_bundler(this, SEND_ACK_IF_PENDING);
   packet_generator_.AddControlFrame(
       QuicFrame(new QuicWindowUpdateFrame(id, byte_offset)));
 }
 
 void QuicConnection::SendBlocked(QuicStreamId id) {
   // Opportunistically bundle an ack with this outgoing packet.
-  ScopedPacketBundler ack_bundler(this, BUNDLE_PENDING_ACK);
+  ScopedPacketBundler ack_bundler(this, SEND_ACK_IF_PENDING);
   packet_generator_.AddControlFrame(QuicFrame(new QuicBlockedFrame(id)));
 }
 
 void QuicConnection::SendPathClose(QuicPathId path_id) {
   // Opportunistically bundle an ack with this outgoing packet.
-  ScopedPacketBundler ack_bundler(this, BUNDLE_PENDING_ACK);
+  ScopedPacketBundler ack_bundler(this, SEND_ACK_IF_PENDING);
   packet_generator_.AddControlFrame(QuicFrame(new QuicPathCloseFrame(path_id)));
   OnPathClosed(path_id);
 }
@@ -1285,8 +1285,8 @@ void QuicConnection::OnCanWrite() {
     return;
   }
 
-  {  // Limit the scope of the bundler. ACK inclusion happens elsewhere.
-    ScopedPacketBundler bundler(this, NO_ACK);
+  {
+    ScopedPacketBundler bundler(this, SEND_ACK_IF_QUEUED);
     visitor_->OnCanWrite();
     visitor_->PostProcessAfterData();
   }
@@ -1310,7 +1310,7 @@ void QuicConnection::WriteIfNotBlocked() {
 
 void QuicConnection::WriteAndBundleAcksIfNotBlocked() {
   if (!writer_->IsWriteBlocked()) {
-    ScopedPacketBundler bundler(this, ack_queued_ ? SEND_ACK : NO_ACK);
+    ScopedPacketBundler bundler(this, SEND_ACK_IF_QUEUED);
     OnCanWrite();
   }
 }
@@ -1760,7 +1760,7 @@ void QuicConnection::OnPingTimeout() {
 }
 
 void QuicConnection::SendPing() {
-  ScopedPacketBundler bundler(this, ack_queued_ ? SEND_ACK : NO_ACK);
+  ScopedPacketBundler bundler(this, SEND_ACK_IF_QUEUED);
   packet_generator_.AddControlFrame(QuicFrame(QuicPingFrame()));
   // Send PING frame immediately, without checking for congestion window bounds.
   packet_generator_.FlushAllQueuedFrames();
@@ -1956,7 +1956,7 @@ void QuicConnection::SendGoAway(QuicErrorCode error,
            << QuicUtils::ErrorToString(error) << " (" << error << ")";
 
   // Opportunistically bundle an ack with this outgoing packet.
-  ScopedPacketBundler ack_bundler(this, BUNDLE_PENDING_ACK);
+  ScopedPacketBundler ack_bundler(this, SEND_ACK_IF_PENDING);
   packet_generator_.AddControlFrame(
       QuicFrame(new QuicGoAwayFrame(error, last_good_stream_id, reason)));
 }
@@ -2121,7 +2121,7 @@ void QuicConnection::MaybeSetMtuAlarm() {
 
 QuicConnection::ScopedPacketBundler::ScopedPacketBundler(
     QuicConnection* connection,
-    AckBundling send_ack)
+    AckBundling ack_mode)
     : connection_(connection),
       already_in_batch_mode_(connection != nullptr &&
                              connection->packet_generator_.InBatchMode()) {
@@ -2134,18 +2134,27 @@ QuicConnection::ScopedPacketBundler::ScopedPacketBundler(
     DVLOG(1) << "Entering Batch Mode.";
     connection_->packet_generator_.StartBatchOperations();
   }
-  // Bundle an ack if the alarm is set or with every second packet if we need to
-  // raise the peer's least unacked.
-  bool ack_pending =
-      connection_->ack_alarm_->IsSet() || connection_->stop_waiting_count_ > 1;
-  if (send_ack == SEND_ACK || (send_ack == BUNDLE_PENDING_ACK && ack_pending)) {
+  if (ShouldSendAck(ack_mode)) {
     DVLOG(1) << "Bundling ack with outgoing packet.";
-    DCHECK(send_ack == SEND_ACK || connection_->ack_frame_updated() ||
+    DCHECK(ack_mode == SEND_ACK || connection_->ack_frame_updated() ||
            connection_->stop_waiting_count_ > 1);
     connection_->SendAck();
   }
 }
 
+bool QuicConnection::ScopedPacketBundler::ShouldSendAck(
+    AckBundling ack_mode) const {
+  switch (ack_mode) {
+    case SEND_ACK:
+      return true;
+    case SEND_ACK_IF_QUEUED:
+      return connection_->ack_queued();
+    case SEND_ACK_IF_PENDING:
+      return connection_->ack_alarm_->IsSet() ||
+             connection_->stop_waiting_count_ > 1;
+  }
+}
+
 QuicConnection::ScopedPacketBundler::~ScopedPacketBundler() {
   if (connection_ == nullptr) {
     return;