Revert D16555673: [RN][TurboModule] Move ownership of TurboModule jni ref to JavaTurboModule

Differential Revision:
D16555673

Original commit changeset: 2778fc5a372c

fbshipit-source-id: fac3a6ea185acaa750f58e19d24c194668749636

ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.cpp

@@ -24,23 +24,23 @@ TurboModuleManager::TurboModuleManager(
   jni::alias_ref<TurboModuleManager::javaobject> jThis,
   jsi::Runtime* rt,
   std::shared_ptr<JSCallInvoker> jsCallInvoker,
-  jni::alias_ref<TurboModuleManagerDelegate::javaobject> delegate
+  jni::alias_ref<TurboModuleManagerDelegate::javaobject> tmmDelegate
 ):
   javaPart_(jni::make_global(jThis)),
   runtime_(rt),
   jsCallInvoker_(jsCallInvoker),
-  delegate_(jni::make_global(delegate))
+  turboModuleManagerDelegate_(jni::make_global(tmmDelegate))
   {}
 
 jni::local_ref<TurboModuleManager::jhybriddata> TurboModuleManager::initHybrid(
   jni::alias_ref<jhybridobject> jThis,
   jlong jsContext,
   jni::alias_ref<JSCallInvokerHolder::javaobject> jsCallInvokerHolder,
-  jni::alias_ref<TurboModuleManagerDelegate::javaobject> delegate
+  jni::alias_ref<TurboModuleManagerDelegate::javaobject> tmmDelegate
 ) {
   auto jsCallInvoker = jsCallInvokerHolder->cthis()->getJSCallInvoker();
 
-  return makeCxxInstance(jThis, (jsi::Runtime *) jsContext, jsCallInvoker, delegate);
+  return makeCxxInstance(jThis, (jsi::Runtime *) jsContext, jsCallInvoker, tmmDelegate);
 }
 
 void TurboModuleManager::registerNatives() {
@@ -61,26 +61,23 @@ void TurboModuleManager::installJSIBindings() {
           return turboModuleLookup->second;
         }
 
-        auto cxxModule = delegate_->cthis()->getTurboModule(name, jsCallInvoker_);
+        auto cxxModule = turboModuleManagerDelegate_->cthis()->getTurboModule(name, jsCallInvoker_);
         if (cxxModule) {
           turboModuleCache_.insert({name, cxxModule});
           return cxxModule;
         }
 
-        static auto getLegacyCxxModule = delegate_->getClass()->getMethod<jni::alias_ref<CxxModuleWrapper::javaobject>(const std::string&)>("getLegacyCxxModule");
-        auto legacyCxxModule = getLegacyCxxModule(delegate_.get(), name);
-
+        auto legacyCxxModule = getLegacyCxxJavaModule(name);
         if (legacyCxxModule) {
           auto turboModule = std::make_shared<react::TurboCxxModule>(legacyCxxModule->cthis()->getModule(), jsCallInvoker_);
           turboModuleCache_.insert({name, turboModule});
           return turboModule;
         }
 
-        static auto getJavaModule = javaClassStatic()->getMethod<jni::alias_ref<JTurboModule>(const std::string&)>("getJavaModule");
-        auto moduleInstance = getJavaModule(javaPart_.get(), name);
+        auto moduleInstance = getJavaModule(name);
 
         if (moduleInstance) {
-          auto turboModule = delegate_->cthis()->getTurboModule(name, moduleInstance, jsCallInvoker_);
+          auto turboModule = turboModuleManagerDelegate_->cthis()->getTurboModule(name, moduleInstance, jsCallInvoker_);
           turboModuleCache_.insert({name, turboModule});
           return turboModule;
         }
@@ -90,5 +87,19 @@ void TurboModuleManager::installJSIBindings() {
   );
 }
 
+jni::global_ref<JTurboModule> TurboModuleManager::getJavaModule(std::string name) {
+  static auto method = javaClassStatic()->getMethod<jni::alias_ref<JTurboModule>(const std::string&)>("getJavaModule");
+
+  auto module = jni::make_global(method(javaPart_.get(), name));
+
+  return module;
+}
+
+jni::global_ref<CxxModuleWrapper::javaobject> TurboModuleManager::getLegacyCxxJavaModule(std::string name) {
+  static auto method = turboModuleManagerDelegate_->getClass()->getMethod<jni::alias_ref<CxxModuleWrapper::javaobject>(const std::string&)>("getLegacyCxxModule");
+  auto module = jni::make_global(method(turboModuleManagerDelegate_.get(), name));
+  return module;
+}
+
 } // namespace react
 } // namespace facebook

ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManager.h

@@ -28,7 +28,7 @@ public:
     jni::alias_ref<jhybridobject> jThis,
     jlong jsContext,
     jni::alias_ref<JSCallInvokerHolder::javaobject> jsCallInvokerHolder,
-    jni::alias_ref<TurboModuleManagerDelegate::javaobject> delegate
+    jni::alias_ref<TurboModuleManagerDelegate::javaobject> tmmDelegate
   );
   static void registerNatives();
 private:
@@ -36,7 +36,7 @@ private:
   jni::global_ref<TurboModuleManager::javaobject> javaPart_;
   jsi::Runtime* runtime_;
   std::shared_ptr<JSCallInvoker> jsCallInvoker_;
-  jni::global_ref<TurboModuleManagerDelegate::javaobject> delegate_;
+  jni::global_ref<TurboModuleManagerDelegate::javaobject> turboModuleManagerDelegate_;
 
   /**
    * TODO(T48018690):
@@ -46,12 +46,14 @@ private:
    */
   std::unordered_map<std::string, std::shared_ptr<react::TurboModule>> turboModuleCache_;
 
+  jni::global_ref<JTurboModule> getJavaModule(std::string name);
+  jni::global_ref<CxxModuleWrapper::javaobject> getLegacyCxxJavaModule(std::string name);
   void installJSIBindings();
   explicit TurboModuleManager(
     jni::alias_ref<TurboModuleManager::jhybridobject> jThis,
     jsi::Runtime *rt,
     std::shared_ptr<JSCallInvoker> jsCallInvoker,
-    jni::alias_ref<TurboModuleManagerDelegate::javaobject> delegate
+    jni::alias_ref<TurboModuleManagerDelegate::javaobject> tmmDelegate
   );
 };
 

ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManagerDelegate.h

@@ -20,7 +20,7 @@ class TurboModuleManagerDelegate : public jni::HybridClass<TurboModuleManagerDel
 public:
   static auto constexpr kJavaDescriptor = "Lcom/facebook/react/turbomodule/core/TurboModuleManagerDelegate;";
 
-  virtual std::shared_ptr<TurboModule> getTurboModule(std::string name, jni::alias_ref<JTurboModule> turboModule, std::shared_ptr<JSCallInvoker> jsInvoker) = 0;
+  virtual std::shared_ptr<TurboModule> getTurboModule(std::string name, jni::global_ref<JTurboModule> turboModule, std::shared_ptr<JSCallInvoker> jsInvoker) = 0;
   virtual std::shared_ptr<TurboModule> getTurboModule(std::string name, std::shared_ptr<JSCallInvoker> jsInvoker) = 0;
 
 private:

ReactCommon/turbomodule/core/platform/android/JavaTurboModule.cpp

@@ -25,11 +25,8 @@
 namespace facebook {
 namespace react {
 
-JavaTurboModule::JavaTurboModule(
-    const std::string &name,
-    jni::alias_ref<JTurboModule> instance,
-    std::shared_ptr<JSCallInvoker> jsInvoker)
-    : TurboModule(name, jsInvoker), instance_(jni::make_global(instance)) {}
+JavaTurboModule::JavaTurboModule(const std::string &name, jni::global_ref<JTurboModule> instance, std::shared_ptr<JSCallInvoker> jsInvoker)
+  : TurboModule(name, jsInvoker), instance_(instance) {}
 
 jni::local_ref<JCxxCallbackImpl::JavaPart> createJavaCallbackFromJSIFunction(
     jsi::Function &function,
@@ -325,20 +322,16 @@ std::vector<jvalue> convertJSIArgsToJNIArgs(
 }
 
 jsi::Value convertFromJMapToValue(JNIEnv *env, jsi::Runtime &rt, jobject arg) {
-  // We currently use Java Argument.makeNativeMap() method to do this conversion
-  // This could also be done purely in C++, but iterative over map methods
-  // but those may end up calling reflection methods anyway
-  // TODO (axe) Investigate the best way to convert Java Map to Value
-  jclass jArguments = env->FindClass("com/facebook/react/bridge/Arguments");
-  static jmethodID jMakeNativeMap = env->GetStaticMethodID(
-      jArguments,
-      "makeNativeMap",
-      "(Ljava/util/Map;)Lcom/facebook/react/bridge/WritableNativeMap;");
-  auto constants =
-      (jobject)env->CallStaticObjectMethod(jArguments, jMakeNativeMap, arg);
-  auto jResult = jni::adopt_local(constants);
-  auto result = jni::static_ref_cast<NativeMap::jhybridobject>(jResult);
-  return jsi::valueFromDynamic(rt, result->cthis()->consume());
+    // We currently use Java Argument.makeNativeMap() method to do this conversion
+    // This could also be done purely in C++, but iterative over map methods
+    // but those may end up calling reflection methods anyway
+    // TODO (axe) Investigate the best way to convert Java Map to Value
+    jclass jArguments = env->FindClass("com/facebook/react/bridge/Arguments");
+    static jmethodID jMakeNativeMap = env->GetStaticMethodID(jArguments, "makeNativeMap", "(Ljava/util/Map;)Lcom/facebook/react/bridge/WritableNativeMap;");
+    auto constants = (jobject) env->CallStaticObjectMethod(jArguments, jMakeNativeMap, arg);
+    auto jResult = jni::adopt_local(constants);
+    auto result = jni::static_ref_cast<NativeMap::jhybridobject>(jResult);
+    return jsi::valueFromDynamic(rt, result->cthis()->consume());
 }
 
 jsi::Value JavaTurboModule::invokeJavaMethod(

ReactCommon/turbomodule/core/platform/android/JavaTurboModule.h

@@ -17,16 +17,12 @@ namespace facebook {
 namespace react {
 
 struct JTurboModule : jni::JavaClass<JTurboModule> {
-  static auto constexpr kJavaDescriptor =
-      "Lcom/facebook/react/turbomodule/core/interfaces/TurboModule;";
+  static auto constexpr kJavaDescriptor = "Lcom/facebook/react/turbomodule/core/interfaces/TurboModule;";
 };
 
 class JSI_EXPORT JavaTurboModule : public TurboModule {
- public:
-  JavaTurboModule(
-      const std::string &name,
-      jni::alias_ref<JTurboModule> instance,
-      std::shared_ptr<JSCallInvoker> jsInvoker);
+public:
+  JavaTurboModule(const std::string &name, jni::global_ref<JTurboModule> instance, std::shared_ptr<JSCallInvoker> jsInvoker);
   jsi::Value invokeJavaMethod(
       jsi::Runtime &runtime,
       TurboModuleMethodValueKind valueKind,
@@ -34,8 +30,7 @@ class JSI_EXPORT JavaTurboModule : public TurboModule {
       const std::string &methodSignature,
       const jsi::Value *args,
       size_t count);
-
- private:
+private:
   jni::global_ref<JTurboModule> instance_;
   jclass findClass(JNIEnv *env) const;
 };