From ad582f5e8a8e35a14a0b3ba0a5e18941ca4b4e47 Mon Sep 17 00:00:00 2001 From: Kudo Chien Date: Wed, 7 Sep 2022 12:52:37 -0700 Subject: [PATCH] add debugging settings (#34489) Summary: For brownfield apps, it is possible to have multiple hermes runtimes serving different JS bundles. Hermes inspector currently only supports single JS bundle. The latest loaded JS bundle will overwrite previous JS bundle. This is because we always use the ["Hermes React Native" as the inspector page name](https://github.com/facebook/react-native/blob/de75a7a22eebbe6b7106377bdd697a2d779b91b0/ReactCommon/hermes/executor/HermesExecutorFactory.cpp#L157) and [the latest page name will overwrite previous one](https://github.com/facebook/react-native/blob/de75a7a22eebbe6b7106377bdd697a2d779b91b0/ReactCommon/hermes/inspector/chrome/ConnectionDemux.cpp#L77-L86). This PR adds more customization for HermesExecutorFactory: - `setEnableDebugger`: provide a way to disable debugging features for the hermes runtime - `setDebuggerName`: provide a way to customize inspector page name other than the default "Hermes React Native" [General] [Added] - Add more debugging settings for *HermesExecutorFactory* Pull Request resolved: https://github.com/facebook/react-native/pull/34489 Test Plan: Verify the features by RNTester. 1. `setEnableDebugger` ```diff --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java @@ -10,10 +10,12 @@ package com.facebook.react.uiapp; import android.app.Application; import androidx.annotation.NonNull; import com.facebook.fbreact.specs.SampleTurboModule; +import com.facebook.hermes.reactexecutor.HermesExecutorFactory; import com.facebook.react.ReactApplication; import com.facebook.react.ReactNativeHost; import com.facebook.react.ReactPackage; import com.facebook.react.TurboReactPackage; +import com.facebook.react.bridge.JavaScriptExecutorFactory; import com.facebook.react.bridge.NativeModule; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.config.ReactFeatureFlags; @@ -50,6 +52,13 @@ public class RNTesterApplication extends Application implements ReactApplication return BuildConfig.DEBUG; } + Override + protected JavaScriptExecutorFactory getJavaScriptExecutorFactory() { + HermesExecutorFactory factory = new HermesExecutorFactory(); + factory.setEnableDebugger(false); + return factory; + } + Override public List getPackages() { return Arrays.asList( ``` after app launched, the metro inspector should return empty array. Run `curl http://localhost:8081/json` and returns `[]` 2. `setDebuggerName` ```diff --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java @@ -10,10 +10,12 @@ package com.facebook.react.uiapp; import android.app.Application; import androidx.annotation.NonNull; import com.facebook.fbreact.specs.SampleTurboModule; +import com.facebook.hermes.reactexecutor.HermesExecutorFactory; import com.facebook.react.ReactApplication; import com.facebook.react.ReactNativeHost; import com.facebook.react.ReactPackage; import com.facebook.react.TurboReactPackage; +import com.facebook.react.bridge.JavaScriptExecutorFactory; import com.facebook.react.bridge.NativeModule; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.config.ReactFeatureFlags; @@ -50,6 +52,13 @@ public class RNTesterApplication extends Application implements ReactApplication return BuildConfig.DEBUG; } + Override + protected JavaScriptExecutorFactory getJavaScriptExecutorFactory() { + HermesExecutorFactory factory = new HermesExecutorFactory(); + factory.setDebuggerName("Custom Hermes Debugger"); + return factory; + } + Override public List getPackages() { return Arrays.asList( ``` after app launched, the metro inspector should return an entry with *Custom Hermes Debugger* Run `curl http://localhost:8081/json` and returns ```json [ { "id": "2-1", "description": "com.facebook.react.uiapp", "title": "Custom Hermes Debugger", "faviconUrl": "https://reactjs.org/favicon.ico", "devtoolsFrontendUrl": "devtools://devtools/bundled/js_app.html?experiments=true&v8only=true&ws=%5B%3A%3A1%5D%3A8081%2Finspector%2Fdebug%3Fdevice%3D2 (https://github.com/facebook/react-native/commit/e5c5dcd9e26e9443f59864d9763b049e0bda98e7)%26page%3D1 (https://github.com/facebook/react-native/commit/ea93151f21003df6f65dd173dd5dcb3135b0ae94)", "type": "node", "webSocketDebuggerUrl": "ws://[::1]:8081/inspector/debug?device=2&page=1", "vm": "Hermes" } ] ``` Reviewed By: mdvacca Differential Revision: D38982104 Pulled By: cipolleschi fbshipit-source-id: 78003c173db55448a751145986985b3e1d1c71bb --- .../hermes/reactexecutor/HermesExecutor.java | 13 ++++--- .../reactexecutor/HermesExecutorFactory.java | 12 ++++++- .../facebook/hermes/reactexecutor/OnLoad.cpp | 23 +++++++++--- .../hermes/executor/HermesExecutorFactory.cpp | 36 +++++++++++++++---- .../hermes/executor/HermesExecutorFactory.h | 6 ++++ 5 files changed, 73 insertions(+), 17 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutor.java b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutor.java index a9a474510fb..48eaaa4c066 100644 --- a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutor.java +++ b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutor.java @@ -33,8 +33,11 @@ public class HermesExecutor extends JavaScriptExecutor { } } - HermesExecutor(@Nullable RuntimeConfig config) { - super(config == null ? initHybridDefaultConfig() : initHybrid(config.heapSizeMB)); + HermesExecutor(@Nullable RuntimeConfig config, boolean enableDebugger, String debuggerName) { + super( + config == null + ? initHybridDefaultConfig(enableDebugger, debuggerName) + : initHybrid(enableDebugger, debuggerName, config.heapSizeMB)); } @Override @@ -51,7 +54,9 @@ public class HermesExecutor extends JavaScriptExecutor { */ public static native boolean canLoadFile(String path); - private static native HybridData initHybridDefaultConfig(); + private static native HybridData initHybridDefaultConfig( + boolean enableDebugger, String debuggerName); - private static native HybridData initHybrid(long heapSizeMB); + private static native HybridData initHybrid( + boolean enableDebugger, String debuggerName, long heapSizeMB); } diff --git a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutorFactory.java b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutorFactory.java index 944afe6b645..a51d5825a26 100644 --- a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutorFactory.java +++ b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutorFactory.java @@ -15,6 +15,8 @@ public class HermesExecutorFactory implements JavaScriptExecutorFactory { private static final String TAG = "Hermes"; private final RuntimeConfig mConfig; + private boolean mEnableDebugger = true; + private String mDebuggerName = ""; public HermesExecutorFactory() { this(null); @@ -24,9 +26,17 @@ public class HermesExecutorFactory implements JavaScriptExecutorFactory { mConfig = config; } + public void setEnableDebugger(boolean enableDebugger) { + mEnableDebugger = enableDebugger; + } + + public void setDebuggerName(String debuggerName) { + mDebuggerName = debuggerName; + } + @Override public JavaScriptExecutor create() { - return new HermesExecutor(mConfig); + return new HermesExecutor(mConfig, mEnableDebugger, mDebuggerName); } @Override diff --git a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/OnLoad.cpp b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/OnLoad.cpp index 304603fd12e..2a102d5d265 100644 --- a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/OnLoad.cpp +++ b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/OnLoad.cpp @@ -69,26 +69,39 @@ class HermesExecutorHolder "Lcom/facebook/hermes/reactexecutor/HermesExecutor;"; static jni::local_ref initHybridDefaultConfig( - jni::alias_ref) { + jni::alias_ref, + bool enableDebugger, + std::string debuggerName) { JReactMarker::setLogPerfMarkerIfNeeded(); std::call_once(flag, []() { facebook::hermes::HermesRuntime::setFatalHandler(hermesFatalHandler); }); - return makeCxxInstance( - std::make_unique(installBindings)); + auto factory = std::make_unique(installBindings); + factory->setEnableDebugger(enableDebugger); + if (!debuggerName.empty()) { + factory->setDebuggerName(debuggerName); + } + return makeCxxInstance(std::move(factory)); } static jni::local_ref initHybrid( jni::alias_ref, + bool enableDebugger, + std::string debuggerName, jlong heapSizeMB) { JReactMarker::setLogPerfMarkerIfNeeded(); auto runtimeConfig = makeRuntimeConfig(heapSizeMB); std::call_once(flag, []() { facebook::hermes::HermesRuntime::setFatalHandler(hermesFatalHandler); }); - return makeCxxInstance(std::make_unique( - installBindings, JSIExecutor::defaultTimeoutInvoker, runtimeConfig)); + auto factory = std::make_unique( + installBindings, JSIExecutor::defaultTimeoutInvoker, runtimeConfig); + factory->setEnableDebugger(enableDebugger); + if (!debuggerName.empty()) { + factory->setDebuggerName(debuggerName); + } + return makeCxxInstance(std::move(factory)); } static bool canLoadFile(jni::alias_ref, const std::string &path) { diff --git a/ReactCommon/hermes/executor/HermesExecutorFactory.cpp b/ReactCommon/hermes/executor/HermesExecutorFactory.cpp index 4e503514856..ffd4f2b48e2 100644 --- a/ReactCommon/hermes/executor/HermesExecutorFactory.cpp +++ b/ReactCommon/hermes/executor/HermesExecutorFactory.cpp @@ -155,15 +155,20 @@ class DecoratedRuntime : public jsi::WithRuntimeDecorator { DecoratedRuntime( std::unique_ptr runtime, HermesRuntime &hermesRuntime, - std::shared_ptr jsQueue) + std::shared_ptr jsQueue, + bool enableDebugger, + const std::string &debuggerName) : jsi::WithRuntimeDecorator(*runtime, reentrancyCheck_), runtime_(std::move(runtime)), hermesRuntime_(hermesRuntime) { #ifdef HERMES_ENABLE_DEBUGGER - auto adapter = std::make_unique( - runtime_, hermesRuntime_, jsQueue); - facebook::hermes::inspector::chrome::enableDebugging( - std::move(adapter), "Hermes React Native"); + enableDebugger_ = enableDebugger; + if (enableDebugger_) { + auto adapter = std::make_unique( + runtime_, hermesRuntime_, jsQueue); + facebook::hermes::inspector::chrome::enableDebugging( + std::move(adapter), debuggerName); + } #else (void)hermesRuntime_; #endif @@ -171,7 +176,9 @@ class DecoratedRuntime : public jsi::WithRuntimeDecorator { ~DecoratedRuntime() { #ifdef HERMES_ENABLE_DEBUGGER - facebook::hermes::inspector::chrome::disableDebugging(*runtime_); + if (enableDebugger_) { + facebook::hermes::inspector::chrome::disableDebugging(*runtime_); + } #endif } @@ -186,10 +193,21 @@ class DecoratedRuntime : public jsi::WithRuntimeDecorator { std::shared_ptr runtime_; ReentrancyCheck reentrancyCheck_; HermesRuntime &hermesRuntime_; +#ifdef HERMES_ENABLE_DEBUGGER + bool enableDebugger_; +#endif }; } // namespace +void HermesExecutorFactory::setEnableDebugger(bool enableDebugger) { + enableDebugger_ = enableDebugger; +} + +void HermesExecutorFactory::setDebuggerName(const std::string &debuggerName) { + debuggerName_ = debuggerName; +} + std::unique_ptr HermesExecutorFactory::createJSExecutor( std::shared_ptr delegate, std::shared_ptr jsQueue) { @@ -197,7 +215,11 @@ std::unique_ptr HermesExecutorFactory::createJSExecutor( makeHermesRuntimeSystraced(runtimeConfig_); HermesRuntime &hermesRuntimeRef = *hermesRuntime; auto decoratedRuntime = std::make_shared( - std::move(hermesRuntime), hermesRuntimeRef, jsQueue); + std::move(hermesRuntime), + hermesRuntimeRef, + jsQueue, + enableDebugger_, + debuggerName_); // So what do we have now? // DecoratedRuntime -> HermesRuntime diff --git a/ReactCommon/hermes/executor/HermesExecutorFactory.h b/ReactCommon/hermes/executor/HermesExecutorFactory.h index d06323ec7d4..d0d5352ba54 100644 --- a/ReactCommon/hermes/executor/HermesExecutorFactory.h +++ b/ReactCommon/hermes/executor/HermesExecutorFactory.h @@ -28,6 +28,10 @@ class HermesExecutorFactory : public JSExecutorFactory { assert(timeoutInvoker_ && "Should not have empty timeoutInvoker"); } + void setEnableDebugger(bool enableDebugger); + + void setDebuggerName(const std::string &debuggerName); + std::unique_ptr createJSExecutor( std::shared_ptr delegate, std::shared_ptr jsQueue) override; @@ -38,6 +42,8 @@ class HermesExecutorFactory : public JSExecutorFactory { JSIExecutor::RuntimeInstaller runtimeInstaller_; JSIScopedTimeoutInvoker timeoutInvoker_; ::hermes::vm::RuntimeConfig runtimeConfig_; + bool enableDebugger_ = true; + std::string debuggerName_ = "Hermes React Native"; }; class HermesExecutor : public JSIExecutor {