From 0e0a6e8a29857438a623a1f114844349298658f4 Mon Sep 17 00:00:00 2001 From: Aaron Chiu Date: Mon, 30 Jan 2017 06:41:42 -0800 Subject: [PATCH] move NativeModuleRegistry creation logic into it's own class Reviewed By: javache Differential Revision: D4479604 fbshipit-source-id: 297fccd25c7400176bcb7821b644d9b05e465ffa --- .../react/testing/ReactTestHelper.java | 20 +-- .../react/NativeModuleRegistryBuilder.java | 132 ++++++++++++++++++ .../facebook/react/ReactInstanceManager.java | 56 ++------ .../react/cxxbridge/LegacyModuleInfo.java | 40 ++++++ .../react/cxxbridge/ModuleHolder.java | 38 +---- .../react/cxxbridge/NativeModuleRegistry.java | 53 +------ 6 files changed, 197 insertions(+), 142 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java create mode 100644 ReactAndroid/src/main/java/com/facebook/react/cxxbridge/LegacyModuleInfo.java diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java index 269cc43fe..11c7bc230 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java @@ -10,22 +10,17 @@ package com.facebook.react.testing; import javax.annotation.Nullable; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - import android.app.Instrumentation; import android.content.Context; import android.support.test.InstrumentationRegistry; import android.view.View; import android.view.ViewGroup; -import com.facebook.react.EagerModuleProvider; +import com.facebook.react.NativeModuleRegistryBuilder; import com.facebook.react.ReactInstanceManager; import com.facebook.react.ReactInstanceManagerBuilder; import com.facebook.react.bridge.CatalystInstance; import com.facebook.react.bridge.JavaScriptModuleRegistry; -import com.facebook.react.bridge.ModuleSpec; import com.facebook.react.bridge.NativeModule; import com.facebook.react.bridge.NativeModuleCallExceptionHandler; import com.facebook.react.bridge.WritableNativeMap; @@ -34,8 +29,6 @@ import com.facebook.react.cxxbridge.CatalystInstanceImpl; import com.facebook.react.cxxbridge.JSBundleLoader; import com.facebook.react.cxxbridge.JSCJavaScriptExecutor; import com.facebook.react.cxxbridge.JavaScriptExecutor; -import com.facebook.react.cxxbridge.NativeModuleRegistry; -import com.facebook.react.module.model.ReactModuleInfo; import com.android.internal.util.Predicate; @@ -43,7 +36,8 @@ public class ReactTestHelper { private static class DefaultReactTestFactory implements ReactTestFactory { private static class ReactInstanceEasyBuilderImpl implements ReactInstanceEasyBuilder { - private final List mModuleSpecList = new ArrayList<>(); + private final NativeModuleRegistryBuilder mNativeModuleRegistryBuilder = + new NativeModuleRegistryBuilder(null, false); private final JavaScriptModuleRegistry.Builder mJSModuleRegistryBuilder = new JavaScriptModuleRegistry.Builder(); @@ -57,8 +51,7 @@ public class ReactTestHelper { @Override public ReactInstanceEasyBuilder addNativeModule(NativeModule nativeModule) { - mModuleSpecList.add( - new ModuleSpec(nativeModule.getClass(), new EagerModuleProvider(nativeModule))); + mNativeModuleRegistryBuilder.addNativeModule(nativeModule); return this; } @@ -79,10 +72,7 @@ public class ReactTestHelper { return new CatalystInstanceImpl.Builder() .setReactQueueConfigurationSpec(ReactQueueConfigurationSpec.createDefault()) .setJSExecutor(executor) - .setRegistry(new NativeModuleRegistry( - mModuleSpecList, - Collections.emptyMap(), - false)) + .setRegistry(mNativeModuleRegistryBuilder.build()) .setJSModuleRegistry(mJSModuleRegistryBuilder.build()) .setJSBundleLoader(JSBundleLoader.createAssetLoader( mContext, diff --git a/ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java b/ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java new file mode 100644 index 000000000..76ca0f00c --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java @@ -0,0 +1,132 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +package com.facebook.react; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import com.facebook.common.logging.FLog; +import com.facebook.react.bridge.BaseJavaModule; +import com.facebook.react.bridge.ModuleSpec; +import com.facebook.react.bridge.NativeModule; +import com.facebook.react.bridge.OnBatchCompleteListener; +import com.facebook.react.bridge.ReactApplicationContext; +import com.facebook.react.common.ReactConstants; +import com.facebook.react.cxxbridge.LegacyModuleInfo; +import com.facebook.react.cxxbridge.ModuleHolder; +import com.facebook.react.cxxbridge.NativeModuleRegistry; +import com.facebook.react.module.model.ReactModuleInfo; + +/** + * Helper class to build NativeModuleRegistry. + */ +public class NativeModuleRegistryBuilder { + + private final ReactApplicationContext mReactApplicationContext; + private final boolean mLazyNativeModulesEnabled; + + private final Map, ModuleHolder> mModules = new HashMap<>(); + private final Map> namesToType = new HashMap<>(); + + public NativeModuleRegistryBuilder( + ReactApplicationContext reactApplicationContext, + boolean lazyNativeModulesEnabled) { + mReactApplicationContext = reactApplicationContext; + mLazyNativeModulesEnabled = lazyNativeModulesEnabled; + } + + public void processPackage(ReactPackage reactPackage) { + if (mLazyNativeModulesEnabled) { + if (!(reactPackage instanceof LazyReactPackage)) { + throw new IllegalStateException("Lazy native modules requires all ReactPackage to " + + "inherit from LazyReactPackage"); + } + + LazyReactPackage lazyReactPackage = (LazyReactPackage) reactPackage; + List moduleSpecs = lazyReactPackage.getNativeModules(mReactApplicationContext); + Map reactModuleInfoMap = lazyReactPackage.getReactModuleInfoProvider() + .getReactModuleInfos(); + + for (ModuleSpec moduleSpec : moduleSpecs) { + Class type = moduleSpec.getType(); + ReactModuleInfo reactModuleInfo = reactModuleInfoMap.get(type); + ModuleHolder moduleHolder; + if (reactModuleInfo == null) { + if (BaseJavaModule.class.isAssignableFrom(type)) { + throw new IllegalStateException("Native Java module " + type.getSimpleName() + + " should be annotated with @ReactModule and added to a @ReactModuleList."); + } + NativeModule nativeModule = moduleSpec.getProvider().get(); + LegacyModuleInfo legacyModuleInfo = new LegacyModuleInfo(type, nativeModule); + moduleHolder = new ModuleHolder(legacyModuleInfo, nativeModule); + } else { + moduleHolder = new ModuleHolder(reactModuleInfo, moduleSpec.getProvider()); + } + + String name = moduleHolder.getInfo().name(); + if (namesToType.containsKey(name)) { + Class existingNativeModule = namesToType.get(name); + if (!moduleHolder.getInfo().canOverrideExistingModule()) { + throw new IllegalStateException("Native module " + type.getSimpleName() + + " tried to override " + existingNativeModule.getSimpleName() + " for module name " + + name + ". If this was your intention, set canOverrideExistingModule=true"); + } + + mModules.remove(existingNativeModule); + } + + namesToType.put(name, type); + mModules.put(type, moduleHolder); + } + } else { + FLog.d( + ReactConstants.TAG, + reactPackage.getClass().getSimpleName() + + " is not a LazyReactPackage, falling back to old version."); + for (NativeModule nativeModule : reactPackage.createNativeModules(mReactApplicationContext)) { + addNativeModule(nativeModule); + } + } + } + + public void addNativeModule(NativeModule nativeModule) { + String name = nativeModule.getName(); + Class type = nativeModule.getClass(); + if (namesToType.containsKey(name)) { + Class existingModule = namesToType.get(name); + if (!nativeModule.canOverrideExistingModule()) { + throw new IllegalStateException("Native module " + type.getSimpleName() + + " tried to override " + existingModule.getSimpleName() + " for module name " + + name + ". If this was your intention, set canOverrideExistingModule=true"); + } + + mModules.remove(existingModule); + } + + namesToType.put(name, type); + LegacyModuleInfo legacyModuleInfo = new LegacyModuleInfo(type, nativeModule); + ModuleHolder moduleHolder = new ModuleHolder(legacyModuleInfo, nativeModule); + mModules.put(type, moduleHolder); + } + + public NativeModuleRegistry build() { + ArrayList batchCompleteListenerModules = new ArrayList<>(); + for (Map.Entry, ModuleHolder> entry : mModules.entrySet()) { + Class type = entry.getKey(); + if (OnBatchCompleteListener.class.isAssignableFrom(type)) { + final ModuleHolder moduleHolder = entry.getValue(); + batchCompleteListenerModules.add(new OnBatchCompleteListener() { + @Override + public void onBatchComplete() { + OnBatchCompleteListener listener = (OnBatchCompleteListener) moduleHolder.getModule(); + listener.onBatchComplete(); + } + }); + } + } + + return new NativeModuleRegistry(mModules, batchCompleteListenerModules); + } +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index 2fac9bd2e..0d0d09486 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -14,10 +14,8 @@ import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import android.app.Activity; import android.app.Application; @@ -35,8 +33,6 @@ import com.facebook.react.bridge.CatalystInstance; import com.facebook.react.bridge.JavaJSExecutor; import com.facebook.react.bridge.JavaScriptModule; import com.facebook.react.bridge.JavaScriptModuleRegistry; -import com.facebook.react.bridge.ModuleSpec; -import com.facebook.react.bridge.NativeModule; import com.facebook.react.bridge.NativeModuleCallExceptionHandler; import com.facebook.react.bridge.NotThreadSafeBridgeIdleDebugListener; import com.facebook.react.bridge.ReactApplicationContext; @@ -62,8 +58,6 @@ import com.facebook.react.devsupport.DevSupportManager; import com.facebook.react.devsupport.DevSupportManagerFactory; import com.facebook.react.devsupport.ReactInstanceDevCommandsHandler; import com.facebook.react.devsupport.RedBoxHandler; -import com.facebook.react.module.model.ReactModuleInfo; -import com.facebook.react.module.model.ReactModuleInfoProvider; import com.facebook.react.modules.core.DefaultHardwareBackBtnHandler; import com.facebook.react.modules.core.DeviceEventManagerModule; import com.facebook.react.modules.debug.DeveloperSettings; @@ -863,11 +857,11 @@ public class ReactInstanceManager { JSBundleLoader jsBundleLoader) { FLog.i(ReactConstants.TAG, "Creating react context."); ReactMarker.logMarker(CREATE_REACT_CONTEXT_START); - List moduleSpecs = new ArrayList<>(); - Map reactModuleInfoMap = new HashMap<>(); - JavaScriptModuleRegistry.Builder jsModulesBuilder = new JavaScriptModuleRegistry.Builder(); - final ReactApplicationContext reactContext = new ReactApplicationContext(mApplicationContext); + NativeModuleRegistryBuilder nativeModuleRegistryBuilder = new NativeModuleRegistryBuilder( + reactContext, + mLazyNativeModulesEnabled); + JavaScriptModuleRegistry.Builder jsModulesBuilder = new JavaScriptModuleRegistry.Builder(); if (mUseDeveloperSupport) { reactContext.setNativeModuleCallExceptionHandler(mDevSupportManager); } @@ -883,12 +877,7 @@ public class ReactInstanceManager { mBackBtnHandler, mUIImplementationProvider, mLazyViewManagersEnabled); - processPackage( - coreModulesPackage, - reactContext, - moduleSpecs, - reactModuleInfoMap, - jsModulesBuilder); + processPackage(coreModulesPackage, nativeModuleRegistryBuilder, jsModulesBuilder); } finally { Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); } @@ -899,12 +888,7 @@ public class ReactInstanceManager { TRACE_TAG_REACT_JAVA_BRIDGE, "createAndProcessCustomReactPackage"); try { - processPackage( - reactPackage, - reactContext, - moduleSpecs, - reactModuleInfoMap, - jsModulesBuilder); + processPackage(reactPackage, nativeModuleRegistryBuilder, jsModulesBuilder); } finally { Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); } @@ -915,10 +899,7 @@ public class ReactInstanceManager { Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "buildNativeModuleRegistry"); NativeModuleRegistry nativeModuleRegistry; try { - nativeModuleRegistry = new NativeModuleRegistry( - moduleSpecs, - reactModuleInfoMap, - mLazyNativeModulesEnabled); + nativeModuleRegistry = nativeModuleRegistryBuilder.build(); } finally { Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); ReactMarker.logMarker(BUILD_NATIVE_MODULE_REGISTRY_END); @@ -958,9 +939,7 @@ public class ReactInstanceManager { private void processPackage( ReactPackage reactPackage, - ReactApplicationContext reactContext, - List moduleSpecs, - Map reactModuleInfoMap, + NativeModuleRegistryBuilder nativeModuleRegistryBuilder, JavaScriptModuleRegistry.Builder jsModulesBuilder) { SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "processPackage") .arg("className", reactPackage.getClass().getSimpleName()) @@ -968,24 +947,7 @@ public class ReactInstanceManager { if (reactPackage instanceof ReactPackageLogger) { ((ReactPackageLogger) reactPackage).startProcessPackage(); } - if (mLazyNativeModulesEnabled && reactPackage instanceof LazyReactPackage) { - LazyReactPackage lazyReactPackage = (LazyReactPackage) reactPackage; - ReactModuleInfoProvider instance = lazyReactPackage.getReactModuleInfoProvider(); - Map map = instance.getReactModuleInfos(); - if (!map.isEmpty()) { - reactModuleInfoMap.putAll(map); - } - moduleSpecs.addAll(lazyReactPackage.getNativeModules(reactContext)); - } else { - FLog.d( - ReactConstants.TAG, - reactPackage.getClass().getSimpleName() + - " is not a LazyReactPackage, falling back to old version."); - for (NativeModule nativeModule : reactPackage.createNativeModules(reactContext)) { - moduleSpecs.add( - new ModuleSpec(nativeModule.getClass(), new EagerModuleProvider(nativeModule))); - } - } + nativeModuleRegistryBuilder.processPackage(reactPackage); for (Class jsModuleClass : reactPackage.createJSModules()) { jsModulesBuilder.add(jsModuleClass); diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/LegacyModuleInfo.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/LegacyModuleInfo.java new file mode 100644 index 000000000..01457572c --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/LegacyModuleInfo.java @@ -0,0 +1,40 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +package com.facebook.react.cxxbridge; + +import com.facebook.react.bridge.NativeModule; +import com.facebook.react.module.model.Info; + +/** + * Module info for non-lazy native modules. + */ +public class LegacyModuleInfo implements Info { + + public final Class mType; + public final NativeModule mNativeModule; + + public LegacyModuleInfo(Class type, NativeModule nativeModule) { + mType = type; + mNativeModule = nativeModule; + } + + @Override + public String name() { + return mNativeModule.getName(); + } + + @Override + public boolean canOverrideExistingModule() { + return mNativeModule.canOverrideExistingModule(); + } + + @Override + public boolean supportsWebWorkers() { + return mNativeModule.supportsWebWorkers(); + } + + @Override + public boolean needsEagerInit() { + return true; + } +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java index e7918bc7a..4daf267af 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java @@ -36,18 +36,19 @@ public class ModuleHolder { private @Nullable NativeModule mModule; private boolean mInitializeNeeded; - public ModuleHolder( - Class clazz, - @Nullable ReactModuleInfo reactModuleInfo, - Provider provider) { - mInfo = reactModuleInfo == null ? new LegacyModuleInfo(clazz) : reactModuleInfo; + public ModuleHolder(ReactModuleInfo info, Provider provider) { + mInfo = info; mProvider = provider; - if (mInfo.needsEagerInit()) { mModule = doCreate(); } } + public ModuleHolder(LegacyModuleInfo info, NativeModule nativeModule) { + mInfo = info; + mModule = nativeModule; + } + public synchronized void initialize() { if (mModule != null) { doInitialize(mModule); @@ -139,29 +140,4 @@ public class ModuleHolder { throw new RuntimeException(e); } } - - private class LegacyModuleInfo implements Info { - - public final Class mType; - - public LegacyModuleInfo(Class type) { - mType = type; - } - - public String name() { - return getModule().getName(); - } - - public boolean canOverrideExistingModule() { - return getModule().canOverrideExistingModule(); - } - - public boolean supportsWebWorkers() { - return getModule().supportsWebWorkers(); - } - - public boolean needsEagerInit() { - return true; - } - } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java index f80c61f00..2cb0149ba 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java @@ -10,20 +10,15 @@ package com.facebook.react.cxxbridge; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; -import android.util.Pair; - import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.BaseJavaModule; -import com.facebook.react.bridge.ModuleSpec; import com.facebook.react.bridge.NativeModule; import com.facebook.react.bridge.OnBatchCompleteListener; import com.facebook.react.bridge.ReactMarker; import com.facebook.react.bridge.ReactMarkerConstants; -import com.facebook.react.module.model.ReactModuleInfo; import com.facebook.systrace.Systrace; /** @@ -35,50 +30,10 @@ public class NativeModuleRegistry { private final ArrayList mBatchCompleteListenerModules; public NativeModuleRegistry( - List moduleSpecList, - Map reactModuleInfoMap, - boolean isLazyNativeModulesEnabled) { - Map, ModuleHolder>> namesToSpecs = new HashMap<>(); - for (ModuleSpec module : moduleSpecList) { - Class type = module.getType(); - ReactModuleInfo reactModuleInfo = reactModuleInfoMap.get(type); - if (isLazyNativeModulesEnabled && - reactModuleInfo == null && - BaseJavaModule.class.isAssignableFrom(type)) { - throw new IllegalStateException("Native Java module " + type.getSimpleName() + - " should be annotated with @ReactModule and added to a @ReactModuleList."); - } - ModuleHolder holder = new ModuleHolder(type, reactModuleInfo, module.getProvider()); - String name = holder.getInfo().name(); - Class existing = namesToSpecs.containsKey(name) ? - namesToSpecs.get(name).first : - null; - if (existing != null && !holder.getInfo().canOverrideExistingModule()) { - throw new IllegalStateException("Native module " + type.getSimpleName() + - " tried to override " + existing.getSimpleName() + " for module name " + name + - ". If this was your intention, set canOverrideExistingModule=true"); - } - namesToSpecs.put(name, new Pair, ModuleHolder>(type, holder)); - } - - mModules = new HashMap<>(); - for (Pair, ModuleHolder> pair : namesToSpecs.values()) { - mModules.put(pair.first, pair.second); - } - - mBatchCompleteListenerModules = new ArrayList<>(); - for (Class type : mModules.keySet()) { - if (OnBatchCompleteListener.class.isAssignableFrom(type)) { - final ModuleHolder holder = mModules.get(type); - mBatchCompleteListenerModules.add(new OnBatchCompleteListener() { - @Override - public void onBatchComplete() { - OnBatchCompleteListener listener = (OnBatchCompleteListener) holder.getModule(); - listener.onBatchComplete(); - } - }); - } - } + Map, ModuleHolder> modules, + ArrayList batchCompleteListenerModules) { + mModules = modules; + mBatchCompleteListenerModules = batchCompleteListenerModules; } /* package */ ModuleRegistryHolder getModuleRegistryHolder(