From c9006ce5fb8bfb040b307d0bb573d076f8cf3f32 Mon Sep 17 00:00:00 2001
From: Ramanpreet Nara <ramanpreet@fb.com>
Date: Fri, 3 May 2019 11:56:58 -0700
Subject: [PATCH] Refactor getConstants implementation

Summary: Previously, we'd override the `TurboModule::get` method inside the `JavaTurboModule` class to return a special `jsi::Function` in the case that the property being accessed was "getConstants". We really don't need to do this because we can simply special-case the invocation of the `getConstants` method inside the `JavaTurboModule::invokeJavaMethod` method.

Reviewed By: mdvacca

Differential Revision: D15174822

fbshipit-source-id: 0ee705be841757d3870c908da911c3872b977a9f
---
 .../core/platform/android/JavaTurboModule.cpp | 44 ++++++-------------
 .../core/platform/android/JavaTurboModule.h   |  2 -
 2 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.cpp b/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.cpp
index 988b82dfd..d30a93c54 100644
--- a/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.cpp
+++ b/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.cpp
@@ -117,33 +117,6 @@ jsi::Value convertFromJMapToValue(JNIEnv *env, jsi::Runtime &rt, jobject arg) {
     return jsi::valueFromDynamic(rt, result->cthis()->consume());
 }
 
-jsi::Value JavaTurboModule::get(jsi::Runtime& runtime, const jsi::PropNameID& propName) {
-  std::string propNameUtf8 = propName.utf8(runtime);
-  if (propNameUtf8 == "getConstants") {
-    // This is the special method to get the constants from the module.
-    // Since `getConstants` in Java only returns a Map, this function takes the map
-    // and converts it to a WritableMap.
-    return jsi::Function::createFromHostFunction(
-      runtime,
-      propName,
-      0,
-      [this](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) {
-        JNIEnv *env = jni::Environment::current();
-        auto instance = instance_.get();
-        jclass cls = env->GetObjectClass(instance);
-        static jmethodID methodID = env->GetMethodID(cls, "getConstants", "()Ljava/util/Map;");
-        auto constantsMap = (jobject) env->CallObjectMethod(instance_.get(), methodID);
-        if (constantsMap == nullptr) {
-          return jsi::Value::undefined();
-        }
-        return convertFromJMapToValue(env, rt, constantsMap);
-      }
-    );
-  } else {
-    return TurboModule::get(runtime, propName);
-  }
-}
-
 static void throwIfJNIReportsPendingException() {
   JNIEnv *env = jni::Environment::current();
   if (env->ExceptionCheck()) {
@@ -168,15 +141,24 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
     const std::string &methodSignature,
     const jsi::Value *args,
     size_t count) {
-  // We are using JNI directly instead of fbjni since we don't want template functiosn
-  // when finding methods.
   JNIEnv *env = jni::Environment::current();
   auto instance = instance_.get();
 
   jclass cls = env->GetObjectClass(instance);
+  jmethodID methodID =
+      env->GetMethodID(cls, methodName.c_str(), methodSignature.c_str());
 
-  // TODO (axe) Memoize method call, so we don't look it up each time the method is called
-  jmethodID methodID = env->GetMethodID(cls, methodName.c_str(), methodSignature.c_str());
+  // TODO(T43933641): Refactor to remove this special-casing
+  if (methodName == "getConstants") {
+    auto constantsMap = (jobject)env->CallObjectMethod(instance, methodID);
+    throwIfJNIReportsPendingException();
+
+    if (constantsMap == nullptr) {
+      return jsi::Value::undefined();
+    }
+
+    return convertFromJMapToValue(env, runtime, constantsMap);
+  }
 
   std::vector<jvalue> jargs =
       convertJSIArgsToJNIArgs(env, runtime, args, count, jsInvoker_, valueKind);
diff --git a/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.h b/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.h
index ce4fd0f5e..fa31b0c73 100644
--- a/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.h
+++ b/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.h
@@ -30,8 +30,6 @@ public:
       const std::string &methodSignature,
       const jsi::Value *args,
       size_t count);
-
-  virtual facebook::jsi::Value get(facebook::jsi::Runtime& runtime, const facebook::jsi::PropNameID& propName) override;
 private:
   jni::global_ref<JTurboModule> instance_;
   jclass findClass(JNIEnv *env) const;