Use HybridClassBase for frequently constructed objects (#46706)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46706

Simplifies object construction and collection, and avoids additional JNI roundtrips to set HybridData.

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D63537781

fbshipit-source-id: 55df2785840ec8fcd5a7a08d2c7dd73c5b5fca82

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

@@ -744,7 +744,7 @@ public class com/facebook/react/bridge/ColorPropConverter {
 	public static fun resolveResourcePath (Landroid/content/Context;Ljava/lang/String;)Ljava/lang/Integer;
 }
 
-public class com/facebook/react/bridge/CxxCallbackImpl : com/facebook/react/bridge/Callback {
+public class com/facebook/react/bridge/CxxCallbackImpl : com/facebook/jni/HybridClassBase, com/facebook/react/bridge/Callback {
 	public fun invoke ([Ljava/lang/Object;)V
 }
 
@@ -1047,8 +1047,8 @@ public class com/facebook/react/bridge/ModuleSpec {
 	public static fun viewManagerSpec (Ljavax/inject/Provider;)Lcom/facebook/react/bridge/ModuleSpec;
 }
 
-public abstract class com/facebook/react/bridge/NativeArray : com/facebook/react/bridge/NativeArrayInterface {
-	protected fun <init> (Lcom/facebook/jni/HybridData;)V
+public abstract class com/facebook/react/bridge/NativeArray : com/facebook/jni/HybridClassBase, com/facebook/react/bridge/NativeArrayInterface {
+	protected fun <init> ()V
 	public fun toString ()Ljava/lang/String;
 }
 
@@ -1056,8 +1056,8 @@ public abstract interface class com/facebook/react/bridge/NativeArrayInterface {
 	public abstract fun toString ()Ljava/lang/String;
 }
 
-public abstract class com/facebook/react/bridge/NativeMap {
-	public fun <init> (Lcom/facebook/jni/HybridData;)V
+public abstract class com/facebook/react/bridge/NativeMap : com/facebook/jni/HybridClassBase {
+	public fun <init> ()V
 	public fun toString ()Ljava/lang/String;
 }
 
@@ -1485,7 +1485,7 @@ public abstract interface class com/facebook/react/bridge/ReadableMapKeySetItera
 }
 
 public class com/facebook/react/bridge/ReadableNativeArray : com/facebook/react/bridge/NativeArray, com/facebook/react/bridge/ReadableArray {
-	protected fun <init> (Lcom/facebook/jni/HybridData;)V
+	protected fun <init> ()V
 	public fun equals (Ljava/lang/Object;)Z
 	public synthetic fun getArray (I)Lcom/facebook/react/bridge/ReadableArray;
 	public fun getArray (I)Lcom/facebook/react/bridge/ReadableNativeArray;
@@ -1506,7 +1506,7 @@ public class com/facebook/react/bridge/ReadableNativeArray : com/facebook/react/
 }
 
 public class com/facebook/react/bridge/ReadableNativeMap : com/facebook/react/bridge/NativeMap, com/facebook/react/bridge/ReadableMap {
-	protected fun <init> (Lcom/facebook/jni/HybridData;)V
+	protected fun <init> ()V
 	public fun equals (Ljava/lang/Object;)Z
 	public fun getArray (Ljava/lang/String;)Lcom/facebook/react/bridge/ReadableArray;
 	public fun getBoolean (Ljava/lang/String;)Z
@@ -2717,7 +2717,7 @@ public abstract interface class com/facebook/react/fabric/ReactNativeConfig {
 public final class com/facebook/react/fabric/ReactNativeConfig$Companion {
 }
 
-public class com/facebook/react/fabric/StateWrapperImpl : com/facebook/react/uimanager/StateWrapper {
+public class com/facebook/react/fabric/StateWrapperImpl : com/facebook/jni/HybridClassBase, com/facebook/react/uimanager/StateWrapper {
 	public fun destroyState ()V
 	public fun getStateData ()Lcom/facebook/react/bridge/ReadableNativeMap;
 	public fun getStateDataMapBuffer ()Lcom/facebook/react/common/mapbuffer/ReadableMapBuffer;
@@ -2745,7 +2745,7 @@ public final class com/facebook/react/fabric/events/EventBeatManager : com/faceb
 	public fun onBatchEventDispatched ()V
 }
 
-public class com/facebook/react/fabric/events/EventEmitterWrapper {
+public class com/facebook/react/fabric/events/EventEmitterWrapper : com/facebook/jni/HybridClassBase {
 	public fun destroy ()V
 	public fun dispatch (Ljava/lang/String;Lcom/facebook/react/bridge/WritableMap;I)V
 	public fun dispatchEventSynchronously (Ljava/lang/String;Lcom/facebook/react/bridge/WritableMap;)V

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

@@ -9,18 +9,15 @@ package com.facebook.react.bridge;
 
 import static com.facebook.react.bridge.Arguments.*;
 
-import com.facebook.jni.HybridData;
+import com.facebook.jni.HybridClassBase;
 import com.facebook.proguard.annotations.DoNotStrip;
 
 /** Callback impl that calls directly into the cxx bridge. Created from C++. */
 @DoNotStrip
-public class CxxCallbackImpl implements Callback {
-  @DoNotStrip private final HybridData mHybridData;
+public class CxxCallbackImpl extends HybridClassBase implements Callback {
 
   @DoNotStrip
-  private CxxCallbackImpl(HybridData hybridData) {
-    mHybridData = hybridData;
-  }
+  private CxxCallbackImpl() {}
 
   @Override
   public void invoke(Object... args) {

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

@@ -7,8 +7,7 @@
 
 package com.facebook.react.bridge
 
-import com.facebook.jni.HybridData
-import com.facebook.proguard.annotations.DoNotStrip
+import com.facebook.jni.HybridClassBase
 import com.facebook.proguard.annotations.DoNotStripAny
 
 /**
@@ -18,9 +17,7 @@ import com.facebook.proguard.annotations.DoNotStripAny
  * and are using [Dynamic] as a parameter type.
  */
 @DoNotStripAny
-private class DynamicNative(
-    @Suppress("NoHungarianNotation") @field:DoNotStrip private val mHybridData: HybridData?
-) : Dynamic {
+private class DynamicNative : HybridClassBase(), Dynamic {
 
   override val type: ReadableType
     get() = getTypeNative()

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

@@ -7,22 +7,18 @@
 
 package com.facebook.react.bridge;
 
-import com.facebook.jni.HybridData;
+import com.facebook.jni.HybridClassBase;
 import com.facebook.proguard.annotations.DoNotStrip;
 
 /** Base class for an array whose members are stored in native code (C++). */
 @DoNotStrip
-public abstract class NativeArray implements NativeArrayInterface {
+public abstract class NativeArray extends HybridClassBase implements NativeArrayInterface {
   static {
     ReactBridge.staticInit();
   }
 
-  protected NativeArray(HybridData hybridData) {
-    mHybridData = hybridData;
-  }
+  protected NativeArray() {}
 
   @Override
   public native String toString();
-
-  @DoNotStrip private HybridData mHybridData;
 }

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

@@ -7,12 +7,12 @@
 
 package com.facebook.react.bridge
 
-import com.facebook.jni.HybridData
+import com.facebook.jni.HybridClassBase
 import com.facebook.proguard.annotations.DoNotStrip
 
 /** Base class for a Map whose keys and values are stored in native code (C++). */
 @DoNotStrip
-public abstract class NativeMap(@field:DoNotStrip private val mHybridData: HybridData?) {
+public abstract class NativeMap : HybridClassBase() {
   external override fun toString(): String
 
   private companion object {

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

@@ -10,7 +10,6 @@ package com.facebook.react.bridge;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import com.facebook.infer.annotation.Assertions;
-import com.facebook.jni.HybridData;
 import com.facebook.proguard.annotations.DoNotStrip;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -25,9 +24,7 @@ public class ReadableNativeArray extends NativeArray implements ReadableArray {
     ReactBridge.staticInit();
   }
 
-  protected ReadableNativeArray(HybridData hybridData) {
-    super(hybridData);
-  }
+  protected ReadableNativeArray() {}
 
   // WriteOnce but not in the constructor fields
   private @Nullable Object[] mLocalArray;

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

@@ -9,7 +9,6 @@ package com.facebook.react.bridge
 
 import android.annotation.SuppressLint
 import com.facebook.infer.annotation.Assertions
-import com.facebook.jni.HybridData
 import com.facebook.proguard.annotations.DoNotStripAny
 
 /**
@@ -17,8 +16,7 @@ import com.facebook.proguard.annotations.DoNotStripAny
  * in native code so you shouldn't construct one yourself.
  */
 @DoNotStripAny
-public open class ReadableNativeMap protected constructor(hybridData: HybridData?) :
-    NativeMap(hybridData), ReadableMap {
+public open class ReadableNativeMap protected constructor() : NativeMap(), ReadableMap {
   private val keys: Array<String> by
       lazy(LazyThreadSafetyMode.SYNCHRONIZED) { importKeys().also { jniPassCounter++ } }
 

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

@@ -9,7 +9,6 @@ package com.facebook.react.bridge;
 
 import androidx.annotation.Nullable;
 import com.facebook.infer.annotation.Assertions;
-import com.facebook.jni.HybridData;
 import com.facebook.proguard.annotations.DoNotStrip;
 
 /**
@@ -23,7 +22,7 @@ public class WritableNativeArray extends ReadableNativeArray implements Writable
   }
 
   public WritableNativeArray() {
-    super(initHybrid());
+    initHybrid();
   }
 
   @Override
@@ -60,7 +59,7 @@ public class WritableNativeArray extends ReadableNativeArray implements Writable
     pushNativeMap((ReadableNativeMap) map);
   }
 
-  private static native HybridData initHybrid();
+  private native void initHybrid();
 
   private native void pushNativeArray(ReadableNativeArray array);
 

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

@@ -8,7 +8,6 @@
 package com.facebook.react.bridge
 
 import com.facebook.infer.annotation.Assertions
-import com.facebook.jni.HybridData
 import com.facebook.proguard.annotations.DoNotStrip
 
 /**
@@ -16,7 +15,11 @@ import com.facebook.proguard.annotations.DoNotStrip
  * to stub out creating this class in a test. TODO(5815532): Check if consumed on read
  */
 @DoNotStrip
-public class WritableNativeMap : ReadableNativeMap(initHybrid()), WritableMap {
+public class WritableNativeMap : ReadableNativeMap(), WritableMap {
+  init {
+    initHybrid()
+  }
+
   external override fun putBoolean(key: String, value: Boolean)
 
   external override fun putDouble(key: String, value: Double)
@@ -59,11 +62,11 @@ public class WritableNativeMap : ReadableNativeMap(initHybrid()), WritableMap {
 
   private external fun mergeNativeMap(source: ReadableNativeMap)
 
+  private external fun initHybrid()
+
   private companion object {
     init {
       ReactBridge.staticInit()
     }
-
-    @JvmStatic private external fun initHybrid(): HybridData?
   }
 }

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

@@ -12,7 +12,7 @@ import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import com.facebook.common.logging.FLog;
 import com.facebook.infer.annotation.Nullsafe;
-import com.facebook.jni.HybridData;
+import com.facebook.jni.HybridClassBase;
 import com.facebook.proguard.annotations.DoNotStrip;
 import com.facebook.react.bridge.NativeMap;
 import com.facebook.react.bridge.ReadableNativeMap;
@@ -27,21 +27,18 @@ import com.facebook.react.uimanager.StateWrapper;
 @Nullsafe(Nullsafe.Mode.LOCAL)
 @SuppressLint("MissingNativeLoadLibrary")
 @DoNotStrip
-public class StateWrapperImpl implements StateWrapper {
+public class StateWrapperImpl extends HybridClassBase implements StateWrapper {
   static {
     FabricSoLoader.staticInit();
   }
 
   private static final String TAG = "StateWrapperImpl";
 
-  @DoNotStrip private final HybridData mHybridData;
-  private volatile boolean mDestroyed = false;
-
   private StateWrapperImpl() {
-    mHybridData = initHybrid();
+    initHybrid();
   }
 
-  private static native HybridData initHybrid();
+  private native void initHybrid();
 
   private native ReadableNativeMap getStateDataImpl();
 
@@ -52,7 +49,7 @@ public class StateWrapperImpl implements StateWrapper {
   @Override
   @Nullable
   public ReadableMapBuffer getStateDataMapBuffer() {
-    if (mDestroyed) {
+    if (!isValid()) {
       FLog.e(TAG, "Race between StateWrapperImpl destruction and getState");
       return null;
     }
@@ -62,7 +59,7 @@ public class StateWrapperImpl implements StateWrapper {
   @Override
   @Nullable
   public ReadableNativeMap getStateData() {
-    if (mDestroyed) {
+    if (!isValid()) {
       FLog.e(TAG, "Race between StateWrapperImpl destruction and getState");
       return null;
     }
@@ -71,7 +68,7 @@ public class StateWrapperImpl implements StateWrapper {
 
   @Override
   public void updateState(@NonNull WritableMap map) {
-    if (mDestroyed) {
+    if (!isValid()) {
       FLog.e(TAG, "Race between StateWrapperImpl destruction and updateState");
       return;
     }
@@ -80,15 +77,14 @@ public class StateWrapperImpl implements StateWrapper {
 
   @Override
   public void destroyState() {
-    if (!mDestroyed) {
-      mDestroyed = true;
-      mHybridData.resetNative();
+    if (isValid()) {
+      resetNative();
     }
   }
 
   @Override
   public String toString() {
-    if (mDestroyed) {
+    if (!isValid()) {
       return "<destroyed>";
     }
 

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/events/EventEmitterWrapper.java

@@ -10,7 +10,7 @@ package com.facebook.react.fabric.events;
 import android.annotation.SuppressLint;
 import androidx.annotation.Nullable;
 import com.facebook.infer.annotation.Nullsafe;
-import com.facebook.jni.HybridData;
+import com.facebook.jni.HybridClassBase;
 import com.facebook.proguard.annotations.DoNotStrip;
 import com.facebook.react.bridge.NativeMap;
 import com.facebook.react.bridge.WritableMap;
@@ -24,18 +24,13 @@ import com.facebook.react.uimanager.events.EventCategoryDef;
 @Nullsafe(Nullsafe.Mode.LOCAL)
 @DoNotStrip
 @SuppressLint("MissingNativeLoadLibrary")
-public class EventEmitterWrapper {
-
+public class EventEmitterWrapper extends HybridClassBase {
   static {
     FabricSoLoader.staticInit();
   }
 
-  @DoNotStrip private final HybridData mHybridData;
-
   @DoNotStrip
-  private EventEmitterWrapper(HybridData hybridData) {
-    mHybridData = hybridData;
-  }
+  private EventEmitterWrapper() {}
 
   private native void dispatchEvent(
       String eventName, @Nullable NativeMap params, @EventCategoryDef int category);
@@ -81,15 +76,8 @@ public class EventEmitterWrapper {
   }
 
   public synchronized void destroy() {
-    if (mHybridData != null) {
-      mHybridData.resetNative();
+    if (isValid()) {
+      resetNative();
     }
   }
-
-  private boolean isValid() {
-    if (mHybridData != null) {
-      return mHybridData.isValid();
-    }
-    return false;
-  }
 }

packages/react-native/ReactAndroid/src/main/jni/react/fabric/StateWrapperImpl.cpp

@@ -19,9 +19,8 @@ namespace facebook::react {
 /**
  * Called from Java constructor through the JNI.
  */
-jni::local_ref<StateWrapperImpl::jhybriddata> StateWrapperImpl::initHybrid(
-    jni::alias_ref<jclass>) {
-  return makeCxxInstance();
+void StateWrapperImpl::initHybrid(jni::alias_ref<jhybridobject> jobj) {
+  return setCxxInstance(jobj);
 }
 
 jni::local_ref<ReadableNativeMap::jhybridobject>

packages/react-native/ReactAndroid/src/main/jni/react/fabric/StateWrapperImpl.h

@@ -31,10 +31,9 @@ class StateWrapperImpl : public jni::HybridClass<StateWrapperImpl> {
   void setState(std::shared_ptr<const State> state);
 
  private:
-  jni::alias_ref<StateWrapperImpl::jhybriddata> jhybridobject_;
   std::shared_ptr<const State> state_;
 
-  static jni::local_ref<jhybriddata> initHybrid(jni::alias_ref<jclass>);
+  static void initHybrid(jni::alias_ref<jhybridobject> jobj);
 };
 
 } // namespace facebook::react

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

@@ -29,7 +29,7 @@ class JCxxCallbackImpl : public jni::HybridClass<JCxxCallbackImpl, JCallback> {
       "Lcom/facebook/react/bridge/CxxCallbackImpl;";
 
   static void registerNatives() {
-    javaClassStatic()->registerNatives({
+    registerHybrid({
         makeNativeMethod("nativeInvoke", JCxxCallbackImpl::invoke),
     });
   }

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

@@ -23,9 +23,8 @@ WritableNativeArray::WritableNativeArray(folly::dynamic&& val)
   }
 }
 
-local_ref<WritableNativeArray::jhybriddata> WritableNativeArray::initHybrid(
-    alias_ref<jclass>) {
-  return makeCxxInstance();
+void WritableNativeArray::initHybrid(alias_ref<jhybridobject> jobj) {
+  setCxxInstance(jobj);
 }
 
 void WritableNativeArray::pushNull() {

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

@@ -30,7 +30,7 @@ struct WritableNativeArray
   WritableNativeArray();
   WritableNativeArray(folly::dynamic&& val);
 
-  static jni::local_ref<jhybriddata> initHybrid(jni::alias_ref<jclass>);
+  static void initHybrid(jni::alias_ref<jhybridobject> jobj);
 
   void pushNull();
   void pushBoolean(jboolean value);

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

@@ -20,9 +20,8 @@ WritableNativeMap::WritableNativeMap(folly::dynamic&& val)
   }
 }
 
-local_ref<WritableNativeMap::jhybriddata> WritableNativeMap::initHybrid(
-    alias_ref<jclass>) {
-  return makeCxxInstance();
+void WritableNativeMap::initHybrid(alias_ref<jhybridobject> jobj) {
+  setCxxInstance(jobj);
 }
 
 void WritableNativeMap::putNull(std::string key) {

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

@@ -29,7 +29,7 @@ struct WritableNativeMap
   WritableNativeMap();
   WritableNativeMap(folly::dynamic&& val);
 
-  static jni::local_ref<jhybriddata> initHybrid(jni::alias_ref<jclass>);
+  static void initHybrid(jni::alias_ref<jhybridobject> jobj);
 
   void putNull(std::string key);
   void putBoolean(std::string key, bool val);

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.kt

@@ -8,6 +8,7 @@
 package com.facebook.react.bridge
 
 import com.facebook.react.turbomodule.core.interfaces.TurboModule
+import com.facebook.testutils.shadows.ShadowNativeLoader
 import com.facebook.testutils.shadows.ShadowSoLoader
 import org.junit.Before
 import org.junit.Test
@@ -18,7 +19,7 @@ import org.robolectric.RobolectricTestRunner
 import org.robolectric.annotation.Config
 
 /** Tests for [BaseJavaModule] and [JavaModuleWrapper] */
-@Config(shadows = [ShadowSoLoader::class])
+@Config(shadows = [ShadowSoLoader::class, ShadowNativeLoader::class])
 @RunWith(RobolectricTestRunner::class)
 class BaseJavaModuleTest {
   private lateinit var methods: List<JavaModuleWrapper.MethodDescriptor>

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/runtime/BridgelessReactContextTest.kt

@@ -15,6 +15,7 @@ import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
 import com.facebook.react.uimanager.UIManagerModule
 import com.facebook.testutils.shadows.ShadowArguments
 import com.facebook.testutils.shadows.ShadowNativeArray
+import com.facebook.testutils.shadows.ShadowNativeLoader
 import com.facebook.testutils.shadows.ShadowSoLoader
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Before
@@ -34,7 +35,12 @@ import org.robolectric.annotation.Config
 /** Tests [BridgelessReactContext] */
 @RunWith(RobolectricTestRunner::class)
 @Config(
-    shadows = [ShadowSoLoader::class, ShadowArguments::class, ShadowNativeArray.Writable::class])
+    shadows =
+        [
+            ShadowSoLoader::class,
+            ShadowNativeLoader::class,
+            ShadowArguments::class,
+            ShadowNativeArray.Writable::class])
 class BridgelessReactContextTest {
   private lateinit var context: Context
   private lateinit var reactHost: ReactHostImpl

packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/shadows/ShadowNativeLoader.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.shadows
+
+import com.facebook.soloader.nativeloader.NativeLoader
+import kotlin.jvm.JvmStatic
+import org.robolectric.annotation.Implementation
+import org.robolectric.annotation.Implements
+
+@Suppress("UNUSED_PARAMETER")
+@Implements(NativeLoader::class)
+class ShadowNativeLoader {
+  companion object {
+    @JvmStatic @Implementation fun loadLibrary(shortName: String?): Boolean = true
+  }
+}