Fix dependency loop and improve namings

Summary:
Fixing dependency loop introduced by D37799050 (96abc8160d)

Running `autodeps` yields the following patch:

```
 --- a/xplat/quic/state/TARGETS
+++ b/xplat/quic/state/TARGETS
@@ -43,8 +43,8 @@
     exported_deps = [
         "//folly:random",
         "//quic:constants",
+        "//quic/codec:codec",
         "//quic/codec:types",
-        "//quic/common:circular_deque",
         "//quic/common:interval_set",
     ],
 )
```

If this patch is applied, there is a circular dependency loop between `//quic/codec:codec` and `//quic/state:ack_states` by way of `//quic/codec:types`; this loop was introduced by D37799050 (96abc8160d).

Fixed by separating out headers files and targets. In parallel, renamed structures used for writing ACK frames (which were the reason this loop occurred) to make their role clear.

Differential Revision: D42281359

fbshipit-source-id: 8514c99f3fe72ff1d942d7f303e4a209838c7623

quic/api/QuicPacketScheduler.cpp

@@ -586,7 +586,7 @@ folly::Optional<PacketNum> AckScheduler::writeNextAcks(
                  ackingTime - receivedTime)
            : 0us);
 
-  AckFrameMetaData meta = {
+  WriteAckFrameMetaData meta = {
       ackState_, /* ackState*/
       ackDelay, /* ackDelay */
       static_cast<uint8_t>(ackDelayExponentToUse), /* ackDelayExponent */
@@ -594,7 +594,7 @@ folly::Optional<PacketNum> AckScheduler::writeNextAcks(
       folly::none, /* recvTimestampsConfig */
       folly::none /* maxAckReceiveTimestampsToSend */};
 
-  folly::Optional<AckFrameWriteResult> ackWriteResult;
+  folly::Optional<WriteAckFrameResult> ackWriteResult;
 
   bool isAckReceiveTimestampsSupported =
       conn_.transportSettings.maybeAckReceiveTimestampsConfigSentToPeer &&

quic/api/test/QuicPacketSchedulerTest.cpp

@@ -646,8 +646,8 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
   AckBlocks ackBlocks;
   ackBlocks.insert(10, 100);
   ackBlocks.insert(200, 1000);
-  WriteAckState writeAckState = {.acks = ackBlocks};
+  WriteAckFrameState writeAckState = {.acks = ackBlocks};
-  AckFrameMetaData ackMeta = {
+  WriteAckFrameMetaData ackMeta = {
       .ackState = writeAckState,
       .ackDelay = 0us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent)};

quic/codec/PacketNumber.cpp

@@ -5,7 +5,13 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+#include <folly/Conv.h>
+#include <folly/lang/Bits.h>
+#include <glog/logging.h>
+#include <quic/QuicConstants.h>
+#include <quic/QuicException.h>
 #include <quic/codec/PacketNumber.h>
+#include <string>
 
 namespace quic {
 

quic/codec/PacketNumber.h

@@ -7,10 +7,13 @@
 
 #pragma once
 
-#include <quic/codec/Types.h>
+#include <cstddef>
+#include <cstdint>
 
 namespace quic {
 
+using PacketNum = uint64_t;
+
 /**
  * Returns a decoded packet number by using the expectedNextPacketNum to
  * search for the most probable packet number that could satisfy that condition.

quic/codec/QuicPacketRebuilder.cpp

@@ -229,7 +229,7 @@ folly::Optional<PacketEvent> PacketRebuilder::rebuildFromPacket(
                    ackingTime - receivedTime)
              : 0us);
 
-    AckFrameMetaData meta = {
+    WriteAckFrameMetaData meta = {
         ackState_, /* ackState*/
         ackDelay, /* ackDelay */
         static_cast<uint8_t>(ackDelayExponent), /* ackDelayExponent */
@@ -237,7 +237,7 @@ folly::Optional<PacketEvent> PacketRebuilder::rebuildFromPacket(
         folly::none, /* recvTimestampsConfig */
         folly::none /* maxAckReceiveTimestampsToSend */};
 
-    folly::Optional<AckFrameWriteResult> ackWriteResult;
+    folly::Optional<WriteAckFrameResult> ackWriteResult;
 
     uint64_t peerRequestedTimestampsCount =
         conn_.maybePeerAckReceiveTimestampsConfig.has_value()

quic/codec/QuicWriteCodec.cpp

@@ -282,7 +282,7 @@ size_t computeSizeUsedByRecvdTimestamps(WriteAckFrame& ackFrame) {
 }
 
 static size_t fillPacketReceiveTimestamps(
-    const quic::AckFrameMetaData& ackFrameMetaData,
+    const quic::WriteAckFrameMetaData& ackFrameMetaData,
     WriteAckFrame& ackFrame,
     uint64_t largestAckedPacketNum,
     uint64_t spaceLeft,
@@ -376,13 +376,13 @@ static size_t fillPacketReceiveTimestamps(
 }
 
 folly::Optional<WriteAckFrame> writeAckFrameToPacketBuilder(
-    const quic::AckFrameMetaData& ackFrameMetaData,
+    const quic::WriteAckFrameMetaData& ackFrameMetaData,
     PacketBuilderInterface& builder,
     FrameType frameType) {
   if (ackFrameMetaData.ackState.acks.empty()) {
     return folly::none;
   }
-  const WriteAckState& ackState = ackFrameMetaData.ackState;
+  const WriteAckFrameState& ackState = ackFrameMetaData.ackState;
   // The last block must be the largest block.
   auto largestAckedPacket = ackState.acks.back().end;
   // ackBlocks are already an interval set so each value is naturally
@@ -469,8 +469,8 @@ folly::Optional<WriteAckFrame> writeAckFrameToPacketBuilder(
   return ackFrame;
 }
 
-folly::Optional<AckFrameWriteResult> writeAckFrame(
+folly::Optional<WriteAckFrameResult> writeAckFrame(
-    const quic::AckFrameMetaData& ackFrameMetaData,
+    const quic::WriteAckFrameMetaData& ackFrameMetaData,
     PacketBuilderInterface& builder,
     FrameType frameType) {
   uint64_t beginningSpace = builder.remainingSpaceInPkt();
@@ -479,7 +479,7 @@ folly::Optional<AckFrameWriteResult> writeAckFrame(
 
   if (maybeWriteAckFrame.has_value()) {
     builder.appendFrame(std::move(maybeWriteAckFrame.value()));
-    return AckFrameWriteResult(
+    return WriteAckFrameResult(
         beginningSpace - builder.remainingSpaceInPkt(),
         maybeWriteAckFrame.value(),
         maybeWriteAckFrame.value().ackBlocks.size());
@@ -488,8 +488,8 @@ folly::Optional<AckFrameWriteResult> writeAckFrame(
   }
 }
 
-folly::Optional<AckFrameWriteResult> writeAckFrameWithReceivedTimestamps(
+folly::Optional<WriteAckFrameResult> writeAckFrameWithReceivedTimestamps(
-    const quic::AckFrameMetaData& ackFrameMetaData,
+    const quic::WriteAckFrameMetaData& ackFrameMetaData,
     PacketBuilderInterface& builder,
     FrameType frameType) {
   auto beginningSpace = builder.remainingSpaceInPkt();
@@ -499,7 +499,7 @@ folly::Optional<AckFrameWriteResult> writeAckFrameWithReceivedTimestamps(
     return folly::none;
   }
   auto ackFrame = maybeAckFrame.value();
-  const WriteAckState& ackState = ackFrameMetaData.ackState;
+  const WriteAckFrameState& ackState = ackFrameMetaData.ackState;
   uint64_t spaceLeft = builder.remainingSpaceInPkt();
   uint64_t lastPktNum = 0;
   std::chrono::microseconds lastPktTsDelta = 0us;
@@ -559,7 +559,7 @@ folly::Optional<AckFrameWriteResult> writeAckFrameWithReceivedTimestamps(
       builder.write(timeStampRangeCountInt);
     }
   }
-  auto ackFrameWriteResult = AckFrameWriteResult(
+  auto ackFrameWriteResult = WriteAckFrameResult(
       beginningSpace - builder.remainingSpaceInPkt(),
       ackFrame,
       ackFrame.ackBlocks.size(),

quic/codec/QuicWriteCodec.h

@@ -19,75 +19,6 @@
 
 namespace quic {
 
-// Ack and PacketNumber states. This is per-packet number space.
-struct WriteAckState {
-  AckBlocks acks;
-
-  // Receive timestamp and packet number for the largest received packet.
-  //
-  // Updated whenever we receive a packet with a larger packet number
-  // than all previously received packets in the packet number space
-  // tracked by this AckState.
-  folly::Optional<RecvdPacketInfo> largestRecvdPacketInfo;
-  // Receive timestamp and packet number for the last received packet.
-  //
-  // Will be different from the value stored in largestRecvdPacketInfo
-  // if the last packet was received out of order and thus had a packet
-  // number less than that of a previously received packet in the packet
-  // number space tracked by this AckState.
-  folly::Optional<RecvdPacketInfo> lastRecvdPacketInfo;
-
-  // Packet number and timestamp of recently received packets.
-  //
-  // The maximum number of packets stored in pktsReceivedTimestamps is
-  // controlled by kMaxReceivedPktsTimestampsStored.
-  //
-  // The packet number of entries in the deque is guarenteed to increase
-  // monotonically because an entry is only added for a received packet
-  // if the packet number is greater than the packet number of the last
-  // element in the deque (e.g., entries are not added for packets that
-  // arrive out of order relative to previously received packets).
-  CircularDeque<RecvdPacketInfo> recvdPacketInfos;
-};
-
-struct AckFrameMetaData {
-  // ACK state.
-  const WriteAckState& ackState;
-
-  // Delay in sending ack from time that packet was received.
-  std::chrono::microseconds ackDelay;
-  // The ack delay exponent to use.
-  uint8_t ackDelayExponent;
-
-  // Receive timestamps basis
-  TimePoint connTime;
-
-  folly::Optional<AckReceiveTimestampsConfig> recvTimestampsConfig =
-      folly::none;
-
-  folly::Optional<uint64_t> maxAckReceiveTimestampsToSend = folly::none;
-};
-
-struct AckFrameWriteResult {
-  uint64_t bytesWritten;
-  WriteAckFrame writeAckFrame;
-  // This includes the first ack block
-  size_t ackBlocksWritten;
-  size_t timestampRangesWritten;
-  size_t timestampsWritten;
-  AckFrameWriteResult(
-      uint64_t bytesWrittenIn,
-      WriteAckFrame writeAckFrameIn,
-      size_t ackBlocksWrittenIn,
-      size_t timestampRangesWrittenIn = 0,
-      size_t timestampsWrittenIn = 0)
-      : bytesWritten(bytesWrittenIn),
-        writeAckFrame(writeAckFrameIn),
-        ackBlocksWritten(ackBlocksWrittenIn),
-        timestampRangesWritten(timestampRangesWrittenIn),
-        timestampsWritten(timestampsWrittenIn) {}
-};
-
 /**
  * Write a simple QuicFrame into builder
  *
@@ -178,12 +109,12 @@ folly::Optional<WriteCryptoFrame> writeCryptoFrame(
  * of the packet sequence numbers. Exception will be thrown if they are not
  * sorted.
  *
- * Return: A AckFrameWriteResult to indicate how many bytes and ack blocks are
+ * Return: A WriteAckFrameResult to indicate how many bytes and ack blocks are
  * written to the appender. Returns an empty optional if an ack block could not
  * be written.
  */
-folly::Optional<AckFrameWriteResult> writeAckFrame(
+folly::Optional<WriteAckFrameResult> writeAckFrame(
-    const AckFrameMetaData& ackFrameMetaData,
+    const WriteAckFrameMetaData& ackFrameMetaData,
     PacketBuilderInterface& builder,
     FrameType frameType = FrameType::ACK);
 
@@ -192,13 +123,13 @@ folly::Optional<AckFrameWriteResult> writeAckFrame(
  */
 size_t computeSizeUsedByRecvdTimestamps(quic::WriteAckFrame& writeAckFrame);
 
-folly::Optional<AckFrameWriteResult> writeAckFrameWithReceivedTimestamps(
+folly::Optional<WriteAckFrameResult> writeAckFrameWithReceivedTimestamps(
-    const AckFrameMetaData& ackFrameMetaData,
+    const WriteAckFrameMetaData& ackFrameMetaData,
     PacketBuilderInterface& builder,
     FrameType frameType = FrameType::ACK_RECEIVE_TIMESTAMPS);
 
 folly::Optional<quic::WriteAckFrame> writeAckFrameToPacketBuilder(
-    const quic::AckFrameMetaData& ackFrameMetaData,
+    const WriteAckFrameMetaData& ackFrameMetaData,
     quic::PacketBuilderInterface& builder,
     quic::FrameType frameType);
 

quic/codec/Types.h

@@ -13,12 +13,15 @@
 #include <folly/io/Cursor.h>
 #include <quic/QuicConstants.h>
 #include <quic/QuicException.h>
+#include <quic/codec/PacketNumber.h>
 #include <quic/codec/QuicConnectionId.h>
 #include <quic/codec/QuicInteger.h>
 #include <quic/common/BufUtil.h>
+#include <quic/common/CircularDeque.h>
 #include <quic/common/IntervalSet.h>
 #include <quic/common/SmallCollections.h>
 #include <quic/common/Variant.h>
+#include <quic/state/TransportSettings.h>
 
 /**
  * This details the types of objects that can be serialized or deserialized
@@ -29,7 +32,6 @@ namespace quic {
 
 using StreamId = uint64_t;
 using StreamGroupId = uint64_t;
-using PacketNum = uint64_t;
 
 enum class PacketNumberSpace : uint8_t {
   Initial,
@@ -218,6 +220,74 @@ struct WriteAckFrame {
   }
 };
 
+struct WriteAckFrameState {
+  AckBlocks acks;
+
+  // Receive timestamp and packet number for the largest received packet.
+  //
+  // Updated whenever we receive a packet with a larger packet number
+  // than all previously received packets in the packet number space
+  // tracked by this AckState.
+  folly::Optional<RecvdPacketInfo> largestRecvdPacketInfo;
+  // Receive timestamp and packet number for the last received packet.
+  //
+  // Will be different from the value stored in largestRecvdPacketInfo
+  // if the last packet was received out of order and thus had a packet
+  // number less than that of a previously received packet in the packet
+  // number space tracked by this AckState.
+  folly::Optional<RecvdPacketInfo> lastRecvdPacketInfo;
+
+  // Packet number and timestamp of recently received packets.
+  //
+  // The maximum number of packets stored in pktsReceivedTimestamps is
+  // controlled by kMaxReceivedPktsTimestampsStored.
+  //
+  // The packet number of entries in the deque is guarenteed to increase
+  // monotonically because an entry is only added for a received packet
+  // if the packet number is greater than the packet number of the last
+  // element in the deque (e.g., entries are not added for packets that
+  // arrive out of order relative to previously received packets).
+  CircularDeque<RecvdPacketInfo> recvdPacketInfos;
+};
+
+struct WriteAckFrameMetaData {
+  // ACK state.
+  const WriteAckFrameState& ackState;
+
+  // Delay in sending ack from time that packet was received.
+  std::chrono::microseconds ackDelay;
+  // The ack delay exponent to use.
+  uint8_t ackDelayExponent;
+
+  // Receive timestamps basis
+  TimePoint connTime;
+
+  folly::Optional<AckReceiveTimestampsConfig> recvTimestampsConfig =
+      folly::none;
+
+  folly::Optional<uint64_t> maxAckReceiveTimestampsToSend = folly::none;
+};
+
+struct WriteAckFrameResult {
+  uint64_t bytesWritten;
+  WriteAckFrame writeAckFrame;
+  // This includes the first ack block
+  size_t ackBlocksWritten;
+  size_t timestampRangesWritten;
+  size_t timestampsWritten;
+  WriteAckFrameResult(
+      uint64_t bytesWrittenIn,
+      WriteAckFrame writeAckFrameIn,
+      size_t ackBlocksWrittenIn,
+      size_t timestampRangesWrittenIn = 0,
+      size_t timestampsWrittenIn = 0)
+      : bytesWritten(bytesWrittenIn),
+        writeAckFrame(std::move(writeAckFrameIn)),
+        ackBlocksWritten(ackBlocksWrittenIn),
+        timestampRangesWritten(timestampRangesWrittenIn),
+        timestampsWritten(timestampsWrittenIn) {}
+};
+
 struct RstStreamFrame {
   StreamId streamId;
   ApplicationErrorCode errorCode;

quic/codec/test/QuicPacketRebuilderTest.cpp

@@ -75,9 +75,9 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) {
   AckBlocks ackBlocks;
   ackBlocks.insert(10, 100);
   ackBlocks.insert(200, 1000);
-  WriteAckState writeAckState = {.acks = ackBlocks};
+  WriteAckFrameState writeAckState = {.acks = ackBlocks};
-  //   AckFrameMetaData ackMeta(ackBlocks, 0us, kDefaultAckDelayExponent);
+  //   WriteAckFrameMetaData ackMeta(ackBlocks, 0us, kDefaultAckDelayExponent);
-  AckFrameMetaData ackMeta = {
+  WriteAckFrameMetaData ackMeta = {
       .ackState = writeAckState,
       .ackDelay = 0us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent)};
@@ -407,8 +407,8 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuild) {
   AckBlocks ackBlocks;
   ackBlocks.insert(10, 100);
   ackBlocks.insert(200, 1000);
-  WriteAckState writeAckState = {.acks = ackBlocks};
+  WriteAckFrameState writeAckState = {.acks = ackBlocks};
-  AckFrameMetaData ackMeta = {
+  WriteAckFrameMetaData ackMeta = {
       .ackState = writeAckState,
       .ackDelay = 0us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent)};

quic/codec/test/QuicWriteCodecTest.cpp

@@ -175,8 +175,8 @@ PacketsReceivedTimestampsDeque populateReceiveTimestamps(
 }
 
 size_t computeBytesForAckReceivedTimestamps(
-    AckFrameMetaData ackFrameMetadata,
+    const WriteAckFrameMetaData& ackFrameMetadata,
-    AckFrameWriteResult ackFrameWriteResult,
+    WriteAckFrameResult ackFrameWriteResult,
     FrameType frameType) {
   if (frameType != FrameType::ACK_RECEIVE_TIMESTAMPS) {
     return 0;
@@ -212,12 +212,12 @@ size_t computeBytesForAckReceivedTimestamps(
   return sizeConsumed;
 }
 
-WriteAckState createTestWriteAckState(
+WriteAckFrameState createTestWriteAckState(
     FrameType frameType,
     const TimePoint& connTime,
     AckBlocks& ackBlocks,
     uint64_t countTimestampsToStore = kMaxReceivedPktsTimestampsStored) {
-  WriteAckState ackState = {.acks = ackBlocks};
+  WriteAckFrameState ackState = {.acks = ackBlocks};
   ackState.acks = ackBlocks;
   if (frameType == FrameType::ACK_RECEIVE_TIMESTAMPS) {
     ackState.recvdPacketInfos =
@@ -231,7 +231,7 @@ WriteAckState createTestWriteAckState(
 }
 
 void assertsOnDecodedReceiveTimestamps(
-    const AckFrameMetaData& ackFrameMetaData,
+    const WriteAckFrameMetaData& ackFrameMetaData,
     const WriteAckFrame& writeAckFrame,
     const ReadAckFrame& readAckFrame,
     uint64_t expectedTimestampRangesCount,
@@ -887,9 +887,9 @@ TEST_P(QuicWriteCodecTest, AckFrameGapExceedsRepresentation) {
   AckBlocks ackBlocks = {{max - 10, max - 10}, {1, 1}};
   auto frameType = GetParam();
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = 0us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -919,7 +919,7 @@ TEST_P(QuicWriteCodecTest, AckFrameVeryLargeAckRange) {
 
   TimePoint connTime = Clock::now();
 
-  WriteAckState ackState = {.acks = ackBlocks};
+  WriteAckFrameState ackState = {.acks = ackBlocks};
   ackState.acks = ackBlocks;
   if (frameType == FrameType::ACK_RECEIVE_TIMESTAMPS) {
     auto lastPacketDelta =
@@ -947,7 +947,7 @@ TEST_P(QuicWriteCodecTest, AckFrameVeryLargeAckRange) {
     });
   }
 
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = 0us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -1007,9 +1007,9 @@ TEST_P(QuicWriteCodecTest, AckFrameNotEnoughForAnything) {
   AckBlocks ackBlocks = {{1000, 1000}, {500, 700}, {100, 200}};
   auto frameType = GetParam();
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = 0us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -1031,9 +1031,9 @@ TEST_P(QuicWriteCodecTest, WriteSimpleAckFrame) {
   AckBlocks ackBlocks = {{501, 1000}, {101, 400}};
   auto frameType = GetParam();
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = ackDelay,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -1103,9 +1103,9 @@ TEST_P(QuicWriteCodecTest, WriteAckFrameWillSaveAckDelay) {
   AckBlocks ackBlocks = {{501, 1000}, {101, 400}};
   auto frameType = GetParam();
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = ackDelay,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -1150,9 +1150,9 @@ TEST_P(QuicWriteCodecTest, VerifyNumAckBlocksSizeAccounted) {
   }
   ackBlocks.insert({largest, largest});
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = 0us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -1220,9 +1220,9 @@ TEST_P(QuicWriteCodecTest, WriteWithDifferentAckDelayExponent) {
   auto frameType = GetParam();
 
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = 1240us,
       .ackDelayExponent = static_cast<uint8_t>(ackDelayExponent),
@@ -1256,9 +1256,9 @@ TEST_P(QuicWriteCodecTest, WriteExponentInLongHeaderPacket) {
   uint8_t ackDelayExponent = 6;
   auto frameType = GetParam();
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = 1240us,
       .ackDelayExponent = static_cast<uint8_t>(ackDelayExponent),
@@ -1301,9 +1301,9 @@ TEST_P(QuicWriteCodecTest, OnlyAckLargestPacket) {
   // generated as the first block to cover largestAcked => 2 bytes
   auto frameType = GetParam();
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = 555us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -1388,9 +1388,9 @@ TEST_P(QuicWriteCodecTest, WriteSomeAckBlocks) {
     pktBuilder.remaining_ = 42;
   }
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = 555us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -1453,9 +1453,9 @@ TEST_P(QuicWriteCodecTest, NoSpaceForAckBlockSection) {
   AckBlocks ackBlocks = {{1000, 1000}, {701, 900}, {501, 600}};
   auto frameType = GetParam();
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = 555us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -1489,9 +1489,9 @@ TEST_P(QuicWriteCodecTest, OnlyHasSpaceForFirstAckBlock) {
   // 1 byte for first ack block length
   AckBlocks ackBlocks = {{1000, 1000}, {701, 900}, {501, 600}};
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = 555us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -1555,9 +1555,9 @@ TEST_P(QuicWriteCodecTest, WriteAckFrameWithMultipleTimestampRanges) {
   // total 11 bytes
   auto frameType = GetParam();
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks, 50);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = ackDelay,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),
@@ -1632,9 +1632,9 @@ TEST_P(
   // total 11 bytes
   auto frameType = GetParam();
   TimePoint connTime = Clock::now();
-  WriteAckState ackState =
+  WriteAckFrameState ackState =
       createTestWriteAckState(frameType, connTime, ackBlocks, 100);
-  AckFrameMetaData ackFrameMetaData = {
+  WriteAckFrameMetaData ackFrameMetaData = {
       .ackState = ackState,
       .ackDelay = ackDelay,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent),

quic/common/test/TestPacketBuilders.cpp

@@ -131,9 +131,9 @@ RegularQuicPacketBuilder::Packet AckPacketBuilder::build() && {
     builder.accountForCipherOverhead(maybeAead.value()->getCipherOverhead());
   }
   DCHECK(builder.canBuildPacket());
-  WriteAckState ackState;
+  WriteAckFrameState ackState;
   ackState.acks = *CHECK_NOTNULL(maybeAckBlocks.get_pointer());
-  AckFrameMetaData ackData = {
+  WriteAckFrameMetaData ackData = {
       .ackState = ackState,
       .ackDelay = *CHECK_NOTNULL(maybeAckDelay.get_pointer()),
       .ackDelayExponent = static_cast<uint8_t>(

quic/d6d/Types.h

@@ -7,7 +7,7 @@
 
 #pragma once
 
-#include <quic/codec/Types.h>
+#include <quic/codec/PacketNumber.h>
 #include <string>
 
 namespace quic {

quic/fizz/client/test/QuicClientTransportTest.cpp

@@ -4086,9 +4086,9 @@ TEST_F(QuicClientTransportVersionAndRetryTest, UnencryptedAckData) {
       kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */);
   builder.encodePacketHeader();
   DCHECK(builder.canBuildPacket());
-  // AckFrameMetaData ackData(acks, 0us, 0);
+  // WriteAckFrameMetaData ackData(acks, 0us, 0);
-  WriteAckState writeAckState = {.acks = acks};
+  WriteAckFrameState writeAckState = {.acks = acks};
-  AckFrameMetaData ackData = {
+  WriteAckFrameMetaData ackData = {
       .ackState = writeAckState,
       .ackDelay = 0us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent)};

quic/server/test/QuicServerTransportTest.cpp

@@ -3287,8 +3287,8 @@ TEST_F(QuicUnencryptedServerTransportTest, TestUnencryptedAck) {
       kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */);
   builder.encodePacketHeader();
   DCHECK(builder.canBuildPacket());
-  WriteAckState writeAckState = {.acks = acks};
+  WriteAckFrameState writeAckState = {.acks = acks};
-  AckFrameMetaData ackData = {
+  WriteAckFrameMetaData ackData = {
       .ackState = writeAckState,
       .ackDelay = 0us,
       .ackDelayExponent = static_cast<uint8_t>(kDefaultAckDelayExponent)};

quic/state/AckStates.h

@@ -7,17 +7,15 @@
 
 #pragma once
 
+#include <folly/Random.h>
 #include <quic/QuicConstants.h>
-#include <quic/codec/QuicWriteCodec.h>
 #include <quic/codec/Types.h>
 #include <quic/common/IntervalSet.h>
 
-#include <folly/Random.h>
-
 namespace quic {
 
 // Ack and PacketNumber states. This is per-packet number space.
-struct AckState : WriteAckState {
+struct AckState : WriteAckFrameState {
   // Largest ack that has been written to a packet
   folly::Optional<PacketNum> largestAckScheduled;
   // Count of outstanding packets received with only non-retransmittable data.

quic/state/TransportSettings.h

@@ -8,7 +8,6 @@
 #pragma once
 
 #include <quic/QuicConstants.h>
-#include <quic/codec/QuicConnectionId.h>
 #include <quic/d6d/Types.h>
 #include <chrono>
 #include <cstdint>