commit f08778a58e717a38fcf59eb1a2ca01b8bf836ad2
author QUICHE team <quiche-dev@google.com> Thu Mar 14 08:10:26 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Mar 14 08:10:55 2019 -0700
tree 5b436af0ba0160f5f31dbffffb6ccf600041f3ba
parent 62446bcd3c0851334d81fcf91c86ee774bc2398b

Pass data_size into QuicPacketCreator::CreateStreamFrame instead of calculating it inside the method and no longer special case the 0 length case, except the existing QUIC_BUG.

gfe-relnote: n/a (Code refactor)
PiperOrigin-RevId: 238444568
Change-Id: I66c6ea5046639e91390bb64f88e5ab4e479a1b36

quic/core/quic_packet_creator.cc

diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index f5c7684..ec7248d 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -179,10 +179,14 @@
                                     bool needs_full_padding,
                                     TransmissionType transmission_type,
                                     QuicFrame* frame) {
-  if (!HasRoomForStreamFrame(id, offset, write_length - iov_offset)) {
+  // TODO(ianswett): Remove write_length once the multi-packet CHLO check is
+  // done higher in the stack.
+  DCHECK_GE(write_length, iov_offset);
+  const size_t data_size = write_length - iov_offset;
+  if (!HasRoomForStreamFrame(id, offset, data_size)) {
     return false;
   }
-  CreateStreamFrame(id, write_length, iov_offset, offset, fin, frame);
+  CreateStreamFrame(id, data_size, offset, fin, frame);
   // Explicitly disallow multi-packet CHLOs.
   if (FLAGS_quic_enforce_single_packet_chlo &&
       StreamFrameStartsWithChlo(frame->stream_frame) &&
@@ -267,12 +271,10 @@
 }
 
 void QuicPacketCreator::CreateStreamFrame(QuicStreamId id,
-                                          size_t write_length,
-                                          size_t iov_offset,
+                                          size_t data_size,
                                           QuicStreamOffset offset,
                                           bool fin,
                                           QuicFrame* frame) {
-  const size_t data_size = write_length - iov_offset;
   DCHECK_GT(
       max_packet_length_,
       StreamFramePacketOverhead(
@@ -287,13 +289,9 @@
       << QuicFramer::GetMinStreamFrameSize(framer_->transport_version(), id,
                                            offset, true, data_size);
 
-  if (iov_offset == write_length) {
-    QUIC_BUG_IF(!fin) << "Creating a stream frame with no data or fin.";
-    // Create a new packet for the fin, if necessary.
-    *frame = QuicFrame(QuicStreamFrame(id, true, offset, QuicStringPiece()));
-    return;
-  }
-
+  QUIC_BUG_IF(data_size == 0 && !fin)
+      << "Creating a stream frame for stream ID:" << id
+      << " with no data or fin.";
   size_t min_frame_size = QuicFramer::GetMinStreamFrameSize(
       framer_->transport_version(), id, offset,
       /* last_frame_in_packet= */ true, data_size);

quic/core/quic_packet_creator.h

diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h
index 7e129e1..76e7726 100644
--- a/quic/core/quic_packet_creator.h
+++ b/quic/core/quic_packet_creator.h
@@ -277,11 +277,10 @@
   friend class test::QuicPacketCreatorPeer;
 
   // Creates a stream frame which fits into the current open packet. If
-  // |write_length| is 0 and fin is true, the expected behavior is to consume
-  // the fin but return 0.
+  // |data_size| is 0 and fin is true, the expected behavior is to consume
+  // the fin.
   void CreateStreamFrame(QuicStreamId id,
-                         size_t write_length,
-                         size_t iov_offset,
+                         size_t data_size,
                          QuicStreamOffset offset,
                          bool fin,
                          QuicFrame* frame);

quic/core/quic_packet_creator_test.cc

diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc
index efd7485..6727fca 100644
--- a/quic/core/quic_packet_creator_test.cc
+++ b/quic/core/quic_packet_creator_test.cc
@@ -390,7 +390,7 @@
   QuicPacketCreatorPeer::CreateStreamFrame(
       &creator_,
       QuicUtils::GetCryptoStreamId(client_framer_.transport_version()),
-      iov_.iov_len, 0u, 0u, false, &frame);
+      iov_.iov_len, 0u, false, &frame);
   QuicFrames frames;
   frames.push_back(frame);
   char buffer[kMaxPacketSize];
@@ -413,7 +413,7 @@
   QuicPacketCreatorPeer::CreateStreamFrame(
       &creator_,
       QuicUtils::GetCryptoStreamId(client_framer_.transport_version()),
-      iov_.iov_len, 0u, 0u, false, &frame);
+      iov_.iov_len, 0u, false, &frame);
 
   const int kNumPaddingBytes1 = 4;
   int packet_size = 0;
@@ -480,7 +480,7 @@
     QuicPacketCreatorPeer::CreateStreamFrame(
         &creator_,
         QuicUtils::GetCryptoStreamId(client_framer_.transport_version()),
-        iov_.iov_len, 0, kOffset, false, &frame);
+        iov_.iov_len, kOffset, false, &frame);
     QuicFrames frames;
     frames.push_back(frame);
     char buffer[kMaxPacketSize];
@@ -1375,7 +1375,7 @@
   QuicPacketCreatorPeer::CreateStreamFrame(
       &creator_,
       QuicUtils::GetCryptoStreamId(client_framer_.transport_version()),
-      iov_.iov_len, 0u, 0u, false, &frame);
+      iov_.iov_len, 0u, false, &frame);
   QuicFrames frames;
   frames.push_back(frame);
   char buffer[kMaxPacketSize];