From 11394e7c88fc09fb27e315810543cd600cabdbb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Wed, 28 Feb 2024 09:53:18 -0800 Subject: [PATCH] Register ReactInstance with modern CDP backend Summary: Changelog: [Internal] This implements the integration of `ReactInstance` with the modern CDP backend. This is the last missing piece to complete the integration of bridgeless with the modern CDP backend, and now we can test it end to end. Reviewed By: huntie Differential Revision: D51459050 fbshipit-source-id: 54e8972ee199cbcc8e5e73d7215a34f008feeaa3 --- .../facebook/react/runtime/ReactHostImpl.java | 11 ++- .../facebook/react/runtime/ReactInstance.java | 12 ++- .../jni/react/runtime/jni/JReactInstance.cpp | 22 ++++-- .../jni/react/runtime/jni/JReactInstance.h | 11 ++- .../react/runtime/ReactInstance.cpp | 73 +++++++++++++------ 5 files changed, 95 insertions(+), 34 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java index 93fe93ec08f..e3f9a864883 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java @@ -915,7 +915,8 @@ public class ReactHostImpl implements ReactHost { devSupportManager, mQueueThreadExceptionHandler, mReactJsExceptionHandler, - mUseDevSupport); + mUseDevSupport, + mReactHostInspectorTarget); if (ReactFeatureFlags .unstable_bridgelessArchitectureMemoryPressureHackyBoltsFix) { @@ -1200,6 +1201,10 @@ public class ReactHostImpl implements ReactHost { final ReactInstance reactInstance = reactInstanceTaskUnwrapper.unwrap(task, "1: Starting reload"); + if (reactInstance != null) { + reactInstance.unregisterFromInspector(); + } + final ReactContext reactContext = mBridgelessReactContextRef.getNullable(); if (reactContext == null) { raiseSoftException(method, "ReactContext is null. Reload reason: " + reason); @@ -1373,6 +1378,10 @@ public class ReactHostImpl implements ReactHost { final ReactInstance reactInstance = reactInstanceTaskUnwrapper.unwrap(task, "1: Starting destroy"); + if (reactInstance != null) { + reactInstance.unregisterFromInspector(); + } + // Step 1: Destroy DevSupportManager if (mUseDevSupport) { log(method, "DevSupportManager cleanup"); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java index 37d8d44d830..8f71f3ca724 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java @@ -114,7 +114,8 @@ final class ReactInstance { DevSupportManager devSupportManager, QueueThreadExceptionHandler exceptionHandler, ReactJsExceptionHandler reactExceptionManager, - boolean useDevSupport) { + boolean useDevSupport, + ReactHostInspectorTarget reactHostInspectorTarget) { mBridgelessReactContext = bridgelessReactContext; mDelegate = delegate; @@ -182,7 +183,8 @@ final class ReactInstance { jsTimerExecutor, reactExceptionManager, bindingsInstaller, - isProfiling); + isProfiling, + reactHostInspectorTarget); mJavaScriptContextHolder = new JavaScriptContextHolder(getJavaScriptContext()); @@ -463,7 +465,8 @@ final class ReactInstance { JSTimerExecutor jsTimerExecutor, ReactJsExceptionHandler jReactExceptionsManager, @Nullable BindingsInstaller jBindingsInstaller, - boolean isProfiling); + boolean isProfiling, + ReactHostInspectorTarget reactHostInspectorTarget); @DoNotStrip private static native JSTimerExecutor createJSTimerExecutor(); @@ -494,6 +497,9 @@ final class ReactInstance { private native void handleMemoryPressureJs(int pressureLevel); + @ThreadConfined(ThreadConfined.UI) + /* package */ native void unregisterFromInspector(); + public void handleMemoryPressure(int level) { try { handleMemoryPressureJs(level); diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.cpp index ce3bc7b613e..0e789a5eec5 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.cpp @@ -36,7 +36,9 @@ JReactInstance::JReactInstance( jni::alias_ref jsTimerExecutor, jni::alias_ref jReactExceptionManager, jni::alias_ref jBindingsInstaller, - bool isProfiling) noexcept { + bool isProfiling, + jni::alias_ref + jReactHostInspectorTarget) noexcept { // TODO(janzer): Lazily create runtime auto sharedJSMessageQueueThread = std::make_shared(jsMessageQueueThread); @@ -64,7 +66,8 @@ JReactInstance::JReactInstance( jsRuntimeFactory->cthis()->createJSRuntime(sharedJSMessageQueueThread), sharedJSMessageQueueThread, timerManager, - std::move(jsErrorHandlingFunc)); + std::move(jsErrorHandlingFunc), + jReactHostInspectorTarget->cthis()->getInspectorTarget()); auto bufferedRuntimeExecutor = instance_->getBufferedRuntimeExecutor(); timerManager->setRuntimeExecutor(bufferedRuntimeExecutor); @@ -115,7 +118,9 @@ jni::local_ref JReactInstance::initHybrid( jni::alias_ref jsTimerExecutor, jni::alias_ref jReactExceptionManager, jni::alias_ref jBindingsInstaller, - bool isProfiling) { + bool isProfiling, + jni::alias_ref + jReactHostInspectorTarget) { return makeCxxInstance( jsRuntimeFactory, jsMessageQueueThread, @@ -124,7 +129,8 @@ jni::local_ref JReactInstance::initHybrid( jsTimerExecutor, jReactExceptionManager, jBindingsInstaller, - isProfiling); + isProfiling, + jReactHostInspectorTarget); } void JReactInstance::loadJSBundleFromAssets( @@ -205,6 +211,10 @@ jlong JReactInstance::getJavaScriptContext() { return (jlong)(intptr_t)instance_->getJavaScriptContext(); } +void JReactInstance::unregisterFromInspector() { + instance_->unregisterFromInspector(); +} + void JReactInstance::registerNatives() { registerHybrid({ makeNativeMethod("initHybrid", JReactInstance::initHybrid), @@ -229,14 +239,14 @@ void JReactInstance::registerNatives() { JReactInstance::getBufferedRuntimeExecutor), makeNativeMethod( "getRuntimeScheduler", JReactInstance::getRuntimeScheduler), - makeNativeMethod( "registerSegmentNative", JReactInstance::registerSegment), makeNativeMethod( "handleMemoryPressureJs", JReactInstance::handleMemoryPressureJs), makeNativeMethod( "getJavaScriptContext", JReactInstance::getJavaScriptContext), + makeNativeMethod( + "unregisterFromInspector", JReactInstance::unregisterFromInspector), }); } - } // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.h b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.h index 6a28ea50c73..d6552a8a978 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.h @@ -27,6 +27,7 @@ #include "JJSTimerExecutor.h" #include "JJavaTimerManager.h" #include "JReactExceptionManager.h" +#include "JReactHostInspectorTarget.h" namespace facebook::react { @@ -45,7 +46,9 @@ class JReactInstance : public jni::HybridClass { jni::alias_ref jsTimerExecutor, jni::alias_ref jReactExceptionManager, jni::alias_ref jBindingsInstaller, - bool isProfiling); + bool isProfiling, + jni::alias_ref + jReactHostInspectorTarget); /* * Instantiates and returns an instance of `JSTimerExecutor`. @@ -78,6 +81,8 @@ class JReactInstance : public jni::HybridClass { void handleMemoryPressureJs(jint level); + void unregisterFromInspector(); + private: friend HybridBase; @@ -90,7 +95,9 @@ class JReactInstance : public jni::HybridClass { jni::alias_ref jsTimerExecutor, jni::alias_ref jReactExceptionManager, jni::alias_ref jBindingsInstaller, - bool isProfiling) noexcept; + bool isProfiling, + jni::alias_ref + jReactHostInspectorTarget) noexcept; jni::alias_ref getJSCallInvokerHolder(); jni::alias_ref diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 09e97cfb5ca..c2ade6b2e7c 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -7,6 +7,7 @@ #include "ReactInstance.h" +#include #include #include #include @@ -15,11 +16,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include @@ -37,16 +40,13 @@ ReactInstance::ReactInstance( jsErrorHandler_(jsErrorHandlingFunc), hasFatalJsError_(std::make_shared(false)), parentInspectorTarget_(parentInspectorTarget) { - auto runtimeExecutor = [weakRuntime = std::weak_ptr(runtime_), - weakTimerManager = - std::weak_ptr(timerManager_), - weakJsMessageQueueThread = - std::weak_ptr( - jsMessageQueueThread_), - weakHasFatalJsError = - std::weak_ptr(hasFatalJsError_)]( - std::function&& - callback) { + RuntimeExecutor runtimeExecutor = [weakRuntime = std::weak_ptr(runtime_), + weakTimerManager = + std::weak_ptr(timerManager_), + weakJsMessageQueueThread = + std::weak_ptr(jsMessageQueueThread_), + weakHasFatalJsError = std::weak_ptr( + hasFatalJsError_)](auto callback) { if (std::shared_ptr sharedHasFatalJsError = weakHasFatalJsError.lock()) { if (*sharedHasFatalJsError) { @@ -85,13 +85,40 @@ ReactInstance::ReactInstance( }; if (parentInspectorTarget_) { - inspectorTarget_ = &parentInspectorTarget_->registerInstance(*this); - runtimeInspectorTarget_ = - &inspectorTarget_->registerRuntime(*runtime_, runtimeExecutor); + auto executor = parentInspectorTarget_->executorFromThis(); + + auto runtimeExecutorThatWaitsForInspectorSetup = + std::make_shared(runtimeExecutor); + + // This code can execute from any thread, so we need to make sure we set up + // the inspector logic in the right one. The callback executes immediately + // if we are already in the right thread. + executor([this, runtimeExecutor, runtimeExecutorThatWaitsForInspectorSetup]( + jsinspector_modern::HostTarget& hostTarget) { + // Callbacks scheduled through the page target executor are generally + // not guaranteed to run (e.g.: if the page target is destroyed) + // but in this case it is because the page target cannot be destroyed + // before the instance finishes its setup: + // * On iOS it's because we do the setup synchronously. + // * On Android it's because we explicitly wait for the instance + // creation task to finish before starting the destruction. + inspectorTarget_ = &hostTarget.registerInstance(*this); + runtimeInspectorTarget_ = + &inspectorTarget_->registerRuntime(*runtime_, runtimeExecutor); + runtimeExecutorThatWaitsForInspectorSetup->flush(); + }); + + // We decorate the runtime executor used everywhere else to wait for the + // inspector to finish its setup. + runtimeExecutor = + [runtimeExecutorThatWaitsForInspectorSetup]( + std::function&& callback) { + runtimeExecutorThatWaitsForInspectorSetup->execute( + std::move(callback)); + }; } - runtimeScheduler_ = - std::make_shared(std::move(runtimeExecutor)); + runtimeScheduler_ = std::make_shared(runtimeExecutor); auto pipedRuntimeExecutor = [runtimeScheduler = runtimeScheduler_.get()]( @@ -104,13 +131,15 @@ ReactInstance::ReactInstance( } void ReactInstance::unregisterFromInspector() { - if (inspectorTarget_) { - assert(runtimeInspectorTarget_); - inspectorTarget_->unregisterRuntime(*runtimeInspectorTarget_); - assert(parentInspectorTarget_); - parentInspectorTarget_->unregisterInstance(*inspectorTarget_); - inspectorTarget_ = nullptr; - } + assert(inspectorTarget_); + + assert(runtimeInspectorTarget_); + inspectorTarget_->unregisterRuntime(*runtimeInspectorTarget_); + + assert(parentInspectorTarget_); + parentInspectorTarget_->unregisterInstance(*inspectorTarget_); + + inspectorTarget_ = nullptr; } RuntimeExecutor ReactInstance::getUnbufferedRuntimeExecutor() noexcept {