[core][Android] Introduce base JS class for shared objects (#27331)

# Why A follow-up to the #27038. Currently, on the JS side, shared objects inherit only from Object. To provide some common functionalities, we're introducing a SharedObject class in JS (global.expo.SharedObject). # Test Plan - android tests ✅
expo · Feb 27, 2024 · 01a47a8 · 01a47a8
1 parent 0abd3b4
commit 01a47a8
Show file tree

Hide file tree

Showing 15 changed files with 187 additions and 83 deletions.
diff --git a/packages/expo-modules-core/CHANGELOG.md b/packages/expo-modules-core/CHANGELOG.md
@@ -13,7 +13,7 @@
 - Add iOS support for `PlatformColor` and `DynamicColorIOS` color props. ([#26724](https://github.com/expo/expo/pull/26724) by [@dlindenkreuz](https://github.com/dlindenkreuz))
 - `BarCodeScannerResult` interface now declares an additional `raw` field corresponding to the barcode value as it was encoded in the barcode without parsing. Will always be undefined on iOS. ([#25391](https://github.com/expo/expo/pull/25391) by [@ajacquierbret](https://github.com/ajacquierbret))
 - [Android] Added syntactic sugar for defining a prop group. ([#27004](https://github.com/expo/expo/pull/27004) by [@lukmccall](https://github.com/lukmccall))
-- Introduced a base class for all shared objects (`expo.SharedObject`) with a simple mechanism to release native pointer from JS. ([#27038](https://github.com/expo/expo/pull/27038) by [@tsapeta](https://github.com/tsapeta))
+- Introduced a base class for all shared objects (`expo.SharedObject`) with a simple mechanism to release native pointer from JS. ([#27038](https://github.com/expo/expo/pull/27038) by [@tsapeta](https://github.com/tsapeta) & [#27331](https://github.com/expo/expo/pull/27331) by [@lukmccall](https://github.com/lukmccall))
 - [iOS] Added native implementation of the JS EventEmitter. ([#27092](https://github.com/expo/expo/pull/27092) by [@tsapeta](https://github.com/tsapeta))
 - [iOS] Allow for the export of views that conform to `AnyExpoView` ([#27284](https://github.com/expo/expo/pull/27284) by [@dominicstop](https://github.com/dominicstop))
 - [iOS] Added support for native modules in bridgeless mode in React Native 0.74 ([#27242](https://github.com/expo/expo/pull/27242) by [@tsapeta](https://github.com/tsapeta))

diff --git a/...s/expo-modules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JSIModuleMock.kt b/...s/expo-modules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JSIModuleMock.kt
@@ -91,15 +91,13 @@ internal inline fun withJSIInterop(
       register(it)
     }
   }
-  val sharedObjectRegistry = SharedObjectRegistry()
+  val sharedObjectRegistry = SharedObjectRegistry(appContextMock)
   every { appContextMock.registry } answers { registry }
   every { appContextMock.sharedObjectRegistry } answers { sharedObjectRegistry }
 
-  val jsiIterop = JSIInteropModuleRegistry(appContextMock).apply {
-    installJSIForTests(jniDeallocator)
-  }
-
+  val jsiIterop = JSIInteropModuleRegistry()
   every { appContextMock.jsiInterop } answers { jsiIterop }
+  jsiIterop.installJSIForTests(appContextMock, jniDeallocator)
 
   block(jsiIterop, methodQueue)
 
@@ -119,7 +117,7 @@ open class TestContext(
 }
 
 class SingleTestContext(
-  private val moduleName: String,
+  moduleName: String,
   jsiInterop: JSIInteropModuleRegistry,
   methodQueue: TestScope
 ) : TestContext(jsiInterop, methodQueue) {

diff --git a/...-modules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JavaScriptClassTest.kt b/...-modules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JavaScriptClassTest.kt
@@ -41,7 +41,7 @@ class JavaScriptClassTest {
     }
   }) {
     val jsObject = callClass("MyClass").getObject()
-    Truth.assertThat(jsObject.getPropertyNames()).asList().containsExactly("foo")
+    Truth.assertThat(jsObject.getPropertyNames()).asList().contains("foo")
     Truth.assertThat(jsObject.getProperty("foo").getString()).isEqualTo("bar")
   }
 

diff --git a/...dules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JavaScriptFunctionTest.kt b/...dules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JavaScriptFunctionTest.kt
@@ -9,8 +9,8 @@ class JavaScriptFunctionTest {
 
   @Before
   fun before() {
-    jsiInterop = JSIInteropModuleRegistry(defaultAppContextMock()).apply {
-      installJSIForTests()
+    jsiInterop = JSIInteropModuleRegistry().apply {
+      installJSIForTests(defaultAppContextMock())
     }
   }
 

diff --git a/...modules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JavaScriptObjectTest.kt b/...modules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JavaScriptObjectTest.kt
@@ -16,8 +16,8 @@ class JavaScriptObjectTest {
 
   @Before
   fun before() {
-    jsiInterop = JSIInteropModuleRegistry(defaultAppContextMock()).apply {
-      installJSIForTests()
+    jsiInterop = JSIInteropModuleRegistry().apply {
+      installJSIForTests(defaultAppContextMock())
     }
   }
 

diff --git a/...odules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JavaScriptRuntimeTest.kt b/...odules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JavaScriptRuntimeTest.kt
@@ -11,8 +11,8 @@ class JavaScriptRuntimeTest {
 
   @Before
   fun before() {
-    jsiInterop = JSIInteropModuleRegistry(defaultAppContextMock()).apply {
-      installJSIForTests()
+    jsiInterop = JSIInteropModuleRegistry().apply {
+      installJSIForTests(defaultAppContextMock())
     }
   }
 

diff --git a/...-modules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JavaScriptValueTest.kt b/...-modules-core/android/src/androidTest/java/expo/modules/kotlin/jni/JavaScriptValueTest.kt
@@ -10,8 +10,8 @@ class JavaScriptValueTest {
 
   @Before
   fun before() {
-    jsiInterop = JSIInteropModuleRegistry(defaultAppContextMock()).apply {
-      installJSIForTests()
+    jsiInterop = JSIInteropModuleRegistry().apply {
+      installJSIForTests(defaultAppContextMock())
     }
   }
 

diff --git a/packages/expo-modules-core/android/src/main/cpp/JSIInteropModuleRegistry.cpp b/packages/expo-modules-core/android/src/main/cpp/JSIInteropModuleRegistry.cpp
@@ -4,6 +4,8 @@
 #include "ExpoModulesHostObject.h"
 #include "JavaReferencesCache.h"
 #include "JSReferencesCache.h"
+#include "SharedObject.h"
+#include <android/log.h>
 
 #include <fbjni/detail/Meta.h>
 #include <fbjni/fbjni.h>
@@ -31,12 +33,21 @@ void JSIInteropModuleRegistry::registerNatives() {
                    makeNativeMethod("createObject", JSIInteropModuleRegistry::createObject),
                    makeNativeMethod("drainJSEventLoop", JSIInteropModuleRegistry::drainJSEventLoop),
                    makeNativeMethod("wasDeallocated", JSIInteropModuleRegistry::jniWasDeallocated),
+                   makeNativeMethod("setNativeStateForSharedObject",
+                                    JSIInteropModuleRegistry::jniSetNativeStateForSharedObject),
                  });
 }
 
 JSIInteropModuleRegistry::JSIInteropModuleRegistry(jni::alias_ref<jhybridobject> jThis)
   : javaPart_(jni::make_global(jThis)) {}
 
+JSIInteropModuleRegistry::~JSIInteropModuleRegistry() {
+  // The runtime would be deallocated automatically.
+  // However, we need to enforce the order of deallocations.
+  // The runtime has to be deallocated before the JNI part.
+  runtimeHolder.reset();
+}
+
 void JSIInteropModuleRegistry::installJSI(
   jlong jsRuntimePointer,
   jni::alias_ref<JNIDeallocator::javaobject> jniDeallocator,
@@ -82,19 +93,36 @@ void JSIInteropModuleRegistry::installJSIForTests(
 
   jsRegistry = std::make_unique<JSReferencesCache>(jsiRuntime);
 
+  prepareRuntime();
+#endif // !UNIT_TEST
+}
+
+void JSIInteropModuleRegistry::prepareRuntime() {
   runtimeHolder->installMainObject();
 
   auto expoModules = std::make_shared<ExpoModulesHostObject>(this);
-  auto expoModulesObject = jsi::Object::createFromHostObject(jsiRuntime, expoModules);
+  auto expoModulesObject = jsi::Object::createFromHostObject(
+    runtimeHolder->get(),
+    expoModules
+  );
 
+  EventEmitter::installClass(runtimeHolder->get());
+
+  // Define the `global.expo.modules` object.
   runtimeHolder
     ->getMainObject()
     ->setProperty(
-      jsiRuntime,
+      runtimeHolder->get(),
       "modules",
-      std::move(expoModulesObject)
+      expoModulesObject
     );
-#endif // !UNIT_TEST
+
+  SharedObject::installBaseClass(
+    runtimeHolder->get(),
+    [this](const SharedObject::ObjectId objectId) {
+      deleteSharedObject(objectId);
+    }
+  );
 }
 
 jni::local_ref<JavaScriptModuleObject::javaobject>
@@ -181,6 +209,14 @@ void JSIInteropModuleRegistry::registerSharedObject(
   method(javaPart_, std::move(native), std::move(js));
 }
 
+void JSIInteropModuleRegistry::deleteSharedObject(int objectId) {
+  const static auto method = expo::JSIInteropModuleRegistry::javaClassLocal()
+    ->getMethod<void(int)>(
+      "deleteSharedObject"
+    );
+  method(javaPart_, objectId);
+}
+
 void JSIInteropModuleRegistry::registerClass(
   jni::local_ref<jclass> native,
   jni::local_ref<JavaScriptObject::javaobject> jsClass
@@ -205,4 +241,21 @@ jni::local_ref<JavaScriptObject::javaobject> JSIInteropModuleRegistry::getJavasc
 void JSIInteropModuleRegistry::jniWasDeallocated() {
   wasDeallocated = true;
 }
+
+void JSIInteropModuleRegistry::jniSetNativeStateForSharedObject(
+  int id,
+  jni::alias_ref<JavaScriptObject::javaobject> jsObject
+) {
+  auto nativeState = std::make_shared<expo::SharedObject::NativeState>(
+    id,
+    [this](int id) {
+      deleteSharedObject(id);
+    }
+  );
+
+  jsObject
+    ->cthis()
+    ->get()
+    ->setNativeState(runtimeHolder->get(), std::move(nativeState));
+}
 } // namespace expo
diff --git a/packages/expo-modules-core/android/src/main/cpp/JSIInteropModuleRegistry.h b/packages/expo-modules-core/android/src/main/cpp/JSIInteropModuleRegistry.h
@@ -14,8 +14,11 @@
 #include <jsi/jsi.h>
 #include <ReactCommon/CallInvokerHolder.h>
 #include <ReactCommon/CallInvoker.h>
+
 #if REACT_NATIVE_TARGET_VERSION >= 73
+
 #include <ReactCommon/NativeMethodCallInvokerHolder.h>
+
 #endif
 
 #include <memory>
@@ -100,6 +103,10 @@ class JSIInteropModuleRegistry : public jni::HybridClass<JSIInteropModuleRegistr
     jni::local_ref<JavaScriptObject::javaobject> js
   );
 
+  void deleteSharedObject(
+    int objectId
+  );
+
   /**
    * Exposes a `JavaScriptRuntime::drainJSEventLoop` function to Kotlin
    */
@@ -112,9 +119,13 @@ class JSIInteropModuleRegistry : public jni::HybridClass<JSIInteropModuleRegistr
 
   bool wasDeallocated = false;
 
-  void registerClass(jni::local_ref<jclass> native,jni::local_ref<JavaScriptObject::javaobject> jsClass);
+  void registerClass(jni::local_ref<jclass> native,
+                     jni::local_ref<JavaScriptObject::javaobject> jsClass);
+
   jni::local_ref<JavaScriptObject::javaobject> getJavascriptClass(jni::local_ref<jclass> native);
 
+  ~JSIInteropModuleRegistry();
+
 private:
   friend HybridBase;
   jni::global_ref<JSIInteropModuleRegistry::javaobject> javaPart_;
@@ -131,5 +142,9 @@ class JSIInteropModuleRegistry : public jni::HybridClass<JSIInteropModuleRegistr
   inline bool callHasModule(const std::string &moduleName) const;
 
   void jniWasDeallocated();
+
+  void prepareRuntime();
+
+  void jniSetNativeStateForSharedObject(int id, jni::alias_ref<JavaScriptObject::javaobject> jsObject);
 };
 } // namespace expo
diff --git a/packages/expo-modules-core/android/src/main/cpp/JavaScriptModuleObject.cpp b/packages/expo-modules-core/android/src/main/cpp/JavaScriptModuleObject.cpp
@@ -3,6 +3,7 @@
 #include "JavaScriptModuleObject.h"
 #include "JSIInteropModuleRegistry.h"
 #include "JSIUtils.h"
+#include "SharedObject.h"
 
 #include <folly/dynamic.h>
 #include <jsi/JSIDynamic.h>
@@ -143,44 +144,15 @@ std::shared_ptr<jsi::Object> JavaScriptModuleObject::getJSIObject(jsi::Runtime &
     auto classObject = classRef->cthis();
     classObject->jsiInteropModuleRegistry = jsiInteropModuleRegistry;
 
-    std::string nativeConstructorKey("__native_constructor__");
-
-    // Create a string buffer of the source code to evaluate.
-    std::stringstream source;
-    source << "(function " << name << "(...args) { this." << nativeConstructorKey
-           << "(...args); return this; })";
-    std::shared_ptr<jsi::StringBuffer> sourceBuffer = std::make_shared<jsi::StringBuffer>(
-      source.str());
-
-    // Evaluate the code and obtain returned value (the constructor function).
-    jsi::Object klass = runtime.evaluateJavaScript(sourceBuffer, "").asObject(runtime);
-    auto klassSharedPtr = std::make_shared<jsi::Object>(std::move(klass));
-
-    auto jsThisObject = JavaScriptObject::newInstance(
-      jsiInteropModuleRegistry,
-      jsiInteropModuleRegistry->runtimeHolder,
-      klassSharedPtr
-    );
-
-    if(ownerClass != nullptr) {
-      jsiInteropModuleRegistry->registerClass(jni::make_local(ownerClass), jsThisObject);
-    }
-
-    // Set the native constructor in the prototype.
-    jsi::Object prototype = klassSharedPtr->getPropertyAsObject(runtime, "prototype");
-    jsi::PropNameID nativeConstructorPropId = jsi::PropNameID::forAscii(runtime,
-                                                                        nativeConstructorKey);
-    jsi::Function nativeConstructor = jsi::Function::createFromHostFunction(
+    auto klass = SharedObject::createClass(
       runtime,
-      nativeConstructorPropId,
-      // The paramCount is not obligatory to match, it only affects the `length` property of the function.
-      0,
+      name.c_str(),
       [classObject, &constructor = constructor, jsiInteropModuleRegistry = jsiInteropModuleRegistry](
         jsi::Runtime &runtime,
         const jsi::Value &thisValue,
         const jsi::Value *args,
         size_t count
-      ) -> jsi::Value {
+      ) {
         auto thisObject = std::make_shared<jsi::Object>(thisValue.asObject(runtime));
         decorateObjectWithProperties(runtime, jsiInteropModuleRegistry, thisObject.get(),
                                      classObject);
@@ -201,7 +173,7 @@ std::shared_ptr<jsi::Object> JavaScriptModuleObject::getJSIObject(jsi::Runtime &
             count
           );
           if (result == nullptr) {
-            return jsi::Value::undefined();
+            return;
           }
           jobject unpackedResult = result.get();
           jclass resultClass = env->GetObjectClass(unpackedResult);
@@ -220,24 +192,36 @@ std::shared_ptr<jsi::Object> JavaScriptModuleObject::getJSIObject(jsi::Runtime &
         } catch (jni::JniException &jniException) {
           rethrowAsCodedError(runtime, jniException);
         }
-        return jsi::Value::undefined();
-      });
+      }
+    );
 
-    auto descriptor = JavaScriptObject::preparePropertyDescriptor(runtime, 0);
-    descriptor.setProperty(runtime, "value", jsi::Value(runtime, nativeConstructor));
+    auto klassSharedPtr = std::make_shared<jsi::Function>(std::move(klass));
 
-    common::defineProperty(runtime, &prototype, nativeConstructorKey.c_str(), std::move(descriptor));
+    auto jsThisObject = JavaScriptObject::newInstance(
+      jsiInteropModuleRegistry,
+      jsiInteropModuleRegistry->runtimeHolder,
+      klassSharedPtr
+    );
+
+    if (ownerClass != nullptr) {
+      jsiInteropModuleRegistry->registerClass(jni::make_local(ownerClass), jsThisObject);
+    }
 
     moduleObject->setProperty(
       runtime,
       jsi::String::createFromUtf8(runtime, name),
-      jsi::Value(runtime, klassSharedPtr->asFunction(runtime))
+      jsi::Value(runtime, *klassSharedPtr.get())
     );
 
+    jsi::PropNameID prototypePropNameId = jsi::PropNameID::forAscii(runtime, "prototype", 9);
+    jsi::Object klassPrototype = klassSharedPtr
+      ->getProperty(runtime, prototypePropNameId)
+      .asObject(runtime);
+
     decorateObjectWithFunctions(
       runtime,
       jsiInteropModuleRegistry,
-      &prototype,
+      &klassPrototype,
       classObject
     );
   }
@@ -316,7 +300,11 @@ void JavaScriptModuleObject::registerClass(
     jni::make_global(body)
   );
 
-  auto classTuple = std::make_tuple(jni::make_global(classObject), std::move(constructor), jni::make_global(ownerClass));
+  auto classTuple = std::make_tuple(
+    jni::make_global(classObject),
+    std::move(constructor),
+    jni::make_global(ownerClass)
+  );
 
   classes.try_emplace(
     cName,

diff --git a/packages/expo-modules-core/android/src/main/java/expo/modules/kotlin/AppContext.kt b/packages/expo-modules-core/android/src/main/java/expo/modules/kotlin/AppContext.kt
@@ -79,7 +79,7 @@ class AppContext(
     ModuleHolder(module)
   }
 
-  internal val sharedObjectRegistry = SharedObjectRegistry()
+  internal val sharedObjectRegistry = SharedObjectRegistry(this)
 
   internal val classRegistry = ClassRegistry()
 
@@ -154,7 +154,7 @@ class AppContext(
 
     trace("AppContext.installJSIInterop") {
       try {
-        jsiInterop = JSIInteropModuleRegistry(this)
+        jsiInterop = JSIInteropModuleRegistry()
         val reactContext = reactContextHolder.get() ?: return@trace
         val jsContextHolder = reactContext.javaScriptContextHolder?.get() ?: return@trace
 
@@ -168,6 +168,7 @@ class AppContext(
           .takeIf { it != 0L }
           ?.let {
             jsiInterop.installJSI(
+              this,
               it,
               jniDeallocator,
               jsCallInvokerHolder