Stream background traces to multiple RNDT sessions if present (facebook#55351)

Summary:

Changelog: [Internal]

## Behaviour

Fixes an edge case in the way background traces are emitted when multiple RNDT frontends (or other clients with the `ReactNativeApplication` domain enabled) are connected to a single Host. Previously, only one of the clients would receive the CDP events for the trace. With this diff, *all* of them will.

NOTE: This leaves another, even smaller edge case / inconsistency: if there are *no* RNDT frontends at the time a background trace ends, we still only send it to the first frontend (if any) that connects. This is logically more defensible than picking one client out of multiple active connections ( = what's fixed in this diff). 

## Implementation

Tracing functionality is somewhat awkwardly split between `HostTarget` (background traces) and `TracingAgent` (CDP-initiated traces). This diff doesn't attempt to fully clean this up[1], but we do reduce duplication and boilerplate by creating a helper function used by both. This function encapsulates the gnarly/surprising details of dealing with `tracing::HostTracingProfile` objects in a memory-efficient way:

1. As per the existing implementation, we serialise them to JSON chunk-by-chunk, destroying the original object in the process.
2. Each chunk is sent to all relevant sessions *and destroyed* before we begin serialising the next one.

[1] In future work, we should probably move the Host's tracing state and logic into its own `TracingTarget` class. The naturally close coupling of a Target with its corresponding Agent would be helpful here.

Reviewed By: huntie

Differential Revision: D90888852

Author: motiz88

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

@@ -379,15 +379,8 @@ class HostAgent::Impl final {
     }
   }
 
-  bool hasFuseboxClientConnected() const {
-    return fuseboxClientType_ == FuseboxClientType::Fusebox;
-  }
-
-  void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) {
-    assert(
-        hasFuseboxClientConnected() &&
-        "Attempted to emit a trace recording to a non-Fusebox client");
-    tracingAgent_.emitExternalHostTracingProfile(std::move(tracingProfile));
+  bool isEligibleForBackgroundTrace() const {
+    return sessionState_.isReactNativeApplicationDomainEnabled;
   }
 
   void emitSystemStateChanged(bool isSingleHost) {
@@ -500,10 +493,9 @@ class HostAgent::Impl final {
 
   void handleRequest(const cdp::PreparsedRequest& req) {}
   void setCurrentInstanceAgent(std::shared_ptr<InstanceAgent> agent) {}
-  bool hasFuseboxClientConnected() const {
+  bool isEligibleForBackgroundTrace() const {
     return false;
   }
-  void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) {}
   void emitSystemStateChanged(bool isSingleHost) {}
 };
 
@@ -535,17 +527,8 @@ void HostAgent::setCurrentInstanceAgent(
   impl_->setCurrentInstanceAgent(std::move(instanceAgent));
 }
 
-bool HostAgent::hasFuseboxClientConnected() const {
-  return impl_->hasFuseboxClientConnected();
-}
-
-void HostAgent::emitExternalTracingProfile(
-    tracing::HostTracingProfile tracingProfile) {
-  impl_->emitExternalTracingProfile(std::move(tracingProfile));
-}
-
-void HostAgent::emitSystemStateChanged(bool isSingleHost) {
-  impl_->emitSystemStateChanged(isSingleHost);
+bool HostAgent::isEligibleForBackgroundTrace() const {
+  return impl_->isEligibleForBackgroundTrace();
 }
 
 #pragma mark - Tracing

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

@@ -67,17 +67,10 @@ class HostAgent final {
   void setCurrentInstanceAgent(std::shared_ptr<InstanceAgent> agent);
 
   /**
-   * Returns whether this HostAgent is part of the session that has an active
-   * Fusebox client connecte, i.e. with Chrome DevTools Frontend fork for React
-   * Native.
+   * Returns whether this HostAgent is eligible to receive notifications about
+   * background traces.
    */
-  bool hasFuseboxClientConnected() const;
-
-  /**
-   * Emits the HostTracingProfile that was captured externally, not via the
-   * CDP-initiated request.
-   */
-  void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile);
+  bool isEligibleForBackgroundTrace() const;
 
   /**
    * Emits a system state changed event when the number of ReactHost instances

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

@@ -8,6 +8,7 @@
 #include "HostTarget.h"
 #include "HostAgent.h"
 #include "HostTargetTraceRecording.h"
+#include "HostTargetTracing.h"
 #include "InspectorInterfaces.h"
 #include "InspectorUtilities.h"
 #include "InstanceTarget.h"
@@ -106,6 +107,10 @@ class HostTargetSession {
     return hostAgent_;
   }
 
+  FrontendChannel dangerouslyGetFrontendChannel() {
+    return frontendChannel_;
+  }
+
  private:
   // Owned by this instance, but shared (weakly) with the frontend channel
   std::shared_ptr<RAIIRemoteConnection> remote_;
@@ -370,20 +375,27 @@ folly::dynamic createHostMetadataPayload(const HostTargetMetadata& metadata) {
 }
 
 bool HostTarget::maybeEmitStashedBackgroundTrace() {
-  bool emitted = false;
-  sessions_.forEach([&](auto& session) {
-    if (emitted) {
-      return;
-    }
-    if (session.agent().hasFuseboxClientConnected()) {
-      auto stashedTrace = std::exchange(stashedTracingProfile_, std::nullopt);
-      if (stashedTrace) {
-        session.agent().emitExternalTracingProfile(std::move(*stashedTrace));
-      }
-      emitted = true;
+  std::vector<FrontendChannel> eligibleFrontendChannels;
+  eligibleFrontendChannels.reserve(sessions_.size());
+  sessions_.forEach([&eligibleFrontendChannels](auto& session) {
+    if (session.agent().isEligibleForBackgroundTrace()) {
+      eligibleFrontendChannels.push_back(
+          session.dangerouslyGetFrontendChannel());
     }
   });
-  return emitted;
+
+  if (eligibleFrontendChannels.empty()) {
+    return false;
+  }
+
+  auto stashedTrace = std::exchange(stashedTracingProfile_, std::nullopt);
+  if (stashedTrace) {
+    emitNotificationsForTracingProfile(
+        std::move(*stashedTrace),
+        eligibleFrontendChannels,
+        /* isBackgroundTrace */ true);
+  }
+  return true;
 }
 
 bool HostTarget::stopAndMaybeEmitBackgroundTrace() {

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

@@ -244,7 +244,7 @@ class HostTargetController final {
   tracing::HostTracingProfile stopTracing();
 
   /**
-   * If there is a stashed background trace, emit it to the first eligible session.
+   * If there is a stashed background trace, emit it to all eligible sessions.
    * \return true if an eligible session is found (even if there was no stashed background trace).
    */
   bool maybeEmitStashedBackgroundTrace();

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

@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+#include "InspectorInterfaces.h"
+
+#include "cdp/CdpJson.h"
+#include "tracing/HostTracingProfile.h"
+#include "tracing/HostTracingProfileSerializer.h"
+
+#include <array>
+#include <concepts>
+#include <cstdint>
+#include <ranges>
+
+namespace facebook::react::jsinspector_modern {
+
+/**
+ * Emits a captured HostTracingProfile in a series of
+ * Tracing.dataCollected events, followed by a Tracing.tracingComplete event, to zero or more
+ * FrontendChannels. If \p isBackgroundTrace is true, a ReactNativeApplication.traceRequested
+ * notification is sent to each FrontendChannel before the trace events are emitted.
+ */
+template <typename ChannelsRange>
+void emitNotificationsForTracingProfile(
+    tracing::HostTracingProfile &&hostTracingProfile,
+    const ChannelsRange &channels,
+    bool isBackgroundTrace)
+  requires std::ranges::range<ChannelsRange> &&
+    std::convertible_to<std::ranges::range_value_t<ChannelsRange>, FrontendChannel>
+{
+  /**
+   * Threshold for the size Trace Event chunk, that will be flushed out with
+   * Tracing.dataCollected event.
+   */
+  static constexpr uint16_t TRACE_EVENT_CHUNK_SIZE = 1000;
+
+  /**
+   * The maximum number of ProfileChunk trace events
+   * that will be sent in a single CDP Tracing.dataCollected message.
+   */
+  static constexpr uint16_t PROFILE_TRACE_EVENT_CHUNK_SIZE = 10;
+
+  if (std::ranges::empty(channels)) {
+    return;
+  }
+
+  if (isBackgroundTrace) {
+    for (auto &frontendChannel : channels) {
+      frontendChannel(cdp::jsonNotification("ReactNativeApplication.traceRequested"));
+    }
+  }
+
+  // Serialize each chunk once and send it to all eligible sessions.
+  tracing::HostTracingProfileSerializer::emitAsDataCollectedChunks(
+      std::move(hostTracingProfile),
+      [&](folly::dynamic &&serializedChunk) {
+        for (auto &frontendChannel : channels) {
+          frontendChannel(
+              cdp::jsonNotification("Tracing.dataCollected", folly::dynamic::object("value", serializedChunk)));
+        }
+      },
+      TRACE_EVENT_CHUNK_SIZE,
+      PROFILE_TRACE_EVENT_CHUNK_SIZE);
+
+  for (auto &frontendChannel : channels) {
+    frontendChannel(
+        cdp::jsonNotification("Tracing.tracingComplete", folly::dynamic::object("dataLossOccurred", false)));
+  }
+}
+
+/**
+ * Convenience overload of emitNotificationsForTracingProfile() for a single FrontendChannel.
+ */
+inline void emitNotificationsForTracingProfile(
+    tracing::HostTracingProfile &&hostTracingProfile,
+    const FrontendChannel &channel,
+    bool isBackgroundTrace)
+{
+  std::array<FrontendChannel, 1> channels{channel};
+  emitNotificationsForTracingProfile(std::move(hostTracingProfile), channels, isBackgroundTrace);
+}
+
+} // namespace facebook::react::jsinspector_modern

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

@@ -6,31 +6,15 @@
  */
 
 #include "TracingAgent.h"
+#include "HostTargetTracing.h"
 
-#include <jsinspector-modern/tracing/HostTracingProfileSerializer.h>
 #include <jsinspector-modern/tracing/PerformanceTracer.h>
 #include <jsinspector-modern/tracing/RuntimeSamplingProfileTraceEventSerializer.h>
 #include <jsinspector-modern/tracing/TraceEventSerializer.h>
 #include <jsinspector-modern/tracing/TracingMode.h>
 
 namespace facebook::react::jsinspector_modern {
 
-namespace {
-
-/**
- * Threshold for the size Trace Event chunk, that will be flushed out with
- * Tracing.dataCollected event.
- */
-const uint16_t TRACE_EVENT_CHUNK_SIZE = 1000;
-
-/**
- * The maximum number of ProfileChunk trace events
- * that will be sent in a single CDP Tracing.dataCollected message.
- */
-const uint16_t PROFILE_TRACE_EVENT_CHUNK_SIZE = 10;
-
-} // namespace
-
 TracingAgent::TracingAgent(
     FrontendChannel frontendChannel,
     SessionState& sessionState,
@@ -112,38 +96,14 @@ bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) {
     // Send response to Tracing.end request.
     frontendChannel_(cdp::jsonResult(req.id));
 
-    emitHostTracingProfile(std::move(tracingProfile));
+    emitNotificationsForTracingProfile(
+        std::move(tracingProfile),
+        frontendChannel_,
+        /* isBackgroundTrace */ false);
     return true;
   }
 
   return false;
 }
 
-void TracingAgent::emitExternalHostTracingProfile(
-    tracing::HostTracingProfile tracingProfile) {
-  frontendChannel_(
-      cdp::jsonNotification("ReactNativeApplication.traceRequested"));
-  emitHostTracingProfile(std::move(tracingProfile));
-}
-
-void TracingAgent::emitHostTracingProfile(
-    tracing::HostTracingProfile tracingProfile) {
-  auto dataCollectedCallback = [this](folly::dynamic&& eventsChunk) {
-    frontendChannel_(
-        cdp::jsonNotification(
-            "Tracing.dataCollected",
-            folly::dynamic::object("value", std::move(eventsChunk))));
-  };
-  tracing::HostTracingProfileSerializer::emitAsDataCollectedChunks(
-      std::move(tracingProfile),
-      dataCollectedCallback,
-      TRACE_EVENT_CHUNK_SIZE,
-      PROFILE_TRACE_EVENT_CHUNK_SIZE);
-
-  frontendChannel_(
-      cdp::jsonNotification(
-          "Tracing.tracingComplete",
-          folly::dynamic::object("dataLossOccurred", false)));
-}
-
 } // namespace facebook::react::jsinspector_modern

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

@@ -41,11 +41,6 @@ class TracingAgent {
    */
   bool handleRequest(const cdp::PreparsedRequest &req);
 
-  /**
-   * Emits the HostTracingProfile that was stashed externally by the HostTarget.
-   */
-  void emitExternalHostTracingProfile(tracing::HostTracingProfile tracingProfile);
-
  private:
   /**
    * A channel used to send responses and events to the frontend.
@@ -55,12 +50,6 @@ class TracingAgent {
   SessionState &sessionState_;
 
   HostTargetController &hostTargetController_;
-
-  /**
-   * Emits captured HostTracingProfile in a series of
-   * Tracing.dataCollected events, followed by a Tracing.tracingComplete event.
-   */
-  void emitHostTracingProfile(tracing::HostTracingProfile tracingProfile);
 };
 
 } // namespace facebook::react::jsinspector_modern

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

@@ -185,7 +185,7 @@ TEST_F(TracingTest, BackgroundTracingIsRejectedWhileCDPTracingIsRunning) {
   page_->stopTracing();
 }
 
-TEST_F(TracingTest, EmitsToFirstEligibleSessionWhenMultipleSessionsEnabled) {
+TEST_F(TracingTest, EmitsToAllSessionsWithReactNativeApplicationDomainEnabled) {
   auto secondaryFusebox = this->connectSecondary();
   auto secondaryNonFusebox = this->connectSecondary();
 
@@ -230,9 +230,11 @@ TEST_F(TracingTest, EmitsToFirstEligibleSessionWhenMultipleSessionsEnabled) {
           now + HighResDuration::fromNanoseconds(10),
           now + HighResDuration::fromNanoseconds(50)));
 
-  // Primary session should receive the trace (first eligible session).
-  // Events within a session are ordered.
+  // Primary and secondaryFusebox sessions should receive the trace.
+  // Events within each session are ordered, but order between sessions is
+  // arbitrary.
   Sequence primarySeq;
+  Sequence secondarySeq;
 
   EXPECT_CALL(
       this->fromPage(),
@@ -249,12 +251,26 @@ TEST_F(TracingTest, EmitsToFirstEligibleSessionWhenMultipleSessionsEnabled) {
       onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.tracingComplete"))))
       .InSequence(primarySeq);
 
-  // secondaryFusebox and secondaryNonFusebox should NOT receive anything
-  // (only first eligible session receives the trace)
-  EXPECT_CALL(secondaryFusebox.fromPage(), onMessage(_)).Times(0);
+  EXPECT_CALL(
+      secondaryFusebox.fromPage(),
+      onMessage(JsonParsed(
+          AtJsonPtr("/method", "ReactNativeApplication.traceRequested"))))
+      .InSequence(secondarySeq);
+  EXPECT_CALL(
+      secondaryFusebox.fromPage(),
+      onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.dataCollected"))))
+      .Times(AtLeast(1))
+      .InSequence(secondarySeq);
+  EXPECT_CALL(
+      secondaryFusebox.fromPage(),
+      onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.tracingComplete"))))
+      .InSequence(secondarySeq);
+
+  // secondaryNonFusebox should NOT receive anything (it did not enable the
+  // domain)
   EXPECT_CALL(secondaryNonFusebox.fromPage(), onMessage(_)).Times(0);
 
-  // Stop tracing and emit to first eligible session
+  // Stop tracing and emit to all eligible sessions
   EXPECT_TRUE(this->page_->stopAndMaybeEmitBackgroundTrace());
 }