Stop requiring registration of callable JS modules

Reviewed By: AaaChiuuu

Differential Revision: D5229073

fbshipit-source-id: d6d1967982ae379733a7e9667515ca9f074aadd4

Author: javache

ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java

@@ -75,7 +75,7 @@ public PendingJSCall(
   private final ArrayList<PendingJSCall> mJSCallsPendingInit = new ArrayList<PendingJSCall>();
   private final Object mJSCallsPendingInitLock = new Object();
 
-  private final NativeModuleRegistry mJavaRegistry;
+  private final NativeModuleRegistry mNativeModuleRegistry;
   private final NativeModuleCallExceptionHandler mNativeModuleCallExceptionHandler;
   private final MessageQueueThread mNativeModulesQueueThread;
   private final @Nullable MessageQueueThread mUIBackgroundQueueThread;
@@ -92,8 +92,7 @@ public PendingJSCall(
   private CatalystInstanceImpl(
       final ReactQueueConfigurationSpec reactQueueConfigurationSpec,
       final JavaScriptExecutor jsExecutor,
-      final NativeModuleRegistry registry,
-      final JavaScriptModuleRegistry jsModuleRegistry,
+      final NativeModuleRegistry nativeModuleRegistry,
       final JSBundleLoader jsBundleLoader,
       NativeModuleCallExceptionHandler nativeModuleCallExceptionHandler) {
     Log.d(ReactConstants.TAG, "Initializing React Xplat Bridge.");
@@ -103,8 +102,8 @@ private CatalystInstanceImpl(
         reactQueueConfigurationSpec,
         new NativeExceptionHandler());
     mBridgeIdleListeners = new CopyOnWriteArrayList<>();
-    mJavaRegistry = registry;
-    mJSModuleRegistry = jsModuleRegistry;
+    mNativeModuleRegistry = nativeModuleRegistry;
+    mJSModuleRegistry = new JavaScriptModuleRegistry();
     mJSBundleLoader = jsBundleLoader;
     mNativeModuleCallExceptionHandler = nativeModuleCallExceptionHandler;
     mNativeModulesQueueThread = mReactQueueConfiguration.getNativeModulesQueueThread();
@@ -118,8 +117,8 @@ private CatalystInstanceImpl(
       mReactQueueConfiguration.getJSQueueThread(),
       mNativeModulesQueueThread,
       mUIBackgroundQueueThread,
-      mJavaRegistry.getJavaModules(this),
-      mJavaRegistry.getCxxModules());
+      mNativeModuleRegistry.getJavaModules(this),
+      mNativeModuleRegistry.getCxxModules());
     Log.d(ReactConstants.TAG, "Initializing React Xplat Bridge after initializeBridge");
   }
 
@@ -137,7 +136,7 @@ public BridgeCallback(CatalystInstanceImpl outer) {
     public void onBatchComplete() {
       CatalystInstanceImpl impl = mOuter.get();
       if (impl != null) {
-        impl.mJavaRegistry.onBatchComplete();
+        impl.mNativeModuleRegistry.onBatchComplete();
       }
     }
 
@@ -294,7 +293,7 @@ public void destroy() {
     mNativeModulesQueueThread.runOnQueue(new Runnable() {
       @Override
       public void run() {
-        mJavaRegistry.notifyJSInstanceDestroy();
+        mNativeModuleRegistry.notifyJSInstanceDestroy();
         boolean wasIdle = (mPendingJSCalls.getAndSet(0) == 0);
         if (!wasIdle && !mBridgeIdleListeners.isEmpty()) {
           for (NotThreadSafeBridgeIdleDebugListener listener : mBridgeIdleListeners) {
@@ -342,7 +341,7 @@ public void initialize() {
     mNativeModulesQueueThread.runOnQueue(new Runnable() {
       @Override
       public void run() {
-        mJavaRegistry.notifyJSInstanceInitialized();
+        mNativeModuleRegistry.notifyJSInstanceInitialized();
       }
     });
   }
@@ -359,19 +358,19 @@ public <T extends JavaScriptModule> T getJSModule(Class<T> jsInterface) {
 
   @Override
   public <T extends NativeModule> boolean hasNativeModule(Class<T> nativeModuleInterface) {
-    return mJavaRegistry.hasModule(nativeModuleInterface);
+    return mNativeModuleRegistry.hasModule(nativeModuleInterface);
   }
 
   // This is only ever called with UIManagerModule or CurrentViewerModule.
   @Override
   public <T extends NativeModule> T getNativeModule(Class<T> nativeModuleInterface) {
-    return mJavaRegistry.getModule(nativeModuleInterface);
+    return mNativeModuleRegistry.getModule(nativeModuleInterface);
   }
 
   // This is only used by com.facebook.react.modules.common.ModuleDataCleaner
   @Override
   public Collection<NativeModule> getNativeModules() {
-    return mJavaRegistry.getAllModules();
+    return mNativeModuleRegistry.getAllModules();
   }
 
   private native void handleMemoryPressureUiHidden();
@@ -528,7 +527,6 @@ public static class Builder {
     private @Nullable ReactQueueConfigurationSpec mReactQueueConfigurationSpec;
     private @Nullable JSBundleLoader mJSBundleLoader;
     private @Nullable NativeModuleRegistry mRegistry;
-    private @Nullable JavaScriptModuleRegistry mJSModuleRegistry;
     private @Nullable JavaScriptExecutor mJSExecutor;
     private @Nullable NativeModuleCallExceptionHandler mNativeModuleCallExceptionHandler;
 
@@ -544,7 +542,6 @@ public Builder setRegistry(NativeModuleRegistry registry) {
     }
 
     public Builder setJSModuleRegistry(JavaScriptModuleRegistry jsModuleRegistry) {
-      mJSModuleRegistry = jsModuleRegistry;
       return this;
     }
 
@@ -569,7 +566,6 @@ public CatalystInstanceImpl build() {
           Assertions.assertNotNull(mReactQueueConfigurationSpec),
           Assertions.assertNotNull(mJSExecutor),
           Assertions.assertNotNull(mRegistry),
-          Assertions.assertNotNull(mJSModuleRegistry),
           Assertions.assertNotNull(mJSBundleLoader),
           Assertions.assertNotNull(mNativeModuleCallExceptionHandler));
     }

ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistration.java

@@ -11,35 +11,25 @@
 
 import javax.annotation.Nullable;
 
-import java.lang.ref.WeakReference;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
-import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.List;
-import java.util.WeakHashMap;
+import java.util.HashSet;
+import java.util.Set;
 
-import com.facebook.common.logging.FLog;
-import com.facebook.infer.annotation.Assertions;
-import com.facebook.react.common.ReactConstants;
+import com.facebook.react.common.build.ReactBuildConfig;
 
 /**
- * Class responsible for holding all the {@link JavaScriptModule}s registered to this
- * {@link CatalystInstance}. Uses Java proxy objects to dispatch method calls on JavaScriptModules
- * to the bridge using the corresponding module and method ids so the proper function is executed in
- * JavaScript.
+ * Class responsible for holding all the {@link JavaScriptModule}s.  Uses Java proxy objects
+ * to dispatch method calls on JavaScriptModules to the bridge using the corresponding
+ * module and method ids so the proper function is executed in JavaScript.
  */
-public class JavaScriptModuleRegistry {
+public final class JavaScriptModuleRegistry {
   private final HashMap<Class<? extends JavaScriptModule>, JavaScriptModule> mModuleInstances;
-  private final HashMap<Class<? extends JavaScriptModule>, JavaScriptModuleRegistration> mModuleRegistrations;
 
-  public JavaScriptModuleRegistry(List<JavaScriptModuleRegistration> config) {
+  public JavaScriptModuleRegistry() {
     mModuleInstances = new HashMap<>();
-    mModuleRegistrations = new HashMap<>();
-    for (JavaScriptModuleRegistration registration : config) {
-      mModuleRegistrations.put(registration.getModuleInterface(), registration);
-    }
   }
 
   public synchronized <T extends JavaScriptModule> T getJavaScriptModule(
@@ -50,51 +40,70 @@ public synchronized <T extends JavaScriptModule> T getJavaScriptModule(
       return (T) module;
     }
 
-    JavaScriptModuleRegistration registration =
-        Assertions.assertNotNull(
-            mModuleRegistrations.get(moduleInterface),
-            "JS module " + moduleInterface.getSimpleName() + " hasn't been registered!");
     JavaScriptModule interfaceProxy = (JavaScriptModule) Proxy.newProxyInstance(
         moduleInterface.getClassLoader(),
         new Class[]{moduleInterface},
-        new JavaScriptModuleInvocationHandler(instance, registration));
+        new JavaScriptModuleInvocationHandler(instance, moduleInterface));
     mModuleInstances.put(moduleInterface, interfaceProxy);
     return (T) interfaceProxy;
   }
 
   public static class Builder {
-    private List<JavaScriptModuleRegistration> mModules =
-      new ArrayList<JavaScriptModuleRegistration>();
-
     public Builder add(Class<? extends JavaScriptModule> moduleInterfaceClass) {
-      mModules.add(new JavaScriptModuleRegistration(moduleInterfaceClass));
       return this;
     }
 
     public JavaScriptModuleRegistry build() {
-      return new JavaScriptModuleRegistry(mModules);
+      return new JavaScriptModuleRegistry();
     }
   }
 
   private static class JavaScriptModuleInvocationHandler implements InvocationHandler {
     private final CatalystInstance mCatalystInstance;
-    private final JavaScriptModuleRegistration mModuleRegistration;
+    private final Class<? extends JavaScriptModule> mModuleInterface;
+    private @Nullable String mName;
 
     public JavaScriptModuleInvocationHandler(
         CatalystInstance catalystInstance,
-        JavaScriptModuleRegistration moduleRegistration) {
+        Class<? extends JavaScriptModule> moduleInterface) {
       mCatalystInstance = catalystInstance;
-      mModuleRegistration = moduleRegistration;
+      mModuleInterface = moduleInterface;
+
+      if (ReactBuildConfig.DEBUG) {
+        Set<String> methodNames = new HashSet<>();
+        for (Method method : mModuleInterface.getDeclaredMethods()) {
+          if (!methodNames.add(method.getName())) {
+            throw new AssertionError(
+              "Method overloading is unsupported: " + mModuleInterface.getName() +
+                "#" + method.getName());
+          }
+        }
+      }
+    }
+
+    private String getJSModuleName() {
+      if (mName == null) {
+        // With proguard obfuscation turned on, proguard apparently (poorly) emulates inner
+        // classes or something because Class#getSimpleName() no longer strips the outer
+        // class name. We manually strip it here if necessary.
+        String name = mModuleInterface.getSimpleName();
+        int dollarSignIndex = name.lastIndexOf('$');
+        if (dollarSignIndex != -1) {
+          name = name.substring(dollarSignIndex + 1);
+        }
+
+        // getting the class name every call is expensive, so cache it
+        mName = name;
+      }
+      return mName;
     }
 
     @Override
     public @Nullable Object invoke(Object proxy, Method method, @Nullable Object[] args) throws Throwable {
-      NativeArray jsArgs = args != null ? Arguments.fromJavaArgs(args) : new WritableNativeArray();
-      mCatalystInstance.callFunction(
-        mModuleRegistration.getName(),
-        method.getName(),
-        jsArgs
-      );
+      NativeArray jsArgs = args != null
+        ? Arguments.fromJavaArgs(args)
+        : new WritableNativeArray();
+      mCatalystInstance.callFunction(getJSModuleName(), method.getName(), jsArgs);
       return null;
     }
   }