From 53d5504f4077bf7fb7cbf7c1edac7e2cbde5c964 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Tue, 13 Jun 2017 05:44:10 -0700 Subject: [PATCH] Stop requiring registration of callable JS modules Reviewed By: AaaChiuuu Differential Revision: D5229073 fbshipit-source-id: d6d1967982ae379733a7e9667515ca9f074aadd4 --- .../react/bridge/CatalystInstanceImpl.java | 28 +++---- .../bridge/JavaScriptModuleRegistration.java | 71 ---------------- .../bridge/JavaScriptModuleRegistry.java | 83 ++++++++++--------- 3 files changed, 58 insertions(+), 124 deletions(-) delete mode 100644 ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistration.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java index b6c350093..0395b00d7 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -75,7 +75,7 @@ public class CatalystInstanceImpl implements CatalystInstance { private final ArrayList mJSCallsPendingInit = new ArrayList(); 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 class CatalystInstanceImpl implements CatalystInstance { 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 @@ public class CatalystInstanceImpl implements CatalystInstance { 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 @@ public class CatalystInstanceImpl implements CatalystInstance { 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 class CatalystInstanceImpl implements CatalystInstance { public void onBatchComplete() { CatalystInstanceImpl impl = mOuter.get(); if (impl != null) { - impl.mJavaRegistry.onBatchComplete(); + impl.mNativeModuleRegistry.onBatchComplete(); } } @@ -294,7 +293,7 @@ public class CatalystInstanceImpl implements CatalystInstance { 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 class CatalystInstanceImpl implements CatalystInstance { mNativeModulesQueueThread.runOnQueue(new Runnable() { @Override public void run() { - mJavaRegistry.notifyJSInstanceInitialized(); + mNativeModuleRegistry.notifyJSInstanceInitialized(); } }); } @@ -359,19 +358,19 @@ public class CatalystInstanceImpl implements CatalystInstance { @Override public boolean hasNativeModule(Class nativeModuleInterface) { - return mJavaRegistry.hasModule(nativeModuleInterface); + return mNativeModuleRegistry.hasModule(nativeModuleInterface); } // This is only ever called with UIManagerModule or CurrentViewerModule. @Override public T getNativeModule(Class nativeModuleInterface) { - return mJavaRegistry.getModule(nativeModuleInterface); + return mNativeModuleRegistry.getModule(nativeModuleInterface); } // This is only used by com.facebook.react.modules.common.ModuleDataCleaner @Override public Collection getNativeModules() { - return mJavaRegistry.getAllModules(); + return mNativeModuleRegistry.getAllModules(); } private native void handleMemoryPressureUiHidden(); @@ -528,7 +527,6 @@ public class CatalystInstanceImpl implements CatalystInstance { 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 class CatalystInstanceImpl implements CatalystInstance { } public Builder setJSModuleRegistry(JavaScriptModuleRegistry jsModuleRegistry) { - mJSModuleRegistry = jsModuleRegistry; return this; } @@ -569,7 +566,6 @@ public class CatalystInstanceImpl implements CatalystInstance { Assertions.assertNotNull(mReactQueueConfigurationSpec), Assertions.assertNotNull(mJSExecutor), Assertions.assertNotNull(mRegistry), - Assertions.assertNotNull(mJSModuleRegistry), Assertions.assertNotNull(mJSBundleLoader), Assertions.assertNotNull(mNativeModuleCallExceptionHandler)); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistration.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistration.java deleted file mode 100644 index 53ed41d20..000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistration.java +++ /dev/null @@ -1,71 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ - -package com.facebook.react.bridge; - -import javax.annotation.concurrent.Immutable; -import javax.annotation.Nullable; - -import java.lang.reflect.Method; -import java.util.Arrays; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Set; - -import com.facebook.react.common.build.ReactBuildConfig; - -/** - * Registration info for a {@link JavaScriptModule}. Maps its methods to method ids. - */ -@Immutable -public class JavaScriptModuleRegistration { - - private final Class mModuleInterface; - private @Nullable String mName; - - public JavaScriptModuleRegistration(Class moduleInterface) { - mModuleInterface = moduleInterface; - - if (ReactBuildConfig.DEBUG) { - Set methodNames = new LinkedHashSet<>(); - for (Method method : mModuleInterface.getDeclaredMethods()) { - if (!methodNames.add(method.getName())) { - throw new AssertionError( - "Method overloading is unsupported: " + mModuleInterface.getName() + "#" + - method.getName()); - } - } - } - } - - public Class getModuleInterface() { - return mModuleInterface; - } - - public String getName() { - 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; - } - - public List getMethods() { - return Arrays.asList(mModuleInterface.getDeclaredMethods()); - } -} diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistry.java index 4048636fe..a758c3610 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptModuleRegistry.java @@ -11,35 +11,25 @@ package com.facebook.react.bridge; 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, JavaScriptModule> mModuleInstances; - private final HashMap, JavaScriptModuleRegistration> mModuleRegistrations; - public JavaScriptModuleRegistry(List config) { + public JavaScriptModuleRegistry() { mModuleInstances = new HashMap<>(); - mModuleRegistrations = new HashMap<>(); - for (JavaScriptModuleRegistration registration : config) { - mModuleRegistrations.put(registration.getModuleInterface(), registration); - } } public synchronized T getJavaScriptModule( @@ -50,51 +40,70 @@ public class JavaScriptModuleRegistry { 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 mModules = - new ArrayList(); - public Builder add(Class 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 mModuleInterface; + private @Nullable String mName; public JavaScriptModuleInvocationHandler( CatalystInstance catalystInstance, - JavaScriptModuleRegistration moduleRegistration) { + Class moduleInterface) { mCatalystInstance = catalystInstance; - mModuleRegistration = moduleRegistration; + mModuleInterface = moduleInterface; + + if (ReactBuildConfig.DEBUG) { + Set 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; } }