Define HostTargetTracingDelegate (facebook#54622)

Summary:
Pull Request resolved: facebook#54622

# Changelog: [Internal]

Introduces a TracingDelegate that will receive tracing-related events on the Host side.

We can't give synchronous access to tracing state to Host directly, since there is no guarantee that this call will happen on the inspector thread.

Example:
- `HostTarget::tracingState()` call may happen on UI thread, or whatever thread the Host is using, which reads `traceRecoding_` value.
- At the same time, the `stopTracing()` call that will invalidate `traceRecording_` could happen on inspector thread.

This is a pre-requisite for a better threading model, since we are going to be doing much frequent write operations from the Host side to record frame timings or screenshots.

Reviewed By: sbuggay

Differential Revision: D86205686

fbshipit-source-id: b6e40ff60a1427fdff1aa30f89297d64b99bef75

Author: hoxyq

packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h

@@ -55,6 +55,36 @@ struct HostTargetMetadata {
   std::optional<std::string> reactNativeVersion{};
 };
 
+/**
+ * Receives any performance-related events from a HostTarget: could be Tracing, Performance Monitor, etc.
+ */
+class HostTargetTracingDelegate {
+ public:
+  HostTargetTracingDelegate() = default;
+  virtual ~HostTargetTracingDelegate() = default;
+
+  /**
+   * Fired when the corresponding HostTarget started recording a tracing session.
+   * The tracing state is expected to be initialized at this point and the delegate should be able to record events
+   * through HostTarget.
+   */
+  virtual void onTracingStarted(tracing::Mode /* tracingMode */, bool /* screenshotsCategoryEnabled */) {}
+
+  /**
+   * Fired when the corresponding HostTarget is about to end recording a tracing session.
+   * The tracing state is expected to be still initialized during the call and the delegate should be able to record
+   * events through HostTarget.
+   *
+   * Any attempts to record events after this callback is finished will fail.
+   */
+  virtual void onTracingStopped() {}
+
+  HostTargetTracingDelegate(const HostTargetTracingDelegate &) = delete;
+  HostTargetTracingDelegate(HostTargetTracingDelegate &&) = delete;
+  HostTargetTracingDelegate &operator=(const HostTargetTracingDelegate &) = delete;
+  HostTargetTracingDelegate &operator=(HostTargetTracingDelegate &&) = delete;
+};
+
 /**
  * Receives events from a HostTarget. This is a shared interface that each
  * React Native platform needs to implement in order to integrate with the
@@ -161,6 +191,14 @@ class HostTargetDelegate : public LoadNetworkResourceDelegate {
   {
     return std::nullopt;
   }
+
+  /**
+   * An optional delegate that will be used by HostTarget to notify about tracing-related events.
+   */
+  virtual HostTargetTracingDelegate *getTracingDelegate()
+  {
+    return nullptr;
+  }
 };
 
 /**
@@ -230,12 +268,15 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis<HostTarget>
  public:
   /**
    * Constructs a new HostTarget.
+   *
    * \param delegate The HostTargetDelegate that will
    * receive events from this HostTarget. The caller is responsible for ensuring
    * that the HostTargetDelegate outlives this object.
+   *
    * \param executor An executor that may be used to call methods on this
    * HostTarget while it exists. \c create additionally guarantees that the
    * executor will not be called after the HostTarget is destroyed.
+   *
    * \note Copies of the provided executor may be destroyed on arbitrary
    * threads, including after the HostTarget is destroyed. Callers must ensure
    * that such destructor calls are safe - e.g. if using a lambda as the
@@ -330,6 +371,7 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis<HostTarget>
   /**
    * Constructs a new HostTarget.
    * The caller must call setExecutor immediately afterwards.
+   *
    * \param delegate The HostTargetDelegate that will
    * receive events from this HostTarget. The caller is responsible for ensuring
    * that the HostTargetDelegate outlives this object.

packages/react-native/ReactCommon/jsinspector-modern/HostTargetTracing.cpp

@@ -35,23 +35,34 @@ bool HostTarget::startTracing(
   if (traceRecording_ != nullptr) {
     if (traceRecording_->isBackgroundInitiated() &&
         tracingMode == tracing::Mode::CDP) {
-      traceRecording_.reset();
+      stopTracing();
     } else {
       return false;
     }
   }
 
+  auto screenshotsCategoryEnabled =
+      enabledCategories.contains(tracing::Category::Screenshot);
+
   traceRecording_ = std::make_unique<HostTargetTraceRecording>(
       *this, tracingMode, std::move(enabledCategories));
   traceRecording_->setTracedInstance(currentInstance_.get());
   traceRecording_->start();
 
+  if (auto tracingDelegate = delegate_.getTracingDelegate()) {
+    tracingDelegate->onTracingStarted(tracingMode, screenshotsCategoryEnabled);
+  }
+
   return true;
 }
 
 tracing::TraceRecordingState HostTarget::stopTracing() {
   assert(traceRecording_ != nullptr && "No tracing in progress");
 
+  if (auto tracingDelegate = delegate_.getTracingDelegate()) {
+    tracingDelegate->onTracingStopped();
+  }
+
   auto state = traceRecording_->stop();
   traceRecording_.reset();
 

packages/react-native/ReactCommon/jsinspector-modern/tests/HostTargetTest.cpp

@@ -1526,4 +1526,57 @@ TEST_F(HostTargetTest, IOReadSizeValidation) {
       })");
 }
 
+TEST_F(HostTargetTest, TracingDelegateIsNotifiedOnCDPRequest) {
+  connect();
+  InSequence s;
+
+  EXPECT_CALL(
+      hostTargetDelegate_.getTracingDelegateMock(),
+      onTracingStarted(Eq(tracing::Mode::CDP), Eq(false)))
+      .Times(1)
+      .RetiresOnSaturation();
+  EXPECT_CALL(fromPage(), onMessage(JsonEq(R"({
+                                               "id": 1,
+                                               "result": {}
+                                             })")));
+  toPage_->sendMessage(R"({
+                        "id": 1,
+                        "method": "Tracing.start"
+                      })");
+
+  EXPECT_CALL(hostTargetDelegate_.getTracingDelegateMock(), onTracingStopped())
+      .Times(1)
+      .RetiresOnSaturation();
+  EXPECT_CALL(fromPage(), onMessage(JsonEq(R"({
+                                               "id": 1,
+                                               "result": {}
+                                             })")));
+  EXPECT_CALL(
+      fromPage(),
+      onMessage(JsonParsed(
+          testing::AllOf(
+              AtJsonPtr("/method", "Tracing.tracingComplete"),
+              AtJsonPtr("/params/dataLossOccurred", false)))));
+  toPage_->sendMessage(R"({
+                        "id": 1,
+                        "method": "Tracing.end"
+                      })");
+}
+
+TEST_F(HostTargetTest, TracingDelegateIsNotifiedOnDirectTracingCall) {
+  connect();
+
+  EXPECT_CALL(
+      hostTargetDelegate_.getTracingDelegateMock(),
+      onTracingStarted(Eq(tracing::Mode::Background), Eq(false)))
+      .Times(1)
+      .RetiresOnSaturation();
+  page_->startTracing(tracing::Mode::Background, {});
+
+  EXPECT_CALL(hostTargetDelegate_.getTracingDelegateMock(), onTracingStopped())
+      .Times(1)
+      .RetiresOnSaturation();
+  page_->stopTracing();
+}
+
 } // namespace facebook::react::jsinspector_modern

packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h

@@ -113,6 +113,12 @@ class MockInspectorPackagerConnectionDelegate : public InspectorPackagerConnecti
   folly::Executor &executor_;
 };
 
+class MockHostTargetTracingDelegate : public HostTargetTracingDelegate {
+ public:
+  MOCK_METHOD(void, onTracingStarted, (tracing::Mode tracingMode, bool screenshotsCategoryEnabled), (override));
+  MOCK_METHOD(void, onTracingStopped, (), (override));
+};
+
 class MockHostTargetDelegate : public HostTargetDelegate {
  public:
   // HostTargetDelegate methods
@@ -131,6 +137,20 @@ class MockHostTargetDelegate : public HostTargetDelegate {
       loadNetworkResource,
       (const LoadNetworkResourceRequest &params, ScopedExecutor<NetworkRequestListener> executor),
       (override));
+
+  HostTargetTracingDelegate *getTracingDelegate() override
+  {
+    return mockTracingDelegate_.get();
+  }
+
+  MockHostTargetTracingDelegate &getTracingDelegateMock()
+  {
+    return *mockTracingDelegate_;
+  }
+
+ private:
+  std::unique_ptr<MockHostTargetTracingDelegate> mockTracingDelegate_ =
+      std::make_unique<MockHostTargetTracingDelegate>();
 };
 
 class MockInstanceTargetDelegate : public InstanceTargetDelegate {};