move NativeModuleRegistry creation logic into it's own class

Reviewed By: javache

Differential Revision: D4479604

fbshipit-source-id: 297fccd25c7400176bcb7821b644d9b05e465ffa

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<ModuleSpec> 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(
+        mNativeModuleRegistryBuilder.addNativeModule(nativeModule);
-          new ModuleSpec(nativeModule.getClass(), new EagerModuleProvider(nativeModule)));
         return this;
       }
 
@@ -79,10 +72,7 @@ public class ReactTestHelper {
         return new CatalystInstanceImpl.Builder()
           .setReactQueueConfigurationSpec(ReactQueueConfigurationSpec.createDefault())
           .setJSExecutor(executor)
-          .setRegistry(new NativeModuleRegistry(
+          .setRegistry(mNativeModuleRegistryBuilder.build())
-            mModuleSpecList,
-            Collections.<Class, ReactModuleInfo>emptyMap(),
-            false))
           .setJSModuleRegistry(mJSModuleRegistryBuilder.build())
           .setJSBundleLoader(JSBundleLoader.createAssetLoader(
                                mContext,

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<Class<? extends NativeModule>, ModuleHolder> mModules = new HashMap<>();
+  private final Map<String, Class<? extends NativeModule>> 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<ModuleSpec> moduleSpecs = lazyReactPackage.getNativeModules(mReactApplicationContext);
+      Map<Class, ReactModuleInfo> reactModuleInfoMap = lazyReactPackage.getReactModuleInfoProvider()
+        .getReactModuleInfos();
+
+      for (ModuleSpec moduleSpec : moduleSpecs) {
+        Class<? extends NativeModule> 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<? extends NativeModule> 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<? extends NativeModule> type = nativeModule.getClass();
+    if (namesToType.containsKey(name)) {
+      Class<? extends NativeModule> 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<OnBatchCompleteListener> batchCompleteListenerModules = new ArrayList<>();
+    for (Map.Entry<Class<? extends NativeModule>, ModuleHolder> entry : mModules.entrySet()) {
+      Class<? extends NativeModule> 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);
+  }
+}

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<ModuleSpec> moduleSpecs = new ArrayList<>();
-    Map<Class, ReactModuleInfo> 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(
+      processPackage(coreModulesPackage, nativeModuleRegistryBuilder, jsModulesBuilder);
-        coreModulesPackage,
-        reactContext,
-        moduleSpecs,
-        reactModuleInfoMap,
-        jsModulesBuilder);
     } finally {
       Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
     }
@@ -899,12 +888,7 @@ public class ReactInstanceManager {
           TRACE_TAG_REACT_JAVA_BRIDGE,
           "createAndProcessCustomReactPackage");
       try {
-        processPackage(
+        processPackage(reactPackage, nativeModuleRegistryBuilder, jsModulesBuilder);
-          reactPackage,
-          reactContext,
-          moduleSpecs,
-          reactModuleInfoMap,
-          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(
+       nativeModuleRegistry = nativeModuleRegistryBuilder.build();
-         moduleSpecs,
-         reactModuleInfoMap,
-         mLazyNativeModulesEnabled);
     } 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,
+    NativeModuleRegistryBuilder nativeModuleRegistryBuilder,
-    List<ModuleSpec> moduleSpecs,
-    Map<Class, ReactModuleInfo> reactModuleInfoMap,
     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) {
+    nativeModuleRegistryBuilder.processPackage(reactPackage);
-      LazyReactPackage lazyReactPackage = (LazyReactPackage) reactPackage;
-      ReactModuleInfoProvider instance = lazyReactPackage.getReactModuleInfoProvider();
-      Map<Class, ReactModuleInfo> 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)));
-      }
-    }
 
     for (Class<? extends JavaScriptModule> jsModuleClass : reactPackage.createJSModules()) {
       jsModulesBuilder.add(jsModuleClass);

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;
+  }
+}

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(
+  public ModuleHolder(ReactModuleInfo info, Provider<? extends NativeModule> provider) {
-    Class<? extends NativeModule> clazz,
+    mInfo = info;
-    @Nullable ReactModuleInfo reactModuleInfo,
-    Provider<? extends NativeModule> provider) {
-    mInfo = reactModuleInfo == null ? new LegacyModuleInfo(clazz) : reactModuleInfo;
     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;
-    }
-  }
 }

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<OnBatchCompleteListener> mBatchCompleteListenerModules;
 
   public NativeModuleRegistry(
-    List<ModuleSpec> moduleSpecList,
+    Map<Class<? extends NativeModule>, ModuleHolder> modules,
-    Map<Class, ReactModuleInfo> reactModuleInfoMap,
+    ArrayList<OnBatchCompleteListener> batchCompleteListenerModules) {
-    boolean isLazyNativeModulesEnabled) {
+    mModules = modules;
-    Map<String, Pair<Class<? extends NativeModule>, ModuleHolder>> namesToSpecs = new HashMap<>();
+    mBatchCompleteListenerModules = batchCompleteListenerModules;
-    for (ModuleSpec module : moduleSpecList) {
-      Class<? extends NativeModule> 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<? extends NativeModule> 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<Class<? extends NativeModule>, ModuleHolder>(type, holder));
-    }
-
-    mModules = new HashMap<>();
-    for (Pair<Class<? extends NativeModule>, ModuleHolder> pair : namesToSpecs.values()) {
-      mModules.put(pair.first, pair.second);
-    }
-
-    mBatchCompleteListenerModules = new ArrayList<>();
-    for (Class<? extends NativeModule> 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();
-          }
-        });
-      }
-    }
   }
 
   /* package */ ModuleRegistryHolder getModuleRegistryHolder(