Stash background trace directly in HostTarget, simplify API

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostInspectorTarget.kt

@@ -41,7 +41,7 @@ internal class ReactHostInspectorTarget(reactHostImpl: ReactHostImpl) :
 
   external fun stopAndMaybeEmitBackgroundTrace(): Boolean
 
-  external fun stopAndDiscardBackgroundTrace()
+  external fun stopTracing()
 
   external override fun getTracingState(): TracingState
 
@@ -65,7 +65,7 @@ internal class ReactHostInspectorTarget(reactHostImpl: ReactHostImpl) :
   }
 
   override fun stopBackgroundTrace() {
-    stopAndDiscardBackgroundTrace()
+    stopTracing()
   }
 
   fun handleNativePerfIssueAdded(

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp

@@ -73,13 +73,16 @@ JReactHostInspectorTarget::initHybrid(
 }
 
 void JReactHostInspectorTarget::sendDebuggerResumeCommand() {
+  inspectorTarget().sendCommand(HostCommand::DebuggerResume);
+}
+
+jsinspector_modern::HostTarget& JReactHostInspectorTarget::inspectorTarget() {
   if (inspectorTarget_) {
-    inspectorTarget_->sendCommand(HostCommand::DebuggerResume);
-  } else {
-    jni::throwNewJavaException(
-        "java/lang/IllegalStateException",
-        "Cannot send command while the Fusebox backend is not enabled");
+    return *inspectorTarget_;
   }
+  jni::throwNewJavaException(
+      "java/lang/IllegalStateException",
+      "Inspector method called while the Fusebox backend is not enabled.");
 }
 
 jsinspector_modern::HostTargetMetadata
@@ -147,58 +150,22 @@ HostTarget* JReactHostInspectorTarget::getInspectorTarget() {
 }
 
 bool JReactHostInspectorTarget::startBackgroundTrace() {
-  if (inspectorTarget_) {
-    return inspectorTarget_->startTracing(
-        tracing::Mode::Background,
-        {
-            tracing::Category::HiddenTimeline,
-            tracing::Category::RuntimeExecution,
-            tracing::Category::Timeline,
-            tracing::Category::UserTiming,
-        });
-  } else {
-    jni::throwNewJavaException(
-        "java/lang/IllegalStateException",
-        "Cannot start Tracing session while the Fusebox backend is not enabled.");
-  }
+  return inspectorTarget().startTracing(
+      tracing::Mode::Background,
+      {
+          tracing::Category::HiddenTimeline,
+          tracing::Category::RuntimeExecution,
+          tracing::Category::Timeline,
+          tracing::Category::UserTiming,
+      });
 }
 
-tracing::HostTracingProfile JReactHostInspectorTarget::stopTracing() {
-  if (inspectorTarget_) {
-    return inspectorTarget_->stopTracing();
-  } else {
-    jni::throwNewJavaException(
-        "java/lang/IllegalStateException",
-        "Cannot stop Tracing session while the Fusebox backend is not enabled.");
-  }
+void JReactHostInspectorTarget::stopTracing() {
+  inspectorTarget().stopTracing();
 }
 
 jboolean JReactHostInspectorTarget::stopAndMaybeEmitBackgroundTrace() {
-  auto capturedTrace = inspectorTarget_->stopTracing();
-  if (inspectorTarget_->hasActiveSessionWithFuseboxClient()) {
-    inspectorTarget_->emitTracingProfileForFirstFuseboxClient(
-        std::move(capturedTrace));
-    return jboolean(true);
-  }
-
-  stashTracingProfile(std::move(capturedTrace));
-  return jboolean(false);
-}
-
-void JReactHostInspectorTarget::stopAndDiscardBackgroundTrace() {
-  inspectorTarget_->stopTracing();
-}
-
-void JReactHostInspectorTarget::stashTracingProfile(
-    tracing::HostTracingProfile&& hostTracingProfile) {
-  stashedTracingProfile_ = std::move(hostTracingProfile);
-}
-
-std::optional<tracing::HostTracingProfile> JReactHostInspectorTarget::
-    unstable_getHostTracingProfileThatWillBeEmittedOnInitialization() {
-  auto tracingProfile = std::move(stashedTracingProfile_);
-  stashedTracingProfile_.reset();
-  return tracingProfile;
+  return jboolean(inspectorTarget().stopAndMaybeEmitBackgroundTrace());
 }
 
 void JReactHostInspectorTarget::registerNatives() {
@@ -213,9 +180,7 @@ void JReactHostInspectorTarget::registerNatives() {
       makeNativeMethod(
           "stopAndMaybeEmitBackgroundTrace",
           JReactHostInspectorTarget::stopAndMaybeEmitBackgroundTrace),
-      makeNativeMethod(
-          "stopAndDiscardBackgroundTrace",
-          JReactHostInspectorTarget::stopAndDiscardBackgroundTrace),
+      makeNativeMethod("stopTracing", JReactHostInspectorTarget::stopTracing),
       makeNativeMethod(
           "getTracingState", JReactHostInspectorTarget::getTracingState),
       makeNativeMethod(
@@ -256,7 +221,7 @@ HostTargetTracingDelegate* JReactHostInspectorTarget::getTracingDelegate() {
 
 void JReactHostInspectorTarget::recordFrameTimings(
     jni::alias_ref<JFrameTimingSequence::javaobject> frameTimingSequence) {
-  inspectorTarget_->recordFrameTimings({
+  inspectorTarget().recordFrameTimings({
       frameTimingSequence->getId(),
       frameTimingSequence->getThreadId(),
       frameTimingSequence->getBeginDrawingTimestamp(),

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h

@@ -240,7 +240,7 @@ class JReactHostInspectorTarget : public jni::HybridClass<JReactHostInspectorTar
   /**
    * Stops previously started trace recording and discards the captured trace.
    */
-  void stopAndDiscardBackgroundTrace();
+  void stopTracing();
 
   jsinspector_modern::HostTarget *getInspectorTarget();
 
@@ -277,15 +277,20 @@ class JReactHostInspectorTarget : public jni::HybridClass<JReactHostInspectorTar
   void loadNetworkResource(
       const jsinspector_modern::LoadNetworkResourceRequest &params,
       jsinspector_modern::ScopedExecutor<jsinspector_modern::NetworkRequestListener> executor) override;
-  std::optional<jsinspector_modern::tracing::HostTracingProfile>
-  unstable_getHostTracingProfileThatWillBeEmittedOnInitialization() override;
   jsinspector_modern::HostTargetTracingDelegate *getTracingDelegate() override;
 
  private:
   JReactHostInspectorTarget(
       jni::alias_ref<JReactHostInspectorTarget::javaobject> jobj,
       jni::alias_ref<JReactHostImpl> reactHostImpl,
       jni::alias_ref<JExecutor::javaobject> javaExecutor);
+
+  /**
+   * Returns a reference to the HostTarget, throwing a Java IllegalStateException
+   * if the Fusebox backend is not enabled (i.e., inspectorTarget_ is null).
+   */
+  jsinspector_modern::HostTarget &inspectorTarget();
+
   jni::global_ref<JReactHostInspectorTarget::javaobject> jobj_;
   // This weak reference breaks the cycle between the C++ HostTarget and the
   // Java ReactHostImpl, preventing memory leaks in apps that create multiple
@@ -296,20 +301,6 @@ class JReactHostInspectorTarget : public jni::HybridClass<JReactHostInspectorTar
   std::shared_ptr<jsinspector_modern::HostTarget> inspectorTarget_;
   std::optional<int> inspectorPageId_;
 
-  /**
-   * Stops previously started trace recording and returns the captured HostTracingProfile.
-   */
-  jsinspector_modern::tracing::HostTracingProfile stopTracing();
-  /**
-   * Stashes previously recorded HostTracingProfile that will be emitted when
-   * CDP session is created. Once emitted, the value will be cleared from this
-   * instance.
-   */
-  void stashTracingProfile(jsinspector_modern::tracing::HostTracingProfile &&hostTracingProfile);
-  /**
-   * Previously recorded HostTracingProfile that will be emitted when CDP session is created.
-   */
-  std::optional<jsinspector_modern::tracing::HostTracingProfile> stashedTracingProfile_;
   /**
    * Encapsulates the logic around tracing for this HostInspectorTarget.
    */

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

@@ -236,13 +236,11 @@ class HostAgent::Impl final {
         emitSystemStateChanged(isSingleHost);
       }
 
-      auto stashedTraceRecording =
-          targetController_.getDelegate()
-              .unstable_getHostTracingProfileThatWillBeEmittedOnInitialization();
-      if (stashedTraceRecording.has_value()) {
-        tracingAgent_.emitExternalHostTracingProfile(
-            std::move(stashedTraceRecording.value()));
-      }
+      auto emitted = targetController_.maybeEmitStashedBackgroundTrace();
+      assert(
+          emitted &&
+          "Expected to find at least one session eligible to receive a background trace after ReactNativeApplication.enable");
+      (void)emitted;
 
       return {
           .isFinishedHandlingRequest = true,
@@ -385,8 +383,7 @@ class HostAgent::Impl final {
     return fuseboxClientType_ == FuseboxClientType::Fusebox;
   }
 
-  void emitExternalTracingProfile(
-      tracing::HostTracingProfile tracingProfile) const {
+  void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) {
     assert(
         hasFuseboxClientConnected() &&
         "Attempted to emit a trace recording to a non-Fusebox client");
@@ -543,11 +540,11 @@ bool HostAgent::hasFuseboxClientConnected() const {
 }
 
 void HostAgent::emitExternalTracingProfile(
-    tracing::HostTracingProfile tracingProfile) const {
+    tracing::HostTracingProfile tracingProfile) {
   impl_->emitExternalTracingProfile(std::move(tracingProfile));
 }
 
-void HostAgent::emitSystemStateChanged(bool isSingleHost) const {
+void HostAgent::emitSystemStateChanged(bool isSingleHost) {
   impl_->emitSystemStateChanged(isSingleHost);
 }
 

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

@@ -77,13 +77,13 @@ class HostAgent final {
    * Emits the HostTracingProfile that was captured externally, not via the
    * CDP-initiated request.
    */
-  void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) const;
+  void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile);
 
   /**
    * Emits a system state changed event when the number of ReactHost instances
    * changes.
    */
-  void emitSystemStateChanged(bool isSingleHost) const;
+  void emitSystemStateChanged(bool isSingleHost);
 
  private:
   // We use the private implementation idiom to ensure this class has the same

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

@@ -102,18 +102,8 @@ class HostTargetSession {
     }
   }
 
-  /**
-   * Returns whether the ReactNativeApplication CDP domain is enabled.
-   *
-   * Chrome DevTools Frontend enables this domain as a client.
-   */
-  bool hasFuseboxClient() const {
-    return hostAgent_.hasFuseboxClientConnected();
-  }
-
-  void emitHostTracingProfile(
-      tracing::HostTracingProfile tracingProfile) const {
-    hostAgent_.emitExternalTracingProfile(std::move(tracingProfile));
+  HostAgent& agent() {
+    return hostAgent_;
   }
 
  private:
@@ -324,6 +314,10 @@ bool HostTargetController::decrementPauseOverlayCounter() {
   return --pauseOverlayCounter_ != 0;
 }
 
+bool HostTargetController::maybeEmitStashedBackgroundTrace() {
+  return target_.maybeEmitStashedBackgroundTrace();
+}
+
 namespace {
 
 struct StaticHostTargetMetadata {
@@ -375,32 +369,26 @@ folly::dynamic createHostMetadataPayload(const HostTargetMetadata& metadata) {
   return result;
 }
 
-bool HostTarget::hasActiveSessionWithFuseboxClient() const {
-  bool hasActiveFuseboxSession = false;
-  sessions_.forEach([&](HostTargetSession& session) {
-    hasActiveFuseboxSession |= session.hasFuseboxClient();
-  });
-  return hasActiveFuseboxSession;
-}
-
-void HostTarget::emitTracingProfileForFirstFuseboxClient(
-    tracing::HostTracingProfile tracingProfile) const {
+bool HostTarget::maybeEmitStashedBackgroundTrace() {
   bool emitted = false;
-  sessions_.forEach([&](HostTargetSession& session) {
+  sessions_.forEach([&](auto& session) {
     if (emitted) {
-      /**
-       * HostTracingProfile object is not copiable for performance reasons,
-       * because it could contain large Runtime sampling profile object.
-       *
-       * This approach would not work with multi-client debugger setup.
-       */
       return;
     }
-    if (session.hasFuseboxClient()) {
-      session.emitHostTracingProfile(std::move(tracingProfile));
+    if (session.agent().hasFuseboxClientConnected()) {
+      auto stashedTrace = std::exchange(stashedTracingProfile_, std::nullopt);
+      if (stashedTrace) {
+        session.agent().emitExternalTracingProfile(std::move(*stashedTrace));
+      }
       emitted = true;
     }
   });
+  return emitted;
+}
+
+bool HostTarget::stopAndMaybeEmitBackgroundTrace() {
+  stashedTracingProfile_ = stopTracing();
+  return maybeEmitStashedBackgroundTrace();
 }
 
 } // namespace facebook::react::jsinspector_modern