Fix a QUIC bug where previously undecryptable packets were not decrypted

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 f1e25d6e24b4989a0fc8062db44263385b8777b9..32c44f44a5e81a6c4e19e23e284d0c07570b0a31 100644
--- a/net/quic/quic_connection.cc
+++ b/net/quic/quic_connection.cc
@@ -167,6 +167,7 @@ QuicConnection::QuicConnection(QuicGuid guid,
       largest_seen_packet_with_ack_(0),
       pending_version_negotiation_packet_(false),
       received_packet_manager_(kTCP),
+      ack_queued_(false),
       ack_alarm_(helper->CreateAlarm(new AckAlarm(this))),
       retransmission_alarm_(helper->CreateAlarm(new RetransmissionAlarm(this))),
       send_alarm_(helper->CreateAlarm(new SendAlarm(this))),
@@ -636,13 +637,7 @@ void QuicConnection::OnPacketComplete() {
              << last_stream_frames_.size()
              << " stream frames for " << last_header_.public_header.guid;
 
-  // Must called before ack processing, because processing acks removes entries
-  // from unacket_packets_, increasing the least_unacked.
-  const bool last_packet_should_instigate_ack = ShouldLastPacketInstigateAck();
-
-  // If the incoming packet was missing, send an ack immediately.
-  bool send_ack_immediately = received_packet_manager_.IsMissing(
-      last_header_.packet_sequence_number);
+  MaybeQueueAck();
 
   // Discard the packet if the visitor fails to process the stream frames.
   if (!last_stream_frames_.empty() &&
@@ -680,15 +675,36 @@ void QuicConnection::OnPacketComplete() {
 
   // If there are new missing packets to report, send an ack immediately.
   if (received_packet_manager_.HasNewMissingPackets()) {
-    send_ack_immediately = true;
+    ack_queued_ = true;
+    ack_alarm_->Cancel();
   }
 
-  MaybeSendInResponseToPacket(send_ack_immediately,
-                              last_packet_should_instigate_ack);
-
   ClearLastFrames();
 }
 
+void QuicConnection::MaybeQueueAck() {
+  // If the incoming packet was missing, send an ack immediately.
+  ack_queued_ = received_packet_manager_.IsMissing(
+      last_header_.packet_sequence_number);
+
+  // ShouldLastPacketInstigateAck must called before ack processing, because
+  // processing acks removes entries from unacket_packets_, increasing the
+  // least_unacked.
+  if (!ack_queued_ && ShouldLastPacketInstigateAck()) {
+    if (ack_alarm_->IsSet()) {
+      ack_queued_ = true;
+    } else {
+      ack_alarm_->Set(clock_->ApproximateNow().Add(
+          sent_packet_manager_.DelayedAckTime()));
+      DVLOG(1) << "Ack timer set; next packet or timer will trigger ACK.";
+    }
+  }
+
+  if (ack_queued_) {
+    ack_alarm_->Cancel();
+  }
+}
+
 void QuicConnection::ClearLastFrames() {
   last_stream_frames_.clear();
   last_goaway_frames_.clear();
@@ -717,9 +733,8 @@ bool QuicConnection::ShouldLastPacketInstigateAck() {
     return true;
   }
 
-  // If the peer is still waiting for a packet that we are no
-  // longer planning to send, we should send an ack to raise
-  // the high water mark.
+  // If the peer is still waiting for a packet that we are no longer planning to
+  // send, send an ack to raise the high water mark.
   if (!last_ack_frames_.empty() &&
       !last_ack_frames_.back().received_info.missing_packets.empty()) {
     return sent_packet_manager_.GetLeastUnackedSentPacket() >
@@ -728,39 +743,26 @@ bool QuicConnection::ShouldLastPacketInstigateAck() {
   return false;
 }
 
-void QuicConnection::MaybeSendInResponseToPacket(
-    bool send_ack_immediately,
-    bool last_packet_should_instigate_ack) {
-  // |include_ack| is false since we decide about ack bundling below.
+void QuicConnection::MaybeSendInResponseToPacket() {
+  if (!connected_) {
+    return;
+  }
   ScopedPacketBundler bundler(this, false);
-
-  if (last_packet_should_instigate_ack) {
-    // In general, we ack every second packet.  When we don't ack the first
-    // packet, we set the delayed ack alarm.  Thus, if the ack alarm is set
-    // then we know this is the second packet, and we should send an ack.
-    if (send_ack_immediately || ack_alarm_->IsSet()) {
-      SendAck();
-      DCHECK(!ack_alarm_->IsSet());
-    } else {
-      ack_alarm_->Set(clock_->ApproximateNow().Add(
-          sent_packet_manager_.DelayedAckTime()));
-      DVLOG(1) << "Ack timer set; next packet or timer will trigger ACK.";
-    }
+  if (ack_queued_) {
+    SendAck();
   }
 
-  if (!last_ack_frames_.empty()) {
-    // Now the we have received an ack, we might be able to send packets which
-    // are queued locally, or drain streams which are blocked.
-    QuicTime::Delta delay = sent_packet_manager_.TimeUntilSend(
-        time_of_last_received_packet_, NOT_RETRANSMISSION,
-        HAS_RETRANSMITTABLE_DATA, NOT_HANDSHAKE);
-    if (delay.IsZero()) {
-      send_alarm_->Cancel();
-      WriteIfNotBlocked();
-    } else if (!delay.IsInfinite()) {
-      send_alarm_->Cancel();
-      send_alarm_->Set(time_of_last_received_packet_.Add(delay));
-    }
+  // Now that we have received an ack, we might be able to send packets which
+  // are queued locally, or drain streams which are blocked.
+  QuicTime::Delta delay = sent_packet_manager_.TimeUntilSend(
+      time_of_last_received_packet_, NOT_RETRANSMISSION,
+      HAS_RETRANSMITTABLE_DATA, NOT_HANDSHAKE);
+  if (delay.IsZero()) {
+    send_alarm_->Cancel();
+    WriteIfNotBlocked();
+  } else if (!delay.IsInfinite()) {
+    send_alarm_->Cancel();
+    send_alarm_->Set(time_of_last_received_packet_.Add(delay));
   }
 }
 
@@ -873,8 +875,10 @@ void QuicConnection::ProcessUdpPacket(const IPEndPoint& self_address,
              << last_header_.packet_sequence_number;
     return;
   }
+
   MaybeProcessUndecryptablePackets();
   MaybeProcessRevivedPacket();
+  MaybeSendInResponseToPacket();
 }
 
 bool QuicConnection::OnCanWrite() {