Stop using next_packet_sequence_number_length in QuicPacketCreator. No functional change. Protect…

net/quic/quic_packet_creator_test.cc

Index: net/quic/quic_packet_creator_test.cc
diff --git a/net/quic/quic_packet_creator_test.cc b/net/quic/quic_packet_creator_test.cc
index 0effaeff36dfa140858f49f0d5c210102d5bdc85..df2e399ddc3675880bce35a2aca7ad885c0bce89 100644
--- a/net/quic/quic_packet_creator_test.cc
+++ b/net/quic/quic_packet_creator_test.cc
@@ -179,10 +179,17 @@ class QuicPacketCreatorTest : public ::testing::TestWithParam<TestParams> {
   // Returns the number of bytes consumed by the header of packet, including
   // the version.
   size_t GetPacketHeaderOverhead(QuicVersion version) {
-    return GetPacketHeaderSize(
-        version, creator_.connection_id_length(), kIncludeVersion,
-        !kIncludePathId, !kIncludeDiversificationNonce,
-        QuicPacketCreatorPeer::NextPacketNumberLength(&creator_));
+    if (FLAGS_quic_simple_packet_number_length) {
+      return GetPacketHeaderSize(
+          version, creator_.connection_id_length(), kIncludeVersion,
+          !kIncludePathId, !kIncludeDiversificationNonce,
+          QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+    } else {
+      return GetPacketHeaderSize(
+          version, creator_.connection_id_length(), kIncludeVersion,
+          !kIncludePathId, !kIncludeDiversificationNonce,
+          QuicPacketCreatorPeer::NextPacketNumberLength(&creator_));
+    }
   }
 
   // Returns the number of bytes of overhead that will be added to a packet
@@ -273,6 +280,7 @@ TEST_P(QuicPacketCreatorTest, SerializeFrames) {
 }
 
 TEST_P(QuicPacketCreatorTest, SerializeChangingSequenceNumberLength) {
+  FLAGS_quic_simple_packet_number_length = false;
   frames_.push_back(QuicFrame(new QuicAckFrame(MakeAckFrame(0u))));
   creator_.AddSavedFrame(frames_[0]);
   QuicPacketCreatorPeer::SetNextPacketNumberLength(&creator_,
@@ -321,6 +329,7 @@ TEST_P(QuicPacketCreatorTest, SerializeChangingSequenceNumberLength) {
 }
 
 TEST_P(QuicPacketCreatorTest, ChangeSequenceNumberLengthMidPacket) {
+  FLAGS_quic_simple_packet_number_length = false;
   // Changing the packet number length with queued frames in the creator
   // should hold the change until after any currently queued frames are
   // serialized.
@@ -402,12 +411,33 @@ TEST_P(QuicPacketCreatorTest, ChangeSequenceNumberLengthMidPacket) {
   delete ack_frame.ack_frame;
 }
 
+TEST_P(QuicPacketCreatorTest, ChangeSequenceNumberLengthMidPacketDoesNothing) {
+  FLAGS_quic_simple_packet_number_length = true;
+  EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+  // Changing the packet number length with queued frames in the creator
+  // should do nothing.
+  EXPECT_FALSE(creator_.HasPendingFrames());
+  QuicFrame ack_frame = QuicFrame(new QuicAckFrame(MakeAckFrame(0u)));
+  creator_.AddSavedFrame(ack_frame);
+
+  // Now change packet number length and expect a QUIC_BUG and no change in
+  // packet number length.
+  EXPECT_DFATAL(creator_.UpdatePacketNumberLength(0, 256),
+                "Called UpdatePacketNumberLength with 1 queued_frames.");
+  EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+  delete ack_frame.ack_frame;
+}
+
 TEST_P(QuicPacketCreatorTest, ReserializeFramesWithSequenceNumberLength) {
   // If the original packet number length, the current packet number
   // length, and the configured send packet number length are different, the
   // retransmit must sent with the original length and the others do not change.
-  QuicPacketCreatorPeer::SetNextPacketNumberLength(&creator_,
-                                                   PACKET_4BYTE_PACKET_NUMBER);
+  if (!FLAGS_quic_simple_packet_number_length) {
+    QuicPacketCreatorPeer::SetNextPacketNumberLength(
+        &creator_, PACKET_4BYTE_PACKET_NUMBER);
+  }
   QuicPacketCreatorPeer::SetPacketNumberLength(&creator_,
                                                PACKET_2BYTE_PACKET_NUMBER);
   QuicStreamFrame* stream_frame =
@@ -421,10 +451,17 @@ TEST_P(QuicPacketCreatorTest, ReserializeFramesWithSequenceNumberLength) {
   EXPECT_CALL(delegate_, OnSerializedPacket(_))
       .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket));
   creator_.ReserializeAllFrames(retransmission, buffer, kMaxPacketSize);
-  EXPECT_EQ(PACKET_4BYTE_PACKET_NUMBER,
-            QuicPacketCreatorPeer::NextPacketNumberLength(&creator_));
-  EXPECT_EQ(PACKET_2BYTE_PACKET_NUMBER,
-            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+  if (FLAGS_quic_simple_packet_number_length) {
+    // The packet number length is updated after every packet is sent,
+    // so there is no need to restore the old length after sending.
+    EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
+              QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+  } else {
+    EXPECT_EQ(PACKET_4BYTE_PACKET_NUMBER,
+              QuicPacketCreatorPeer::NextPacketNumberLength(&creator_));
+    EXPECT_EQ(PACKET_2BYTE_PACKET_NUMBER,
+              QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+  }
   EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
             serialized_packet_.packet_number_length);
 
@@ -513,7 +550,9 @@ TEST_P(QuicPacketCreatorTest, ReserializeFramesWithSpecifiedPadding) {
     PendingRetransmission retransmission(CreateRetransmission(
         frames, false /* has_crypto_handshake */,
         kNumPaddingBytes1 /* padding bytes */, ENCRYPTION_NONE,
-        QuicPacketCreatorPeer::NextPacketNumberLength(&creator_)));
+        FLAGS_quic_simple_packet_number_length
+            ? QuicPacketCreatorPeer::GetPacketNumberLength(&creator_)
+            : QuicPacketCreatorPeer::NextPacketNumberLength(&creator_)));
     EXPECT_CALL(delegate_, OnSerializedPacket(_))
         .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket));
     creator_.ReserializeAllFrames(retransmission, buffer, kMaxPacketSize);
@@ -772,7 +811,8 @@ TEST_P(QuicPacketCreatorTest, SerializeVersionNegotiationPacket) {
   client_framer_.ProcessPacket(*encrypted);
 }
 
-TEST_P(QuicPacketCreatorTest, UpdatePacketSequenceNumberLengthLeastAwaiting) {
+TEST_P(QuicPacketCreatorTest, UpdatePacketNumberLengthLeastAwaiting_Old) {
+  FLAGS_quic_simple_packet_number_length = false;
   EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
             QuicPacketCreatorPeer::NextPacketNumberLength(&creator_));
 
@@ -798,7 +838,8 @@ TEST_P(QuicPacketCreatorTest, UpdatePacketSequenceNumberLengthLeastAwaiting) {
             QuicPacketCreatorPeer::NextPacketNumberLength(&creator_));
 }
 
-TEST_P(QuicPacketCreatorTest, UpdatePacketSequenceNumberLengthBandwidth) {
+TEST_P(QuicPacketCreatorTest, UpdatePacketSequenceNumberLengthBandwidth_old) {
+  FLAGS_quic_simple_packet_number_length = false;
   EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
             QuicPacketCreatorPeer::NextPacketNumberLength(&creator_));
 
@@ -821,6 +862,57 @@ TEST_P(QuicPacketCreatorTest, UpdatePacketSequenceNumberLengthBandwidth) {
             QuicPacketCreatorPeer::NextPacketNumberLength(&creator_));
 }
 
+TEST_P(QuicPacketCreatorTest, UpdatePacketSequenceNumberLengthLeastAwaiting) {
+  FLAGS_quic_simple_packet_number_length = true;
+  EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+
+  QuicPacketCreatorPeer::SetPacketNumber(&creator_, 64);
+  creator_.UpdatePacketNumberLength(2, 10000 / kDefaultMaxPacketSize);
+  EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+
+  QuicPacketCreatorPeer::SetPacketNumber(&creator_, 64 * 256);
+  creator_.UpdatePacketNumberLength(2, 10000 / kDefaultMaxPacketSize);
+  EXPECT_EQ(PACKET_2BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+
+  QuicPacketCreatorPeer::SetPacketNumber(&creator_, 64 * 256 * 256);
+  creator_.UpdatePacketNumberLength(2, 10000 / kDefaultMaxPacketSize);
+  EXPECT_EQ(PACKET_4BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+
+  QuicPacketCreatorPeer::SetPacketNumber(&creator_,
+                                         UINT64_C(64) * 256 * 256 * 256 * 256);
+  creator_.UpdatePacketNumberLength(2, 10000 / kDefaultMaxPacketSize);
+  EXPECT_EQ(PACKET_6BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+}
+
+TEST_P(QuicPacketCreatorTest, UpdatePacketSequenceNumberLengthCwnd) {
+  FLAGS_quic_simple_packet_number_length = true;
+  EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+
+  creator_.UpdatePacketNumberLength(1, 10000 / kDefaultMaxPacketSize);
+  EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+
+  creator_.UpdatePacketNumberLength(1, 10000 * 256 / kDefaultMaxPacketSize);
+  EXPECT_EQ(PACKET_2BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+
+  creator_.UpdatePacketNumberLength(1,
+                                    10000 * 256 * 256 / kDefaultMaxPacketSize);
+  EXPECT_EQ(PACKET_4BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+
+  creator_.UpdatePacketNumberLength(
+      1, UINT64_C(1000) * 256 * 256 * 256 * 256 / kDefaultMaxPacketSize);
+  EXPECT_EQ(PACKET_6BYTE_PACKET_NUMBER,
+            QuicPacketCreatorPeer::GetPacketNumberLength(&creator_));
+}
+
 TEST_P(QuicPacketCreatorTest, SerializeFrame) {
   if (!GetParam().version_serialization) {
     creator_.StopSendingVersion();
@@ -1129,6 +1221,7 @@ TEST_P(QuicPacketCreatorTest, SetCurrentPath) {
 
 TEST_P(QuicPacketCreatorTest,
        SetCurrentPathAndUpdatePacketSequenceNumberLength) {
+  FLAGS_quic_simple_packet_number_length = false;
   // Current path is the default path.
   EXPECT_EQ(kDefaultPathId, QuicPacketCreatorPeer::GetCurrentPath(&creator_));
   EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER,