From 54c5920397d84bc0d1694757aa87d1eefb2de0ae Mon Sep 17 00:00:00 2001
From: Subodh Iyengar <subodh@fb.com>
Date: Mon, 9 Dec 2019 20:59:32 -0800
Subject: [PATCH] dont append padding frames to write frames

Summary:
If the packet is too small we might automatically add padding frames
to make it large enough. However we used to add padding frames to the
frame list as well.

We dont need this, lets just add the regular frame types.

Reviewed By: mjoras

Differential Revision: D18903074

fbshipit-source-id: f73f82f96f833347c84a38eb1035c46e35ba3b2f
---
 quic/api/test/QuicTransportTest.cpp       | 2 +-
 quic/codec/QuicPacketBuilder.cpp          | 2 +-
 quic/codec/test/QuicPacketBuilderTest.cpp | 5 ++---
 quic/logging/test/QLoggerTest.cpp         | 4 ----
 4 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp
index a0a975570..02da75d13 100644
--- a/quic/api/test/QuicTransportTest.cpp
+++ b/quic/api/test/QuicTransportTest.cpp
@@ -914,7 +914,7 @@ TEST_F(QuicTransportTest, StopSending) {
       getLastOutstandingPacket(
           transport_->getConnectionState(), PacketNumberSpace::AppData)
           ->packet;
-  EXPECT_EQ(14, packet.frames.size());
+  EXPECT_EQ(1, packet.frames.size());
   bool foundStopSending = false;
   for (auto& frame : packet.frames) {
     const QuicSimpleFrame* simpleFrame = frame.asQuicSimpleFrame();
diff --git a/quic/codec/QuicPacketBuilder.cpp b/quic/codec/QuicPacketBuilder.cpp
index 2946afa06..6785306bb 100644
--- a/quic/codec/QuicPacketBuilder.cpp
+++ b/quic/codec/QuicPacketBuilder.cpp
@@ -201,7 +201,7 @@ RegularQuicPacketBuilder::Packet RegularQuicPacketBuilder::buildPacket() && {
   size_t bodyLength = body_->computeChainDataLength();
   while (bodyLength + extraDataWritten + cipherOverhead_ < minBodySize &&
          !quicFrames_.empty() && remainingBytes_ > kMaxPacketLenSize) {
-    quicFrames_.push_back(PaddingFrame());
+    // We can add padding frames, but we don't need to store them.
     QuicInteger paddingType(static_cast<uint8_t>(FrameType::PADDING));
     write(paddingType);
     extraDataWritten++;
diff --git a/quic/codec/test/QuicPacketBuilderTest.cpp b/quic/codec/test/QuicPacketBuilderTest.cpp
index 0ccfe5013..76f33415f 100644
--- a/quic/codec/test/QuicPacketBuilderTest.cpp
+++ b/quic/codec/test/QuicPacketBuilderTest.cpp
@@ -311,8 +311,7 @@ TEST_F(QuicPacketBuilderTest, TestPaddingAccountsForCipherOverhead) {
   // We should have padded the remaining bytes with Padding frames.
   size_t expectedOutputSize =
       sizeof(Sample) + kMaxPacketNumEncodingSize - encodedPacketNum.length;
-  EXPECT_EQ(
-      resultRegularPacket.frames.size(), expectedOutputSize - cipherOverhead);
+  EXPECT_EQ(resultRegularPacket.frames.size(), 1);
   EXPECT_EQ(
       builtOut.body->computeChainDataLength(),
       expectedOutputSize - cipherOverhead);
@@ -335,7 +334,7 @@ TEST_F(QuicPacketBuilderTest, TestPaddingRespectsRemainingBytes) {
 
   size_t headerSize = 13;
   // We should have padded the remaining bytes with Padding frames.
-  EXPECT_EQ(resultRegularPacket.frames.size(), totalPacketSize - headerSize);
+  EXPECT_EQ(resultRegularPacket.frames.size(), 1);
   EXPECT_EQ(
       builtOut.body->computeChainDataLength(), totalPacketSize - headerSize);
 }
diff --git a/quic/logging/test/QLoggerTest.cpp b/quic/logging/test/QLoggerTest.cpp
index 33f3553c7..274291f06 100644
--- a/quic/logging/test/QLoggerTest.cpp
+++ b/quic/logging/test/QLoggerTest.cpp
@@ -606,10 +606,6 @@ TEST_F(QLoggerTest, AddingMultiplePacketEvents) {
                  "id": 10,
                  "length": 5,
                  "offset": 0
-               },
-               {
-                 "frame_type": "PADDING",
-                 "num_frames": 11
                }
              ],
              "header": {