Handle Runtime.addBinding and Runtime.removeBinding before a RuntimeAgent is present (facebook#52813)

motiz88 · facebook-github-bot · commit 3271e57c751e · 2025-07-25T07:45:57.000-07:00
Summary: Pull Request resolved: facebook#52813 Changelog: [General][Changed] CDP backend now accepts addBinding and removeBinding methods earlier, before a Runtime exists. Allows managing CDP binding subscriptions at any point in time, including at the very beginning of a session when a `RuntimeAgent` doesn't yet exist (due to `registerRuntime` not having been called yet). "Adding" a binding with the `Runtime.addBinding` method is actually two actions in one: 1. Subscribing the current session to events from a particular binding name, from one or more Runtime(s) that may exist now or in the future (optionally targetable by execution context ID/name). 2. Installing a JSI function with the given `name` on the global object of the targeted Runtime(s), if they exist at the time of the method call. NOTE: "Removing" a binding only involves managing the subscription and never causes the corresponding JSI function to be uninstalled - presumably because it may have been captured by user code anyway. We currently do both (1) and (2) in `RuntimeAgent`, but this means subscriptions can't be managed before a `RuntimeTarget` has been created, which can cause problems if we want to set up certain bindings programmatically during React Native's initialisation. As this is an unnecessary restriction, here we move (1) to `HostAgent` and keep only (2) in `RuntimeAgent`. Reviewed By: huntie Differential Revision: D78739067 fbshipit-source-id: 9a39f4503d1b5e7c7a6e4c80dfbaabdd2549fb8d
diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp
@@ -141,9 +141,11 @@ class HostAgent::Impl final {
           .shouldSendOKResponse = true,
       };
     }
-    // Methods other than domain enables/disables: handle anything we know how
-    // to handle, and delegate to the InstanceAgent otherwise.
 
+    // Methods other than domain enables/disables: handle anything we know how
+    // to handle, and delegate to the InstanceAgent otherwise. (In some special
+    // cases we may handle the request *and* delegate to the InstanceAgent for
+    // some side effect.)
     if (req.method == "Page.reload") {
       targetController_.getDelegate().onReload({
           .ignoreCache =
@@ -207,6 +209,72 @@ class HostAgent::Impl final {
           .shouldSendOKResponse = true,
       };
     }
+    if (req.method == "Runtime.addBinding") {
+      // @cdp Runtime.addBinding and @cdp Runtime.removeBinding are explicitly
+      // supported at any time during a session, even while the JS runtime
+      // hasn't been created yet. For this reason they are handled by the
+      // HostAgent.
+      std::string bindingName = req.params["name"].getString();
+
+      ExecutionContextSelector contextSelector =
+          ExecutionContextSelector::all();
+
+      if (req.params.count("executionContextId") != 0u) {
+        auto executionContextId = req.params["executionContextId"].getInt();
+        if (executionContextId < (int64_t)std::numeric_limits<int32_t>::min() ||
+            executionContextId > (int64_t)std::numeric_limits<int32_t>::max()) {
+          frontendChannel_(cdp::jsonError(
+              req.id,
+              cdp::ErrorCode::InvalidParams,
+              "Invalid execution context id"));
+          return {
+              .isFinishedHandlingRequest = true,
+              .shouldSendOKResponse = false,
+          };
+        }
+        contextSelector =
+            ExecutionContextSelector::byId((int32_t)executionContextId);
+
+        if (req.params.count("executionContextName") != 0u) {
+          frontendChannel_(cdp::jsonError(
+              req.id,
+              cdp::ErrorCode::InvalidParams,
+              "executionContextName is mutually exclusive with executionContextId"));
+          return {
+              .isFinishedHandlingRequest = true,
+              .shouldSendOKResponse = false,
+          };
+        }
+      } else if (req.params.count("executionContextName") != 0u) {
+        contextSelector = ExecutionContextSelector::byName(
+            req.params["executionContextName"].getString());
+      }
+
+      sessionState_.subscribedBindings[bindingName].insert(contextSelector);
+
+      // We need this request to percolate down to the RuntimeAgent via the
+      // InstanceAgent. If there isn't a RuntimeAgent, it's OK: the next
+      // RuntimeAgent will pick up the binding via session state.
+      return {
+          .isFinishedHandlingRequest = false,
+          .shouldSendOKResponse = true,
+      };
+    }
+    if (req.method == "Runtime.removeBinding") {
+      // @cdp Runtime.removeBinding has no targeting by execution context. We
+      // interpret it to mean "unsubscribe, and stop installing the binding on
+      // all new contexts". This diverges slightly from V8, which continues
+      // to install the binding on new contexts after it's "removed", but *only*
+      // if the subscription is targeted by context name.
+      sessionState_.subscribedBindings.erase(req.params["name"].getString());
+
+      // Because of the above, we don't need to pass this request down to the
+      // RuntimeAgent.
+      return {
+          .isFinishedHandlingRequest = true,
+          .shouldSendOKResponse = true,
+      };
+    }
 
     return {
         .isFinishedHandlingRequest = false,
diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeAgent.cpp
@@ -39,56 +39,21 @@ bool RuntimeAgent::handleRequest(const cdp::PreparsedRequest& req) {
   if (req.method == "Runtime.addBinding") {
     std::string bindingName = req.params["name"].getString();
 
-    ExecutionContextSelector contextSelector = ExecutionContextSelector::all();
-
-    // TODO: Eventually, move execution context targeting out of RuntimeAgent.
-    // Right now, there's only ever one context (Runtime) in a Host, so we can
-    // handle it here for simplicity, and use session state to propagate
-    // bindings to the next RuntimeAgent.
-    if (req.params.count("executionContextId") != 0u) {
-      auto executionContextId = req.params["executionContextId"].getInt();
-      if (executionContextId < (int64_t)std::numeric_limits<int32_t>::min() ||
-          executionContextId > (int64_t)std::numeric_limits<int32_t>::max()) {
-        frontendChannel_(cdp::jsonError(
-            req.id,
-            cdp::ErrorCode::InvalidParams,
-            "Invalid execution context id"));
-        return true;
+    // Check if the binding has a context selector that matches the current
+    // context. The state will have been updated by HostAgent by the time we
+    // receive the request.
+    // NOTE: We DON'T do the reverse for removeBinding, for reasons explained
+    // in the implementation of HostAgent.
+    auto it = sessionState_.subscribedBindings.find(bindingName);
+    if (it != sessionState_.subscribedBindings.end()) {
+      auto contextSelectors = it->second;
+      if (matchesAny(executionContextDescription_, contextSelectors)) {
+        targetController_.installBindingHandler(bindingName);
       }
-      contextSelector =
-          ExecutionContextSelector::byId((int32_t)executionContextId);
-
-      if (req.params.count("executionContextName") != 0u) {
-        frontendChannel_(cdp::jsonError(
-            req.id,
-            cdp::ErrorCode::InvalidParams,
-            "executionContextName is mutually exclusive with executionContextId"));
-        return true;
-      }
-    } else if (req.params.count("executionContextName") != 0u) {
-      contextSelector = ExecutionContextSelector::byName(
-          req.params["executionContextName"].getString());
-    }
-    if (contextSelector.matches(executionContextDescription_)) {
-      targetController_.installBindingHandler(bindingName);
     }
-    sessionState_.subscribedBindings[bindingName].insert(contextSelector);
-
-    frontendChannel_(cdp::jsonResult(req.id));
-
-    return true;
-  }
-  if (req.method == "Runtime.removeBinding") {
-    // @cdp Runtime.removeBinding has no targeting by execution context. We
-    // interpret it to mean "unsubscribe, and stop installing the binding on
-    // all new contexts". This diverges slightly from V8, which continues
-    // to install the binding on new contexts after it's "removed", but *only*
-    // if the subscription is targeted by context name.
-    sessionState_.subscribedBindings.erase(req.params["name"].getString());
-
-    frontendChannel_(cdp::jsonResult(req.id));
 
-    return true;
+    // We are not responding to this request, just processing a side effect.
+    return false;
   }
   if (req.method == "Runtime.enable" && sessionState_.isLogDomainEnabled) {
     targetController_.notifyDebuggerSessionCreated();
diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/JsiIntegrationTest.cpp
@@ -385,6 +385,55 @@ TYPED_TEST(JsiIntegrationPortableTest, ReactNativeApplicationDisable) {
                                })");
 }
 
+#pragma region JsiIntegrationPortableTestWithEarlyAddedBinding
+// A test with some special setup to cover an edge case in the CDP backend:
+// bindings can be added before there's even a Runtime in the session, and
+// they should still work seamlessly once there is one.
+
+TYPED_TEST_SUITE(JsiIntegrationPortableTestWithEarlyAddedBinding, AllEngines);
+
+template <typename EngineAdapter>
+class JsiIntegrationPortableTestWithEarlyAddedBinding
+    : public JsiIntegrationPortableTest<EngineAdapter> {
+  void setupRuntimeBeforeRegistration(jsi::Runtime& /*unused*/) override {
+    // Connect early, before the RuntimeTarget is created. This session will not
+    // have a RuntimeAgent yet.
+    this->connect();
+
+    // Add a binding.
+    this->expectMessageFromPage(JsonEq(R"({
+                                          "id": 1,
+                                          "result": {}
+                                        })"));
+    this->toPage_->sendMessage(R"({
+                                 "id": 1,
+                                 "method": "Runtime.addBinding",
+                                 "params": {"name": "foo"}
+                               })");
+
+    // The binding is not installed yet, because the runtime is not registered
+    // with the CDP backend yet.
+    EXPECT_TRUE(this->eval("typeof globalThis.foo === 'undefined'").getBool());
+  }
+};
+
+TYPED_TEST(
+    JsiIntegrationPortableTestWithEarlyAddedBinding,
+    AddBindingBeforeRuntimeRegistered) {
+  // Now there is a RuntimeTarget / RuntimeAgent in the session, as per the
+  // normal test setup. Note that we're already connect()ed - not repeating that
+  // here.
+
+  // The binding is now installed, callable and working.
+  this->expectMessageFromPage(JsonParsed(AllOf(
+      AtJsonPtr("/method", "Runtime.bindingCalled"),
+      AtJsonPtr("/params/name", "foo"),
+      AtJsonPtr("/params/payload", "bar"))));
+  this->eval("globalThis.foo('bar');");
+}
+
+#pragma endregion // JsiIntegrationPortableTestWithEarlyAddedBinding
+
 #pragma endregion // AllEngines
 #pragma region AllHermesVariants
 
