Stop tracking pure ack packets in Quic

Summary:
Previously we track them since we thought we can get some additional
RTT samples. But these are bad RTT samples since peer can delays the acking of
pure acks. Now we no longer trust such RTT samples, there is no reason to keep
tracking pure ack packets.

Reviewed By: mjoras

Differential Revision: D18946081

fbshipit-source-id: 0a92d88e709edf8475d67791ba064c3e8b7f627a

quic/api/QuicPacketScheduler.cpp

@@ -526,9 +526,8 @@ CloningScheduler::CloningScheduler(
 bool CloningScheduler::hasData() const {
   return frameScheduler_.hasData() ||
       (!conn_.outstandingPackets.empty() &&
-       (conn_.outstandingPackets.size() !=
+       conn_.outstandingPackets.size() !=
-        conn_.outstandingHandshakePacketsCount +
+           conn_.outstandingHandshakePacketsCount);
-            conn_.outstandingPureAckPacketsCount));
 }
 
 std::pair<
@@ -568,9 +567,8 @@ CloningScheduler::scheduleFramesForPacket(
         getAckState(conn_, builderPnSpace).largestAckedByPeer,
         conn_.version.value_or(*conn_.originalVersion));
     PacketRebuilder rebuilder(regularBuilder, conn_);
-    // We shouldn't clone Handshake packet. For PureAcks, cloning them bring
+    // We shouldn't clone Handshake packet.
-    // perf down as shown by load test.
+    if (iter->isHandshake) {
-    if (iter->isHandshake || iter->pureAck) {
       continue;
     }
     // If the packet is already a clone that has been processed, we don't clone

quic/api/QuicTransportBase.cpp

@@ -382,7 +382,6 @@ void QuicTransportBase::closeImpl(
   // Don't need outstanding packets.
   conn_->outstandingPackets.clear();
   conn_->outstandingHandshakePacketsCount = 0;
-  conn_->outstandingPureAckPacketsCount = 0;
 
   // We don't need no congestion control.
   conn_->congestionController = nullptr;

quic/api/QuicTransportFunctions.cpp

@@ -339,16 +339,26 @@ void updateConnection(
     }
   }
 
-  // TODO: Now pureAck is equivalent to non retransmittable packet. This might
+  increaseNextPacketNum(conn, packetNumberSpace);
-  // change in the future.
+  conn.lossState.largestSent = std::max(conn.lossState.largestSent, packetNum);
-  auto pureAck = !retransmittable;
+  // updateConnection may be called multiple times during write. If before or
+  // during any updateConnection, setLossDetectionAlarm is already set, we
+  // shouldn't clear it:
+  if (!conn.pendingEvents.setLossDetectionAlarm) {
+    conn.pendingEvents.setLossDetectionAlarm = retransmittable;
+  }
+  conn.lossState.totalBytesSent += encodedSize;
+
+  if (!retransmittable) {
+    DCHECK(!packetEvent);
+    return;
+  }
   OutstandingPacket pkt(
       std::move(packet),
       std::move(sentTime),
       encodedSize,
       isHandshake,
-      pureAck,
+      conn.lossState.totalBytesSent);
-      conn.lossState.totalBytesSent + encodedSize);
   pkt.isAppLimited = conn.congestionController
       ? conn.congestionController->isAppLimited()
       : false;
@@ -362,17 +372,12 @@ void updateConnection(
   }
   if (packetEvent) {
     DCHECK(conn.outstandingPacketEvents.count(*packetEvent));
-    // CloningScheduler doesn't clone handshake packets or pureAck, and the
-    // clone result cannot be pureAck either.
     DCHECK(!isHandshake);
-    DCHECK(!pureAck);
     pkt.associatedEvent = std::move(packetEvent);
     conn.lossState.totalBytesCloned += encodedSize;
   }
 
-  increaseNextPacketNum(conn, packetNumberSpace);
+  if (conn.congestionController) {
-  conn.lossState.largestSent = std::max(conn.lossState.largestSent, packetNum);
-  if (conn.congestionController && !pureAck) {
     conn.congestionController->onPacketSent(pkt);
     // An approximation of the app being blocked. The app
     // technically might not have bytes to write.
@@ -386,7 +391,7 @@ void updateConnection(
           conn.congestionController->getCongestionWindow());
     }
   }
-  if (conn.pacer && !pureAck) {
+  if (conn.pacer) {
     conn.pacer->onPacketSent();
   }
   if (conn.pathValidationLimiter &&
@@ -397,11 +402,7 @@ void updateConnection(
     ++conn.outstandingHandshakePacketsCount;
     conn.lossState.lastHandshakePacketSentTime = pkt.time;
   }
-  if (pureAck) {
-    ++conn.outstandingPureAckPacketsCount;
-  } else {
   conn.lossState.lastRetransmittablePacketSentTime = pkt.time;
-  }
   if (pkt.associatedEvent) {
     CHECK_EQ(packetNumberSpace, PacketNumberSpace::AppData);
     ++conn.outstandingClonedPacketsCount;
@@ -420,16 +421,8 @@ void updateConnection(
   conn.outstandingPackets.insert(packetIt, std::move(pkt));
 
   auto opCount = conn.outstandingPackets.size();
-  DCHECK_GE(opCount, conn.outstandingPureAckPacketsCount);
   DCHECK_GE(opCount, conn.outstandingHandshakePacketsCount);
   DCHECK_GE(opCount, conn.outstandingClonedPacketsCount);
-  // updateConnection may be called multiple times during write. If before or
-  // during any updateConnection, setLossDetectionAlarm is already set, we
-  // shouldn't clear it:
-  if (!conn.pendingEvents.setLossDetectionAlarm) {
-    conn.pendingEvents.setLossDetectionAlarm = retransmittable;
-  }
-  conn.lossState.totalBytesSent += encodedSize;
 }
 
 uint64_t congestionControlWritableBytes(const QuicConnectionStateBase& conn) {

quic/api/test/QuicPacketSchedulerTest.cpp

@@ -36,7 +36,7 @@ PacketNum addInitialOutstandingPacket(QuicConnectionStateBase& conn) {
       nextPacketNum,
       QuicVersion::QUIC_DRAFT);
   RegularQuicWritePacket packet(std::move(header));
-  conn.outstandingPackets.emplace_back(packet, Clock::now(), 0, true, false, 0);
+  conn.outstandingPackets.emplace_back(packet, Clock::now(), 0, true, 0);
   conn.outstandingHandshakePacketsCount++;
   increaseNextPacketNum(conn, PacketNumberSpace::Handshake);
   return nextPacketNum;
@@ -54,24 +54,12 @@ PacketNum addHandshakeOutstandingPacket(QuicConnectionStateBase& conn) {
       nextPacketNum,
       QuicVersion::QUIC_DRAFT);
   RegularQuicWritePacket packet(std::move(header));
-  conn.outstandingPackets.emplace_back(packet, Clock::now(), 0, true, false, 0);
+  conn.outstandingPackets.emplace_back(packet, Clock::now(), 0, true, 0);
   conn.outstandingHandshakePacketsCount++;
   increaseNextPacketNum(conn, PacketNumberSpace::Handshake);
   return nextPacketNum;
 }
 
-PacketNum addPureAckOutstandingPacket(QuicConnectionStateBase& conn) {
-  PacketNum nextPacketNum = getNextPacketNum(conn, PacketNumberSpace::AppData);
-  ShortHeader header(
-      ProtectionType::KeyPhaseOne,
-      conn.clientConnectionId.value_or(quic::test::getTestConnectionId()),
-      nextPacketNum);
-  RegularQuicWritePacket packet(std::move(header));
-  conn.outstandingPackets.emplace_back(packet, Clock::now(), 0, false, true, 0);
-  increaseNextPacketNum(conn, PacketNumberSpace::AppData);
-  return nextPacketNum;
-}
-
 PacketNum addOutstandingPacket(QuicConnectionStateBase& conn) {
   PacketNum nextPacketNum = getNextPacketNum(conn, PacketNumberSpace::AppData);
   ShortHeader header(
@@ -79,8 +67,7 @@ PacketNum addOutstandingPacket(QuicConnectionStateBase& conn) {
       conn.clientConnectionId.value_or(quic::test::getTestConnectionId()),
       nextPacketNum);
   RegularQuicWritePacket packet(std::move(header));
-  conn.outstandingPackets.emplace_back(
+  conn.outstandingPackets.emplace_back(packet, Clock::now(), 0, false, 0);
-      packet, Clock::now(), 0, false, false, 0);
   increaseNextPacketNum(conn, PacketNumberSpace::AppData);
   return nextPacketNum;
 }
@@ -439,33 +426,6 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) {
   EXPECT_EQ(expected, *result.first);
 }
 
-TEST_F(QuicPacketSchedulerTest, DoNotClonePureAck) {
-  QuicClientConnectionState conn(
-      FizzClientQuicHandshakeContext::Builder().build());
-  FrameScheduler noopScheduler("frame");
-  CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
-  // Add two outstanding packets, with second one being pureAck
-  auto expected = addOutstandingPacket(conn);
-  // There needs to have retransmittable frame for the rebuilder to work
-  conn.outstandingPackets.back().packet.frames.push_back(
-      MaxDataFrame(conn.flowControlState.advertisedMaxOffset));
-  addPureAckOutstandingPacket(conn);
-  conn.outstandingPackets.back().packet.frames.push_back(WriteAckFrame());
-
-  ShortHeader header(
-      ProtectionType::KeyPhaseOne,
-      conn.clientConnectionId.value_or(getTestConnectionId()),
-      getNextPacketNum(conn, PacketNumberSpace::AppData));
-  RegularQuicPacketBuilder builder(
-      conn.udpSendPacketLen,
-      std::move(header),
-      conn.ackStates.appDataAckState.largestAckedByPeer);
-  auto result = cloningScheduler.scheduleFramesForPacket(
-      std::move(builder), kDefaultUDPSendPacketLen);
-  EXPECT_TRUE(result.first.hasValue() && result.second.hasValue());
-  EXPECT_EQ(expected, *result.first);
-}
-
 TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasDataIgnoresNonAppData) {
   QuicClientConnectionState conn(
       FizzClientQuicHandshakeContext::Builder().build());

quic/api/test/QuicTransportBaseTest.cpp

@@ -1090,10 +1090,8 @@ TEST_F(QuicTransportImplTest, CancelAllDeliveryCallbacksMap) {
 }
 
 TEST_F(QuicTransportImplTest, CloseTransportCleansupOutstandingCounters) {
-  transport->transportConn->outstandingPureAckPacketsCount = 100;
   transport->transportConn->outstandingHandshakePacketsCount = 200;
   transport->closeNow(folly::none);
-  EXPECT_EQ(0, transport->transportConn->outstandingPureAckPacketsCount);
   EXPECT_EQ(0, transport->transportConn->outstandingHandshakePacketsCount);
 }
 

quic/api/test/QuicTransportFunctionsTest.cpp

@@ -351,6 +351,16 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionPacketSorting) {
   auto handshakePacket = buildEmptyPacket(*conn, PacketNumberSpace::Handshake);
   auto appDataPacket = buildEmptyPacket(*conn, PacketNumberSpace::AppData);
 
+  auto stream = conn->streamManager->createNextBidirectionalStream().value();
+  writeDataToQuicStream(
+      *stream,
+      folly::IOBuf::copyBuffer("The sun is cold and the rain is hard."),
+      true);
+  WriteStreamFrame writeStreamFrame(stream->id, 0, 5, false);
+  initialPacket.packet.frames.push_back(writeStreamFrame);
+  handshakePacket.packet.frames.push_back(writeStreamFrame);
+  appDataPacket.packet.frames.push_back(writeStreamFrame);
+
   updateConnection(
       *conn,
       folly::none,
@@ -539,7 +549,6 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionPureAckCounter) {
   packet.packet.frames.push_back(std::move(ackFrame));
   updateConnection(
       *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet));
-  EXPECT_EQ(1, conn->outstandingPureAckPacketsCount);
 
   auto nonHandshake = buildEmptyPacket(*conn, PacketNumberSpace::Handshake);
   packetEncodedSize =
@@ -557,7 +566,6 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionPureAckCounter) {
 
   updateConnection(
       *conn, folly::none, packet2.packet, TimePoint(), getEncodedSize(packet));
-  EXPECT_EQ(1, conn->outstandingPureAckPacketsCount);
 
   //  verify QLogger contains correct packet and frame information
   std::shared_ptr<quic::FileQLogger> qLogger =
@@ -588,7 +596,6 @@ TEST_F(QuicTransportFunctionsTest, TestPaddingPureAckPacketIsStillPureAck) {
   packet.packet.frames.push_back(PaddingFrame());
   updateConnection(
       *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet));
-  EXPECT_EQ(1, conn->outstandingPureAckPacketsCount);
 
   // verify QLogger contains correct packet and frames information
   std::shared_ptr<quic::FileQLogger> qLogger =
@@ -761,9 +768,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithPureAck) {
   EXPECT_CALL(*rawPacer, onPacketSent()).Times(0);
   updateConnection(
       *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet));
-  EXPECT_EQ(1, conn->outstandingPackets.size());
+  EXPECT_EQ(0, conn->outstandingPackets.size());
-  EXPECT_TRUE(
-      getFirstOutstandingPacket(*conn, PacketNumberSpace::Handshake)->pureAck);
   EXPECT_EQ(0, conn->lossState.totalBytesAcked);
   std::shared_ptr<quic::FileQLogger> qLogger =
       std::dynamic_pointer_cast<quic::FileQLogger>(conn->qLogger);
@@ -785,7 +790,13 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithPureAck) {
 TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithBytesStats) {
   auto conn = createConn();
   conn->qLogger = std::make_shared<quic::FileQLogger>(VantagePoint::CLIENT);
+  auto stream = conn->streamManager->createNextBidirectionalStream().value();
   auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake);
+  // This is clearly not 555 bytes. I just need some data inside the packet.
+  writeDataToQuicStream(
+      *stream, folly::IOBuf::copyBuffer("Im gonna cut your hair."), true);
+  WriteStreamFrame writeStreamFrame(stream->id, 0, 5, false);
+  packet.packet.frames.push_back(std::move(writeStreamFrame));
   conn->lossState.totalBytesSent = 13579;
   conn->lossState.totalBytesAcked = 8642;
   auto currentTime = Clock::now();
@@ -1585,6 +1596,7 @@ TEST_F(QuicTransportFunctionsTest, WritePureAckWhenNoWritableBytes) {
         return iobuf->computeChainDataLength();
       }));
   EXPECT_CALL(*rawCongestionController, onPacketSent(_)).Times(0);
+  EXPECT_GT(
       writeQuicDataToSocket(
           *rawSocket,
           *conn,
@@ -1593,10 +1605,9 @@ TEST_F(QuicTransportFunctionsTest, WritePureAckWhenNoWritableBytes) {
           *aead,
           *headerCipher,
           getVersion(*conn),
-      conn->transportSettings.writeConnectionDataPacketsLimit);
+          conn->transportSettings.writeConnectionDataPacketsLimit),
-  EXPECT_EQ(1, conn->outstandingPackets.size());
+      0);
-  auto packet = *getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData);
+  EXPECT_EQ(0, conn->outstandingPackets.size());
-  EXPECT_TRUE(packet.pureAck);
 }
 
 TEST_F(QuicTransportFunctionsTest, ShouldWriteDataTest) {
@@ -1872,10 +1883,7 @@ TEST_F(QuicTransportFunctionsTest, ClearBlockedFromPendingEvents) {
       *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet));
   EXPECT_FALSE(conn->streamManager->hasBlocked());
   EXPECT_FALSE(conn->outstandingPackets.empty());
-  EXPECT_EQ(0, conn->outstandingPureAckPacketsCount);
   EXPECT_EQ(0, conn->outstandingClonedPacketsCount);
-  EXPECT_FALSE(
-      getLastOutstandingPacket(*conn, PacketNumberSpace::Handshake)->pureAck);
 }
 
 TEST_F(QuicTransportFunctionsTest, ClonedBlocked) {
@@ -1890,10 +1898,7 @@ TEST_F(QuicTransportFunctionsTest, ClonedBlocked) {
   updateConnection(
       *conn, packetEvent, packet.packet, TimePoint(), getEncodedSize(packet));
   EXPECT_FALSE(conn->outstandingPackets.empty());
-  EXPECT_EQ(0, conn->outstandingPureAckPacketsCount);
   EXPECT_EQ(1, conn->outstandingClonedPacketsCount);
-  EXPECT_FALSE(
-      getLastOutstandingPacket(*conn, PacketNumberSpace::AppData)->pureAck);
 }
 
 TEST_F(QuicTransportFunctionsTest, TwoConnWindowUpdateWillCrash) {
@@ -1925,9 +1930,6 @@ TEST_F(QuicTransportFunctionsTest, WriteStreamFrameIsNotPureAck) {
   updateConnection(
       *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet));
   EXPECT_FALSE(conn->outstandingPackets.empty());
-  EXPECT_EQ(0, conn->outstandingPureAckPacketsCount);
-  EXPECT_FALSE(
-      getLastOutstandingPacket(*conn, PacketNumberSpace::Handshake)->pureAck);
 }
 
 TEST_F(QuicTransportFunctionsTest, ClearRstFromPendingEvents) {
@@ -1942,10 +1944,7 @@ TEST_F(QuicTransportFunctionsTest, ClearRstFromPendingEvents) {
       *conn, folly::none, packet.packet, TimePoint(), getEncodedSize(packet));
   EXPECT_TRUE(conn->pendingEvents.resets.empty());
   EXPECT_FALSE(conn->outstandingPackets.empty());
-  EXPECT_EQ(0, conn->outstandingPureAckPacketsCount);
   EXPECT_EQ(0, conn->outstandingClonedPacketsCount);
-  EXPECT_FALSE(
-      getLastOutstandingPacket(*conn, PacketNumberSpace::Handshake)->pureAck);
 }
 
 TEST_F(QuicTransportFunctionsTest, ClonedRst) {
@@ -1961,10 +1960,7 @@ TEST_F(QuicTransportFunctionsTest, ClonedRst) {
   updateConnection(
       *conn, packetEvent, packet.packet, TimePoint(), getEncodedSize(packet));
   EXPECT_FALSE(conn->outstandingPackets.empty());
-  EXPECT_EQ(0, conn->outstandingPureAckPacketsCount);
   EXPECT_EQ(1, conn->outstandingClonedPacketsCount);
-  EXPECT_FALSE(
-      getLastOutstandingPacket(*conn, PacketNumberSpace::AppData)->pureAck);
 }
 
 TEST_F(QuicTransportFunctionsTest, TotalBytesSentUpdate) {

quic/api/test/QuicTransportTest.cpp

@@ -784,24 +784,7 @@ TEST_F(QuicTransportTest, WriteImmediateAcks) {
       *headerCipher_,
       transport_->getVersion(),
       conn.transportSettings.writeConnectionDataPacketsLimit);
-  EXPECT_EQ(conn.outstandingPackets.size(), 1);
+  EXPECT_TRUE(conn.outstandingPackets.empty());
-  auto& packet =
-      getFirstOutstandingPacket(conn, PacketNumberSpace::AppData)->packet;
-  EXPECT_GE(packet.frames.size(), 1);
-
-  bool ackFound = false;
-  for (auto& frame : packet.frames) {
-    auto ackFrame = frame.asWriteAckFrame();
-    if (!ackFrame) {
-      continue;
-    }
-    EXPECT_EQ(ackFrame->ackBlocks.size(), 1);
-    EXPECT_EQ(start, ackFrame->ackBlocks.front().start);
-    EXPECT_EQ(end, ackFrame->ackBlocks.front().end);
-    ackFound = true;
-  }
-  EXPECT_TRUE(ackFound);
-
   EXPECT_EQ(conn.ackStates.appDataAckState.largestAckScheduled, end);
   EXPECT_FALSE(conn.ackStates.appDataAckState.needsToSendAckImmediately);
   EXPECT_EQ(0, conn.ackStates.appDataAckState.numNonRxPacketsRecvd);
@@ -1044,7 +1027,6 @@ TEST_F(QuicTransportTest, ClonePathChallenge) {
   auto& conn = transport_->getConnectionState();
   // knock every handshake outstanding packets out
   conn.outstandingHandshakePacketsCount = 0;
-  conn.outstandingPureAckPacketsCount = 0;
   conn.outstandingPackets.clear();
   conn.lossState.initialLossTime.clear();
   conn.lossState.handshakeLossTime.clear();
@@ -1080,7 +1062,6 @@ TEST_F(QuicTransportTest, OnlyClonePathValidationIfOutstanding) {
   auto& conn = transport_->getConnectionState();
   // knock every handshake outstanding packets out
   conn.outstandingHandshakePacketsCount = 0;
-  conn.outstandingPureAckPacketsCount = 0;
   conn.outstandingPackets.clear();
   conn.lossState.initialLossTime.clear();
   conn.lossState.handshakeLossTime.clear();
@@ -1223,7 +1204,6 @@ TEST_F(QuicTransportTest, ClonePathResponse) {
   auto& conn = transport_->getConnectionState();
   // knock every handshake outstanding packets out
   conn.outstandingHandshakePacketsCount = 0;
-  conn.outstandingPureAckPacketsCount = 0;
   conn.outstandingPackets.clear();
   conn.lossState.initialLossTime.clear();
   conn.lossState.handshakeLossTime.clear();
@@ -1307,7 +1287,6 @@ TEST_F(QuicTransportTest, CloneNewConnectionIdFrame) {
   auto& conn = transport_->getConnectionState();
   // knock every handshake outstanding packets out
   conn.outstandingHandshakePacketsCount = 0;
-  conn.outstandingPureAckPacketsCount = 0;
   conn.outstandingPackets.clear();
   conn.lossState.initialLossTime.clear();
   conn.lossState.handshakeLossTime.clear();
@@ -1446,7 +1425,6 @@ TEST_F(QuicTransportTest, CloneRetireConnectionIdFrame) {
   auto& conn = transport_->getConnectionState();
   // knock every handshake outstanding packets out
   conn.outstandingHandshakePacketsCount = 0;
-  conn.outstandingPureAckPacketsCount = 0;
   conn.outstandingPackets.clear();
   conn.lossState.initialLossTime.clear();
   conn.lossState.handshakeLossTime.clear();

quic/client/test/QuicClientTransportTest.cpp

@@ -3738,8 +3738,7 @@ TEST_F(QuicClientTransportAfterStartTest, IdleTimerResetNoOutstandingPackets) {
   // Clear out all the outstanding packets to simulate quiescent state.
   client->getNonConstConn().receivedNewPacketBeforeWrite = false;
   client->getNonConstConn().outstandingPackets.clear();
-  client->getNonConstConn().outstandingPureAckPacketsCount =
+  client->getNonConstConn().outstandingHandshakePacketsCount = 0;
-      client->getNonConstConn().outstandingHandshakePacketsCount =
   client->getNonConstConn().outstandingClonedPacketsCount = 0;
   client->idleTimeout().cancelTimeout();
   auto streamId = client->createBidirectionalStream().value();

quic/codec/test/QuicPacketRebuilderTest.cpp

@@ -27,12 +27,7 @@ OutstandingPacket makeDummyOutstandingPacket(
     const RegularQuicWritePacket& writePacket,
     uint64_t totalBytesSentOnConnection) {
   OutstandingPacket packet(
-      writePacket,
+      writePacket, Clock::now(), 1000, false, totalBytesSentOnConnection);
-      Clock::now(),
-      1000,
-      false,
-      false,
-      totalBytesSentOnConnection);
   return packet;
 }
 

quic/common/test/TestUtils.cpp

@@ -522,7 +522,6 @@ OutstandingPacket makeTestingWritePacket(
     PacketNum desiredPacketSeqNum,
     size_t desiredSize,
     uint64_t totalBytesSent,
-    bool pureAck,
     TimePoint sentTime) {
   LongHeader longHeader(
       LongHeader::Types::ZeroRtt,
@@ -532,7 +531,7 @@ OutstandingPacket makeTestingWritePacket(
       QuicVersion::MVFST);
   RegularQuicWritePacket packet(std::move(longHeader));
   return OutstandingPacket(
-      packet, sentTime, desiredSize, false, pureAck, totalBytesSent);
+      packet, sentTime, desiredSize, false, totalBytesSent);
 }
 
 CongestionController::AckEvent makeAck(

quic/common/test/TestUtils.h

@@ -214,7 +214,6 @@ OutstandingPacket makeTestingWritePacket(
     PacketNum desiredPacketSeqNum,
     size_t desiredSize,
     uint64_t totalBytesSent,
-    bool pureAck = false,
     TimePoint sentTime = Clock::now());
 
 // TODO: The way we setup packet sent, ack, loss in test cases can use some

quic/congestion_control/test/BbrBandwidthSamplerTest.cpp

@@ -34,7 +34,7 @@ TEST_F(BbrBandwidthSamplerTest, NoPreviousAckedPacket) {
   CongestionController::AckEvent ackEvent;
   ackEvent.ackedBytes = 5000;
   ackEvent.ackedPackets.push_back(makeAckPacketFromOutstandingPacket(
-      makeTestingWritePacket(0, 1000, 1000, false)));
+      makeTestingWritePacket(0, 1000, 1000)));
   sampler.onPacketAcked(ackEvent, 0);
   EXPECT_EQ(0, sampler.getBandwidth().units);
 }
@@ -46,7 +46,7 @@ TEST_F(BbrBandwidthSamplerTest, NoPreviousAckedPacketFallback) {
   CongestionController::AckEvent ackEvent;
   ackEvent.ackedBytes = 5000;
   ackEvent.ackedPackets.push_back(makeAckPacketFromOutstandingPacket(
-      makeTestingWritePacket(0, 1000, 1000, false, sentTime)));
+      makeTestingWritePacket(0, 1000, 1000, sentTime)));
   ackEvent.ackTime = sentTime + 50ms;
   sampler.onPacketAcked(ackEvent, 0);
   EXPECT_EQ(1000, sampler.getBandwidth().units);
@@ -63,7 +63,7 @@ TEST_F(BbrBandwidthSamplerTest, RateCalculation) {
   auto lastAckedPacketSentTime = ackTime - 200us;
   auto lastAckedPacketAckTime = ackTime - 100us;
   for (PacketNum pn = 0; pn < 5; pn++) {
-    auto packet = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn, false);
+    auto packet = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn);
     packet.lastAckedPacketInfo.emplace(
         lastAckedPacketSentTime, lastAckedPacketAckTime, 0, 0);
     packet.time = ackTime - 50us;
@@ -86,7 +86,7 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) {
   auto lastAckedPacketSentTime = ackTime - 200us;
   auto lastAckedPacketAckTime = ackTime - 100us;
   PacketNum pn = 1;
-  auto packet = makeTestingWritePacket(pn, 1000, 2000, false);
+  auto packet = makeTestingWritePacket(pn, 1000, 2000);
   packet.lastAckedPacketInfo.emplace(
       lastAckedPacketSentTime, lastAckedPacketAckTime, 1000, 1000);
   packet.time = ackTime - 50us;
@@ -96,7 +96,7 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) {
 
   pn++;
   conn_.lossState.totalBytesAcked = 2000;
-  auto packet2 = makeTestingWritePacket(pn, 500, 2500, false);
+  auto packet2 = makeTestingWritePacket(pn, 500, 2500);
   packet2.lastAckedPacketInfo.emplace(packet.time, ackTime, 2000, 1000);
   auto ackTime2 = ackTime + 150us;
   CongestionController::AckEvent ackEvent2;
@@ -109,7 +109,7 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) {
 
   pn++;
   conn_.lossState.totalBytesAcked = 2500;
-  auto packet3 = makeTestingWritePacket(pn, 200, 2700, false);
+  auto packet3 = makeTestingWritePacket(pn, 200, 2700);
   packet3.lastAckedPacketInfo.emplace(packet2.time, ackTime2, 2500, 2000);
   auto ackTime3 = ackTime + 250us;
   CongestionController::AckEvent ackEvent3;
@@ -121,7 +121,7 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) {
 
   pn++;
   conn_.lossState.totalBytesAcked = 2700;
-  auto packet4 = makeTestingWritePacket(pn, 100, 2800, false);
+  auto packet4 = makeTestingWritePacket(pn, 100, 2800);
   packet4.lastAckedPacketInfo.emplace(packet3.time, ackTime3, 2700, 2500);
   auto ackTime4 = ackTime + 350us;
   CongestionController::AckEvent ackEvent4;
@@ -146,7 +146,7 @@ TEST_F(BbrBandwidthSamplerTest, AppLimited) {
   CongestionController::AckEvent ackEvent;
   ackEvent.largestAckedPacket = ++conn.lossState.largestSent;
   auto packet =
-      makeTestingWritePacket(*ackEvent.largestAckedPacket, 1000, 1000, false);
+      makeTestingWritePacket(*ackEvent.largestAckedPacket, 1000, 1000);
   ackEvent.largestAckedPacketSentTime = packet.time;
   ackEvent.ackedPackets.push_back(
       makeAckPacketFromOutstandingPacket(std::move(packet)));
@@ -175,7 +175,7 @@ TEST_F(BbrBandwidthSamplerTest, AppLimitedOutstandingPacket) {
   auto lastAckedPacketSentTime = ackTime - 200us;
   auto lastAckedPacketAckTime = ackTime - 100us;
   PacketNum pn = 0;
-  auto packet = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn, false);
+  auto packet = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn);
   packet.isAppLimited = true;
   packet.lastAckedPacketInfo.emplace(
       lastAckedPacketSentTime, lastAckedPacketAckTime, 0, 0);
@@ -187,7 +187,7 @@ TEST_F(BbrBandwidthSamplerTest, AppLimitedOutstandingPacket) {
   auto bandwidth = sampler.getBandwidth();
 
   pn++;
-  auto packet1 = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn, false);
+  auto packet1 = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn);
   packet1.isAppLimited = true;
   packet1.lastAckedPacketInfo.emplace(packet.time, ackTime, 1000, 0);
   packet1.time = ackTime + 500us;

quic/congestion_control/test/BbrTest.cpp

@@ -130,7 +130,7 @@ TEST_F(BbrTest, StartupCwnd) {
   bbr.setRttSampler(std::move(mockRttSampler));
   bbr.setBandwidthSampler(std::move(mockBandwidthSampler));
 
-  auto packet = makeTestingWritePacket(0, 3000, 3000, false);
+  auto packet = makeTestingWritePacket(0, 3000, 3000);
   bbr.onPacketSent(packet);
   auto startingCwnd = bbr.getCongestionWindow();
   conn.lossState.srtt = 100us;
@@ -162,7 +162,7 @@ TEST_F(BbrTest, LeaveStartup) {
   auto sendAckGrow = [&](bool growFast) {
     conn.lossState.largestSent = currentLatest;
     auto packet = makeTestingWritePacket(
-        conn.lossState.largestSent, 1000, 1000 + totalSent, false);
+        conn.lossState.largestSent, 1000, 1000 + totalSent);
     bbr.onPacketSent(packet);
     totalSent += 1000;
     EXPECT_CALL(*rawBandwidthSampler, getBandwidth())
@@ -206,7 +206,7 @@ TEST_F(BbrTest, RemoveInflightBytes) {
   conn.udpSendPacketLen = 1000;
   BbrCongestionController bbr(conn);
   auto writableBytesAfterInit = bbr.getWritableBytes();
-  bbr.onPacketSent(makeTestingWritePacket(0, 1000, 1000, 0));
+  bbr.onPacketSent(makeTestingWritePacket(0, 1000, 1000));
   EXPECT_EQ(writableBytesAfterInit - 1000, bbr.getWritableBytes());
   bbr.onRemoveBytesFromInflight(1000);
   EXPECT_EQ(writableBytesAfterInit, bbr.getWritableBytes());
@@ -233,8 +233,7 @@ TEST_F(BbrTest, ProbeRtt) {
     auto packet = makeTestingWritePacket(
         conn.lossState.largestSent,
         conn.udpSendPacketLen,
-        totalSent + conn.udpSendPacketLen,
+        totalSent + conn.udpSendPacketLen);
-        false);
     bbr.onPacketSent(packet);
     inflightPackets.push_back(std::make_pair(currentLatest, packet.time));
     inflightBytes += conn.udpSendPacketLen;
@@ -397,7 +396,7 @@ TEST_F(BbrTest, AckAggregation) {
   auto sendAckGrow = [&](bool growFast) {
     conn.lossState.largestSent = currentLatest;
     auto packet = makeTestingWritePacket(
-        conn.lossState.largestSent, 1000, 1000 + totalSent, false);
+        conn.lossState.largestSent, 1000, 1000 + totalSent);
     bbr.onPacketSent(packet);
     totalSent += 1000;
     EXPECT_CALL(*rawBandwidthSampler, getBandwidth())
@@ -438,7 +437,7 @@ TEST_F(BbrTest, AckAggregation) {
 
   conn.lossState.largestSent = currentLatest;
   auto packet = makeTestingWritePacket(
-      conn.lossState.largestSent, 1000, 1000 + totalSent, false);
+      conn.lossState.largestSent, 1000, 1000 + totalSent);
   bbr.onPacketSent(packet);
   totalSent += 1000;
   auto ackEvent = makeAck(currentLatest, 1000, Clock::now(), packet.time);
@@ -463,8 +462,7 @@ TEST_F(BbrTest, AckAggregation) {
   auto packet1 = makeTestingWritePacket(
       conn.lossState.largestSent,
       currentMaxAckHeight * 2 + 100,
-      currentMaxAckHeight * 2 + 100 + totalSent,
+      currentMaxAckHeight * 2 + 100 + totalSent);
-      false);
   bbr.onPacketSent(packet1);
   totalSent += (currentMaxAckHeight * 2 + 100);
   auto ackEvent2 = makeAck(
@@ -483,8 +481,7 @@ TEST_F(BbrTest, AppLimited) {
   auto rawBandwidthSampler = mockBandwidthSampler.get();
   bbr.setBandwidthSampler(std::move(mockBandwidthSampler));
 
-  auto packet =
+  auto packet = makeTestingWritePacket(conn.lossState.largestSent, 1000, 1000);
-      makeTestingWritePacket(conn.lossState.largestSent, 1000, 1000, false);
   bbr.onPacketSent(packet);
   conn.lossState.largestSent++;
   EXPECT_CALL(*rawBandwidthSampler, onAppLimited()).Times(1);
@@ -507,7 +504,7 @@ TEST_F(BbrTest, AppLimitedIgnored) {
     EXPECT_CALL(*rawBandwidthSampler, onAppLimited()).Times(1);
     bbr.setAppLimited();
     auto packet = makeTestingWritePacket(
-        conn.lossState.largestSent++, 1000, 1000 + inflightBytes, 0);
+        conn.lossState.largestSent++, 1000, 1000 + inflightBytes);
     inflightBytes += 1000;
     bbr.onPacketSent(packet);
   }
@@ -529,8 +526,7 @@ TEST_F(BbrTest, ExtendMinRttExpiration) {
   EXPECT_CALL(*rawBandwidthSampler, onAppLimited()).Times(1);
   bbr.setAppLimited();
 
-  auto packet =
+  auto packet = makeTestingWritePacket(conn.lossState.largestSent, 1000, 1000);
-      makeTestingWritePacket(conn.lossState.largestSent, 1000, 1000, 0);
   bbr.onPacketSent(packet);
   EXPECT_CALL(*rawRttSampler, timestampMinRtt(_)).Times(1);
   bbr.onPacketAckOrLoss(
@@ -544,7 +540,7 @@ TEST_F(BbrTest, BytesCounting) {
   bbr.setBandwidthSampler(std::make_unique<BbrBandwidthSampler>(conn));
 
   PacketNum packetNum = 0;
-  auto packet = makeTestingWritePacket(packetNum, 1200, 1200, false);
+  auto packet = makeTestingWritePacket(packetNum, 1200, 1200);
   conn.outstandingPackets.push_back(packet);
   ReadAckFrame ackFrame;
   ackFrame.largestAcked = packetNum;

quic/congestion_control/test/CopaTest.cpp

@@ -29,7 +29,7 @@ class CopaTest : public Test {
           ShortHeader(ProtectionType::KeyPhaseZero, connId, packetData.first));
       totalSentBytes += 10;
       loss.addLostPacket(OutstandingPacket(
-          std::move(packet), Clock::now(), 10, false, false, totalSentBytes));
+          std::move(packet), Clock::now(), 10, false, totalSentBytes));
       loss.lostBytes = packetData.second;
     }
     loss.lostPackets = lostPackets.size();
@@ -42,7 +42,7 @@ class CopaTest : public Test {
     RegularQuicWritePacket packet(
         ShortHeader(ProtectionType::KeyPhaseZero, connId, packetNum));
     return OutstandingPacket(
-        std::move(packet), Clock::now(), size, false, false, totalSent);
+        std::move(packet), Clock::now(), size, false, totalSent);
   }
 
   CongestionController::AckEvent createAckEvent(

quic/congestion_control/test/CubicHystartTest.cpp

@@ -83,19 +83,11 @@ TEST_F(CubicHystartTest, AckTrain) {
   // Packet 0 is sent:
   auto packetSize = kLowSsthreshInMss * conn.udpSendPacketLen;
   auto packet0 = makeTestingWritePacket(
-      0,
+      0, packetSize, packetSize, realNow - std::chrono::milliseconds(10));
-      packetSize,
-      packetSize,
-      false,
-      realNow - std::chrono::milliseconds(10));
   cubic.onPacketSent(packet0);
   // Packet 1 is sent:
   auto packet1 = makeTestingWritePacket(
-      1,
+      1, packetSize, packetSize * 2, realNow - std::chrono::milliseconds(5));
-      packetSize,
-      packetSize * 2,
-      false,
-      realNow - std::chrono::milliseconds(5));
   cubic.onPacketSent(packet1);
   // Packet 0 is acked:
   cubic.onPacketAckOrLoss(
@@ -128,7 +120,7 @@ TEST_F(CubicHystartTest, NoAckTrainNoDelayIncrease) {
   conn.lossState.srtt = 2us;
   auto realNow = quic::Clock::now();
   // One onPacketAcked will not trigger DelayIncrease
-  auto packet = makeTestingWritePacket(0, 1000, 1000, false, realNow);
+  auto packet = makeTestingWritePacket(0, 1000, 1000, realNow);
   cubic.onPacketSent(packet);
   cubic.onPacketAckOrLoss(
       makeAck(0, 1000, realNow + kAckCountingGap + 2us, packet.time),
@@ -157,14 +149,14 @@ TEST_F(CubicHystartTest, DelayIncrease) {
   auto packetsSentTime = Clock::now();
   auto firstPacketNum = packetNum++;
   auto packet0 = makeTestingWritePacket(
-      packetNum, 1000, totalSent + 1000, false, packetsSentTime);
+      packetNum, 1000, totalSent + 1000, packetsSentTime);
   totalSent += 1000;
   conn.lossState.largestSent = firstPacketNum;
   cubic.onPacketSent(packet0);
 
   auto secondPacketNum = packetNum++;
   auto packet1 = makeTestingWritePacket(
-      secondPacketNum, 1000, totalSent + 1000, false, packetsSentTime);
+      secondPacketNum, 1000, totalSent + 1000, packetsSentTime);
   totalSent += 1000;
   // Packet 1 is sent:
   conn.lossState.largestSent = secondPacketNum;
@@ -185,7 +177,7 @@ TEST_F(CubicHystartTest, DelayIncrease) {
   auto estimatedRttEndTarget = Clock::now();
 
   auto packet2 = makeTestingWritePacket(
-      packetNum, 1000, totalSent + 1000, false, estimatedRttEndTarget + 1us);
+      packetNum, 1000, totalSent + 1000, estimatedRttEndTarget + 1us);
   totalSent += 1000;
   conn.lossState.largestSent = packetNum;
   cubic.onPacketSent(packet2);
@@ -203,7 +195,7 @@ TEST_F(CubicHystartTest, DelayIncrease) {
   for (size_t i = 0; i < kAckSampling - 1; i++) {
     conn.lossState.largestSent = i + packetNum;
     auto packet = makeTestingWritePacket(
-        i + packetNum, 1000, totalSent + 1000, false, packetsSentTime);
+        i + packetNum, 1000, totalSent + 1000, packetsSentTime);
     cubic.onPacketSent(packet);
     totalSent += 1000;
     ackTime += ackTimeIncrease;
@@ -217,7 +209,7 @@ TEST_F(CubicHystartTest, DelayIncrease) {
   // DelayIncrease:
   conn.lossState.largestSent = packetNum;
   auto packetEnd = makeTestingWritePacket(
-      packetNum, 1000, totalSent + 1000, false, packetsSentTime);
+      packetNum, 1000, totalSent + 1000, packetsSentTime);
   cubic.onPacketSent(packetEnd);
   totalSent += 1000;
   ackTime += ackTimeIncrease;
@@ -234,13 +226,13 @@ TEST_F(CubicHystartTest, DelayIncreaseCwndTooSmall) {
   auto realNow = quic::Clock::now();
 
   uint64_t totalSent = 0;
-  auto packet0 = makeTestingWritePacket(0, 1, totalSent + 1, false, realNow);
+  auto packet0 = makeTestingWritePacket(0, 1, totalSent + 1, realNow);
   // Packet 0 is sent:
   conn.lossState.largestSent = 0;
   cubic.onPacketSent(packet0);
   totalSent += 1;
 
-  auto packet1 = makeTestingWritePacket(1, 1, totalSent + 1, false, realNow);
+  auto packet1 = makeTestingWritePacket(1, 1, totalSent + 1, realNow);
   conn.lossState.largestSent = 1;
   cubic.onPacketSent(packet1);
   totalSent += 1;
@@ -255,7 +247,7 @@ TEST_F(CubicHystartTest, DelayIncreaseCwndTooSmall) {
   ackTime += ackTimeIncrease;
   cubic.onPacketAckOrLoss(makeAck(1, 1, ackTime, packet1.time), folly::none);
 
-  auto packet2 = makeTestingWritePacket(2, 10, 10 + totalSent, false, realNow);
+  auto packet2 = makeTestingWritePacket(2, 10, 10 + totalSent, realNow);
   conn.lossState.largestSent = 2;
   cubic.onPacketSent(packet2);
   totalSent += 10;
@@ -269,8 +261,7 @@ TEST_F(CubicHystartTest, DelayIncreaseCwndTooSmall) {
   std::vector<CongestionController::AckEvent> moreAcks;
   for (size_t i = 0; i < kAckSampling - 1; i++) {
     conn.lossState.largestSent = i + 3;
-    auto packet =
+    auto packet = makeTestingWritePacket(i + 3, 1, 1 + totalSent, realNow);
-        makeTestingWritePacket(i + 3, 1, 1 + totalSent, false, realNow);
     ackTime += ackTimeIncrease;
     moreAcks.push_back(makeAck(i + 3, 1, ackTime, packet.time));
     cubic.onPacketSent(packet);
@@ -283,7 +274,7 @@ TEST_F(CubicHystartTest, DelayIncreaseCwndTooSmall) {
   // DelayIncrease:
   conn.lossState.largestSent = kAckSampling;
   auto packetEnd =
-      makeTestingWritePacket(kAckSampling, 1, 1 + totalSent, false, realNow);
+      makeTestingWritePacket(kAckSampling, 1, 1 + totalSent, realNow);
   cubic.onPacketSent(packetEnd);
   totalSent += 1;
   ackTime += ackTimeIncrease;

quic/congestion_control/test/NewRenoTest.cpp

@@ -25,8 +25,8 @@ CongestionController::LossEvent createLossEvent(
   for (auto packetData : lostPackets) {
     RegularQuicWritePacket packet(
         ShortHeader(ProtectionType::KeyPhaseZero, connId, packetData.first));
-    loss.addLostPacket(OutstandingPacket(
+    loss.addLostPacket(
-        std::move(packet), Clock::now(), 10, false, false, 10));
+        OutstandingPacket(std::move(packet), Clock::now(), 10, false, 10));
     loss.lostBytes = packetData.second;
   }
   loss.lostPackets = lostPackets.size();
@@ -45,12 +45,7 @@ CongestionController::AckEvent createAckEvent(
   ack.ackedBytes = ackedSize;
   ack.ackedPackets.push_back(
       makeAckPacketFromOutstandingPacket(OutstandingPacket(
-          std::move(packet),
+          std::move(packet), packetSentTime, ackedSize, false, ackedSize)));
-          packetSentTime,
-          ackedSize,
-          false,
-          false,
-          ackedSize)));
   return ack;
 }
 
@@ -59,8 +54,7 @@ createPacket(PacketNum packetNum, uint32_t size, TimePoint sendTime) {
   auto connId = getTestConnectionId();
   RegularQuicWritePacket packet(
       ShortHeader(ProtectionType::KeyPhaseZero, connId, packetNum));
-  return OutstandingPacket(
+  return OutstandingPacket(std::move(packet), sendTime, size, false, size);
-      std::move(packet), sendTime, size, false, false, size);
 }
 
 TEST_F(NewRenoTest, TestLoss) {
@@ -144,7 +138,6 @@ TEST_F(NewRenoTest, TestSteadyStateAck) {
   NewReno reno(conn);
   EXPECT_TRUE(reno.inSlowStart());
 
-
   conn.lossState.largestSent = 5;
   auto originalWritableBytes = reno.getWritableBytes();
   PacketNum loss1 = 4;

quic/loss/QuicLossFunctions.h

@@ -128,8 +128,6 @@ calculateAlarmDuration(const QuicConnectionStateBase& conn) {
  */
 template <class Timeout, class ClockType = Clock>
 void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) {
-  DCHECK_GE(
-      conn.outstandingPackets.size(), conn.outstandingPureAckPacketsCount);
   /*
    * We might have new data or lost data to send even if we don't have any
    * outstanding packets. When we get a PTO event, it is possible that only
@@ -141,16 +139,6 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) {
   bool hasDataToWrite = hasAckDataToWrite(conn) ||
       (hasNonAckDataToWrite(conn) != WriteDataReason::NO_WRITE);
   auto totalPacketsOutstanding = conn.outstandingPackets.size();
-  if (totalPacketsOutstanding == conn.outstandingPureAckPacketsCount) {
-    VLOG(10) << __func__ << " unset alarm pure ack only"
-             << " outstanding=" << totalPacketsOutstanding
-             << " handshakePackets=" << conn.outstandingHandshakePacketsCount
-             << " pureAckPackets=" << conn.outstandingPureAckPacketsCount << " "
-             << nodeToString(conn.nodeType) << " " << conn;
-    conn.pendingEvents.setLossDetectionAlarm = false;
-    timeout.cancelLossTimeout();
-    return;
-  }
   /*
    * We have this condition to disambiguate the case where we have.
    * (1) All outstanding packets that are clones that are processed and there
@@ -164,14 +152,11 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) {
    * so that we can write this data with the slack packet space for the clones.
    */
   if (!hasDataToWrite && conn.outstandingPacketEvents.empty() &&
-      totalPacketsOutstanding ==
+      totalPacketsOutstanding == conn.outstandingClonedPacketsCount) {
-          (conn.outstandingClonedPacketsCount +
-           conn.outstandingPureAckPacketsCount)) {
     VLOG(10) << __func__ << " unset alarm pure ack or processed packets only"
              << " outstanding=" << totalPacketsOutstanding
              << " handshakePackets=" << conn.outstandingHandshakePacketsCount
-             << " pureAckPackets=" << conn.outstandingPureAckPacketsCount << " "
+             << " " << conn;
-             << conn;
     conn.pendingEvents.setLossDetectionAlarm = false;
     timeout.cancelLossTimeout();
     return;
@@ -180,8 +165,7 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) {
     VLOG_IF(10, !timeout.isLossTimeoutScheduled())
         << __func__ << " alarm not scheduled"
         << " outstanding=" << totalPacketsOutstanding
-        << " handshakePackets=" << conn.outstandingHandshakePacketsCount
+        << " handshakePackets=" << conn.outstandingHandshakePacketsCount << " "
-        << " pureAckPackets=" << conn.outstandingPureAckPacketsCount << " "
         << nodeToString(conn.nodeType) << " " << conn;
     return;
   }
@@ -193,8 +177,7 @@ void setLossDetectionAlarm(QuicConnectionStateBase& conn, Timeout& timeout) {
            << " method=" << conn.lossState.currentAlarmMethod
            << " outstanding=" << totalPacketsOutstanding
            << " handshakePackets=" << conn.outstandingHandshakePacketsCount
-           << " pureAckPackets=" << conn.outstandingPureAckPacketsCount << " "
+           << " " << nodeToString(conn.nodeType) << " " << conn;
-           << nodeToString(conn.nodeType) << " " << conn;
   timeout.scheduleLossTimeout(alarmDuration.first);
   conn.pendingEvents.setLossDetectionAlarm = false;
 }
@@ -241,12 +224,7 @@ folly::Optional<CongestionController::LossEvent> detectLossPackets(
       shouldSetTimer = true;
       break;
     }
-    if (!pkt.pureAck) {
     lossEvent.addLostPacket(pkt);
-    } else {
-      DCHECK_GT(conn.outstandingPureAckPacketsCount, 0);
-      --conn.outstandingPureAckPacketsCount;
-    }
     if (pkt.associatedEvent) {
       DCHECK_GT(conn.outstandingClonedPacketsCount, 0);
       --conn.outstandingClonedPacketsCount;
@@ -265,17 +243,15 @@ folly::Optional<CongestionController::LossEvent> detectLossPackets(
       --conn.outstandingHandshakePacketsCount;
     }
     VLOG(10) << __func__ << " lost packetNum=" << currentPacketNum
-             << " pureAck=" << pkt.pureAck << " handshake=" << pkt.isHandshake
+             << " handshake=" << pkt.isHandshake << " " << conn;
-             << " " << conn;
     iter = conn.outstandingPackets.erase(iter);
   }
 
   auto earliest = getFirstOutstandingPacket(conn, pnSpace);
   for (; earliest != conn.outstandingPackets.end();
        earliest = getNextOutstandingPacket(conn, pnSpace, earliest + 1)) {
-    if (!earliest->pureAck &&
+    if (!earliest->associatedEvent ||
-        (!earliest->associatedEvent ||
+        conn.outstandingPacketEvents.count(*earliest->associatedEvent)) {
-         conn.outstandingPacketEvents.count(*earliest->associatedEvent))) {
       break;
     }
   }
@@ -346,7 +322,6 @@ void onHandshakeAlarm(
       auto currentPacketNumSpace = packet.packet.header.getPacketNumberSpace();
       VLOG(10) << "HandshakeAlarm, removing packetNum=" << currentPacketNum
                << " packetNumSpace=" << currentPacketNumSpace << " " << conn;
-      DCHECK(!packet.pureAck);
       lossEvent.addLostPacket(std::move(packet));
       lossVisitor(conn, packet.packet, false, currentPacketNum);
       DCHECK(conn.outstandingHandshakePacketsCount);
@@ -405,14 +380,12 @@ void onLossDetectionAlarm(
   } else {
     onPTOAlarm(conn);
   }
-  conn.pendingEvents.setLossDetectionAlarm =
+  conn.pendingEvents.setLossDetectionAlarm = !conn.outstandingPackets.empty();
-      (conn.outstandingPackets.size() > conn.outstandingPureAckPacketsCount);
   VLOG(10) << __func__ << " setLossDetectionAlarm="
            << conn.pendingEvents.setLossDetectionAlarm
            << " outstanding=" << conn.outstandingPackets.size()
            << " handshakePackets=" << conn.outstandingHandshakePacketsCount
-           << " pureAckPackets=" << conn.outstandingPureAckPacketsCount << " "
+           << " " << conn;
-           << conn;
 }
 
 /*
@@ -446,16 +419,14 @@ folly::Optional<CongestionController::LossEvent> handleAckForLoss(
       lossVisitor,
       ack.ackTime,
       pnSpace);
-  conn.pendingEvents.setLossDetectionAlarm =
+  conn.pendingEvents.setLossDetectionAlarm = !conn.outstandingPackets.empty();
-      (conn.outstandingPackets.size() > conn.outstandingPureAckPacketsCount);
   VLOG(10) << __func__
            << " largestAckedInPacket=" << ack.largestAckedPacket.value_or(0)
            << " setLossDetectionAlarm="
            << conn.pendingEvents.setLossDetectionAlarm
            << " outstanding=" << conn.outstandingPackets.size()
            << " handshakePackets=" << conn.outstandingHandshakePacketsCount
-           << " pureAckPackets=" << conn.outstandingPureAckPacketsCount << " "
+           << " " << conn;
-           << conn;
   return lossEvent;
 }
 
@@ -475,7 +446,6 @@ void markZeroRttPacketsLost(
         iter->packet.header.getProtectionType() == ProtectionType::ZeroRtt;
     if (isZeroRttPacket) {
       auto& pkt = *iter;
-      DCHECK(!pkt.pureAck);
       DCHECK(!pkt.isHandshake);
       auto currentPacketNum = pkt.packet.header.getPacketSequenceNum();
       bool processed = pkt.associatedEvent &&

quic/loss/test/QuicLossFunctionsTest.cpp

@@ -54,7 +54,6 @@ class QuicLossFunctionsTest : public TestWithParam<PacketNumberSpace> {
   PacketNum sendPacket(
       QuicConnectionStateBase& conn,
       TimePoint time,
-      bool pureAck,
       folly::Optional<PacketEvent> associatedEvent,
       PacketType packetType);
 
@@ -131,7 +130,6 @@ auto testingLossMarkFunc(std::vector<PacketNum>& lostPackets) {
 PacketNum QuicLossFunctionsTest::sendPacket(
     QuicConnectionStateBase& conn,
     TimePoint time,
-    bool pureAck,
     folly::Optional<PacketEvent> associatedEvent,
     PacketType packetType) {
   folly::Optional<PacketHeader> header;
@@ -182,18 +180,14 @@ PacketNum QuicLossFunctionsTest::sendPacket(
     encodedSize += packet.body->computeChainDataLength();
   }
   auto outstandingPacket = OutstandingPacket(
-      packet.packet, time, encodedSize, isHandshake, pureAck, encodedSize);
+      packet.packet, time, encodedSize, isHandshake, encodedSize);
   outstandingPacket.associatedEvent = associatedEvent;
   if (isHandshake) {
     conn.outstandingHandshakePacketsCount++;
     conn.lossState.lastHandshakePacketSentTime = time;
   }
-  if (pureAck) {
-    conn.outstandingPureAckPacketsCount++;
-  } else {
   conn.lossState.lastRetransmittablePacketSentTime = time;
-  }
+  if (conn.congestionController) {
-  if (!pureAck && conn.congestionController) {
     conn.congestionController->onPacketSent(outstandingPacket);
   }
   if (associatedEvent) {
@@ -225,11 +219,11 @@ TEST_F(QuicLossFunctionsTest, AllPacketsProcessed) {
   auto conn = createConn();
   EXPECT_CALL(*transportInfoCb_, onPTO()).Times(0);
   auto pkt1 = conn->ackStates.appDataAckState.nextPacketNum;
-  sendPacket(*conn, Clock::now(), false, pkt1, PacketType::OneRtt);
+  sendPacket(*conn, Clock::now(), pkt1, PacketType::OneRtt);
   auto pkt2 = conn->ackStates.appDataAckState.nextPacketNum;
-  sendPacket(*conn, Clock::now(), false, pkt2, PacketType::OneRtt);
+  sendPacket(*conn, Clock::now(), pkt2, PacketType::OneRtt);
   auto pkt3 = conn->ackStates.appDataAckState.nextPacketNum;
-  sendPacket(*conn, Clock::now(), false, pkt3, PacketType::OneRtt);
+  sendPacket(*conn, Clock::now(), pkt3, PacketType::OneRtt);
   EXPECT_CALL(timeout, cancelLossTimeout()).Times(1);
   setLossDetectionAlarm(*conn, timeout);
   EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm);
@@ -238,7 +232,7 @@ TEST_F(QuicLossFunctionsTest, AllPacketsProcessed) {
 TEST_F(QuicLossFunctionsTest, HasDataToWrite) {
   auto conn = createConn();
   // There needs to be at least one outstanding packet.
-  sendPacket(*conn, Clock::now(), false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, Clock::now(), folly::none, PacketType::OneRtt);
   conn->streamManager->addLoss(1);
   conn->pendingEvents.setLossDetectionAlarm = true;
   EXPECT_CALL(timeout, cancelLossTimeout()).Times(1);
@@ -247,47 +241,6 @@ TEST_F(QuicLossFunctionsTest, HasDataToWrite) {
   EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm);
 }
 
-TEST_F(QuicLossFunctionsTest, NoLossTimeoutIfOnlyPureAcksAreOutstanding) {
-  auto conn = createConn();
-  conn->pendingEvents.setLossDetectionAlarm = true;
-  EXPECT_TRUE(conn->outstandingPackets.empty());
-  EXPECT_EQ(0, conn->outstandingPureAckPacketsCount);
-
-  // Empty outstandingPackets: no timer scheduled
-  EXPECT_CALL(timeout, cancelLossTimeout()).Times(1);
-  setLossDetectionAlarm(*conn, timeout);
-  EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm);
-
-  TimePoint now = Clock::now();
-  // Only pure acks outstanding: no timer scheduled
-  sendPacket(*conn, now, true, folly::none, PacketType::OneRtt);
-  EXPECT_TRUE(conn->pendingEvents.setLossDetectionAlarm);
-  EXPECT_EQ(1, conn->outstandingPackets.size());
-  EXPECT_EQ(1, conn->outstandingPureAckPacketsCount);
-  EXPECT_CALL(timeout, cancelLossTimeout()).Times(1);
-  setLossDetectionAlarm(*conn, timeout);
-  EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm);
-
-  // Non-pure ack outstanding: timer scheduled
-  sendPacket(*conn, now, false, folly::none, PacketType::Handshake);
-  EXPECT_TRUE(conn->pendingEvents.setLossDetectionAlarm);
-  EXPECT_EQ(2, conn->outstandingPackets.size());
-  EXPECT_EQ(1, conn->outstandingPureAckPacketsCount);
-  EXPECT_CALL(timeout, cancelLossTimeout()).Times(1);
-  EXPECT_CALL(timeout, scheduleLossTimeout(_)).Times(1);
-  setLossDetectionAlarm(*conn, timeout);
-  EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm);
-
-  // Non-pure poped from outstanding: no timer scheduled
-  conn->outstandingPackets.pop_back();
-  conn->pendingEvents.setLossDetectionAlarm = true;
-  EXPECT_EQ(1, conn->outstandingPackets.size());
-  EXPECT_EQ(1, conn->outstandingPureAckPacketsCount);
-  EXPECT_CALL(timeout, cancelLossTimeout()).Times(1);
-  setLossDetectionAlarm(*conn, timeout);
-  EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm);
-}
-
 TEST_F(QuicLossFunctionsTest, TestOnLossDetectionAlarm) {
   auto conn = createConn();
   auto mockCongestionController = std::make_unique<MockCongestionController>();
@@ -296,7 +249,7 @@ TEST_F(QuicLossFunctionsTest, TestOnLossDetectionAlarm) {
   EXPECT_CALL(*rawCongestionController, onPacketSent(_))
       .WillRepeatedly(Return());
 
-  sendPacket(*conn, Clock::now(), false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, Clock::now(), folly::none, PacketType::OneRtt);
   MockClock::mockNow = []() { return TimePoint(123ms); };
   std::vector<PacketNum> lostPacket;
   MockClock::mockNow = []() { return TimePoint(23ms); };
@@ -312,7 +265,7 @@ TEST_F(QuicLossFunctionsTest, TestOnLossDetectionAlarm) {
 
   MockClock::mockNow = []() { return TimePoint(3ms); };
   EXPECT_CALL(*transportInfoCb_, onPTO());
-  sendPacket(*conn, TimePoint(), false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, TimePoint(), folly::none, PacketType::OneRtt);
   setLossDetectionAlarm<decltype(timeout), MockClock>(*conn, timeout);
   EXPECT_CALL(*rawCongestionController, onPacketAckOrLoss(_, _)).Times(0);
   onLossDetectionAlarm<decltype(testingLossMarkFunc(lostPacket)), MockClock>(
@@ -320,7 +273,6 @@ TEST_F(QuicLossFunctionsTest, TestOnLossDetectionAlarm) {
   EXPECT_EQ(conn->lossState.ptoCount, 2);
   // PTO doesn't take anything out of outstandingPackets
   EXPECT_FALSE(conn->outstandingPackets.empty());
-  EXPECT_EQ(0, conn->outstandingPureAckPacketsCount);
   EXPECT_TRUE(conn->pendingEvents.setLossDetectionAlarm);
   // PTO shouldn't mark loss
   EXPECT_TRUE(lostPacket.empty());
@@ -336,7 +288,7 @@ TEST_F(QuicLossFunctionsTest, TestOnPTOSkipProcessed) {
   // By adding an associatedEvent that doesn't exist in the
   // outstandingPacketEvents, they are all processed and will skip lossVisitor
   for (auto i = 0; i < 10; i++) {
-    sendPacket(*conn, TimePoint(), false, i, PacketType::OneRtt);
+    sendPacket(*conn, TimePoint(), i, PacketType::OneRtt);
   }
   EXPECT_EQ(10, conn->outstandingPackets.size());
   std::vector<PacketNum> lostPackets;
@@ -542,11 +494,9 @@ TEST_F(QuicLossFunctionsTest, TestReorderingThreshold) {
         lostPacket.push_back(packetNum);
       };
   for (int i = 0; i < 6; ++i) {
-    sendPacket(
+    sendPacket(*conn, Clock::now(), folly::none, PacketType::Handshake);
-        *conn, Clock::now(), !(i % 2), folly::none, PacketType::Handshake);
   }
   EXPECT_EQ(6, conn->outstandingHandshakePacketsCount);
-  EXPECT_EQ(3, conn->outstandingPureAckPacketsCount);
   // Assume some packets are already acked
   for (auto iter =
            getFirstOutstandingPacket(*conn, PacketNumberSpace::Handshake) + 2;
@@ -556,9 +506,6 @@ TEST_F(QuicLossFunctionsTest, TestReorderingThreshold) {
     if (iter->isHandshake) {
       conn->outstandingHandshakePacketsCount--;
     }
-    if (iter->pureAck) {
-      conn->outstandingPureAckPacketsCount--;
-    }
   }
   auto firstHandshakeOpIter =
       getFirstOutstandingPacket(*conn, PacketNumberSpace::Handshake);
@@ -580,7 +527,6 @@ TEST_F(QuicLossFunctionsTest, TestReorderingThreshold) {
 
   // Packet 6 is the only thing remaining inflight, it is a handshake pkt
   EXPECT_EQ(1, conn->outstandingHandshakePacketsCount);
-  EXPECT_EQ(0, conn->outstandingPureAckPacketsCount);
 
   // Packet 6 should remain in packet as the delta is less than threshold
   EXPECT_EQ(conn->outstandingPackets.size(), 1);
@@ -605,7 +551,7 @@ TEST_F(QuicLossFunctionsTest, TestHandleAckForLoss) {
   RegularQuicWritePacket outstandingRegularPacket(std::move(longHeader));
   auto now = Clock::now();
   conn->outstandingPackets.emplace_back(
-      OutstandingPacket(outstandingRegularPacket, now, 0, false, false, 0));
+      OutstandingPacket(outstandingRegularPacket, now, 0, false, 0));
 
   bool testLossMarkFuncCalled = false;
   auto testLossMarkFunc = [&](auto& /* conn */, auto&, bool, PacketNum) {
@@ -620,7 +566,6 @@ TEST_F(QuicLossFunctionsTest, TestHandleAckForLoss) {
 
   EXPECT_EQ(0, conn->lossState.ptoCount);
   EXPECT_TRUE(conn->outstandingPackets.empty());
-  EXPECT_EQ(0, conn->outstandingPureAckPacketsCount);
   EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm);
   EXPECT_TRUE(testLossMarkFuncCalled);
 
@@ -642,7 +587,7 @@ TEST_F(QuicLossFunctionsTest, TestHandleAckedPacket) {
   conn->lossState.handshakeAlarmCount = 5;
   conn->lossState.reorderingThreshold = 10;
 
-  sendPacket(*conn, TimePoint(), false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, TimePoint(), folly::none, PacketType::OneRtt);
 
   ReadAckFrame ackFrame;
   ackFrame.largestAcked = conn->lossState.largestSent;
@@ -668,7 +613,6 @@ TEST_F(QuicLossFunctionsTest, TestHandleAckedPacket) {
   EXPECT_EQ(0, conn->lossState.ptoCount);
   EXPECT_EQ(0, conn->lossState.handshakeAlarmCount);
   EXPECT_TRUE(conn->outstandingPackets.empty());
-  EXPECT_EQ(0, conn->outstandingPureAckPacketsCount);
   EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm);
   EXPECT_FALSE(testLossMarkFuncCalled);
   ASSERT_TRUE(conn->outstandingPackets.empty());
@@ -758,8 +702,8 @@ TEST_F(QuicLossFunctionsTest, ReorderingThresholdChecksSamePacketNumberSpace) {
   };
   PacketNum latestSent = 0;
   for (size_t i = 0; i < conn->lossState.reorderingThreshold + 1; i++) {
-    latestSent = sendPacket(
+    latestSent =
-        *conn, Clock::now(), false, folly::none, PacketType::Handshake);
+        sendPacket(*conn, Clock::now(), folly::none, PacketType::Handshake);
   }
 
   detectLossPackets(
@@ -818,7 +762,7 @@ TEST_F(QuicLossFunctionsTest, TestTimeReordering) {
   PacketNum largestSent = 0;
   for (int i = 0; i < 7; ++i) {
     largestSent = sendPacket(
-        *conn, TimePoint(i * 100ms), false, folly::none, PacketType::OneRtt);
+        *conn, TimePoint(i * 100ms), folly::none, PacketType::OneRtt);
   }
   // Some packets are already acked
   conn->lossState.srtt = 400ms;
@@ -855,9 +799,9 @@ TEST_F(QuicLossFunctionsTest, LossTimePreemptsCryptoTimer) {
   auto expectedDelayUntilLost = 900000us / 8;
   auto sendTime = Clock::now();
   // Send two:
-  sendPacket(*conn, sendTime, false, folly::none, PacketType::Handshake);
+  sendPacket(*conn, sendTime, folly::none, PacketType::Handshake);
-  PacketNum second = sendPacket(
+  PacketNum second =
-      *conn, sendTime + 1ms, false, folly::none, PacketType::Handshake);
+      sendPacket(*conn, sendTime + 1ms, folly::none, PacketType::Handshake);
   auto lossTime = sendTime + 50ms;
   detectLossPackets<decltype(testingLossMarkFunc(lostPackets))>(
       *conn,
@@ -904,7 +848,7 @@ TEST_F(QuicLossFunctionsTest, PTONoLongerMarksPacketsToBeRetransmitted) {
   MockClock::mockNow = [&]() { return startTime; };
   std::vector<PacketNum> lostPackets;
   for (auto i = 0; i < kPacketToSendForPTO + 10; i++) {
-    sendPacket(*conn, startTime, false, folly::none, PacketType::OneRtt);
+    sendPacket(*conn, startTime, folly::none, PacketType::OneRtt);
     setLossDetectionAlarm<decltype(timeout), MockClock>(*conn, timeout);
     startTime += 1ms;
   }
@@ -936,7 +880,6 @@ TEST_F(
     auto sentPacketNum = sendPacket(
         *conn,
         TimePoint(100ms),
-        false,
         folly::none,
         (i % 2 ? PacketType::OneRtt : PacketType::Handshake));
     expectedLargestLostNum = std::max(
@@ -980,8 +923,7 @@ TEST_F(QuicLossFunctionsTest, HandshakeAlarmWithOneRttCipher) {
   conn->oneRttWriteCipher = createNoOpAead();
   conn->lossState.currentAlarmMethod = LossState::AlarmMethod::Handshake;
   std::vector<PacketNum> lostPackets;
-  sendPacket(
+  sendPacket(*conn, TimePoint(100ms), folly::none, PacketType::Handshake);
-      *conn, TimePoint(100ms), false, folly::none, PacketType::Handshake);
   onLossDetectionAlarm<decltype(testingLossMarkFunc(lostPackets)), Clock>(
       *conn, testingLossMarkFunc(lostPackets));
 
@@ -1001,45 +943,6 @@ TEST_F(QuicLossFunctionsTest, HandshakeAlarmWithOneRttCipher) {
   EXPECT_EQ(event->type, kHandshakeAlarm);
 }
 
-TEST_F(QuicLossFunctionsTest, PureAckSkipsCongestionControl) {
-  std::vector<PacketNum> lostPacket;
-  auto conn = createConn();
-  auto mockCongestionController = std::make_unique<MockCongestionController>();
-  auto rawCongestionController = mockCongestionController.get();
-  conn->congestionController = std::move(mockCongestionController);
-  EXPECT_CALL(*rawCongestionController, onPacketSent(_))
-      .WillRepeatedly(Return());
-
-  for (int i = 0; i < 5; ++i) {
-    bool pureAck = (i == 3);
-    sendPacket(*conn, TimePoint(), pureAck, folly::none, PacketType::Handshake);
-  }
-
-  // Verify that bytes for pure ack pkt won't be counted in lost bytes
-  auto sumNoPureAckBytes = std::accumulate(
-      conn->outstandingPackets.begin(),
-      conn->outstandingPackets.end(),
-      0,
-      [](uint64_t bytes, const OutstandingPacket& p) {
-        if (p.pureAck) {
-          return bytes;
-        }
-        return bytes + p.encodedSize;
-      });
-
-  // Ack for packet 9 arrives
-  auto lossEvent = detectLossPackets<decltype(testingLossMarkFunc(lostPacket))>(
-      *conn,
-      9,
-      testingLossMarkFunc(lostPacket),
-      TimePoint(80ms),
-      PacketNumberSpace::Handshake);
-  EXPECT_EQ(5, lossEvent->largestLostPacketNum.value());
-  EXPECT_EQ(sumNoPureAckBytes, lossEvent->lostBytes);
-  EXPECT_EQ(TimePoint(80ms), lossEvent->lossTime);
-  EXPECT_EQ(conn->outstandingHandshakePacketsCount, 0);
-}
-
 TEST_F(QuicLossFunctionsTest, EmptyOutstandingNoTimeout) {
   auto conn = createConn();
   EXPECT_CALL(timeout, cancelLossTimeout()).Times(1);
@@ -1053,8 +956,7 @@ TEST_F(QuicLossFunctionsTest, AlarmDurationHandshakeOutstanding) {
   std::chrono::milliseconds packetSentDelay = 10ms;
   auto thisMoment = lastPacketSentTime + packetSentDelay;
   MockClock::mockNow = [=]() { return thisMoment; };
-  sendPacket(
+  sendPacket(*conn, lastPacketSentTime, folly::none, PacketType::Handshake);
-      *conn, lastPacketSentTime, false, folly::none, PacketType::Handshake);
 
   MockClock::mockNow = [=]() { return thisMoment; };
   auto duration = calculateAlarmDuration<MockClock>(*conn);
@@ -1088,7 +990,7 @@ TEST_F(QuicLossFunctionsTest, AlarmDurationHasLossTime) {
   conn->lossState.srtt = 200ms;
   conn->lossState.lrtt = 150ms;
 
-  sendPacket(*conn, lastPacketSentTime, false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, lastPacketSentTime, folly::none, PacketType::OneRtt);
   auto duration = calculateAlarmDuration<MockClock>(*conn);
   EXPECT_EQ(100ms, duration.first);
   EXPECT_EQ(
@@ -1106,7 +1008,7 @@ TEST_F(QuicLossFunctionsTest, AlarmDurationLossTimeIsZero) {
   conn->lossState.srtt = 200ms;
   conn->lossState.lrtt = 150ms;
 
-  sendPacket(*conn, lastPacketSentTime, false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, lastPacketSentTime, folly::none, PacketType::OneRtt);
   auto duration = calculateAlarmDuration<MockClock>(*conn);
   EXPECT_EQ(0ms, duration.first);
   EXPECT_EQ(
@@ -1120,7 +1022,7 @@ TEST_F(QuicLossFunctionsTest, AlarmDurationNonHandshakeOutstanding) {
   conn->lossState.maxAckDelay = 25ms;
   TimePoint lastPacketSentTime = Clock::now();
   MockClock::mockNow = [=]() { return lastPacketSentTime; };
-  sendPacket(*conn, lastPacketSentTime, false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, lastPacketSentTime, folly::none, PacketType::OneRtt);
   auto duration = calculateAlarmDuration<MockClock>(*conn);
   EXPECT_EQ(duration.second, LossState::AlarmMethod::PTO);
   setLossDetectionAlarm<decltype(timeout), MockClock>(*conn, timeout);
@@ -1149,8 +1051,7 @@ TEST_F(QuicLossFunctionsTest, NoSkipLossVisitor) {
   // Send 5 packets, so when we ack the last one, we mark the first one loss
   PacketNum lastSent;
   for (size_t i = 0; i < 5; i++) {
-    lastSent =
+    lastSent = sendPacket(*conn, Clock::now(), folly::none, PacketType::OneRtt);
-        sendPacket(*conn, Clock::now(), false, folly::none, PacketType::OneRtt);
   }
   detectLossPackets(
       *conn,
@@ -1179,7 +1080,7 @@ TEST_F(QuicLossFunctionsTest, SkipLossVisitor) {
   PacketNum lastSent;
   for (size_t i = 0; i < 5; i++) {
     lastSent = conn->ackStates.appDataAckState.nextPacketNum;
-    sendPacket(*conn, Clock::now(), false, lastSent, PacketType::OneRtt);
+    sendPacket(*conn, Clock::now(), lastSent, PacketType::OneRtt);
   }
   detectLossPackets(
       *conn,
@@ -1209,8 +1110,7 @@ TEST_F(QuicLossFunctionsTest, NoDoubleProcess) {
   PacketNum lastSent;
   PacketEvent event = 0;
   for (size_t i = 0; i < 6; i++) {
-    lastSent =
+    lastSent = sendPacket(*conn, Clock::now(), event, PacketType::OneRtt);
-        sendPacket(*conn, Clock::now(), false, event, PacketType::OneRtt);
   }
   EXPECT_EQ(6, conn->outstandingPackets.size());
   // Add the PacketEvent to the outstandingPacketEvents set
@@ -1231,12 +1131,12 @@ TEST_F(QuicLossFunctionsTest, NoDoubleProcess) {
 TEST_F(QuicLossFunctionsTest, DetectPacketLossClonedPacketsCounter) {
   auto conn = createConn();
   auto packet1 = conn->ackStates.appDataAckState.nextPacketNum;
-  sendPacket(*conn, Clock::now(), false, packet1, PacketType::OneRtt);
+  sendPacket(*conn, Clock::now(), packet1, PacketType::OneRtt);
-  sendPacket(*conn, Clock::now(), false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, Clock::now(), folly::none, PacketType::OneRtt);
-  sendPacket(*conn, Clock::now(), false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, Clock::now(), folly::none, PacketType::OneRtt);
-  sendPacket(*conn, Clock::now(), false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, Clock::now(), folly::none, PacketType::OneRtt);
   auto ackedPacket =
-      sendPacket(*conn, Clock::now(), false, folly::none, PacketType::OneRtt);
+      sendPacket(*conn, Clock::now(), folly::none, PacketType::OneRtt);
   auto noopLossMarker = [](auto&, auto&, bool, PacketNum) {};
   detectLossPackets<decltype(noopLossMarker)>(
       *conn,
@@ -1357,7 +1257,7 @@ TEST_F(QuicLossFunctionsTest, TotalLossCount) {
   PacketNum largestSent = 0;
   for (int i = 0; i < 10; i++) {
     largestSent =
-        sendPacket(*conn, Clock::now(), false, folly::none, PacketType::OneRtt);
+        sendPacket(*conn, Clock::now(), folly::none, PacketType::OneRtt);
   }
   EXPECT_EQ(10, conn->outstandingPackets.size());
   uint32_t lostPackets = 0;
@@ -1390,8 +1290,8 @@ TEST_F(QuicLossFunctionsTest, TestZeroRttRejected) {
   // By adding an associatedEvent that doesn't exist in the
   // outstandingPacketEvents, they are all processed and will skip lossVisitor
   for (auto i = 0; i < 2; i++) {
-    sendPacket(*conn, TimePoint(), false, folly::none, PacketType::OneRtt);
+    sendPacket(*conn, TimePoint(), folly::none, PacketType::OneRtt);
-    sendPacket(*conn, TimePoint(), false, folly::none, PacketType::ZeroRtt);
+    sendPacket(*conn, TimePoint(), folly::none, PacketType::ZeroRtt);
   }
   EXPECT_FALSE(conn->outstandingPackets.empty());
   EXPECT_EQ(4, conn->outstandingPackets.size());
@@ -1429,14 +1329,14 @@ TEST_F(QuicLossFunctionsTest, TestZeroRttRejectedWithClones) {
   folly::Optional<PacketNum> lastPacket;
   for (auto i = 0; i < 2; i++) {
     lastPacket =
-        sendPacket(*conn, TimePoint(), false, lastPacket, PacketType::ZeroRtt);
+        sendPacket(*conn, TimePoint(), lastPacket, PacketType::ZeroRtt);
     zeroRttPackets.emplace(*lastPacket);
   }
   zeroRttPackets.emplace(
-      sendPacket(*conn, TimePoint(), false, folly::none, PacketType::ZeroRtt));
+      sendPacket(*conn, TimePoint(), folly::none, PacketType::ZeroRtt));
   for (auto zeroRttPacketNum : zeroRttPackets) {
-    lastPacket = sendPacket(
+    lastPacket =
-        *conn, TimePoint(), false, zeroRttPacketNum, PacketType::OneRtt);
+        sendPacket(*conn, TimePoint(), zeroRttPacketNum, PacketType::OneRtt);
   }
 
   EXPECT_EQ(6, conn->outstandingPackets.size());
@@ -1477,12 +1377,11 @@ TEST_F(QuicLossFunctionsTest, TimeThreshold) {
   auto conn = createConn();
   conn->lossState.srtt = 10ms;
   auto referenceTime = Clock::now();
-  auto packet1 = sendPacket(
+  auto packet1 =
-      *conn, referenceTime - 10ms, false, folly::none, PacketType::OneRtt);
+      sendPacket(*conn, referenceTime - 10ms, folly::none, PacketType::OneRtt);
   auto packet2 = sendPacket(
       *conn,
       referenceTime + conn->lossState.srtt / 2,
-      false,
       folly::none,
       PacketType::OneRtt);
   auto lossVisitor = [&](const auto& /*conn*/,
@@ -1503,7 +1402,7 @@ TEST_P(QuicLossFunctionsTest, CappedShiftNoCrash) {
   auto conn = createConn();
   conn->lossState.handshakeAlarmCount =
       std::numeric_limits<decltype(conn->lossState.handshakeAlarmCount)>::max();
-  sendPacket(*conn, Clock::now(), false, folly::none, PacketType::Handshake);
+  sendPacket(*conn, Clock::now(), folly::none, PacketType::Handshake);
   ASSERT_GT(conn->outstandingHandshakePacketsCount, 0);
   calculateAlarmDuration(*conn);
 
@@ -1512,7 +1411,7 @@ TEST_P(QuicLossFunctionsTest, CappedShiftNoCrash) {
   conn->outstandingPackets.clear();
   conn->lossState.ptoCount =
       std::numeric_limits<decltype(conn->lossState.ptoCount)>::max();
-  sendPacket(*conn, Clock::now(), false, folly::none, PacketType::OneRtt);
+  sendPacket(*conn, Clock::now(), folly::none, PacketType::OneRtt);
   calculateAlarmDuration(*conn);
 }
 

quic/server/test/QuicServerTransportTest.cpp

@@ -829,10 +829,25 @@ TEST_F(QuicServerTransportTest, IdleTimerNotResetWhenDataOutstanding) {
   server->getNonConstConn().receivedNewPacketBeforeWrite = false;
   StreamId streamId = server->createBidirectionalStream().value();
 
-  auto expected = IOBuf::copyBuffer("hello");
   server->idleTimeout().cancelTimeout();
   ASSERT_FALSE(server->idleTimeout().isScheduled());
-  server->writeChain(streamId, expected->clone(), false, false);
+  server->writeChain(
+      streamId,
+      IOBuf::copyBuffer("And if the darkness is to keep us apart"),
+      false,
+      false);
+  loopForWrites();
+  // It was the first packet
+  ASSERT_TRUE(server->idleTimeout().isScheduled());
+
+  // cancel it and write something else. This time idle timer shouldn't set.
+  server->idleTimeout().cancelTimeout();
+  ASSERT_FALSE(server->idleTimeout().isScheduled());
+  server->writeChain(
+      streamId,
+      IOBuf::copyBuffer("And if the daylight feels like it's a long way off"),
+      false,
+      false);
   loopForWrites();
   ASSERT_FALSE(server->idleTimeout().isScheduled());
 }
@@ -1140,7 +1155,6 @@ TEST_F(QuicServerTransportTest, TestOpenAckStreamFrame) {
 
   // Remove any packets that might have been queued.
   server->getNonConstConn().outstandingPackets.clear();
-  server->getNonConstConn().outstandingPureAckPacketsCount = 0;
   server->getNonConstConn().outstandingHandshakePacketsCount = 0;
   server->writeChain(streamId, data->clone(), false, false);
   loopForWrites();
@@ -1725,7 +1739,6 @@ TEST_F(QuicServerTransportTest, TestCloneStopSending) {
   server->getNonConstConn().streamManager->getStream(streamId);
   // knock every handshake outstanding packets out
   server->getNonConstConn().outstandingHandshakePacketsCount = 0;
-  server->getNonConstConn().outstandingPureAckPacketsCount = 0;
   server->getNonConstConn().outstandingPackets.clear();
   server->getNonConstConn().lossState.initialLossTime.clear();
   server->getNonConstConn().lossState.handshakeLossTime.clear();

quic/state/AckHandlers.cpp

@@ -39,8 +39,6 @@ void processAckFrame(
     const AckVisitor& ackVisitor,
     const LossVisitor& lossVisitor,
     const TimePoint& ackReceiveTime) {
-  DCHECK_GE(
-      conn.outstandingPackets.size(), conn.outstandingPureAckPacketsCount);
   // TODO: send error if we get an ack for a packet we've not sent t18721184
   CongestionController::AckEvent ack;
   ack.ackTime = ackReceiveTime;
@@ -51,7 +49,6 @@ void processAckFrame(
   ack.ackedPackets.reserve(kRxPacketsPendingBeforeAckThresh);
   auto currentPacketIt = getLastOutstandingPacket(conn, pnSpace);
   uint64_t handshakePacketAcked = 0;
-  uint64_t pureAckPacketsAcked = 0;
   uint64_t clonedPacketsAcked = 0;
   folly::Optional<decltype(conn.lossState.lastAckedPacketSentTime)>
       lastAckedPacketSentTime;
@@ -109,16 +106,11 @@ void processAckFrame(
       }
       VLOG(10) << __func__ << " acked packetNum=" << currentPacketNum
                << " space=" << currentPacketNumberSpace
-               << " handshake=" << (int)rPacketIt->isHandshake
+               << " handshake=" << (int)rPacketIt->isHandshake << " " << conn;
-               << " pureAck=" << (int)rPacketIt->pureAck << " " << conn;
       if (rPacketIt->isHandshake) {
         ++handshakePacketAcked;
       }
-      if (!rPacketIt->pureAck) {
       ack.ackedBytes += rPacketIt->encodedSize;
-      } else {
-        ++pureAckPacketsAcked;
-      }
       if (rPacketIt->associatedEvent) {
         ++clonedPacketsAcked;
       }
@@ -127,7 +119,7 @@ void processAckFrame(
           ackReceiveTime > rPacketIt->time ? ackReceiveTime : Clock::now();
       auto rttSample = std::chrono::duration_cast<std::chrono::microseconds>(
           ackReceiveTimeOrNow - rPacketIt->time);
-      if (currentPacketNum == frame.largestAcked && !rPacketIt->pureAck) {
+      if (currentPacketNum == frame.largestAcked) {
         updateRtt(conn, rttSample, frame.ackDelay);
       }
       if (conn.qLogger) {
@@ -190,12 +182,9 @@ void processAckFrame(
   }
   DCHECK_GE(conn.outstandingHandshakePacketsCount, handshakePacketAcked);
   conn.outstandingHandshakePacketsCount -= handshakePacketAcked;
-  DCHECK_GE(conn.outstandingPureAckPacketsCount, pureAckPacketsAcked);
-  conn.outstandingPureAckPacketsCount -= pureAckPacketsAcked;
   DCHECK_GE(conn.outstandingClonedPacketsCount, clonedPacketsAcked);
   conn.outstandingClonedPacketsCount -= clonedPacketsAcked;
   auto updatedOustandingPacketsCount = conn.outstandingPackets.size();
-  DCHECK_GE(updatedOustandingPacketsCount, conn.outstandingPureAckPacketsCount);
   DCHECK_GE(
       updatedOustandingPacketsCount, conn.outstandingHandshakePacketsCount);
   DCHECK_GE(updatedOustandingPacketsCount, conn.outstandingClonedPacketsCount);

quic/state/StateData.h

@@ -93,6 +93,7 @@ struct NetworkDataSingle {
  */
 using PacketEvent = PacketNum;
 
+// Data structure to represent outstanding retransmittable packets
 struct OutstandingPacket {
   // Structure representing the frames that are outstanding including the header
   // that was sent.
@@ -103,8 +104,6 @@ struct OutstandingPacket {
   uint32_t encodedSize;
   // Whether this packet has any data from stream 0
   bool isHandshake;
-  // Whether this packet is pure ack
-  bool pureAck;
   // Total sent bytes on this connection including this packet itself when this
   // packet is sent.
   uint64_t totalBytesSent;
@@ -146,13 +145,11 @@ struct OutstandingPacket {
       TimePoint timeIn,
       uint32_t encodedSizeIn,
       bool isHandshakeIn,
-      bool pureAckIn,
       uint64_t totalBytesSentIn)
       : packet(std::move(packetIn)),
         time(std::move(timeIn)),
         encodedSize(encodedSizeIn),
         isHandshake(isHandshakeIn),
-        pureAck(pureAckIn),
         totalBytesSent(totalBytesSentIn) {}
 };
 
@@ -521,9 +518,6 @@ struct QuicConnectionStateBase {
   // TODO: Enforce only AppTraffic packets to be clonable
   folly::F14FastSet<PacketEvent> outstandingPacketEvents;
 
-  // Number of pure ack packets outstanding.
-  uint64_t outstandingPureAckPacketsCount{0};
-
   // Number of handshake packets outstanding.
   uint64_t outstandingHandshakePacketsCount{0};
 

quic/state/test/AckHandlersTest.cpp

@@ -27,8 +27,8 @@ namespace test {
 class AckHandlersTest : public TestWithParam<PacketNumberSpace> {};
 
 auto testLossHandler(std::vector<PacketNum>& lostPackets) -> decltype(auto) {
-  return [&lostPackets](
+  return
-             QuicConnectionStateBase&, auto& packet, bool, PacketNum) {
+      [&lostPackets](QuicConnectionStateBase&, auto& packet, bool, PacketNum) {
         auto packetNum = packet.header.getPacketSequenceNum();
         lostPackets.push_back(packetNum);
       };
@@ -51,7 +51,7 @@ TEST_P(AckHandlersTest, TestAckMultipleSequentialBlocks) {
     WriteStreamFrame frame(currentStreamId++, 0, 0, true);
     regularPacket.frames.emplace_back(std::move(frame));
     conn.outstandingPackets.emplace_back(OutstandingPacket(
-        std::move(regularPacket), sentTime, 1, false, false, packetNum));
+        std::move(regularPacket), sentTime, 1, false, packetNum));
   }
   ReadAckFrame ackFrame;
   ackFrame.largestAcked = 101;
@@ -122,7 +122,7 @@ TEST_P(AckHandlersTest, TestAckBlocksWithGaps) {
     WriteStreamFrame frame(currentStreamId++, 0, 0, true);
     regularPacket.frames.emplace_back(std::move(frame));
     conn.outstandingPackets.emplace_back(OutstandingPacket(
-        std::move(regularPacket), Clock::now(), 1, false, false, packetNum));
+        std::move(regularPacket), Clock::now(), 1, false, packetNum));
   }
 
   ReadAckFrame ackFrame;
@@ -223,7 +223,7 @@ TEST_P(AckHandlersTest, TestNonSequentialPacketNumbers) {
     WriteStreamFrame frame(current++, 0, 0, true);
     regularPacket.frames.emplace_back(std::move(frame));
     conn.outstandingPackets.emplace_back(OutstandingPacket(
-        std::move(regularPacket), Clock::now(), 1, false, false, packetNum));
+        std::move(regularPacket), Clock::now(), 1, false, packetNum));
   }
 
   for (PacketNum packetNum = 20; packetNum < 40; packetNum += 3) {
@@ -232,7 +232,7 @@ TEST_P(AckHandlersTest, TestNonSequentialPacketNumbers) {
     current += 3;
     regularPacket.frames.emplace_back(std::move(frame));
     conn.outstandingPackets.emplace_back(OutstandingPacket(
-        std::move(regularPacket), Clock::now(), 1, false, false, packetNum));
+        std::move(regularPacket), Clock::now(), 1, false, packetNum));
   }
 
   ReadAckFrame ackFrame;
@@ -311,8 +311,8 @@ TEST_P(AckHandlersTest, AckVisitorForAckTest) {
   conn.ackStates.appDataAckState.acks.insert(900, 1000);
   conn.ackStates.appDataAckState.acks.insert(500, 700);
   firstPacket.frames.emplace_back(std::move(firstAckFrame));
-  conn.outstandingPackets.emplace_back(OutstandingPacket(
+  conn.outstandingPackets.emplace_back(
-      std::move(firstPacket), Clock::now(), 0, false, false, 0));
+      OutstandingPacket(std::move(firstPacket), Clock::now(), 0, false, 0));
 
   auto secondPacket = createNewPacket(101 /* packetNum */, GetParam());
   WriteAckFrame secondAckFrame;
@@ -321,8 +321,8 @@ TEST_P(AckHandlersTest, AckVisitorForAckTest) {
   conn.ackStates.appDataAckState.acks.insert(1100, 2000);
   conn.ackStates.appDataAckState.acks.insert(1002, 1090);
   secondPacket.frames.emplace_back(std::move(secondAckFrame));
-  conn.outstandingPackets.emplace_back(OutstandingPacket(
+  conn.outstandingPackets.emplace_back(
-      std::move(secondPacket), Clock::now(), 0, false, false, 0));
+      OutstandingPacket(std::move(secondPacket), Clock::now(), 0, false, 0));
 
   ReadAckFrame firstReceivedAck;
   firstReceivedAck.largestAcked = 100;
@@ -385,8 +385,8 @@ TEST_P(AckHandlersTest, NoNewAckedPacket) {
   conn.lossState.ptoCount = 1;
   PacketNum packetAfterRtoNum = 10;
   auto packetAfterRto = createNewPacket(packetAfterRtoNum, GetParam());
-  conn.outstandingPackets.emplace_back(OutstandingPacket(
+  conn.outstandingPackets.emplace_back(
-      std::move(packetAfterRto), Clock::now(), 0, false, false, 0));
+      OutstandingPacket(std::move(packetAfterRto), Clock::now(), 0, false, 0));
 
   ReadAckFrame ackFrame;
   ackFrame.largestAcked = 5;
@@ -430,12 +430,12 @@ TEST_P(AckHandlersTest, AckPacketNumDoesNotExist) {
   PacketNum packetNum1 = 9;
   auto regularPacket1 = createNewPacket(packetNum1, GetParam());
   conn.outstandingPackets.emplace_back(
-      std::move(regularPacket1), Clock::now(), 0, false, false, 0);
+      std::move(regularPacket1), Clock::now(), 0, false, 0);
 
   PacketNum packetNum2 = 10;
   auto regularPacket2 = createNewPacket(packetNum2, GetParam());
   conn.outstandingPackets.emplace_back(
-      std::move(regularPacket2), Clock::now(), 0, false, false, 0);
+      std::move(regularPacket2), Clock::now(), 0, false, 0);
 
   // Ack a packet one higher than the packet so that we don't trigger reordering
   // threshold.
@@ -466,7 +466,6 @@ TEST_P(AckHandlersTest, TestHandshakeCounterUpdate) {
         Clock::now(),
         0,
         packetNum % 2,
-        false,
         packetNum / 2);
     conn.outstandingHandshakePacketsCount += packetNum % 2;
   }
@@ -507,76 +506,6 @@ TEST_P(AckHandlersTest, PurgeAcks) {
       expectedTime, *conn.ackStates.initialAckState.largestRecvdPacketTime);
 }
 
-TEST_P(AckHandlersTest, PureAckBytesCountedTowardsTotalBytesAcked) {
-  QuicServerConnectionState conn;
-  ASSERT_EQ(0, conn.lossState.totalBytesAcked);
-  auto regularPacket = createNewPacket(10, GetParam());
-  WriteAckFrame ack;
-  ack.ackBlocks.insert(2, 5);
-  conn.outstandingPackets.emplace_back(OutstandingPacket(
-      std::move(regularPacket), Clock::now(), 2, false, true, 2));
-  conn.outstandingPureAckPacketsCount++;
-
-  ReadAckFrame ackFrame;
-  ackFrame.largestAcked = 12;
-  ackFrame.ackBlocks.emplace_back(5, 12);
-
-  std::vector<PacketNum> lostPackets;
-  processAckFrame(
-      conn,
-      GetParam(),
-      ackFrame,
-      [&](const auto&, const auto&, const auto&) {},
-      testLossHandler(lostPackets),
-      Clock::now());
-  // Packet should be removed from outstandingPackets without triggering
-  // onPacketAckOrLoss
-  EXPECT_TRUE(conn.outstandingPackets.empty());
-  EXPECT_GT(conn.lossState.totalBytesAcked, 0);
-}
-
-TEST_P(AckHandlersTest, PureAckBytesSkipsCongestionControl) {
-  QuicServerConnectionState conn;
-  auto mockController = std::make_unique<MockCongestionController>();
-  auto rawController = mockController.get();
-  conn.congestionController = std::move(mockController);
-
-  auto regularPacket = createNewPacket(10, GetParam());
-  WriteAckFrame ack;
-  ack.ackBlocks.insert(2, 5);
-  conn.outstandingPackets.emplace_back(OutstandingPacket(
-      std::move(regularPacket), Clock::now(), 2, false, true, 2));
-  conn.outstandingPureAckPacketsCount++;
-
-  ReadAckFrame ackFrame;
-  ackFrame.largestAcked = 12;
-  ackFrame.ackBlocks.emplace_back(5, 12);
-
-  std::vector<PacketNum> lostPackets;
-  // onPacketAckOrLoss will be called, but the ackedBytes is 0. We only need the
-  // largestAckedPacket
-  EXPECT_CALL(*rawController, onPacketAckOrLoss(_, _))
-      .Times(1)
-      .WillOnce(Invoke([&](auto ackEvent, auto) {
-        EXPECT_EQ(0, ackEvent->ackedBytes);
-        EXPECT_EQ(10, ackEvent->largestAckedPacket.value());
-        EXPECT_FALSE(ackEvent->ackedPackets.empty());
-        EXPECT_EQ(1, ackEvent->ackedPackets.size());
-        EXPECT_EQ(2, ackEvent->ackedPackets.front().encodedSize);
-        EXPECT_EQ(2, ackEvent->ackedPackets.front().totalBytesSentThen);
-      }));
-  processAckFrame(
-      conn,
-      GetParam(),
-      ackFrame,
-      [&](const auto&, const auto&, const auto&) {},
-      testLossHandler(lostPackets),
-      Clock::now());
-  // Packet should be removed from outstandingPackets without triggering
-  // onPacketAckOrLoss
-  EXPECT_TRUE(conn.outstandingPackets.empty());
-}
-
 TEST_P(AckHandlersTest, NoSkipAckVisitor) {
   QuicServerConnectionState conn;
   auto mockCongestionController = std::make_unique<MockCongestionController>();
@@ -595,8 +524,8 @@ TEST_P(AckHandlersTest, NoSkipAckVisitor) {
   // We need to at least have one frame to trigger ackVisitor
   WriteStreamFrame frame(0, 0, 0, true);
   regularPacket.frames.emplace_back(std::move(frame));
-  conn.outstandingPackets.emplace_back(OutstandingPacket(
+  conn.outstandingPackets.emplace_back(
-      std::move(regularPacket), Clock::now(), 1, false, false, 1));
+      OutstandingPacket(std::move(regularPacket), Clock::now(), 1, false, 1));
   ReadAckFrame ackFrame;
   ackFrame.largestAcked = 0;
   ackFrame.ackBlocks.emplace_back(0, 0);
@@ -639,7 +568,7 @@ TEST_P(AckHandlersTest, SkipAckVisitor) {
   WriteStreamFrame frame(0, 0, 0, true);
   regularPacket.frames.emplace_back(std::move(frame));
   OutstandingPacket outstandingPacket(
-      std::move(regularPacket), Clock::now(), 1, false, false, 1);
+      std::move(regularPacket), Clock::now(), 1, false, 1);
   // Give this outstandingPacket an associatedEvent that's not in
   // outstandingPacketEvents
   outstandingPacket.associatedEvent = 0;
@@ -681,11 +610,11 @@ TEST_P(AckHandlersTest, NoDoubleProcess) {
   regularPacket2.frames.push_back(frame);
 
   OutstandingPacket outstandingPacket1(
-      std::move(regularPacket1), Clock::now(), 1, false, false, 1);
+      std::move(regularPacket1), Clock::now(), 1, false, 1);
   outstandingPacket1.associatedEvent = packetNum1;
 
   OutstandingPacket outstandingPacket2(
-      std::move(regularPacket2), Clock::now(), 1, false, false, 1);
+      std::move(regularPacket2), Clock::now(), 1, false, 1);
   // The seconds packet has the same PacketEvent
   outstandingPacket2.associatedEvent = packetNum1;
 
@@ -743,7 +672,7 @@ TEST_P(AckHandlersTest, ClonedPacketsCounter) {
   auto regularPacket1 = createNewPacket(packetNum1, GetParam());
   regularPacket1.frames.push_back(frame);
   OutstandingPacket outstandingPacket1(
-      std::move(regularPacket1), Clock::now(), 1, false, false, 1);
+      std::move(regularPacket1), Clock::now(), 1, false, 1);
   outstandingPacket1.associatedEvent = packetNum1;
 
   conn.ackStates.appDataAckState.nextPacketNum++;
@@ -751,7 +680,7 @@ TEST_P(AckHandlersTest, ClonedPacketsCounter) {
   auto regularPacket2 = createNewPacket(packetNum2, GetParam());
   regularPacket2.frames.push_back(frame);
   OutstandingPacket outstandingPacket2(
-      std::move(regularPacket2), Clock::now(), 1, false, false, 1);
+      std::move(regularPacket2), Clock::now(), 1, false, 1);
 
   conn.outstandingPackets.push_back(std::move(outstandingPacket1));
   conn.outstandingPackets.push_back(std::move(outstandingPacket2));
@@ -789,8 +718,8 @@ TEST_P(AckHandlersTest, UpdateMaxAckDelay) {
   PacketNum packetNum = 0;
   auto regularPacket = createNewPacket(packetNum, GetParam());
   auto sentTime = Clock::now();
-  conn.outstandingPackets.emplace_back(OutstandingPacket(
+  conn.outstandingPackets.emplace_back(
-      std::move(regularPacket), sentTime, 1, false, false, 1));
+      OutstandingPacket(std::move(regularPacket), sentTime, 1, false, 1));
 
   ReadAckFrame ackFrame;
   // ackDelay has no effect on mrtt
@@ -843,7 +772,6 @@ TEST_P(AckHandlersTest, AckNotOutstandingButLoss) {
       Clock::now() - delayUntilLost - 20ms,
       1,
       false,
-      false,
       1);
   conn.outstandingPackets.push_back(std::move(outstandingPacket));
   conn.outstandingClonedPacketsCount++;
@@ -895,7 +823,6 @@ TEST_P(AckHandlersTest, UpdatePendingAckStates) {
       sentTime,
       111,
       false,
-      false,
       conn.lossState.totalBytesSent + 111));
   conn.lossState.totalBytesSent += 111;
 
@@ -918,65 +845,6 @@ TEST_P(AckHandlersTest, UpdatePendingAckStates) {
   EXPECT_EQ(111 + 1357, conn.lossState.totalBytesAcked);
 }
 
-TEST_F(AckHandlersTest, PureAckDoesNotUpdateRtt) {
-  QuicServerConnectionState conn;
-  conn.congestionController = nullptr;
-  PacketNum packetNum = 0;
-  auto regularPacket = createNewPacket(packetNum, PacketNumberSpace::AppData);
-  conn.outstandingPackets.emplace_back(OutstandingPacket(
-      std::move(regularPacket),
-      Clock::now() - 200ms,
-      111,
-      false /* handshake */,
-      true /* pureAck */,
-      111));
-  conn.outstandingPureAckPacketsCount++;
-  ASSERT_FALSE(conn.outstandingPackets.empty());
-  ReadAckFrame ackFrame;
-  ackFrame.largestAcked = packetNum;
-  ackFrame.ackDelay = 100us;
-  ackFrame.ackBlocks.emplace_back(packetNum, packetNum);
-  processAckFrame(
-      conn,
-      PacketNumberSpace::AppData,
-      ackFrame,
-      [&](const auto&, const auto&, const auto&) {},
-      [&](auto&, auto&, bool, auto) {},
-      Clock::now() - 150ms);
-  EXPECT_EQ(std::chrono::microseconds::max(), conn.lossState.mrtt);
-  EXPECT_EQ(0us, conn.lossState.srtt);
-  EXPECT_EQ(0us, conn.lossState.lrtt);
-  EXPECT_EQ(0us, conn.lossState.rttvar);
-  EXPECT_TRUE(conn.outstandingPackets.empty());
-
-  packetNum++;
-  regularPacket = createNewPacket(packetNum, PacketNumberSpace::AppData);
-  conn.outstandingPackets.emplace_back(OutstandingPacket(
-      std::move(regularPacket),
-      Clock::now() - 100ms,
-      111,
-      false /* handshake */,
-      false /* pureAck */,
-      111));
-  ASSERT_FALSE(conn.outstandingPackets.empty());
-  ackFrame.largestAcked = packetNum;
-  ackFrame.ackDelay = 100us;
-  ackFrame.ackBlocks.clear();
-  ackFrame.ackBlocks.emplace_back(packetNum, packetNum);
-  processAckFrame(
-      conn,
-      PacketNumberSpace::AppData,
-      ackFrame,
-      [&](const auto&, const auto&, const auto&) {},
-      [&](auto&, auto&, bool, auto) {},
-      Clock::now());
-  EXPECT_NE(std::chrono::microseconds::max(), conn.lossState.mrtt);
-  EXPECT_NE(0us, conn.lossState.srtt);
-  EXPECT_NE(0us, conn.lossState.lrtt);
-  EXPECT_NE(0us, conn.lossState.rttvar);
-  EXPECT_TRUE(conn.outstandingPackets.empty());
-}
-
 TEST_P(AckHandlersTest, AckEventCreation) {
   QuicServerConnectionState conn;
   auto mockCongestionController = std::make_unique<MockCongestionController>();
@@ -997,13 +865,11 @@ TEST_P(AckHandlersTest, AckEventCreation) {
         largestSentTime,
         1,
         false /* handshake */,
-        (packetNum % 2) /* pureAck */,
         packetNum);
     sentPacket.isAppLimited = (packetNum % 2);
     conn.outstandingPackets.emplace_back(sentPacket);
     packetNum++;
   }
-  conn.outstandingPureAckPacketsCount = 5;
 
   ReadAckFrame ackFrame;
   ackFrame.largestAcked = 9;
@@ -1015,7 +881,7 @@ TEST_P(AckHandlersTest, AckEventCreation) {
         EXPECT_EQ(ackTime, ack->ackTime);
         EXPECT_EQ(9, ack->largestAckedPacket.value());
         EXPECT_EQ(largestSentTime, ack->largestAckedPacketSentTime);
-        EXPECT_EQ(5, ack->ackedBytes);
+        EXPECT_EQ(10, ack->ackedBytes);
         EXPECT_TRUE(ack->largestAckedPacketAppLimited);
         EXPECT_EQ(
             std::chrono::duration_cast<std::chrono::microseconds>(

quic/state/test/QuicStateFunctionsTest.cpp

@@ -482,26 +482,23 @@ TEST_F(QuicStateFunctionsTest, GetOutstandingPackets) {
       Clock::now(),
       135,
       false,
-      false,
       0);
   conn.outstandingPackets.emplace_back(
       makeTestLongPacket(LongHeader::Types::Handshake),
       Clock::now(),
       1217,
       false,
-      false,
       0);
   conn.outstandingPackets.emplace_back(
-      makeTestShortPacket(), Clock::now(), 5556, false, false, 0);
+      makeTestShortPacket(), Clock::now(), 5556, false, 0);
   conn.outstandingPackets.emplace_back(
       makeTestLongPacket(LongHeader::Types::Initial),
       Clock::now(),
       56,
       false,
-      false,
       0);
   conn.outstandingPackets.emplace_back(
-      makeTestShortPacket(), Clock::now(), 6665, false, false, 0);
+      makeTestShortPacket(), Clock::now(), 6665, false, 0);
   EXPECT_EQ(
       135,
       getFirstOutstandingPacket(conn, PacketNumberSpace::Initial)->encodedSize);

quic/state/test/StateDataTest.cpp

@@ -35,8 +35,7 @@ TEST_F(StateDataTest, SingleLostPacketEvent) {
       getTestConnectionId(),
       100,
       kVersion));
-  OutstandingPacket outstandingPacket(
+  OutstandingPacket outstandingPacket(packet, Clock::now(), 1234, false, 1234);
-      packet, Clock::now(), 1234, false, false, 1234);
   CongestionController::LossEvent loss;
   loss.addLostPacket(outstandingPacket);
   EXPECT_EQ(1234, loss.lostBytes);
@@ -51,7 +50,7 @@ TEST_F(StateDataTest, MultipleLostPacketsEvent) {
       100,
       kVersion));
   OutstandingPacket outstandingPacket1(
-      packet1, Clock::now(), 1234, false, false, 1234);
+      packet1, Clock::now(), 1234, false, 1234);
 
   RegularQuicWritePacket packet2(LongHeader(
       LongHeader::Types::Initial,
@@ -60,7 +59,7 @@ TEST_F(StateDataTest, MultipleLostPacketsEvent) {
       110,
       kVersion));
   OutstandingPacket outstandingPacket2(
-      packet2, Clock::now(), 1357, false, false, 1357);
+      packet2, Clock::now(), 1357, false, 1357);
 
   CongestionController::LossEvent loss;
   loss.addLostPacket(outstandingPacket1);