Fix a bug protected by gfe2_reloadable_flag_quic_respect_send_alarm where acks would not be sent im…

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 <ostream>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 1365 matching lines @@
   EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _));
   ProcessAckPacket(&frame);
   EXPECT_EQ(num_packets, manager_->largest_observed());
 }
 
 TEST_P(QuicConnectionTest, AckReceiptCausesAckSendBadEntropy) {
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
 
   ProcessPacket(1);
   // Delay sending, then queue up an ack.
-  EXPECT_CALL(*send_algorithm_,
-              TimeUntilSend(_, _, _)).WillOnce(
-                  testing::Return(QuicTime::Delta::FromMicroseconds(1)));
+  if (!FLAGS_quic_respect_send_alarm2) {
+    EXPECT_CALL(*send_algorithm_, TimeUntilSend(_, _, _))
+        .WillOnce(testing::Return(QuicTime::Delta::FromMicroseconds(1)));
+  }
   QuicConnectionPeer::SendAck(&connection_);
 
   // Process an ack with a least unacked of the received ack.
   // This causes an ack to be sent when TimeUntilSend returns 0.
   EXPECT_CALL(*send_algorithm_,
               TimeUntilSend(_, _, _)).WillRepeatedly(
                   testing::Return(QuicTime::Delta::Zero()));
   // Skip a packet and then record an ack.
   QuicAckFrame frame = InitAckFrame(0);
   ProcessAckPacket(3, &frame);
@@ skipping 813 matching lines @@
   // Simulate the retransmission alarm firing. FEC packet is no longer
   // outstanding.
   clock_.AdvanceTime(rto_time.Subtract(clock_.Now()));
   connection_.GetRetransmissionAlarm()->Fire();
 
   EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
   EXPECT_EQ(0u, QuicSentPacketManagerPeer::GetBytesInFlight(manager_));
 }
 
 TEST_P(QuicConnectionTest, FramePacking) {
-  CongestionBlockWrites();
-
   // Send an ack and two stream frames in 1 packet by queueing them.
-  connection_.SendAck();
-  EXPECT_CALL(visitor_, OnCanWrite()).WillOnce(DoAll(
-      IgnoreResult(InvokeWithoutArgs(&connection_,
-                                     &TestConnection::SendStreamData3)),
-      IgnoreResult(InvokeWithoutArgs(&connection_,
-                                     &TestConnection::SendStreamData5))));
-
-  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
-  CongestionUnblockWrites();
-  connection_.GetSendAlarm()->Fire();
+  {
+    QuicConnection::ScopedPacketBundler bundler(&connection_,
+                                                QuicConnection::SEND_ACK);
+    connection_.SendStreamData3();
+    connection_.SendStreamData5();
+    EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+  }
   EXPECT_EQ(0u, connection_.NumQueuedPackets());
   EXPECT_FALSE(connection_.HasQueuedData());
 
   // Parse the last packet and ensure it's an ack and two stream frames from
   // two different streams.
   EXPECT_EQ(4u, writer_->frame_count());
   EXPECT_FALSE(writer_->stop_waiting_frames().empty());
   EXPECT_FALSE(writer_->ack_frames().empty());
   ASSERT_EQ(2u, writer_->stream_frames().size());
   EXPECT_EQ(kClientDataStreamId1, writer_->stream_frames()[0]->stream_id);
   EXPECT_EQ(kClientDataStreamId2, writer_->stream_frames()[1]->stream_id);
 }
 
 TEST_P(QuicConnectionTest, FramePackingNonCryptoThenCrypto) {
-  CongestionBlockWrites();
-
   // Send an ack and two stream frames (one non-crypto, then one crypto) in 2
   // packets by queueing them.
-  connection_.SendAck();
-  EXPECT_CALL(visitor_, OnCanWrite()).WillOnce(DoAll(
-      IgnoreResult(InvokeWithoutArgs(&connection_,
-                                     &TestConnection::SendStreamData3)),
-      IgnoreResult(InvokeWithoutArgs(&connection_,
-                                     &TestConnection::SendCryptoStreamData))));
-
-  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
-  CongestionUnblockWrites();
-  connection_.GetSendAlarm()->Fire();
+  {
+    EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
+    QuicConnection::ScopedPacketBundler bundler(&connection_,
+                                                QuicConnection::SEND_ACK);
+    connection_.SendStreamData3();
+    connection_.SendCryptoStreamData();
+  }
   EXPECT_EQ(0u, connection_.NumQueuedPackets());
   EXPECT_FALSE(connection_.HasQueuedData());
 
   // 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 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(2);
-  CongestionUnblockWrites();
-  connection_.GetSendAlarm()->Fire();
+  {
+    EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
+    QuicConnection::ScopedPacketBundler bundler(&connection_,
+                                                QuicConnection::SEND_ACK);
+    connection_.SendCryptoStreamData();
+    connection_.SendStreamData3();
+  }
   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);
 }
 
 TEST_P(QuicConnectionTest, FramePackingFEC) {
   EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(creator_));
 
-  CongestionBlockWrites();
-
   // Queue an ack and two stream frames. Ack gets flushed when FEC is turned on
   // for sending protected data; two stream frames are packed in 1 packet.
-  EXPECT_CALL(visitor_, OnCanWrite()).WillOnce(DoAll(
-      IgnoreResult(InvokeWithoutArgs(
-          &connection_, &TestConnection::SendStreamData3WithFec)),
-      IgnoreResult(InvokeWithoutArgs(
-          &connection_, &TestConnection::SendStreamData5WithFec))));
-  connection_.SendAck();
-
-  EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
-  CongestionUnblockWrites();
-  connection_.GetSendAlarm()->Fire();
+  {
+    QuicConnection::ScopedPacketBundler bundler(&connection_,
+                                                QuicConnection::SEND_ACK);
+    connection_.SendStreamData3WithFec();
+    connection_.SendStreamData5WithFec();
+    EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+  }
   EXPECT_EQ(0u, connection_.NumQueuedPackets());
   EXPECT_FALSE(connection_.HasQueuedData());
 
   // Parse the last packet and ensure it's in an fec group.
   EXPECT_EQ(2u, writer_->header().fec_group);
   EXPECT_EQ(2u, writer_->frame_count());
 
   // FEC alarm should be set.
   EXPECT_TRUE(connection_.GetFecAlarm()->IsSet());
 }
@@ skipping 3148 matching lines @@
 TEST_P(QuicConnectionTest, DoNotSendGoAwayTwice) {
   EXPECT_FALSE(connection_.goaway_sent());
   EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
   connection_.SendGoAway(QUIC_PEER_GOING_AWAY, kHeadersStreamId, "Going Away.");
   EXPECT_TRUE(connection_.goaway_sent());
   EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
   connection_.SendGoAway(QUIC_PEER_GOING_AWAY, kHeadersStreamId, "Going Away.");
 }
 
 TEST_P(QuicConnectionTest, ReevaluateTimeUntilSendOnAck) {
-  ValueRestore<bool> old_flag(&FLAGS_quic_respect_send_alarm, true);
+  ValueRestore<bool> old_flag(&FLAGS_quic_respect_send_alarm2, true);
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   connection_.SendStreamDataWithString(kClientDataStreamId1, "foo", 0, !kFin,
                                        nullptr);
 
   // Evaluate CanWrite, and have it return a non-Zero value.
   EXPECT_CALL(*send_algorithm_, TimeUntilSend(_, _, _))
       .WillRepeatedly(Return(QuicTime::Delta::FromMilliseconds(1)));
   connection_.OnCanWrite();
   EXPECT_TRUE(connection_.GetSendAlarm()->IsSet());
   EXPECT_EQ(clock_.Now().Add(QuicTime::Delta::FromMilliseconds(1)),
@@ skipping 12 matching lines @@
       .WillRepeatedly(Return(QuicTime::Delta::FromMilliseconds(2)));
   ProcessAckPacket(&ack);
   EXPECT_EQ(1u, writer_->frame_count());
   EXPECT_EQ(1u, writer_->stream_frames().size());
   EXPECT_TRUE(connection_.GetSendAlarm()->IsSet());
   EXPECT_EQ(clock_.Now().Add(QuicTime::Delta::FromMilliseconds(2)),
             connection_.GetSendAlarm()->deadline());
   writer_->Reset();
 }
 
+TEST_P(QuicConnectionTest, SendAcksImmediately) {
+  FLAGS_quic_respect_send_alarm2 = true;
+  CongestionBlockWrites();
+  SendAckPacketToPeer();
+}
+
 TEST_P(QuicConnectionTest, SendingUnencryptedStreamDataFails) {
   FLAGS_quic_never_write_unencrypted_data = true;
   EXPECT_CALL(visitor_,
               OnConnectionClosed(QUIC_UNENCRYPTED_STREAM_DATA, false));
   EXPECT_DFATAL(connection_.SendStreamDataWithString(3, "", 0, kFin, nullptr),
                 "Cannot send stream data without encryption.");
   EXPECT_FALSE(connection_.connected());
 }
 
 }  // namespace
 }  // namespace test
 }  // namespace net