Pass whole RuntimeTargetDelegate from RN instead of aggregating its m…

…ethods (#43346)

Summary:
Pull Request resolved: #43346

Changelog: [Internal]

(Continuing the theme of reducing integration boilerplate from D54537844.)

This diff changes both `JSExecutor` (Bridge) and `JSRuntime` (Bridgeless) to no longer implement `RuntimeTargetDelegate`. Instead, each of them exposes a `getRuntimeTargetDelegate()` method that returns a stable reference to a target delegate that it *owns*.

To facilitate this, we create a new `FallbackRuntimeTargetDelegate` for use in non-Hermes cases. This replaces *almost* all direct uses of `FallbackRuntimeAgentDelegate` outside of `jsinspector`. I'll follow up in a separate diff to deal with the last case and make the fallback agent delegate fully private.

As a result, changing the `RuntimeTargetDelegate` interface (which we'll need to do for console support) becomes much easier: we only have unit test mocks + two concrete `RuntimeTargetDelegate` implementations (one fallback, one Hermes) to update for each API change.

Reviewed By: huntie

Differential Revision: D54585658

fbshipit-source-id: 08b61c74008ddc36c2b134a40755ef8e43ab21ed

packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp

@@ -35,18 +35,12 @@ double JSExecutor::performanceNow() {
   return duration / NANOSECONDS_IN_MILLISECOND;
 }
 
-std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate>
-JSExecutor::createAgentDelegate(
-    jsinspector_modern::FrontendChannel frontendChannel,
-    jsinspector_modern::SessionState& sessionState,
-    std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate::ExportedState>,
-    const jsinspector_modern::ExecutionContextDescription&
-        executionContextDescription,
-    RuntimeExecutor runtimeExecutor) {
-  (void)executionContextDescription;
-  (void)runtimeExecutor;
-  return std::make_unique<jsinspector_modern::FallbackRuntimeAgentDelegate>(
-      std::move(frontendChannel), sessionState, getDescription());
+jsinspector_modern::RuntimeTargetDelegate&
+JSExecutor::getRuntimeTargetDelegate() {
+  if (!runtimeTargetDelegate_) {
+    runtimeTargetDelegate_.emplace(getDescription());
+  }
+  return *runtimeTargetDelegate_;
 }
 
 } // namespace facebook::react

packages/react-native/ReactCommon/cxxreact/JSExecutor.h

@@ -55,7 +55,7 @@ class JSExecutorFactory {
   virtual ~JSExecutorFactory() {}
 };
 
-class RN_EXPORT JSExecutor : public jsinspector_modern::RuntimeTargetDelegate {
+class RN_EXPORT JSExecutor {
  public:
   /**
    * Prepares the JS runtime for React Native by installing global variables.
@@ -130,7 +130,7 @@ class RN_EXPORT JSExecutor : public jsinspector_modern::RuntimeTargetDelegate {
   virtual void handleMemoryPressure([[maybe_unused]] int pressureLevel) {}
 
   virtual void destroy() {}
-  virtual ~JSExecutor() override {}
+  virtual ~JSExecutor() = default;
 
   virtual void flush() {}
 
@@ -141,17 +141,19 @@ class RN_EXPORT JSExecutor : public jsinspector_modern::RuntimeTargetDelegate {
   static double performanceNow();
 
   /**
-   * Create a RuntimeAgentDelegate that can be used to debug the JS VM instance.
+   * Get a reference to the \c RuntimeTargetDelegate owned (or implemented) by
+   * this executor. This reference must remain valid for the duration of the
+   * executor's lifetime.
    */
-  virtual std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate>
-  createAgentDelegate(
-      jsinspector_modern::FrontendChannel frontendChannel,
-      jsinspector_modern::SessionState& sessionState,
-      std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate::ExportedState>
-          previouslyExportedState,
-      const jsinspector_modern::ExecutionContextDescription&
-          executionContextDescription,
-      RuntimeExecutor runtimeExecutor) override;
+  virtual jsinspector_modern::RuntimeTargetDelegate& getRuntimeTargetDelegate();
+
+ private:
+  /**
+   * Initialized by \c getRuntimeTargetDelegate if not overridden, and then
+   * never changes.
+   */
+  std::optional<jsinspector_modern::FallbackRuntimeTargetDelegate>
+      runtimeTargetDelegate_;
 };
 
 } // namespace facebook::react

packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp

@@ -345,7 +345,7 @@ NativeToJsBridge::getDecoratedNativeMethodCallInvoker(
 
 jsinspector_modern::RuntimeTargetDelegate&
 NativeToJsBridge::getInspectorTargetDelegate() {
-  return *m_executor;
+  return m_executor->getRuntimeTargetDelegate();
 }
 
 } // namespace facebook::react

packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.cpp

@@ -256,21 +256,9 @@ HermesExecutor::HermesExecutor(
       targetDelegate_{
           std::shared_ptr<HermesRuntime>(runtime_, &hermesRuntime)} {}
 
-std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate>
-HermesExecutor::createAgentDelegate(
-    jsinspector_modern::FrontendChannel frontendChannel,
-    jsinspector_modern::SessionState& sessionState,
-    std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate::ExportedState>
-        previouslyExportedState,
-    const jsinspector_modern::ExecutionContextDescription&
-        executionContextDescription,
-    RuntimeExecutor runtimeExecutor) {
-  return targetDelegate_.createAgentDelegate(
-      std::move(frontendChannel),
-      sessionState,
-      std::move(previouslyExportedState),
-      executionContextDescription,
-      std::move(runtimeExecutor));
+jsinspector_modern::RuntimeTargetDelegate&
+HermesExecutor::getRuntimeTargetDelegate() {
+  return targetDelegate_;
 }
 
 } // namespace facebook::react

packages/react-native/ReactCommon/hermes/executor/HermesExecutorFactory.h

@@ -55,15 +55,8 @@ class HermesExecutor : public JSIExecutor {
       RuntimeInstaller runtimeInstaller,
       hermes::HermesRuntime& hermesRuntime);
 
-  virtual std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate>
-  createAgentDelegate(
-      jsinspector_modern::FrontendChannel frontendChannel,
-      jsinspector_modern::SessionState& sessionState,
-      std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate::ExportedState>
-          previouslyExportedState,
-      const jsinspector_modern::ExecutionContextDescription&
-          executionContextDescription,
-      RuntimeExecutor runtimeExecutor) override;
+  jsinspector_modern::RuntimeTargetDelegate& getRuntimeTargetDelegate()
+      override;
 
  private:
   JSIScopedTimeoutInvoker timeoutInvoker_;

packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeAgentDelegate.cpp

@@ -14,6 +14,9 @@
 #include <hermes/inspector/RuntimeAdapter.h>
 #include <hermes/inspector/chrome/CDPHandler.h>
 #else // HERMES_ENABLE_DEBUGGER
+// TODO(moti): FallbackRuntimeAgentDelegate should be private. We should fall
+// back at the *TargetDelegate* level, in HermesRuntimeTargetDelegate, rather
+// than within HermesRuntimeAgentDelegate.
 #include <jsinspector-modern/FallbackRuntimeAgentDelegate.h>
 #endif // HERMES_ENABLE_DEBUGGER
 

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

@@ -0,0 +1,29 @@
+/*
+ * 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.
+ */
+
+#include "FallbackRuntimeTargetDelegate.h"
+#include "FallbackRuntimeAgentDelegate.h"
+
+namespace facebook::react::jsinspector_modern {
+
+FallbackRuntimeTargetDelegate::FallbackRuntimeTargetDelegate(
+    std::string engineDescription)
+    : engineDescription_{std::move(engineDescription)} {}
+
+std::unique_ptr<RuntimeAgentDelegate>
+FallbackRuntimeTargetDelegate::createAgentDelegate(
+    FrontendChannel channel,
+    SessionState& sessionState,
+    std::unique_ptr<RuntimeAgentDelegate::ExportedState>
+    /*previouslyExportedState*/,
+    const ExecutionContextDescription& /*executionContextDescription*/,
+    RuntimeExecutor /*runtimeExecutor*/) {
+  return std::make_unique<jsinspector_modern::FallbackRuntimeAgentDelegate>(
+      std::move(channel), sessionState, engineDescription_);
+}
+
+} // namespace facebook::react::jsinspector_modern

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

@@ -0,0 +1,38 @@
+/*
+ * 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 "RuntimeTarget.h"
+#include "SessionState.h"
+
+#include <string>
+
+namespace facebook::react::jsinspector_modern {
+
+/**
+ * A RuntimeTargetDelegate that stubs out debugging functionality for a
+ * JavaScript runtime that does not natively support debugging.
+ */
+class FallbackRuntimeTargetDelegate : public RuntimeTargetDelegate {
+ public:
+  explicit FallbackRuntimeTargetDelegate(std::string engineDescription);
+
+  std::unique_ptr<RuntimeAgentDelegate> createAgentDelegate(
+      FrontendChannel channel,
+      SessionState& sessionState,
+      std::unique_ptr<RuntimeAgentDelegate::ExportedState>
+          previouslyExportedState,
+      const ExecutionContextDescription& executionContextDescription,
+      RuntimeExecutor runtimeExecutor) override;
+
+ private:
+  std::string engineDescription_;
+};
+
+} // namespace facebook::react::jsinspector_modern

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

@@ -8,7 +8,7 @@
 #pragma once
 
 #include <jsinspector-modern/ExecutionContext.h>
-#include <jsinspector-modern/FallbackRuntimeAgentDelegate.h>
+#include <jsinspector-modern/FallbackRuntimeTargetDelegate.h>
 #include <jsinspector-modern/HostTarget.h>
 #include <jsinspector-modern/InstanceTarget.h>
 #include <jsinspector-modern/RuntimeTarget.h>

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

@@ -55,7 +55,8 @@ class JsiIntegrationPortableTest : public Test, private HostTargetDelegate {
   JsiIntegrationPortableTest() : engineAdapter_{immediateExecutor_} {
     instance_ = &page_->registerInstance(instanceTargetDelegate_);
     runtimeTarget_ = &instance_->registerRuntime(
-        *engineAdapter_, engineAdapter_->getRuntimeExecutor());
+        engineAdapter_->getRuntimeTargetDelegate(),
+        engineAdapter_->getRuntimeExecutor());
   }
 
   ~JsiIntegrationPortableTest() override {
@@ -96,7 +97,8 @@ class JsiIntegrationPortableTest : public Test, private HostTargetDelegate {
     engineAdapter_.emplace(immediateExecutor_);
     instance_ = &page_->registerInstance(instanceTargetDelegate_);
     runtimeTarget_ = &instance_->registerRuntime(
-        *engineAdapter_, engineAdapter_->getRuntimeExecutor());
+        engineAdapter_->getRuntimeTargetDelegate(),
+        engineAdapter_->getRuntimeExecutor());
   }
 
   MockRemoteConnection& fromPage() {

packages/react-native/ReactCommon/jsinspector-modern/tests/engines/JsiIntegrationTestGenericEngineAdapter.cpp

@@ -18,20 +18,14 @@ namespace facebook::react::jsinspector_modern {
 
 JsiIntegrationTestGenericEngineAdapter::JsiIntegrationTestGenericEngineAdapter(
     folly::Executor& jsExecutor)
-    : runtime_{hermes::makeHermesRuntime()}, jsExecutor_{jsExecutor} {}
-
-std::unique_ptr<RuntimeAgentDelegate>
-JsiIntegrationTestGenericEngineAdapter::createAgentDelegate(
-    FrontendChannel frontendChannel,
-    SessionState& sessionState,
-    std::unique_ptr<RuntimeAgentDelegate::ExportedState>,
-    const ExecutionContextDescription& /*executionContextDescription*/,
-    RuntimeExecutor /*runtimeExecutor*/) {
-  return std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate>(
-      new FallbackRuntimeAgentDelegate(
-          frontendChannel,
-          sessionState,
-          "Generic engine (" + runtime_->description() + ")"));
+    : runtime_{hermes::makeHermesRuntime()},
+      jsExecutor_{jsExecutor},
+      runtimeTargetDelegate_{
+          "Generic engine (" + runtime_->description() + ")"} {}
+
+RuntimeTargetDelegate&
+JsiIntegrationTestGenericEngineAdapter::getRuntimeTargetDelegate() {
+  return runtimeTargetDelegate_;
 }
 
 jsi::Runtime& JsiIntegrationTestGenericEngineAdapter::getRuntime()

packages/react-native/ReactCommon/jsinspector-modern/tests/engines/JsiIntegrationTestGenericEngineAdapter.h

@@ -7,6 +7,7 @@
 
 #pragma once
 
+#include <jsinspector-modern/FallbackRuntimeTargetDelegate.h>
 #include <jsinspector-modern/RuntimeTarget.h>
 
 #include <folly/executors/QueuedImmediateExecutor.h>
@@ -21,17 +22,11 @@ namespace facebook::react::jsinspector_modern {
  * JSI-compatible engine, with no engine-specific CDP support. Uses Hermes under
  * the hood, without Hermes's CDP support.
  */
-class JsiIntegrationTestGenericEngineAdapter : public RuntimeTargetDelegate {
+class JsiIntegrationTestGenericEngineAdapter {
  public:
   explicit JsiIntegrationTestGenericEngineAdapter(folly::Executor& jsExecutor);
 
-  virtual std::unique_ptr<RuntimeAgentDelegate> createAgentDelegate(
-      FrontendChannel frontendChannel,
-      SessionState& sessionState,
-      std::unique_ptr<RuntimeAgentDelegate::ExportedState>
-          previouslyExportedState,
-      const ExecutionContextDescription& executionContextDescription,
-      RuntimeExecutor runtimeExecutor) override;
+  RuntimeTargetDelegate& getRuntimeTargetDelegate();
 
   jsi::Runtime& getRuntime() const noexcept;
 
@@ -40,6 +35,7 @@ class JsiIntegrationTestGenericEngineAdapter : public RuntimeTargetDelegate {
  private:
   std::unique_ptr<jsi::Runtime> runtime_;
   folly::Executor& jsExecutor_;
+  jsinspector_modern::FallbackRuntimeTargetDelegate runtimeTargetDelegate_;
 };
 
 } // namespace facebook::react::jsinspector_modern

packages/react-native/ReactCommon/jsinspector-modern/tests/engines/JsiIntegrationTestHermesEngineAdapter.cpp

@@ -17,22 +17,11 @@ JsiIntegrationTestHermesEngineAdapter::JsiIntegrationTestHermesEngineAdapter(
     folly::Executor& jsExecutor)
     : runtime_{hermes::makeHermesRuntime()},
       jsExecutor_{jsExecutor},
-      targetDelegate_{runtime_} {}
-
-std::unique_ptr<RuntimeAgentDelegate>
-JsiIntegrationTestHermesEngineAdapter::createAgentDelegate(
-    FrontendChannel frontendChannel,
-    SessionState& sessionState,
-    std::unique_ptr<RuntimeAgentDelegate::ExportedState>
-        previouslyExportedState,
-    const ExecutionContextDescription& executionContextDescription,
-    RuntimeExecutor runtimeExecutor) {
-  return targetDelegate_.createAgentDelegate(
-      std::move(frontendChannel),
-      sessionState,
-      std::move(previouslyExportedState),
-      executionContextDescription,
-      std::move(runtimeExecutor));
+      runtimeTargetDelegate_{runtime_} {}
+
+RuntimeTargetDelegate&
+JsiIntegrationTestHermesEngineAdapter::getRuntimeTargetDelegate() {
+  return runtimeTargetDelegate_;
 }
 
 jsi::Runtime& JsiIntegrationTestHermesEngineAdapter::getRuntime()

packages/react-native/ReactCommon/jsinspector-modern/tests/engines/JsiIntegrationTestHermesEngineAdapter.h

@@ -22,17 +22,11 @@ namespace facebook::react::jsinspector_modern {
  * An engine adapter for JsiIntegrationTest that uses Hermes (and Hermes's
  * CDP support).
  */
-class JsiIntegrationTestHermesEngineAdapter : public RuntimeTargetDelegate {
+class JsiIntegrationTestHermesEngineAdapter {
  public:
   explicit JsiIntegrationTestHermesEngineAdapter(folly::Executor& jsExecutor);
 
-  virtual std::unique_ptr<RuntimeAgentDelegate> createAgentDelegate(
-      FrontendChannel frontendChannel,
-      SessionState& sessionState,
-      std::unique_ptr<RuntimeAgentDelegate::ExportedState>
-          previouslyExportedState,
-      const ExecutionContextDescription& executionContextDescription,
-      RuntimeExecutor runtimeExecutor) override;
+  RuntimeTargetDelegate& getRuntimeTargetDelegate();
 
   jsi::Runtime& getRuntime() const noexcept;
 
@@ -41,7 +35,7 @@ class JsiIntegrationTestHermesEngineAdapter : public RuntimeTargetDelegate {
  private:
   std::shared_ptr<facebook::hermes::HermesRuntime> runtime_;
   folly::Executor& jsExecutor_;
-  HermesRuntimeTargetDelegate targetDelegate_;
+  HermesRuntimeTargetDelegate runtimeTargetDelegate_;
 };
 
 } // namespace facebook::react::jsinspector_modern

packages/react-native/ReactCommon/react/runtime/JSRuntimeFactory.cpp

@@ -18,18 +18,12 @@ JSIRuntimeHolder::JSIRuntimeHolder(std::unique_ptr<jsi::Runtime> runtime)
   assert(runtime_ != nullptr);
 }
 
-std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate>
-JSIRuntimeHolder::createAgentDelegate(
-    jsinspector_modern::FrontendChannel frontendChannel,
-    jsinspector_modern::SessionState& sessionState,
-    std::unique_ptr<jsinspector_modern::RuntimeAgentDelegate::ExportedState>,
-    const jsinspector_modern::ExecutionContextDescription&
-        executionContextDescription,
-    RuntimeExecutor runtimeExecutor) {
-  (void)executionContextDescription;
-  (void)runtimeExecutor;
-  return std::make_unique<jsinspector_modern::FallbackRuntimeAgentDelegate>(
-      std::move(frontendChannel), sessionState, runtime_->description());
+jsinspector_modern::RuntimeTargetDelegate&
+JSRuntime::getRuntimeTargetDelegate() {
+  if (!runtimeTargetDelegate_) {
+    runtimeTargetDelegate_.emplace(getRuntime().description());
+  }
+  return *runtimeTargetDelegate_;
 }
 
 } // namespace facebook::react