Cleanup in QuicConnection to always cancel the send alarm when CanWrite

net/quic/quic_connection_test.cc

Index: net/quic/quic_connection_test.cc
diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc
index cf6ae8ca9304f62a54af974f6ac48d77d0a6a4bc..54817151c18aa6c956117ff61de8ceae9d406174 100644
--- a/net/quic/quic_connection_test.cc
+++ b/net/quic/quic_connection_test.cc
@@ -914,6 +914,18 @@ class QuicConnectionTest : public ::testing::TestWithParam<QuicVersion> {
     EXPECT_CALL(visitor_, OnWriteBlocked()).Times(AtLeast(1));
   }
 
+  void CongestionBlockWrites() {
+    EXPECT_CALL(*send_algorithm_,
+                TimeUntilSend(_, _, _)).WillRepeatedly(
+                    testing::Return(QuicTime::Delta::FromSeconds(1)));
+  }
+
+  void CongestionUnblockWrites() {
+    EXPECT_CALL(*send_algorithm_,
+                TimeUntilSend(_, _, _)).WillRepeatedly(
+                    testing::Return(QuicTime::Delta::Zero()));
+  }
+
   QuicConnectionId connection_id_;
   QuicFramer framer_;
   QuicPacketCreator peer_creator_;
@@ -1568,9 +1580,7 @@ TEST_P(QuicConnectionTest, AbandonAllFEC) {
 }
 
 TEST_P(QuicConnectionTest, FramePacking) {
-  // Block the connection.
-  connection_.GetSendAlarm()->Set(
-      clock_.ApproximateNow().Add(QuicTime::Delta::FromSeconds(1)));
+  CongestionBlockWrites();
 
   // Send an ack and two stream frames in 1 packet by queueing them.
   connection_.SendAck();
@@ -1581,7 +1591,7 @@ TEST_P(QuicConnectionTest, FramePacking) {
                                      &TestConnection::SendStreamData5))));
 
   EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
-  // Unblock the connection.
+  CongestionUnblockWrites();
   connection_.GetSendAlarm()->Fire();
   EXPECT_EQ(0u, connection_.NumQueuedPackets());
   EXPECT_FALSE(connection_.HasQueuedData());
@@ -1595,15 +1605,13 @@ TEST_P(QuicConnectionTest, FramePacking) {
     EXPECT_EQ(3u, writer_->frame_count());
   }
   EXPECT_FALSE(writer_->ack_frames().empty());
-  EXPECT_EQ(2u, writer_->stream_frames().size());
+  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) {
-  // Block the connection.
-  connection_.GetSendAlarm()->Set(
-      clock_.ApproximateNow().Add(QuicTime::Delta::FromSeconds(1)));
+  CongestionBlockWrites();
 
   // Send an ack and two stream frames (one non-crypto, then one crypto) in 2
   // packets by queueing them.
@@ -1615,21 +1623,19 @@ TEST_P(QuicConnectionTest, FramePackingNonCryptoThenCrypto) {
                                      &TestConnection::SendCryptoStreamData))));
 
   EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
-  // Unblock the connection.
+  CongestionUnblockWrites();
   connection_.GetSendAlarm()->Fire();
   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());
-  EXPECT_EQ(1u, writer_->stream_frames().size());
+  ASSERT_EQ(1u, writer_->stream_frames().size());
   EXPECT_EQ(kCryptoStreamId, writer_->stream_frames()[0].stream_id);
 }
 
 TEST_P(QuicConnectionTest, FramePackingCryptoThenNonCrypto) {
-  // Block the connection.
-  connection_.GetSendAlarm()->Set(
-      clock_.ApproximateNow().Add(QuicTime::Delta::FromSeconds(1)));
+  CongestionBlockWrites();
 
   // Send an ack and two stream frames (one crypto, then one non-crypto) in 3
   // packets by queueing them.
@@ -1641,28 +1647,23 @@ TEST_P(QuicConnectionTest, FramePackingCryptoThenNonCrypto) {
                                      &TestConnection::SendStreamData3))));
 
   EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(3);
-  // Unblock the connection.
+  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());
-  EXPECT_EQ(1u, writer_->stream_frames().size());
+  ASSERT_EQ(1u, writer_->stream_frames().size());
   EXPECT_EQ(kClientDataStreamId1, writer_->stream_frames()[0].stream_id);
 }
 
 TEST_P(QuicConnectionTest, FramePackingFEC) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   // Enable fec.
   EXPECT_TRUE(QuicPacketCreatorPeer::SwitchFecProtectionOn(
       QuicConnectionPeer::GetPacketCreator(&connection_), 6));
 
-  // Block the connection.
-  connection_.GetSendAlarm()->Set(
-      clock_.ApproximateNow().Add(QuicTime::Delta::FromSeconds(1)));
+  CongestionBlockWrites();
 
   // Send an ack and two stream frames in 1 packet by queueing them.
   connection_.SendAck();
@@ -1673,7 +1674,7 @@ TEST_P(QuicConnectionTest, FramePackingFEC) {
                                      &TestConnection::SendStreamData5))));
 
   EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
-  // Unblock the connection.
+  CongestionUnblockWrites();
   connection_.GetSendAlarm()->Fire();
   EXPECT_EQ(0u, connection_.NumQueuedPackets());
   EXPECT_FALSE(connection_.HasQueuedData());
@@ -2107,9 +2108,6 @@ TEST_P(QuicConnectionTest, DontLatchUnackedPacket) {
 }
 
 TEST_P(QuicConnectionTest, ReviveMissingPacketAfterFecPacket) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
 
   // Don't send missing packet 1.
@@ -2119,9 +2117,6 @@ TEST_P(QuicConnectionTest, ReviveMissingPacketAfterFecPacket) {
 }
 
 TEST_P(QuicConnectionTest, ReviveMissingPacketWithVaryingSeqNumLengths) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
 
   // Set up a debug visitor to the connection.
@@ -2151,9 +2146,6 @@ TEST_P(QuicConnectionTest, ReviveMissingPacketWithVaryingSeqNumLengths) {
 }
 
 TEST_P(QuicConnectionTest, ReviveMissingPacketWithVaryingConnectionIdLengths) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
 
   // Set up a debug visitor to the connection.
@@ -2183,9 +2175,6 @@ TEST_P(QuicConnectionTest, ReviveMissingPacketWithVaryingConnectionIdLengths) {
 }
 
 TEST_P(QuicConnectionTest, ReviveMissingPacketAfterDataPacketThenFecPacket) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
 
   ProcessFecProtectedPacket(1, false, kEntropyFlag);
@@ -2196,9 +2185,6 @@ TEST_P(QuicConnectionTest, ReviveMissingPacketAfterDataPacketThenFecPacket) {
 }
 
 TEST_P(QuicConnectionTest, ReviveMissingPacketAfterDataPacketsThenFecPacket) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
 
   ProcessFecProtectedPacket(1, false, !kEntropyFlag);
@@ -2211,9 +2197,6 @@ TEST_P(QuicConnectionTest, ReviveMissingPacketAfterDataPacketsThenFecPacket) {
 }
 
 TEST_P(QuicConnectionTest, ReviveMissingPacketAfterDataPacket) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
 
   // Don't send missing packet 1.
@@ -2225,9 +2208,6 @@ TEST_P(QuicConnectionTest, ReviveMissingPacketAfterDataPacket) {
 }
 
 TEST_P(QuicConnectionTest, ReviveMissingPacketAfterDataPackets) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
 
   ProcessFecProtectedPacket(1, false, !kEntropyFlag);
@@ -2593,9 +2573,6 @@ TEST_P(QuicConnectionTest, UpdateQuicCongestionFeedbackFrame) {
 }
 
 TEST_P(QuicConnectionTest, DontUpdateQuicCongestionFeedbackFrameForRevived) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
   SendAckPacketToPeer();
   // Process an FEC packet, and revive the missing data packet
@@ -2767,8 +2744,7 @@ TEST_P(QuicConnectionTest, SendSchedulerDelayThenSend) {
 }
 
 TEST_P(QuicConnectionTest, SendSchedulerDelayThenRetransmit) {
-  EXPECT_CALL(*send_algorithm_, TimeUntilSend(_, _, _))
-      .WillRepeatedly(testing::Return(QuicTime::Delta::Zero()));
+  CongestionUnblockWrites();
   EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, 1, _, _));
   connection_.SendStreamDataWithString(3, "foo", 0, !kFin, NULL);
   EXPECT_EQ(0u, connection_.NumQueuedPackets());
@@ -2777,17 +2753,13 @@ TEST_P(QuicConnectionTest, SendSchedulerDelayThenRetransmit) {
   // Test that if we send a retransmit with a delay, it ends up queued in the
   // sent packet manager, but not yet serialized.
   EXPECT_CALL(*send_algorithm_, OnRetransmissionTimeout(true));
-  EXPECT_CALL(*send_algorithm_,
-              TimeUntilSend(_, _, _)).WillOnce(
-                  testing::Return(QuicTime::Delta::FromMicroseconds(1)));
+  CongestionBlockWrites();
   connection_.GetRetransmissionAlarm()->Fire();
   EXPECT_EQ(0u, connection_.NumQueuedPackets());
 
   // Advance the clock to fire the alarm, and configure the scheduler
   // to permit the packet to be sent.
-  EXPECT_CALL(*send_algorithm_,
-              TimeUntilSend(_, _, _)).Times(3).
-                  WillRepeatedly(testing::Return(QuicTime::Delta::Zero()));
+  CongestionUnblockWrites();
 
   // Ensure the scheduler is notified this is a retransmit.
   EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _));
@@ -2862,9 +2834,7 @@ TEST_P(QuicConnectionTest, SendSchedulerDelayThenOnCanWrite) {
   // TODO(ianswett): This test is unrealistic, because we would not serialize
   // new data if the send algorithm said not to.
   QuicPacket* packet = ConstructDataPacket(1, 0, !kEntropyFlag);
-  EXPECT_CALL(*send_algorithm_,
-              TimeUntilSend(_, _, _)).WillOnce(
-                  testing::Return(QuicTime::Delta::FromMicroseconds(10)));
+  CongestionBlockWrites();
   connection_.SendPacket(
       ENCRYPTION_NONE, 1, packet, kTestEntropyHash, HAS_RETRANSMITTABLE_DATA);
   EXPECT_EQ(1u, connection_.NumQueuedPackets());
@@ -3194,9 +3164,6 @@ TEST_P(QuicConnectionTest, ReceivedEntropyHashCalculation) {
 }
 
 TEST_P(QuicConnectionTest, ReceivedEntropyHashCalculationHalfFEC) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   // FEC packets should not change the entropy hash calculation.
   EXPECT_CALL(visitor_, OnStreamFrames(_)).Times(AtLeast(1));
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
@@ -3543,9 +3510,6 @@ TEST_P(QuicConnectionTest, CheckSendStats) {
 }
 
 TEST_P(QuicConnectionTest, CheckReceiveStats) {
-  if (version() < QUIC_VERSION_15) {
-    return;
-  }
   EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
 
   size_t received_bytes = 0;