Register CatalystInstanceImpl with modern CDP backend (2/2) (facebook…

…#43251)

Summary:
Pull Request resolved: facebook#43251

Integrates the modern CDP backend with `CatalystInstanceImpl` (the React Native instance implementation) on Android.

This complete the modern CDP integration for Bridge.

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D51458010

fbshipit-source-id: 6f73868da9d0d4cc5d086a4569c78444cb1b83ec

packages/react-native/ReactAndroid/api/ReactAndroid.api

@@ -619,6 +619,7 @@ public class com/facebook/react/bridge/CatalystInstanceImpl : com/facebook/react
 public class com/facebook/react/bridge/CatalystInstanceImpl$Builder {
 	public fun <init> ()V
 	public fun build ()Lcom/facebook/react/bridge/CatalystInstanceImpl;
+	public fun setInspectorTarget (Lcom/facebook/react/bridge/ReactInstanceManagerInspectorTarget;)Lcom/facebook/react/bridge/CatalystInstanceImpl$Builder;
 	public fun setJSBundleLoader (Lcom/facebook/react/bridge/JSBundleLoader;)Lcom/facebook/react/bridge/CatalystInstanceImpl$Builder;
 	public fun setJSExceptionHandler (Lcom/facebook/react/bridge/JSExceptionHandler;)Lcom/facebook/react/bridge/CatalystInstanceImpl$Builder;
 	public fun setJSExecutor (Lcom/facebook/react/bridge/JavaScriptExecutor;)Lcom/facebook/react/bridge/CatalystInstanceImpl$Builder;

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java

@@ -1360,15 +1360,14 @@ private ReactApplicationContext createReactContext(
 
     NativeModuleRegistry nativeModuleRegistry = processPackages(reactContext, mPackages);
 
-    getOrCreateInspectorTarget();
-
     CatalystInstanceImpl.Builder catalystInstanceBuilder =
         new CatalystInstanceImpl.Builder()
             .setReactQueueConfigurationSpec(ReactQueueConfigurationSpec.createDefault())
             .setJSExecutor(jsExecutor)
             .setRegistry(nativeModuleRegistry)
             .setJSBundleLoader(jsBundleLoader)
-            .setJSExceptionHandler(exceptionHandler);
+            .setJSExceptionHandler(exceptionHandler)
+            .setInspectorTarget(getOrCreateInspectorTarget());
 
     ReactMarker.logMarker(CREATE_CATALYST_INSTANCE_START);
     // CREATE_CATALYST_INSTANCE_END is in JSCExecutor.cpp

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java

@@ -119,7 +119,8 @@ private CatalystInstanceImpl(
       final JavaScriptExecutor jsExecutor,
       final NativeModuleRegistry nativeModuleRegistry,
       final JSBundleLoader jsBundleLoader,
-      JSExceptionHandler jSExceptionHandler) {
+      JSExceptionHandler jSExceptionHandler,
+      @Nullable ReactInstanceManagerInspectorTarget inspectorTarget) {
     FLog.d(ReactConstants.TAG, "Initializing React Xplat Bridge.");
     Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "createCatalystInstanceImpl");
 
@@ -146,7 +147,8 @@ private CatalystInstanceImpl(
         mReactQueueConfiguration.getJSQueueThread(),
         mNativeModulesQueueThread,
         mNativeModuleRegistry.getJavaModules(this),
-        mNativeModuleRegistry.getCxxModules());
+        mNativeModuleRegistry.getCxxModules(),
+        inspectorTarget);
     FLog.d(ReactConstants.TAG, "Initializing React Xplat Bridge after initializeBridge");
     Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
 
@@ -214,7 +216,8 @@ private native void initializeBridge(
       MessageQueueThread jsQueue,
       MessageQueueThread moduleQueue,
       Collection<JavaModuleWrapper> javaModules,
-      Collection<ModuleHolder> cxxModules);
+      Collection<ModuleHolder> cxxModules,
+      @Nullable ReactInstanceManagerInspectorTarget inspectorTarget);
 
   @Override
   public void setSourceURLs(String deviceURL, String remoteURL) {
@@ -329,6 +332,8 @@ public void invokeCallback(final int callbackID, final NativeArrayInterface argu
     jniCallJSCallback(callbackID, (NativeArray) arguments);
   }
 
+  private native void unregisterFromInspector();
+
   /**
    * Destroys this catalyst instance, waiting for any other threads in ReactQueueConfiguration
    * (besides the UI thread) to finish running. Must be called from the UI thread so that we can
@@ -343,6 +348,8 @@ public void destroy() {
       return;
     }
 
+    unregisterFromInspector();
+
     // TODO: tell all APIs to shut down
     ReactMarker.logMarker(ReactMarkerConstants.DESTROY_CATALYST_INSTANCE_START);
     mDestroyed = true;
@@ -655,6 +662,7 @@ public static class Builder {
     private @Nullable NativeModuleRegistry mRegistry;
     private @Nullable JavaScriptExecutor mJSExecutor;
     private @Nullable JSExceptionHandler mJSExceptionHandler;
+    private @Nullable ReactInstanceManagerInspectorTarget mInspectorTarget;
 
     public Builder setReactQueueConfigurationSpec(
         ReactQueueConfigurationSpec ReactQueueConfigurationSpec) {
@@ -682,13 +690,20 @@ public Builder setJSExceptionHandler(JSExceptionHandler handler) {
       return this;
     }
 
+    public Builder setInspectorTarget(
+        @Nullable ReactInstanceManagerInspectorTarget inspectorTarget) {
+      mInspectorTarget = inspectorTarget;
+      return this;
+    }
+
     public CatalystInstanceImpl build() {
       return new CatalystInstanceImpl(
           Assertions.assertNotNull(mReactQueueConfigurationSpec),
           Assertions.assertNotNull(mJSExecutor),
           Assertions.assertNotNull(mRegistry),
           Assertions.assertNotNull(mJSBundleLoader),
-          Assertions.assertNotNull(mJSExceptionHandler));
+          Assertions.assertNotNull(mJSExceptionHandler),
+          mInspectorTarget);
     }
   }
 }

packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp

@@ -6,6 +6,7 @@
  */
 
 #include "CatalystInstanceImpl.h"
+#include "ReactInstanceManagerInspectorTarget.h"
 
 #include <fstream>
 #include <memory>
@@ -125,6 +126,9 @@ void CatalystInstanceImpl::registerNatives() {
           "getRuntimeExecutor", CatalystInstanceImpl::getRuntimeExecutor),
       makeNativeMethod(
           "getRuntimeScheduler", CatalystInstanceImpl::getRuntimeScheduler),
+      makeNativeMethod(
+          "unregisterFromInspector",
+          CatalystInstanceImpl::unregisterFromInspector),
   });
 }
 
@@ -157,7 +161,9 @@ void CatalystInstanceImpl::initializeBridge(
     jni::alias_ref<jni::JCollection<JavaModuleWrapper::javaobject>::javaobject>
         javaModules,
     jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject>
-        cxxModules) {
+        cxxModules,
+    jni::alias_ref<ReactInstanceManagerInspectorTarget::javaobject>
+        inspectorTarget) {
   set_react_native_logfunc(&log);
 
   // TODO mhorowitz: how to assert here?
@@ -193,7 +199,10 @@ void CatalystInstanceImpl::initializeBridge(
       std::make_unique<InstanceCallbackImpl>(callback),
       jseh->getExecutorFactory(),
       std::make_unique<JMessageQueueThread>(jsQueue),
-      moduleRegistry_);
+      moduleRegistry_,
+      inspectorTarget != nullptr
+          ? inspectorTarget->cthis()->getInspectorTarget()
+          : nullptr);
 }
 
 void CatalystInstanceImpl::extendNativeModules(
@@ -406,4 +415,8 @@ CatalystInstanceImpl::getRuntimeScheduler() {
   return runtimeScheduler_;
 }
 
+void CatalystInstanceImpl::unregisterFromInspector() {
+  instance_->unregisterFromInspector();
+}
+
 } // namespace facebook::react

packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h

@@ -19,6 +19,7 @@
 #include "JSLoader.h"
 #include "JavaModuleWrapper.h"
 #include "ModuleRegistryBuilder.h"
+#include "ReactInstanceManagerInspectorTarget.h"
 
 namespace facebook::react {
 
@@ -59,7 +60,9 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
           jni::JCollection<JavaModuleWrapper::javaobject>::javaobject>
           javaModules,
       jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject>
-          cxxModules);
+          cxxModules,
+      jni::alias_ref<ReactInstanceManagerInspectorTarget::javaobject>
+          inspectorTarget);
 
   void extendNativeModules(
       jni::alias_ref<jni::JCollection<
@@ -102,6 +105,13 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
 
   void createAndInstallRuntimeSchedulerIfNecessary();
 
+  /**
+   * Unregisters the instance from the inspector. This method must be called
+   * on the main thread, after initializeBridge has finished executing and
+   * before the destructor for Instance has started.
+   */
+  void unregisterFromInspector();
+
   // This should be the only long-lived strong reference, but every C++ class
   // will have a weak reference.
   std::shared_ptr<Instance> instance_;

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

@@ -58,32 +58,67 @@ void Instance::initializeBridge(
   callback_ = std::move(callback);
   moduleRegistry_ = std::move(moduleRegistry);
   parentInspectorTarget_ = parentInspectorTarget;
-  jsQueue->runOnQueueSync(
-      [this, &jsef, jsQueue, parentInspectorTarget]() mutable {
-        nativeToJsBridge_ = std::make_shared<NativeToJsBridge>(
-            jsef.get(), moduleRegistry_, jsQueue, callback_);
-
-        nativeToJsBridge_->initializeRuntime();
-
-        if (parentInspectorTarget) {
-          inspectorTarget_ = &parentInspectorTarget->registerInstance(*this);
-          RuntimeExecutor runtimeExecutorIfJsi = getRuntimeExecutor();
-          runtimeInspectorTarget_ = &inspectorTarget_->registerRuntime(
-              nativeToJsBridge_->getInspectorTargetDelegate(),
-              runtimeExecutorIfJsi ? runtimeExecutorIfJsi : [](auto) {});
+
+  jsQueue->runOnQueueSync([this, &jsef, jsQueue]() mutable {
+    nativeToJsBridge_ = std::make_shared<NativeToJsBridge>(
+        jsef.get(), moduleRegistry_, jsQueue, callback_);
+
+    // If a parent inspector HostTarget is provided, perform inspector
+    // initialization synchronously.
+    if (parentInspectorTarget_ != nullptr) {
+      auto inspectorExecutor = parentInspectorTarget_->executorFromThis();
+      std::mutex inspectorInitializedMutex;
+      std::condition_variable inspectorInitializedCv;
+      bool inspectorInitialized = false;
+
+      // Schedule work on the inspector thread. inspectorExecutor is guaranteed
+      // to execute this callback, so we can safely run this logic
+      // synchronously and reference parentInspectorTarget_.
+      inspectorExecutor([this,
+                         &inspectorInitialized,
+                         &inspectorInitializedMutex,
+                         &inspectorInitializedCv](
+                            jsinspector_modern::HostTarget& hostTarget) {
+        // NOTE: By passing *this, we strongly assume the Instance will still
+        // be alive by the time this executes.
+        // - On iOS, instance creation is done syncrhonously
+        //   (RCTCxxBridge#_initializeBridgeLocked).
+        // - On Android, we explicitly wait for instance creation before
+        //   destruction (ReactInstanceManager#mReactContextLock).
+        inspectorTarget_ = &hostTarget.registerInstance(*this);
+        RuntimeExecutor runtimeExecutorIfJsi = getRuntimeExecutor();
+        runtimeInspectorTarget_ = &inspectorTarget_->registerRuntime(
+            nativeToJsBridge_->getInspectorTargetDelegate(),
+            runtimeExecutorIfJsi ? runtimeExecutorIfJsi : [](auto) {});
+
+        // Signal that initialization is complete
+        {
+          std::lock_guard lock(inspectorInitializedMutex);
+          inspectorInitialized = true;
         }
+        inspectorInitializedCv.notify_one();
+      });
+
+      // Wait for the initialization work to complete
+      {
+        std::unique_lock lock(inspectorInitializedMutex);
+        inspectorInitializedCv.wait(
+            lock, [&inspectorInitialized] { return inspectorInitialized; });
+      }
+    }
 
-        /**
-         * After NativeToJsBridge is created, the jsi::Runtime should exist.
-         * Also, the JS message queue thread exists. So, it's safe to
-         * schedule all queued up js Calls.
-         */
-        jsCallInvoker_->setNativeToJsBridgeAndFlushCalls(nativeToJsBridge_);
+    // Initialize the JavaScript runtime after we've initialized the inspector
+    nativeToJsBridge_->initializeRuntime();
 
-        std::scoped_lock lock(m_syncMutex);
-        m_syncReady = true;
-        m_syncCV.notify_all();
-      });
+    // After NativeToJsBridge is created, the jsi::Runtime should exist. Also,
+    // the JS message queue thread exists. So, it's safe to schedule all queued
+    // up JS calls.
+    jsCallInvoker_->setNativeToJsBridgeAndFlushCalls(nativeToJsBridge_);
+
+    std::scoped_lock lock(m_syncMutex);
+    m_syncReady = true;
+    m_syncCV.notify_all();
+  });
 
   CHECK(nativeToJsBridge_);
 }