[HermesExecutorFactory] add debugging settings

[Kudo]

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 and the latest page name will overwrite previous one.

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"

Changelog

[General] [Added] - Add more debugging settings for HermesExecutorFactory

Test Plan

Verify the features by RNTester.

1. setEnableDebugger

--- 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<ReactPackage> getPackages() {
           return Arrays.<ReactPackage>asList(

after app launched, the metro inspector should return empty array.
Run curl http://localhost:8081/json and returns []

2. setDebuggerName

--- 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<ReactPackage> getPackages() {
           return Arrays.<ReactPackage>asList(

after app launched, the metro inspector should return an entry with Custom Hermes Debugger
Run curl http://localhost:8081/json and returns

[
  {
    "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%26page%3D1",
    "type": "node",
    "webSocketDebuggerUrl": "ws://[::1]:8081/inspector/debug?device=2&page=1",
    "vm": "Hermes"
  }
]

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,616,990	-179
android	hermes	armeabi-v7a	7,031,527	+15
android	hermes	x86	7,917,392	+58
android	hermes	x86_64	7,891,198	+174
android	jsc	arm64-v8a	9,494,852	-18
android	jsc	armeabi-v7a	8,272,559	-26
android	jsc	x86	9,432,741	-19
android	jsc	x86_64	10,025,792	-17

Base commit: de75a7a
Branch: main

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: de75a7a
Branch: main

[facebook-github-bot]

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[RSNara]

Instead of creating setters, can we provide these as constructor arguments to HermesExecutorFactory? We invoke the setters immediately after creating HermesExecutorFactory.

[Kudo]

disabling debugger or changing debugger name is an optional feature and not going to introduce breaking change. there're two options to achieve this.

1. add setters like current approach. we could think this factory as a builder, calling setters are optional.
2. add more constructor arguments with default values:

  explicit HermesExecutorFactory(
      JSIExecutor::RuntimeInstaller runtimeInstaller,
      const JSIScopedTimeoutInvoker &timeoutInvoker =
          JSIExecutor::defaultTimeoutInvoker,
      ::hermes::vm::RuntimeConfig runtimeConfig = defaultRuntimeConfig()
      bool enableDebugger = true,
      const std::string& debuggerName = "Hermes React Native");

i was thinking that there're enough arguments. i just don't want to add more arguments that are optional.

i am open to the implementation. feel free to let me know what's in your mind.

[RSNara]

We default in two locations:

1. In HermesExecutorFactory.java
2. here.

Can we keep the defaults in HermesExecutorFactory.java?

[Kudo]

since HermesExecutorFactory.h is also be used on ios, i'd set the default here.

also in HermesExecutorFactory.java the default value is empty and we use cpp's default if the the value from jni is empty. so we don't have to maintain the "Hermes React Native" string in two places.

[cipolleschi]

What's the outcome here @Kudo @RSNara? Do you think I can try to land this or is there something else that needs to be done?

[facebook-github-bot]

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[Kudo]

i don't have further comments or changes. let me know whether there's any recommendation from you. thanks @cipolleschi for taking care this pr.

[RSNara]

@Kudo, Thanks for answering my questions, and addressing my concerns. The diff looks good to me.

[react-native-bot]

This pull request was successfully merged by @Kudo in 32d12e8.

^{When will my fix make it into a release? | Upcoming Releases}

[Kudo]

cool, thanks for reviewing and landing this pr!

[tomekzaw]

Hey @Kudo, thanks a lot for this PR! @Kwasow is currently working on enabling debugging of Reanimated's runtime using Chrome DevTools and Flipper (software-mansion/react-native-reanimated#3526) and he also faces the same problem as you did. We are really looking forward to this change in 0.71 release.

[Kudo]

hey @tomekzaw! that's a neat feature for reanimated! besides 0.71, i was proposing this change to be included in 0.70.1 for the upcoming expo sdk 47: reactwg/react-native-releases#32 (comment)