Move ByteEventCallback to QuicCallbacks.h

Summary:
The purpose of this is so that we can import this header from the WebTransport implementation and use the `ByteEventCallback` directly instead of creating a wrapper, thereby saving an allocation.

There's no functional change in this commit, it's just moving things around.

Relevant files:
* quic/api/QuicCallbacks.h
* quic/api/QuicSocketLite.h

Reviewed By: hanidamlaj

Differential Revision: D70000563

fbshipit-source-id: 9523cc788f50b4ba218be33e84f7d5b4f44a73c2

quic/api/QuicCallbacks.h

@@ -94,4 +94,56 @@ class ConnectionWriteCallback {
                                       /* error */) noexcept {}
 };
 
+/**
+ * Structure used to communicate TX and ACK/Delivery notifications.
+ */
+struct ByteEvent {
+  enum class Type { ACK = 1, TX = 2 };
+  static constexpr std::array<Type, 2> kByteEventTypes = {
+      {Type::ACK, Type::TX}};
+
+  StreamId id{0};
+  uint64_t offset{0};
+  Type type;
+
+  // sRTT at time of event
+  // TODO(bschlinker): Deprecate, caller can fetch transport state if
+  // desired.
+  std::chrono::microseconds srtt{0us};
+};
+
+/**
+ * Structure used to communicate cancellation of a ByteEvent.
+ *
+ * According to Dictionary.com, cancellation is more frequent in American
+ * English than cancellation. Yet in American English, the preferred style is
+ * typically not to double the final L, so cancel generally becomes canceled.
+ */
+using ByteEventCancellation = ByteEvent;
+
+/**
+ * Callback class for receiving byte event (TX/ACK) notifications.
+ */
+class ByteEventCallback {
+ public:
+  virtual ~ByteEventCallback() = default;
+
+  /**
+   * Invoked when a byte event has been successfully registered.
+   * Since this is a convenience notification and not a mandatory callback,
+   * not marking this as pure virtual.
+   */
+  virtual void onByteEventRegistered(ByteEvent /* byteEvent */) {}
+
+  /**
+   * Invoked when the byte event has occurred.
+   */
+  virtual void onByteEvent(ByteEvent byteEvent) = 0;
+
+  /**
+   * Invoked if byte event is canceled due to reset, shutdown, or other error.
+   */
+  virtual void onByteEventCanceled(ByteEventCancellation cancellation) = 0;
+};
+
 } // namespace quic

quic/api/QuicSocketLite.h

@@ -440,58 +440,6 @@ class QuicSocketLite {
     virtual void onPing() noexcept = 0;
   };
 
-  /**
-   * Structure used to communicate TX and ACK/Delivery notifications.
-   */
-  struct ByteEvent {
-    enum class Type { ACK = 1, TX = 2 };
-    static constexpr std::array<Type, 2> kByteEventTypes = {
-        {Type::ACK, Type::TX}};
-
-    StreamId id{0};
-    uint64_t offset{0};
-    Type type;
-
-    // sRTT at time of event
-    // TODO(bschlinker): Deprecate, caller can fetch transport state if
-    // desired.
-    std::chrono::microseconds srtt{0us};
-  };
-
-  /**
-   * Structure used to communicate cancellation of a ByteEvent.
-   *
-   * According to Dictionary.com, cancellation is more frequent in American
-   * English than cancellation. Yet in American English, the preferred style is
-   * typically not to double the final L, so cancel generally becomes canceled.
-   */
-  using ByteEventCancellation = ByteEvent;
-
-  /**
-   * Callback class for receiving byte event (TX/ACK) notifications.
-   */
-  class ByteEventCallback {
-   public:
-    virtual ~ByteEventCallback() = default;
-
-    /**
-     * Invoked when a byte event has been successfully registered.
-     * Since this is a convenience notification and not a mandatory callback,
-     * not marking this as pure virtual.
-     */
-    virtual void onByteEventRegistered(ByteEvent /* byteEvent */) {}
-
-    /**
-     * Invoked when the byte event has occurred.
-     */
-    virtual void onByteEvent(ByteEvent byteEvent) = 0;
-
-    /**
-     * Invoked if byte event is canceled due to reset, shutdown, or other error.
-     */
-    virtual void onByteEventCanceled(ByteEventCancellation cancellation) = 0;
-  };
-
   /**
    * Callback class for receiving ack notifications
    */

quic/api/test/BUCK

@@ -12,6 +12,7 @@ mvfst_cpp_library(
         "//folly/portability:gmock",
         "//quic:exception",
         "//quic/api:loop_detector_callback",
+        "//quic/api:quic_callbacks",
         "//quic/api:transport",
         "//quic/codec:types",
         "//quic/common:network_data",

quic/api/test/Mocks.h

@@ -11,6 +11,7 @@
 
 #include <quic/QuicException.h>
 #include <quic/api/LoopDetectorCallback.h>
+#include <quic/api/QuicCallbacks.h>
 #include <quic/api/QuicSocket.h>
 #include <quic/codec/QuicConnectionId.h>
 #include <quic/common/NetworkData.h>
@@ -155,29 +156,25 @@ class MockDeliveryCallback : public QuicSocket::DeliveryCallback {
   MOCK_METHOD(void, onCanceled, (StreamId, uint64_t));
 };
 
-class MockByteEventCallback : public QuicSocket::ByteEventCallback {
+class MockByteEventCallback : public ByteEventCallback {
  public:
   ~MockByteEventCallback() override = default;
-  MOCK_METHOD(void, onByteEventRegistered, (QuicSocket::ByteEvent));
+  MOCK_METHOD(void, onByteEventRegistered, (ByteEvent));
-  MOCK_METHOD(void, onByteEvent, (QuicSocket::ByteEvent));
+  MOCK_METHOD(void, onByteEvent, (ByteEvent));
-  MOCK_METHOD(void, onByteEventCanceled, (QuicSocket::ByteEvent));
+  MOCK_METHOD(void, onByteEventCanceled, (ByteEvent));
 
   static auto getTxMatcher(StreamId id, uint64_t offset) {
     return AllOf(
-        testing::Field(
+        testing::Field(&ByteEvent::type, testing::Eq(ByteEvent::Type::TX)),
-            &QuicSocket::ByteEvent::type,
+        testing::Field(&ByteEvent::id, testing::Eq(id)),
-            testing::Eq(QuicSocket::ByteEvent::Type::TX)),
+        testing::Field(&ByteEvent::offset, testing::Eq(offset)));
-        testing::Field(&QuicSocket::ByteEvent::id, testing::Eq(id)),
-        testing::Field(&QuicSocket::ByteEvent::offset, testing::Eq(offset)));
   }
 
   static auto getAckMatcher(StreamId id, uint64_t offset) {
     return AllOf(
-        testing::Field(
+        testing::Field(&ByteEvent::type, testing::Eq(ByteEvent::Type::ACK)),
-            &QuicSocket::ByteEvent::type,
+        testing::Field(&ByteEvent::id, testing::Eq(id)),
-            testing::Eq(QuicSocket::ByteEvent::Type::ACK)),
+        testing::Field(&ByteEvent::offset, testing::Eq(offset)));
-        testing::Field(&QuicSocket::ByteEvent::id, testing::Eq(id)),
-        testing::Field(&QuicSocket::ByteEvent::offset, testing::Eq(offset)));
   }
 };
 

quic/api/test/QuicTransportBaseTest.cpp

@@ -34,8 +34,6 @@ using namespace folly;
 namespace quic::test {
 
 constexpr uint8_t kStreamIncrement = 0x04;
-using ByteEvent = QuicTransportBase::ByteEvent;
-using ByteEventCancellation = QuicTransportBase::ByteEventCancellation;
 
 enum class TestFrameType : uint8_t {
   STREAM,
@@ -166,7 +164,7 @@ class TestPingCallback : public QuicSocket::PingCallback {
   void onPing() noexcept override {}
 };
 
-class TestByteEventCallback : public QuicSocket::ByteEventCallback {
+class TestByteEventCallback : public ByteEventCallback {
  public:
   using HashFn = std::function<size_t(const ByteEvent&)>;
   using ComparatorFn = std::function<bool(const ByteEvent&, const ByteEvent&)>;

quic/api/test/QuicTransportTest.cpp

@@ -4834,7 +4834,7 @@ TEST_F(QuicTransportTest, GetStreamPackestTxedSingleByte) {
   // one
   EXPECT_CALL(firstByteTxCb, onByteEvent(getTxMatcher(stream, 0)))
       .Times(1)
-      .WillOnce(Invoke([&](QuicSocket::ByteEvent /* event */) {
+      .WillOnce(Invoke([&](ByteEvent /* event */) {
         auto info = *transport_->getStreamTransportInfo(stream);
         EXPECT_EQ(info.numPacketsTxWithNewData, 1);
       }));
@@ -4867,13 +4867,13 @@ TEST_F(QuicTransportTest, GetStreamPacketsTxedMultipleBytes) {
   // be 1
   EXPECT_CALL(firstByteTxCb, onByteEvent(getTxMatcher(stream, 0)))
       .Times(1)
-      .WillOnce(Invoke([&](QuicSocket::ByteEvent /* event */) {
+      .WillOnce(Invoke([&](ByteEvent /* event */) {
         auto info = *transport_->getStreamTransportInfo(stream);
         EXPECT_EQ(info.numPacketsTxWithNewData, 1);
       }));
   EXPECT_CALL(lastByteTxCb, onByteEvent(getTxMatcher(stream, lastByte)))
       .Times(1)
-      .WillOnce(Invoke([&](QuicSocket::ByteEvent /* event */) {
+      .WillOnce(Invoke([&](ByteEvent /* event */) {
         auto info = *transport_->getStreamTransportInfo(stream);
         EXPECT_EQ(info.numPacketsTxWithNewData, 1);
       }));
@@ -4939,7 +4939,7 @@ TEST_F(QuicTransportTest, GetStreamPacketsTxedMultiplePackets) {
   // first byte and first packet last bytes get Txed on first loopForWrites
   EXPECT_CALL(firstByteTxCb, onByteEvent(getTxMatcher(stream, 0)))
       .Times(1)
-      .WillOnce(Invoke([&](QuicSocket::ByteEvent /* event */) {
+      .WillOnce(Invoke([&](ByteEvent /* event */) {
         auto info = *transport_->getStreamTransportInfo(stream);
         EXPECT_EQ(info.numPacketsTxWithNewData, 1);
       }));
@@ -4947,7 +4947,7 @@ TEST_F(QuicTransportTest, GetStreamPacketsTxedMultiplePackets) {
       firstPacketNearTailByteTxCb,
       onByteEvent(getTxMatcher(stream, firstPacketNearTailByte)))
       .Times(1)
-      .WillOnce(Invoke([&](QuicSocket::ByteEvent /* even */) {
+      .WillOnce(Invoke([&](ByteEvent /* even */) {
         auto info = *transport_->getStreamTransportInfo(stream);
         EXPECT_EQ(info.numPacketsTxWithNewData, 1);
       }));
@@ -4960,7 +4960,7 @@ TEST_F(QuicTransportTest, GetStreamPacketsTxedMultiplePackets) {
       secondPacketNearHeadByteTxCb,
       onByteEvent(getTxMatcher(stream, secondPacketNearHeadByte)))
       .Times(1)
-      .WillOnce(Invoke([&](QuicSocket::ByteEvent /* even */) {
+      .WillOnce(Invoke([&](ByteEvent /* even */) {
         auto info = *transport_->getStreamTransportInfo(stream);
         EXPECT_EQ(info.numPacketsTxWithNewData, 2);
       }));
@@ -4968,7 +4968,7 @@ TEST_F(QuicTransportTest, GetStreamPacketsTxedMultiplePackets) {
       secondPacketNearTailByteTxCb,
       onByteEvent(getTxMatcher(stream, secondPacketNearTailByte)))
       .Times(1)
-      .WillOnce(Invoke([&](QuicSocket::ByteEvent /* even */) {
+      .WillOnce(Invoke([&](ByteEvent /* even */) {
         auto info = *transport_->getStreamTransportInfo(stream);
         EXPECT_EQ(info.numPacketsTxWithNewData, 2);
       }));
@@ -4979,7 +4979,7 @@ TEST_F(QuicTransportTest, GetStreamPacketsTxedMultiplePackets) {
   // last byte will be sent on the fifth loopForWrites
   EXPECT_CALL(lastByteTxCb, onByteEvent(getTxMatcher(stream, lastByte)))
       .Times(1)
-      .WillOnce(Invoke([&](QuicSocket::ByteEvent /* event */) {
+      .WillOnce(Invoke([&](ByteEvent /* event */) {
         auto info = *transport_->getStreamTransportInfo(stream);
         EXPECT_EQ(info.numPacketsTxWithNewData, 5);
       }));

quic/server/test/QuicServerTransportTest.cpp

@@ -23,7 +23,6 @@ using namespace folly;
 namespace quic::test {
 
 namespace {
-using ByteEvent = QuicTransportBase::ByteEvent;
 auto constexpr kTestMaxPacingRate = std::numeric_limits<uint64_t>::max();
 } // namespace
 

quic/tools/tperf/TperfServer.cpp

@@ -21,7 +21,7 @@ class ServerStreamHandler : public quic::QuicSocket::ConnectionSetupCallback,
                             public quic::QuicSocket::ReadCallback,
                             public quic::QuicSocket::WriteCallback,
                             public quic::QuicTimerCallback,
-                            public QuicSocketLite::ByteEventCallback {
+                            public ByteEventCallback {
  public:
   explicit ServerStreamHandler(
       folly::EventBase* evbIn,
@@ -286,12 +286,12 @@ class ServerStreamHandler : public quic::QuicSocket::ConnectionSetupCallback,
         this, std::chrono::milliseconds(burstDeadlineMs_));
   }
 
-  void onByteEvent(QuicSocketLite::ByteEvent byteEvent) override {
+  void onByteEvent(ByteEvent byteEvent) override {
     CHECK_EQ(byteEvent.id, streamBurstSendResult_.streamId);
     auto now = Clock::now();
-    if (byteEvent.type == QuicSocketLite::ByteEvent::Type::TX) {
+    if (byteEvent.type == ByteEvent::Type::TX) {
       streamBurstSendResult_.trueTxStartTs = now;
-    } else if (byteEvent.type == QuicSocketLite::ByteEvent::Type::ACK) {
+    } else if (byteEvent.type == ByteEvent::Type::ACK) {
       auto ackedLatencyUs =
           std::chrono::duration_cast<std::chrono::microseconds>(
               now - streamBurstSendResult_.startTs);
@@ -311,8 +311,7 @@ class ServerStreamHandler : public quic::QuicSocket::ConnectionSetupCallback,
     }
   }
 
-  void onByteEventCanceled(
+  void onByteEventCanceled(ByteEventCancellation cancellation) override {
-      QuicSocketLite::ByteEventCancellation cancellation) override {
     VLOG(4) << "got stream " << cancellation.id << " offset "
             << cancellation.offset << " cancelled";
   }