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

net/quic/quic_packet_creator.cc

Index: net/quic/quic_packet_creator.cc
diff --git a/net/quic/quic_packet_creator.cc b/net/quic/quic_packet_creator.cc
index 13d357c3048c27ba9155faf517f0ec05e1c658cd..bc012c2af199374cb46e65918dcc035614ae45e2 100644
--- a/net/quic/quic_packet_creator.cc
+++ b/net/quic/quic_packet_creator.cc
@@ -45,7 +45,7 @@ QuicPacketCreator::QuicPacketCreator(QuicConnectionId connection_id,
       connection_id_(connection_id),
       packet_(kDefaultPathId,
               0,
-              next_packet_number_length_,
+              PACKET_1BYTE_PACKET_NUMBER,
               nullptr,
               0,
               0,
@@ -83,6 +83,7 @@ void QuicPacketCreator::SetMaxPacketLength(QuicByteCount length) {
 }
 
 void QuicPacketCreator::MaybeUpdatePacketNumberLength() {
+  DCHECK(!FLAGS_quic_simple_packet_number_length);
   if (!queued_frames_.empty()) {
     // Don't change creator state if there are frames queued.
     return;
@@ -114,12 +115,25 @@ void QuicPacketCreator::SetDiversificationNonce(
 void QuicPacketCreator::UpdatePacketNumberLength(
     QuicPacketNumber least_packet_awaited_by_peer,
     QuicPacketCount max_packets_in_flight) {
+  if (FLAGS_quic_simple_packet_number_length &&
+      !queued_frames_.empty()) {
+    // Don't change creator state if there are frames queued.
+    QUIC_BUG << "Called UpdatePacketNumberLength with " << queued_frames_.size()
+             << " queued_frames.";
+    return;
+  }
+
   DCHECK_LE(least_packet_awaited_by_peer, packet_.packet_number + 1);
   const QuicPacketNumber current_delta =
       packet_.packet_number + 1 - least_packet_awaited_by_peer;
   const uint64_t delta = max(current_delta, max_packets_in_flight);
-  next_packet_number_length_ =
-      QuicFramer::GetMinSequenceNumberLength(delta * 4);
+  if (FLAGS_quic_simple_packet_number_length) {
+    packet_.packet_number_length =
+        QuicFramer::GetMinSequenceNumberLength(delta * 4);
+  } else {
+    next_packet_number_length_ =
+        QuicFramer::GetMinSequenceNumberLength(delta * 4);
+  }
 }
 
 bool QuicPacketCreator::ConsumeData(QuicStreamId id,
@@ -194,9 +208,10 @@ void QuicPacketCreator::CreateStreamFrame(QuicStreamId id,
                                       IncludeNonceInPublicHeader(),
                                       PACKET_6BYTE_PACKET_NUMBER, offset));
 
-  MaybeUpdatePacketNumberLength();
-
-  LOG_IF(DFATAL, !HasRoomForStreamFrame(id, offset))
+  if (!FLAGS_quic_simple_packet_number_length) {
+    MaybeUpdatePacketNumberLength();
+  }
+  QUIC_BUG_IF(!HasRoomForStreamFrame(id, offset))
       << "No room for Stream frame, BytesFree: " << BytesFree()
       << " MinStreamFrameSize: "
       << QuicFramer::GetMinStreamFrameSize(id, offset, true);
@@ -288,7 +303,9 @@ void QuicPacketCreator::ReserializeAllFrames(
 
   // Temporarily set the packet number length and change the encryption level.
   packet_.packet_number_length = retransmission.packet_number_length;
-  next_packet_number_length_ = retransmission.packet_number_length;
+  if (!FLAGS_quic_simple_packet_number_length) {
+    next_packet_number_length_ = retransmission.packet_number_length;
+  }
   packet_.num_padding_bytes = retransmission.num_padding_bytes;
   // Only preserve the original encryption level if it's a handshake packet or
   // if we haven't gone forward secure.
@@ -300,7 +317,12 @@ void QuicPacketCreator::ReserializeAllFrames(
   // Serialize the packet and restore packet number length state.
   for (const QuicFrame& frame : retransmission.retransmittable_frames) {
     bool success = AddFrame(frame, false);
-    DCHECK(success);
+    LOG_IF(DFATAL, !success)
+        << " Failed to add frame of type:" << frame.type
+        << " num_frames:" << retransmission.retransmittable_frames.size()
+        << " retransmission.packet_number_length:"
+        << retransmission.packet_number_length
+        << " packet_.packet_number_length:" << packet_.packet_number_length;
   }
   SerializePacket(buffer, buffer_len);
   packet_.original_path_id = retransmission.path_id;
@@ -308,8 +330,12 @@ void QuicPacketCreator::ReserializeAllFrames(
   packet_.transmission_type = retransmission.transmission_type;
   OnSerializedPacket();
   // Restore old values.
-  packet_.packet_number_length = saved_length;
-  next_packet_number_length_ = saved_next_length;
+  if (!FLAGS_quic_simple_packet_number_length) {
+    // OnSerializedPacket updates the packet_number_length, so it's incorrect to
+    // restore it here.
+    packet_.packet_number_length = saved_length;
+    next_packet_number_length_ = saved_next_length;
+  }
   packet_.encryption_level = default_encryption_level;
 }
 
@@ -458,7 +484,9 @@ size_t QuicPacketCreator::PacketSize() {
     return packet_size_;
   }
   // Update packet number length on packet boundary.
-  packet_.packet_number_length = next_packet_number_length_;
+  if (!FLAGS_quic_simple_packet_number_length) {
+    packet_.packet_number_length = next_packet_number_length_;
+  }
   packet_size_ = GetPacketHeaderSize(
       framer_->version(), connection_id_length_, send_version_in_packet_,
       send_path_id_in_packet_, IncludeNonceInPublicHeader(),
@@ -591,8 +619,9 @@ bool QuicPacketCreator::AddFrame(const QuicFrame& frame,
         ConnectionCloseSource::FROM_SELF);
     return false;
   }
-  MaybeUpdatePacketNumberLength();
-
+  if (!FLAGS_quic_simple_packet_number_length) {
+    MaybeUpdatePacketNumberLength();
+  }
   size_t frame_len = framer_->GetSerializedFrameLength(
       frame, BytesFree(), queued_frames_.empty(), true,
       packet_.packet_number_length);