Fixed integer signing bug in cubic bytes/packet implementation

net/quic/core/congestion_control/cubic_test.cc

Index: net/quic/core/congestion_control/cubic_test.cc
diff --git a/net/quic/core/congestion_control/cubic_test.cc b/net/quic/core/congestion_control/cubic_test.cc
index a048f85cfaf0ac0808374806e8f201c04af5a84d..578b94e78d901d0ca192aff8c7aa87276ab386ac 100644
--- a/net/quic/core/congestion_control/cubic_test.cc
+++ b/net/quic/core/congestion_control/cubic_test.cc
@@ -18,62 +18,97 @@ const float kNConnectionBeta = (kNumConnections - 1 + kBeta) / kNumConnections;
 const float kNConnectionAlpha = 3 * kNumConnections * kNumConnections *
                                 (1 - kNConnectionBeta) / (1 + kNConnectionBeta);
 
-class CubicTest : public ::testing::Test {
+// TODO(jokulik): Once we've rolled out the cubic convex fix, we will
+// no longer need a parameterized test.
+class CubicTest : public ::testing::TestWithParam<bool> {
  protected:
   CubicTest()
       : one_ms_(QuicTime::Delta::FromMilliseconds(1)),
         hundred_ms_(QuicTime::Delta::FromMilliseconds(100)),
-        cubic_(&clock_) {}
+        cubic_(&clock_) {
+    fix_convex_mode_ = GetParam();
+    cubic_.SetFixConvexMode(fix_convex_mode_);
+  }
   const QuicTime::Delta one_ms_;
   const QuicTime::Delta hundred_ms_;
   MockClock clock_;
   Cubic cubic_;
+  bool fix_convex_mode_;
 };
 
-TEST_F(CubicTest, AboveOrigin) {
+INSTANTIATE_TEST_CASE_P(CubicTests, CubicTest, testing::Bool());
+
+TEST_P(CubicTest, AboveOrigin) {
   // Convex growth.
   const QuicTime::Delta rtt_min = hundred_ms_;
+  const float rtt_min_s = rtt_min.ToMilliseconds() / 1000.0;
   QuicPacketCount current_cwnd = 10;
-  QuicPacketCount expected_cwnd = current_cwnd + 1;
+  // Without the signed-integer, cubic-convex fix, we mistakenly
+  // increment cwnd after only one_ms_ and a single ack.
+  QuicPacketCount expected_cwnd =
+      fix_convex_mode_ ? current_cwnd : current_cwnd + 1;
   // Initialize the state.
   clock_.AdvanceTime(one_ms_);
-  EXPECT_EQ(expected_cwnd,
-            cubic_.CongestionWindowAfterAck(current_cwnd, rtt_min));
-  current_cwnd = expected_cwnd;
+  const QuicTime initial_time = clock_.ApproximateNow();
+  current_cwnd = cubic_.CongestionWindowAfterAck(current_cwnd, rtt_min);
+  ASSERT_EQ(expected_cwnd, current_cwnd);
+  const QuicPacketCount initial_cwnd = current_cwnd;
   // Normal TCP phase.
-  for (int i = 0; i < 48; ++i) {
-    for (QuicPacketCount n = 1; n < current_cwnd / kNConnectionAlpha; ++n) {
+  // The maximum number of expected reno RTTs can be calculated by
+  // finding the point where the cubic curve and the reno curve meet.
+  const int max_reno_rtts =
+      std::sqrt(kNConnectionAlpha / (.4 * rtt_min_s * rtt_min_s * rtt_min_s)) -
+      1;
+  for (int i = 0; i < max_reno_rtts; ++i) {
+    const QuicByteCount max_per_ack_cwnd = current_cwnd;
+    for (QuicPacketCount n = 1; n < max_per_ack_cwnd / kNConnectionAlpha; ++n) {
       // Call once per ACK.
-      EXPECT_NEAR(current_cwnd,
-                  cubic_.CongestionWindowAfterAck(current_cwnd, rtt_min), 1);
+      const QuicByteCount next_cwnd =
+          cubic_.CongestionWindowAfterAck(current_cwnd, rtt_min);
+      ASSERT_EQ(current_cwnd, next_cwnd);
     }
     clock_.AdvanceTime(hundred_ms_);
     current_cwnd = cubic_.CongestionWindowAfterAck(current_cwnd, rtt_min);
-    EXPECT_NEAR(expected_cwnd, current_cwnd, 1);
-    expected_cwnd++;
+    if (fix_convex_mode_) {
+      // When we fix convex mode and the uint64 arithmetic, we
+      // increase the expected_cwnd only after after the first 100ms,
+      // rather than after the initial 1ms.
+      expected_cwnd++;
+      ASSERT_EQ(expected_cwnd, current_cwnd);
+    } else {
+      ASSERT_EQ(expected_cwnd, current_cwnd);
+      expected_cwnd++;
+    }
   }
   // Cubic phase.
   for (int i = 0; i < 52; ++i) {
     for (QuicPacketCount n = 1; n < current_cwnd; ++n) {
       // Call once per ACK.
-      EXPECT_EQ(current_cwnd,
+      ASSERT_EQ(current_cwnd,
                 cubic_.CongestionWindowAfterAck(current_cwnd, rtt_min));
     }
     clock_.AdvanceTime(hundred_ms_);
     current_cwnd = cubic_.CongestionWindowAfterAck(current_cwnd, rtt_min);
   }
   // Total time elapsed so far; add min_rtt (0.1s) here as well.
-  float elapsed_time_s = 10.0f + 0.1f;
+  const float elapsed_time_ms =
+      (clock_.ApproximateNow() - initial_time).ToMilliseconds() +
+      rtt_min.ToMilliseconds();
+  const float elapsed_time_s = elapsed_time_ms / 1000.0;
   // |expected_cwnd| is initial value of cwnd + K * t^3, where K = 0.4.
   expected_cwnd =
-      11 + (elapsed_time_s * elapsed_time_s * elapsed_time_s * 410) / 1024;
+      initial_cwnd +
+      (elapsed_time_s * elapsed_time_s * elapsed_time_s * 410) / 1024;
   EXPECT_EQ(expected_cwnd, current_cwnd);
 }
 
-TEST_F(CubicTest, LossEvents) {
+TEST_P(CubicTest, LossEvents) {
   const QuicTime::Delta rtt_min = hundred_ms_;
   QuicPacketCount current_cwnd = 422;
-  QuicPacketCount expected_cwnd = current_cwnd + 1;
+  // Without the signed-integer, cubic-convex fix, we mistakenly
+  // increment cwnd after only one_ms_ and a single ack.
+  QuicPacketCount expected_cwnd =
+      fix_convex_mode_ ? current_cwnd : current_cwnd + 1;
   // Initialize the state.
   clock_.AdvanceTime(one_ms_);
   EXPECT_EQ(expected_cwnd,
@@ -86,11 +121,14 @@ TEST_F(CubicTest, LossEvents) {
             cubic_.CongestionWindowAfterPacketLoss(current_cwnd));
 }
 
-TEST_F(CubicTest, BelowOrigin) {
+TEST_P(CubicTest, BelowOrigin) {
   // Concave growth.
   const QuicTime::Delta rtt_min = hundred_ms_;
   QuicPacketCount current_cwnd = 422;
-  QuicPacketCount expected_cwnd = current_cwnd + 1;
+  // Without the signed-integer, cubic-convex fix, we mistakenly
+  // increment cwnd after only one_ms_ and a single ack.
+  QuicPacketCount expected_cwnd =
+      fix_convex_mode_ ? current_cwnd : current_cwnd + 1;
   // Initialize the state.
   clock_.AdvanceTime(one_ms_);
   EXPECT_EQ(expected_cwnd,