Make NewConnectionIdFrame a QuicSimpleFrame (for now)

Summary:
The plan is to not treat probing frame as simple frame, but that is not
going to happen soon.

Reviewed By: sharma95

Differential Revision: D15176635

fbshipit-source-id: 62ac13cdb82a09161e9148dfc437cf7377a01c96

quic/codec/QuicWriteCodec.cpp

@@ -367,6 +367,30 @@ size_t writeSimpleFrame(
         }
         // no space left in packet
         return size_t(0);
+      },
+      [&](NewConnectionIdFrame& newConnectionIdFrame) {
+        QuicInteger frameType(
+            static_cast<uint8_t>(FrameType::NEW_CONNECTION_ID));
+        QuicInteger sequence(newConnectionIdFrame.sequence);
+        // Include an 8-bit unsigned integer containing the length of the connId
+        auto newConnectionIdFrameSize = frameType.getSize() + sizeof(uint8_t) +
+            sequence.getSize() + newConnectionIdFrame.connectionId.size() +
+            newConnectionIdFrame.token.size();
+        if (packetSpaceCheck(spaceLeft, newConnectionIdFrameSize)) {
+          builder.write(frameType);
+          builder.write(sequence);
+          builder.writeBE(newConnectionIdFrame.connectionId.size());
+          builder.push(
+              newConnectionIdFrame.connectionId.data(),
+              newConnectionIdFrame.connectionId.size());
+          builder.push(
+              newConnectionIdFrame.token.data(),
+              newConnectionIdFrame.token.size());
+          builder.appendFrame(std::move(newConnectionIdFrame));
+          return newConnectionIdFrameSize;
+        }
+        // no space left in packet
+        return size_t(0);
       });
 }
 
@@ -507,30 +531,6 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) {
         // no space left in packet
         return size_t(0);
       },
-      [&](NewConnectionIdFrame& newConnectionIdFrame) {
-        QuicInteger packetType(
-            static_cast<uint8_t>(FrameType::NEW_CONNECTION_ID));
-        QuicInteger sequence(newConnectionIdFrame.sequence);
-        // Include an 8-bit unsigned integer containing the length of the connId
-        auto newConnectionIdFrameSize = packetType.getSize() + sizeof(uint8_t) +
-            sequence.getSize() + newConnectionIdFrame.connectionId.size() +
-            newConnectionIdFrame.token.size();
-        if (packetSpaceCheck(spaceLeft, newConnectionIdFrameSize)) {
-          builder.write(packetType);
-          builder.write(sequence);
-          builder.writeBE(newConnectionIdFrame.connectionId.size());
-          builder.push(
-              newConnectionIdFrame.connectionId.data(),
-              newConnectionIdFrame.connectionId.size());
-          builder.push(
-              newConnectionIdFrame.token.data(),
-              newConnectionIdFrame.token.size());
-          builder.appendFrame(std::move(newConnectionIdFrame));
-          return newConnectionIdFrameSize;
-        }
-        // no space left in packet
-        return size_t(0);
-      },
       [&](ConnectionCloseFrame& connectionCloseFrame) {
         QuicInteger packetType(
             static_cast<uint8_t>(FrameType::CONNECTION_CLOSE));

quic/codec/Types.h

@@ -560,7 +560,8 @@ using QuicSimpleFrame = boost::variant<
     MinStreamDataFrame,
     ExpiredStreamDataFrame,
     PathChallengeFrame,
-    PathResponseFrame>;
+    PathResponseFrame,
+    NewConnectionIdFrame>;
 
 // Types of frames that can be read.
 using QuicFrame = boost::variant<
@@ -575,7 +576,6 @@ using QuicFrame = boost::variant<
     DataBlockedFrame,
     StreamDataBlockedFrame,
     StreamsBlockedFrame,
-    NewConnectionIdFrame,
     ReadAckFrame,
     ReadStreamFrame,
     ReadCryptoFrame,
@@ -596,7 +596,6 @@ using QuicWriteFrame = boost::variant<
     PingFrame,
     DataBlockedFrame,
     StreamDataBlockedFrame,
-    NewConnectionIdFrame,
     WriteAckFrame,
     WriteStreamFrame,
     WriteCryptoFrame,

quic/codec/test/QuicWriteCodecTest.cpp

@@ -1294,16 +1294,16 @@ TEST_F(QuicWriteCodecTest, WriteNewConnId) {
   auto builtOut = std::move(pktBuilder).buildPacket();
   auto regularPacket = builtOut.first;
   EXPECT_EQ(bytesWritten, 27);
-  auto resultNewConnIdFrame =
-      boost::get<NewConnectionIdFrame>(regularPacket.frames[0]);
+  auto resultNewConnIdFrame = boost::get<NewConnectionIdFrame>(
+      boost::get<QuicSimpleFrame>(regularPacket.frames[0]));
   EXPECT_EQ(resultNewConnIdFrame.sequence, 1);
   EXPECT_EQ(resultNewConnIdFrame.connectionId, getTestConnectionId());
   EXPECT_EQ(resultNewConnIdFrame.token, token);
 
   auto wireBuf = std::move(builtOut.second);
   folly::io::Cursor cursor(wireBuf.get());
-  auto wireNewConnIdFrame =
-      boost::get<NewConnectionIdFrame>(parseQuicFrame(cursor));
+  auto wireNewConnIdFrame = boost::get<NewConnectionIdFrame>(
+      boost::get<QuicSimpleFrame>(parseQuicFrame(cursor)));
   EXPECT_EQ(1, wireNewConnIdFrame.sequence);
   EXPECT_EQ(getTestConnectionId(), wireNewConnIdFrame.connectionId);
   EXPECT_TRUE(cursor.isAtEnd());

quic/state/SimpleFrameFunctions.cpp

@@ -58,6 +58,10 @@ folly::Optional<QuicSimpleFrame> updateSimpleFrameOnPacketClone(
       },
       [&](const PathResponseFrame& frame) -> folly::Optional<QuicSimpleFrame> {
         return QuicSimpleFrame(frame);
+      },
+      [&](const NewConnectionIdFrame&) -> folly::Optional<QuicSimpleFrame> {
+        // TODO junqiw
+        return folly::none;
       });
 }
 
@@ -116,6 +120,9 @@ void updateSimpleFrameOnPacketLoss(
       },
       [&](const PathResponseFrame& frame) {
         conn.pendingEvents.frames.push_back(frame);
+      },
+      [&](const NewConnectionIdFrame&) {
+        // TODO junqiw
       });
 }
 
@@ -164,6 +171,10 @@ bool updateSimpleFrameOnPacketReceived(
         conn.pendingEvents.schedulePathValidationTimeout = false;
         conn.writableBytesLimit = folly::none;
         return false;
+      },
+      [&](const NewConnectionIdFrame&) {
+        // TODO junqiw
+        return false;
       });
 }