Fix a bug where QUIC ack frames were not bundled with crypto stream

net/quic/quic_connection_test.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 "base/basictypes.h"
 #include "base/bind.h"
 #include "base/stl_util.h"
 #include "net/base/net_errors.h"
@@ skipping 502 matching lines @@
     EXPECT_TRUE(CanWriteStreamData());
     return SendStreamData5();
   }
 
   // The crypto stream has special semantics so that it is not blocked by a
   // congestion window limitation, and also so that it gets put into a separate
   // packet (so that it is easier to reason about a crypto frame not being
   // split needlessly across packet boundaries).  As a result, we have separate
   // tests for some cases for this stream.
   QuicConsumedData SendCryptoStreamData() {
-    this->Flush();
-    QuicConsumedData consumed =
-        SendStreamDataWithString(kCryptoStreamId, "chlo", 0, !kFin, NULL);
-    this->Flush();
-    return consumed;
+    return SendStreamDataWithString(kCryptoStreamId, "chlo", 0, !kFin, NULL);
   }
 
   bool is_server() {
     return QuicConnectionPeer::IsServer(this);
   }
 
   void set_version(QuicVersion version) {
     QuicConnectionPeer::GetFramer(this)->set_version(version);
   }
 
@@ skipping 1124 matching lines @@
 
   // Parse the last packet and ensure it's the crypto stream frame.
   EXPECT_EQ(1u, writer_->frame_count());
   ASSERT_EQ(1u, writer_->stream_frames().size());
   EXPECT_EQ(kCryptoStreamId, writer_->stream_frames()[0].stream_id);
 }
 
 TEST_P(QuicConnectionTest, FramePackingCryptoThenNonCrypto) {
   CongestionBlockWrites();
 
-  // Send an ack and two stream frames (one crypto, then one non-crypto) in 3
+  // Send an ack and two stream frames (one crypto, then one non-crypto) in 2
   // packets by queueing them.
   connection_.SendAck();
   EXPECT_CALL(visitor_, OnCanWrite()).WillOnce(DoAll(
       IgnoreResult(InvokeWithoutArgs(&connection_,
                                      &TestConnection::SendCryptoStreamData)),
       IgnoreResult(InvokeWithoutArgs(&connection_,
                                      &TestConnection::SendStreamData3))));
 
-  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(3);
+  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
   CongestionUnblockWrites();
   connection_.GetSendAlarm()->Fire();
   EXPECT_EQ(0u, connection_.NumQueuedPackets());
   EXPECT_FALSE(connection_.HasQueuedData());
 
   // Parse the last packet and ensure it's the stream frame from stream 3.
   EXPECT_EQ(1u, writer_->frame_count());
   ASSERT_EQ(1u, writer_->stream_frames().size());
   EXPECT_EQ(kClientDataStreamId1, writer_->stream_frames()[0].stream_id);
 }
@@ skipping 1363 matching lines @@
 TEST_P(QuicConnectionTest, SendDelayedAckOnOutgoingCryptoPacket) {
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   ProcessPacket(1);
   connection_.SendStreamDataWithString(kCryptoStreamId, "foo", 0, !kFin, NULL);
   // Check that ack is bundled with outgoing crypto data.
   EXPECT_EQ(version() <= QUIC_VERSION_15 ? 2u : 3u, writer_->frame_count());
   EXPECT_FALSE(writer_->ack_frames().empty());
   EXPECT_FALSE(connection_.GetAckAlarm()->IsSet());
 }
 
+TEST_P(QuicConnectionTest, BundleAckForSecondCHLO) {
+  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
+  EXPECT_FALSE(connection_.GetAckAlarm()->IsSet());
+  EXPECT_CALL(visitor_, OnCanWrite()).WillOnce(
+      IgnoreResult(InvokeWithoutArgs(&connection_,
+                                     &TestConnection::SendCryptoStreamData)));
+  // Process a packet from the crypto stream, which is frame1_'s default.
+  // Receiving the CHLO as packet 2 first will cause the connection to
+  // immediately send an ack, due to the packet gap.
+  ProcessPacket(2);
+  // Check that ack is sent and that delayed ack alarm is reset.
+  if (version() > QUIC_VERSION_15) {
+    EXPECT_EQ(3u, writer_->frame_count());
+    EXPECT_FALSE(writer_->stop_waiting_frames().empty());
+  } else {
+    EXPECT_EQ(2u, writer_->frame_count());
+  }
+  EXPECT_EQ(1u, writer_->stream_frames().size());
+  EXPECT_FALSE(writer_->ack_frames().empty());
+  EXPECT_FALSE(connection_.GetAckAlarm()->IsSet());
+}
+
 TEST_P(QuicConnectionTest, BundleAckWithDataOnIncomingAck) {
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   connection_.SendStreamDataWithString(kClientDataStreamId1, "foo", 0,
                                        !kFin, NULL);
   connection_.SendStreamDataWithString(kClientDataStreamId1, "foo", 3,
                                        !kFin, NULL);
   // Ack the second packet, which will retransmit the first packet.
   QuicAckFrame ack = InitAckFrame(2, 0);
   NackPacket(1, &ack);
   SequenceNumberSet lost_packets;
@@ skipping 941 matching lines @@
   QuicBlockedFrame blocked;
   blocked.stream_id = 3;
   EXPECT_CALL(visitor_, OnBlockedFrames(_));
   ProcessFramePacket(QuicFrame(&blocked));
   EXPECT_TRUE(ack_alarm->IsSet());
 }
 
 }  // namespace
 }  // namespace test
 }  // namespace net