From cfef30f38660cbd92b6e66880e9fcf04cc24ee1b Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Mon, 15 Jan 2024 10:47:20 -0800 Subject: [PATCH] Make ReactNativeConfig a JNI Class (#42247) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/42247 With this change I make `ReactNativeConfig` a JNI class loaded at Fabric Loading time. This removes the default from `EmptyReactNativeConfig.java` and makes sure we do read the defaults from C++ `ReactNativeConfig.cpp` file. Changelog: [Internal] [Changed] - Make ReactNativeConfig a JNI Class Reviewed By: motiz88 Differential Revision: D52696653 fbshipit-source-id: 99d5e37c65e0e59efcee2c857bb94194fb40d87d --- .../ReactAndroid/proguard-rules.pro | 8 +++- .../react/fabric/EmptyReactNativeConfig.java | 33 +++++++------ .../react/fabric/JEmptyReactNativeConfig.cpp | 47 +++++++++++++++++++ .../react/fabric/JEmptyReactNativeConfig.h | 38 +++++++++++++++ .../src/main/jni/react/fabric/OnLoad.cpp | 2 + .../react/fabric/ReactNativeConfigHolder.h | 2 +- .../react/runtime/ReactHostDelegateTest.kt | 5 +- .../testutils/fakes/FakeReactNativeConfig.kt | 21 +++++++++ .../react/config/ReactNativeConfig.h | 2 +- 9 files changed, 140 insertions(+), 18 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/main/jni/react/fabric/JEmptyReactNativeConfig.cpp create mode 100644 packages/react-native/ReactAndroid/src/main/jni/react/fabric/JEmptyReactNativeConfig.h create mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeReactNativeConfig.kt diff --git a/packages/react-native/ReactAndroid/proguard-rules.pro b/packages/react-native/ReactAndroid/proguard-rules.pro index e169d03a66f..64ab4631c65 100644 --- a/packages/react-native/ReactAndroid/proguard-rules.pro +++ b/packages/react-native/ReactAndroid/proguard-rules.pro @@ -30,10 +30,16 @@ -keepclassmembers class * { @com.facebook.proguard.annotations.DoNotStrip *; } - -keep @com.facebook.proguard.annotations.DoNotStripAny class * { *; } +-keep @com.facebook.jni.annotations.DoNotStrip class * +-keepclassmembers class * { + @com.facebook.jni.annotations.DoNotStrip *; +} +-keep @com.facebook.jni.annotations.DoNotStripAny class * { + *; +} -keepclassmembers @com.facebook.proguard.annotations.KeepGettersAndSetters class * { void set*(***); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/EmptyReactNativeConfig.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/EmptyReactNativeConfig.java index e068e3f4d1d..57f2856dc35 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/EmptyReactNativeConfig.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/EmptyReactNativeConfig.java @@ -7,29 +7,34 @@ package com.facebook.react.fabric; +import androidx.annotation.NonNull; +import com.facebook.jni.HybridData; +import com.facebook.proguard.annotations.DoNotStripAny; + /** - * An empty {@link ReactNativeConfig} that is returning empty responses and false for all the - * requested keys. + * An empty {@link ReactNativeConfig} that is backed by the C++ implementation where the defaults + * are store. */ +@DoNotStripAny public class EmptyReactNativeConfig implements ReactNativeConfig { - @Override - public boolean getBool(final String s) { - return false; + @NonNull private final HybridData mHybridData; + + private static native HybridData initHybrid(); + + public EmptyReactNativeConfig() { + mHybridData = initHybrid(); } @Override - public long getInt64(final String s) { - return 0; - } + public native boolean getBool(final String param); @Override - public String getString(final String s) { - return ""; - } + public native long getInt64(final String param); @Override - public double getDouble(final String s) { - return 0; - } + public native String getString(final String param); + + @Override + public native double getDouble(final String param); } diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JEmptyReactNativeConfig.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JEmptyReactNativeConfig.cpp new file mode 100644 index 00000000000..200ea87ef51 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JEmptyReactNativeConfig.cpp @@ -0,0 +1,47 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "JEmptyReactNativeConfig.h" +#include +#include + +namespace facebook::react { + +jni::local_ref +JEmptyReactNativeConfig::initHybrid(jni::alias_ref) { + return makeCxxInstance(); +} + +jboolean JEmptyReactNativeConfig::getBool(const jni::alias_ref param) { + return reactNativeConfig_.getBool(param->toStdString()); +} + +jni::local_ref JEmptyReactNativeConfig::getString( + const jni::alias_ref param) { + return jni::make_jstring(reactNativeConfig_.getString(param->toStdString())); +} + +jlong JEmptyReactNativeConfig::getInt64(const jni::alias_ref param) { + return reactNativeConfig_.getInt64(param->toStdString()); +} + +jdouble JEmptyReactNativeConfig::getDouble( + const jni::alias_ref param) { + return reactNativeConfig_.getDouble(param->toStdString()); +} + +void JEmptyReactNativeConfig::registerNatives() { + registerHybrid({ + makeNativeMethod("initHybrid", JEmptyReactNativeConfig::initHybrid), + makeNativeMethod("getBool", JEmptyReactNativeConfig::getBool), + makeNativeMethod("getString", JEmptyReactNativeConfig::getString), + makeNativeMethod("getInt64", JEmptyReactNativeConfig::getInt64), + makeNativeMethod("getDouble", JEmptyReactNativeConfig::getDouble), + }); +} + +} // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JEmptyReactNativeConfig.h b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JEmptyReactNativeConfig.h new file mode 100644 index 00000000000..941b082bf31 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JEmptyReactNativeConfig.h @@ -0,0 +1,38 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include +#include +#include + +namespace facebook::react { + +class JEmptyReactNativeConfig + : public jni::HybridClass { + public: + constexpr static const char* const kJavaDescriptor = + "Lcom/facebook/react/fabric/EmptyReactNativeConfig;"; + + static void registerNatives(); + + jboolean getBool(const jni::alias_ref param); + + jni::local_ref getString(const jni::alias_ref param); + + jlong getInt64(const jni::alias_ref param); + + jdouble getDouble(const jni::alias_ref param); + + private: + static jni::local_ref initHybrid(jni::alias_ref); + const EmptyReactNativeConfig reactNativeConfig_ = EmptyReactNativeConfig(); +}; + +} // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/OnLoad.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/OnLoad.cpp index 67176fad7cd..1e4ab75b826 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/OnLoad.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/OnLoad.cpp @@ -12,6 +12,7 @@ #include "CoreComponentsRegistry.h" #include "EventBeatManager.h" #include "EventEmitterWrapper.h" +#include "JEmptyReactNativeConfig.h" #include "StateWrapperImpl.h" #include "SurfaceHandlerBinding.h" @@ -24,5 +25,6 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void*) { facebook::react::ComponentFactory::registerNatives(); facebook::react::CoreComponentsRegistry::registerNatives(); facebook::react::SurfaceHandlerBinding::registerNatives(); + facebook::react::JEmptyReactNativeConfig::registerNatives(); }); } diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/ReactNativeConfigHolder.h b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/ReactNativeConfigHolder.h index 8be8770ddbd..a1692357e0d 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/ReactNativeConfigHolder.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/ReactNativeConfigHolder.h @@ -15,7 +15,7 @@ namespace facebook::react { /** - * Implementation of ReactNativeConfig that wraps a FabricMobileConfig Java + * Implementation of ReactNativeConfig that wraps a ReactNativeConfig Java * object. */ class ReactNativeConfigHolder : public ReactNativeConfig { diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/runtime/ReactHostDelegateTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/runtime/ReactHostDelegateTest.kt index 0407c0ae127..b2b3b17184b 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/runtime/ReactHostDelegateTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/runtime/ReactHostDelegateTest.kt @@ -12,6 +12,7 @@ import com.facebook.react.bridge.JSBundleLoader import com.facebook.react.common.annotations.UnstableReactNativeAPI import com.facebook.react.defaults.DefaultReactHostDelegate import com.facebook.react.runtime.hermes.HermesInstance +import com.facebook.testutils.fakes.FakeReactNativeConfig import com.facebook.testutils.shadows.ShadowSoLoader import org.assertj.core.api.Assertions.assertThat import org.junit.Test @@ -36,12 +37,14 @@ class ReactHostDelegateTest { Mockito.mock(ReactPackageTurboModuleManagerDelegate.Builder::class.java) val hermesInstance: JSRuntimeFactory = Mockito.mock(HermesInstance::class.java) val jsMainModulePathMocked = "mockedJSMainModulePath" + val reactNativeConfig = FakeReactNativeConfig() val delegate = DefaultReactHostDelegate( jsMainModulePath = jsMainModulePathMocked, jsBundleLoader = jsBundleLoader, jsRuntimeFactory = hermesInstance, - turboModuleManagerDelegateBuilder = turboModuleManagerDelegateBuilderMock) + turboModuleManagerDelegateBuilder = turboModuleManagerDelegateBuilderMock, + reactNativeConfig = reactNativeConfig) assertThat(delegate.jsMainModulePath).isEqualTo(jsMainModulePathMocked) } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeReactNativeConfig.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeReactNativeConfig.kt new file mode 100644 index 00000000000..94867908507 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeReactNativeConfig.kt @@ -0,0 +1,21 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.testutils.fakes + +import com.facebook.react.fabric.ReactNativeConfig + +/** A fake [ReactNativeConfig] that returns default values for all methods without accessing JNI. */ +class FakeReactNativeConfig : ReactNativeConfig { + override fun getBool(param: String): Boolean = false + + override fun getInt64(param: String): Long = 0L + + override fun getString(param: String): String = "" + + override fun getDouble(param: String): Double = 0.0 +} diff --git a/packages/react-native/ReactCommon/react/config/ReactNativeConfig.h b/packages/react-native/ReactCommon/react/config/ReactNativeConfig.h index a5cbbc7acd7..13f348d94d1 100644 --- a/packages/react-native/ReactCommon/react/config/ReactNativeConfig.h +++ b/packages/react-native/ReactCommon/react/config/ReactNativeConfig.h @@ -27,7 +27,7 @@ class ReactNativeConfig { }; /** - * Empty configuration that will provide hardcoded values. + * Empty configuration that will provide hardcoded default values. */ class EmptyReactNativeConfig : public ReactNativeConfig { public: