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

net/quic/quic_connection.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/quic/quic_connection.h"
 
 #include <string.h>
 #include <sys/types.h>
 
 #include <algorithm>
@@ skipping 1087 matching lines @@
   // SHLO from the server, leading to two different decrypters at the server.)
   //
   // TODO(jri): Note that ConsumeData may cause a response packet to be sent.
   // We may end up sending stale ack information if there are undecryptable
   // packets hanging around and/or there are revivable packets which may get
   // handled after this packet is sent. Change ScopedPacketBundler to do the
   // right thing: check ack_queued_, and then check undecryptable packets and
   // 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);
 }
 
 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)));
 
   if (error == QUIC_STREAM_NO_ERROR && version() > QUIC_VERSION_28) {
     // All data for streams which are reset with QUIC_STREAM_NO_ERROR must
     // be received by the peer.
     return;
   }
 
   sent_packet_manager_.CancelRetransmissionsForStream(id);
@@ skipping 13 matching lines @@
     }
     delete[] packet_iterator->encrypted_buffer;
     QuicUtils::ClearSerializedPacket(&(*packet_iterator));
     packet_iterator = queued_packets_.erase(packet_iterator);
   }
 }
 
 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);
 }
 
 const QuicConnectionStats& QuicConnection::GetStats() {
   const RttStats* rtt_stats = sent_packet_manager_.GetRttStats();
 
   // Update rtt and estimated bandwidth.
   QuicTime::Delta min_rtt = rtt_stats->min_rtt();
   if (min_rtt.IsZero()) {
@@ skipping 104 matching lines @@
   WritePendingRetransmissions();
 
   // Sending queued packets may have caused the socket to become write blocked,
   // or the congestion manager to prohibit sending.  If we've sent everything
   // we had queued and we're still not blocked, let the visitor know it can
   // write more.
   if (!CanWrite(HAS_RETRANSMITTABLE_DATA)) {
     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();
   }
 
   // After the visitor writes, it may have caused the socket to become write
   // blocked or the congestion manager to prohibit sending, so check again.
   if (visitor_->WillingAndAbleToWrite() && !resume_writes_alarm_->IsSet() &&
       CanWrite(HAS_RETRANSMITTABLE_DATA)) {
     // We're not write blocked, but some stream didn't write out all of its
     // bytes. Register for 'immediate' resumption so we'll keep writing after
     // other connections and events have had a chance to use the thread.
     resume_writes_alarm_->Set(clock_->ApproximateNow());
   }
 }
 
 void QuicConnection::WriteIfNotBlocked() {
   if (!writer_->IsWriteBlocked()) {
     OnCanWrite();
   }
 }
 
 void QuicConnection::WriteAndBundleAcksIfNotBlocked() {
   if (!writer_->IsWriteBlocked()) {
-    ScopedPacketBundler bundler(this, ack_queued_ ? SEND_ACK : NO_ACK);
+    ScopedPacketBundler bundler(this, SEND_ACK_IF_QUEUED);
     OnCanWrite();
   }
 }
 
 bool QuicConnection::ProcessValidatedPacket(const QuicPacketHeader& header) {
   if (header.fec_flag) {
     // Drop any FEC packet.
     return false;
   }
 
@@ skipping 429 matching lines @@
   }
 }
 
 void QuicConnection::OnPingTimeout() {
   if (!retransmission_alarm_->IsSet()) {
     SendPing();
   }
 }
 
 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();
 }
 
 void QuicConnection::SendAck() {
   ack_alarm_->Cancel();
   ack_queued_ = false;
   stop_waiting_count_ = 0;
   num_retransmittable_packets_received_since_last_ack_sent_ = 0;
@@ skipping 175 matching lines @@
                                 const string& reason) {
   if (goaway_sent_) {
     return;
   }
   goaway_sent_ = true;
 
   DVLOG(1) << ENDPOINT << "Going away with 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)));
 }
 
 QuicByteCount QuicConnection::max_packet_length() const {
   return packet_generator_.GetCurrentMaxPacketLength();
 }
 
 void QuicConnection::SetMaxPacketLength(QuicByteCount length) {
   return packet_generator_.SetMaxPacketLength(LimitMaxPacketSize(length));
@@ skipping 144 matching lines @@
 
   if (packet_number_of_last_sent_packet_ >= next_mtu_probe_at_) {
     // Use an alarm to send the MTU probe to ensure that no ScopedPacketBundlers
     // are active.
     mtu_discovery_alarm_->Set(clock_->ApproximateNow());
   }
 }
 
 QuicConnection::ScopedPacketBundler::ScopedPacketBundler(
     QuicConnection* connection,
-    AckBundling send_ack)
+    AckBundling ack_mode)
     : connection_(connection),
       already_in_batch_mode_(connection != nullptr &&
                              connection->packet_generator_.InBatchMode()) {
   if (connection_ == nullptr) {
     return;
   }
   // Move generator into batch mode. If caller wants us to include an ack,
   // check the delayed-ack timer to see if there's ack info to be sent.
   if (!already_in_batch_mode_) {
     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;
   }
   // If we changed the generator's batch state, restore original batch state.
   if (!already_in_batch_mode_) {
     DVLOG(1) << "Leaving Batch Mode.";
     connection_->packet_generator_.FinishBatchOperations();
   }
   DCHECK_EQ(already_in_batch_mode_,
@@ skipping 168 matching lines @@
 }
 
 StringPiece QuicConnection::GetCurrentPacket() {
   if (current_packet_data_ == nullptr) {
     return StringPiece();
   }
   return StringPiece(current_packet_data_, last_size_);
 }
 
 }  // namespace net