Setting bridge up for sharing: allowing native modules to register after init

Reviewed By: javache

Differential Revision: D4945784

fbshipit-source-id: 80e7236e9ccd5d5c9a7fba7c96b98fc38b43a2fc

ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java

@@ -287,6 +287,7 @@ public class ReactInstanceManager {
    *
    * Called from UI thread.
    */
+  @ThreadConfined(UI)
   public void createReactContextInBackground() {
     Log.d(ReactConstants.TAG, "ReactInstanceManager.createReactContextInBackground()");
     Assertions.assertCondition(
@@ -299,6 +300,33 @@ public class ReactInstanceManager {
     recreateReactContextInBackgroundInner();
   }
 
+  @ThreadConfined(UI)
+  public void registerAdditionalPackages(List<ReactPackage> packages) {
+    if (packages == null || packages.isEmpty()) {
+      return;
+    }
+
+    // CatalystInstance hasn't been created, so add packages for later evaluation
+    if (!hasStartedCreatingInitialContext()) {
+      for (ReactPackage p : packages) {
+        if (!mPackages.contains(p)) {
+          mPackages.add(p);
+        }
+      }
+      return;
+    }
+
+    ReactContext context = getCurrentReactContext();
+    CatalystInstance catalystInstance = context != null ? context.getCatalystInstance() : null;
+
+    Assertions.assertNotNull(catalystInstance, "CatalystInstance null after hasStartedCreatingInitialContext true.");
+
+    final ReactApplicationContext reactContext = new ReactApplicationContext(mApplicationContext);
+
+    NativeModuleRegistry nativeModuleRegistry = processPackages(reactContext, packages, true);
+    catalystInstance.extendNativeModules(nativeModuleRegistry);
+  }
+
   /**
    * Recreate the react application and context. This should be called if configuration has
    * changed or the developer has requested the app to be reloaded. It should only be called after
@@ -806,7 +834,6 @@ public class ReactInstanceManager {
     if (!mSetupReactContextInBackgroundEnabled) {
       UiThreadUtil.assertOnUiThread();
     }
-    Assertions.assertCondition(mCurrentReactContext == null);
     mCurrentReactContext = Assertions.assertNotNull(reactContext);
     CatalystInstance catalystInstance =
       Assertions.assertNotNull(reactContext.getCatalystInstance());
@@ -922,56 +949,23 @@ public class ReactInstanceManager {
     Log.d(ReactConstants.TAG, "ReactInstanceManager.createReactContext()");
     ReactMarker.logMarker(CREATE_REACT_CONTEXT_START);
     final ReactApplicationContext reactContext = new ReactApplicationContext(mApplicationContext);
-    NativeModuleRegistryBuilder nativeModuleRegistryBuilder = new NativeModuleRegistryBuilder(
-      reactContext,
-      this,
-      mLazyNativeModulesEnabled);
+
     if (mUseDeveloperSupport) {
       reactContext.setNativeModuleCallExceptionHandler(mDevSupportManager);
     }
 
-    ReactMarker.logMarker(PROCESS_PACKAGES_START);
-    Systrace.beginSection(
-        TRACE_TAG_REACT_JAVA_BRIDGE,
-        "createAndProcessCoreModulesPackage");
-    try {
-      CoreModulesPackage coreModulesPackage =
-        new CoreModulesPackage(
-          this,
-          mBackBtnHandler,
-          mUIImplementationProvider,
-          mLazyViewManagersEnabled);
-      processPackage(coreModulesPackage, nativeModuleRegistryBuilder);
-    } finally {
-      Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
-    }
-
-    // TODO(6818138): Solve use-case of native/js modules overriding
-    for (ReactPackage reactPackage : mPackages) {
-      Systrace.beginSection(
-          TRACE_TAG_REACT_JAVA_BRIDGE,
-          "createAndProcessCustomReactPackage");
-      try {
-        processPackage(reactPackage, nativeModuleRegistryBuilder);
-      } finally {
-        Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
-      }
-    }
-    ReactMarker.logMarker(PROCESS_PACKAGES_END);
-
-    ReactMarker.logMarker(BUILD_NATIVE_MODULE_REGISTRY_START);
-    Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "buildNativeModuleRegistry");
-    NativeModuleRegistry nativeModuleRegistry;
-    try {
-       nativeModuleRegistry = nativeModuleRegistryBuilder.build();
-    } finally {
-      Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
-      ReactMarker.logMarker(BUILD_NATIVE_MODULE_REGISTRY_END);
-    }
+    CoreModulesPackage coreModulesPackage =
+      new CoreModulesPackage(
+        this,
+        mBackBtnHandler,
+        mUIImplementationProvider,
+        mLazyViewManagersEnabled);
+    mPackages.add(coreModulesPackage);
+    NativeModuleRegistry nativeModuleRegistry = processPackages(reactContext, mPackages, false);
 
     NativeModuleCallExceptionHandler exceptionHandler = mNativeModuleCallExceptionHandler != null
-        ? mNativeModuleCallExceptionHandler
-        : mDevSupportManager;
+      ? mNativeModuleCallExceptionHandler
+      : mDevSupportManager;
     CatalystInstanceImpl.Builder catalystInstanceBuilder = new CatalystInstanceImpl.Builder()
       .setReactQueueConfigurationSpec(mUseSeparateUIBackgroundThread ?
         ReactQueueConfigurationSpec.createWithSeparateUIBackgroundThread() :
@@ -1006,6 +1000,49 @@ public class ReactInstanceManager {
     return reactContext;
   }
 
+  private NativeModuleRegistry processPackages(
+    ReactApplicationContext reactContext,
+    List<ReactPackage> packages,
+    boolean checkAndUpdatePackageMembership) {
+    NativeModuleRegistryBuilder nativeModuleRegistryBuilder = new NativeModuleRegistryBuilder(
+      reactContext,
+      this,
+      mLazyNativeModulesEnabled);
+
+    ReactMarker.logMarker(PROCESS_PACKAGES_START);
+
+    // TODO(6818138): Solve use-case of native modules overriding
+    for (ReactPackage reactPackage : packages) {
+      if (checkAndUpdatePackageMembership && mPackages.contains(reactPackage)) {
+        continue;
+      }
+      Systrace.beginSection(
+        TRACE_TAG_REACT_JAVA_BRIDGE,
+        "createAndProcessCustomReactPackage");
+      try {
+        if (checkAndUpdatePackageMembership) {
+          mPackages.add(reactPackage);
+        }
+        processPackage(reactPackage, nativeModuleRegistryBuilder);
+      } finally {
+        Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
+      }
+    }
+    ReactMarker.logMarker(PROCESS_PACKAGES_END);
+
+    ReactMarker.logMarker(BUILD_NATIVE_MODULE_REGISTRY_START);
+    Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "buildNativeModuleRegistry");
+    NativeModuleRegistry nativeModuleRegistry;
+    try {
+      nativeModuleRegistry = nativeModuleRegistryBuilder.build();
+    } finally {
+      Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
+      ReactMarker.logMarker(BUILD_NATIVE_MODULE_REGISTRY_END);
+    }
+
+    return nativeModuleRegistry;
+  }
+
   private void processPackage(
     ReactPackage reactPackage,
     NativeModuleRegistryBuilder nativeModuleRegistryBuilder) {

ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerBuilder.java

@@ -197,8 +197,8 @@ public class ReactInstanceManagerBuilder {
 
   public ReactInstanceManagerBuilder setUseSeparateUIBackgroundThread(
     boolean useSeparateUIBackgroundThread) {
-   mUseSeparateUIBackgroundThread = useSeparateUIBackgroundThread;
-   return this;
+    mUseSeparateUIBackgroundThread = useSeparateUIBackgroundThread;
+    return this;
   }
 
   public ReactInstanceManagerBuilder setMinNumShakes(int minNumShakes) {

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

@@ -68,6 +68,12 @@ public interface CatalystInstance
   <T extends NativeModule> T getNativeModule(Class<T> nativeModuleInterface);
   Collection<NativeModule> getNativeModules();
 
+  /**
+   * This method permits a CatalystInstance to extend the known
+   * Native modules. This provided registry contains only the new modules to load.
+   */
+  void extendNativeModules(NativeModuleRegistry modules);
+
   /**
    * Adds a idle listener for this Catalyst instance. The listener will receive notifications
    * whenever the bridge transitions from idle to busy and vice-versa, where the busy state is

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

@@ -157,6 +157,26 @@ public class CatalystInstanceImpl implements CatalystInstance {
     }
   }
 
+  /**
+   * This method and the native below permits a CatalystInstance to extend the known
+   * Native modules. This registry contains only the new modules to load. The
+   * registry {@code mNativeModuleRegistry} updates internally to contain all the new modules, and generates
+   * the new registry for extracting just the new collections.
+   */
+  @Override
+  public void extendNativeModules(NativeModuleRegistry modules) {
+    //Extend the Java-visible registry of modules
+    mNativeModuleRegistry.registerModules(modules);
+    Collection<JavaModuleWrapper> javaModules = modules.getJavaModules(this);
+    Collection<ModuleHolder> cxxModules = modules.getCxxModules();
+    //Extend the Cxx-visible registry of modules wrapped in appropriate interfaces
+    jniExtendNativeModules(javaModules, cxxModules);
+  }
+
+  private native void jniExtendNativeModules(
+    Collection<JavaModuleWrapper> javaModules,
+    Collection<ModuleHolder> cxxModules);
+
   private native void initializeBridge(
       ReactCallback callback,
       JavaScriptExecutor jsExecutor,

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

@@ -13,6 +13,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.HashMap;
 
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.systrace.Systrace;
@@ -35,6 +36,21 @@ public class NativeModuleRegistry {
     mBatchCompleteListenerModules = batchCompleteListenerModules;
   }
 
+  /**
+   * Private getters for combining NativeModuleRegistrys
+   */
+  private Map<Class<? extends NativeModule>, ModuleHolder> getModuleMap() {
+    return mModules;
+  }
+
+  private ReactApplicationContext getReactApplicationContext() {
+    return mReactApplicationContext;
+  }
+
+  private ArrayList<ModuleHolder> getBatchCompleteListenerModules() {
+    return mBatchCompleteListenerModules;
+  }
+
   /* package */ Collection<JavaModuleWrapper> getJavaModules(
       JSInstance jsInstance) {
     ArrayList<JavaModuleWrapper> javaModules = new ArrayList<>();
@@ -58,6 +74,29 @@ public class NativeModuleRegistry {
     return cxxModules;
   }
 
+  /*
+  * Adds any new modules to the current module regsitry
+  */
+  /* package */ void registerModules(NativeModuleRegistry newRegister) {
+
+    Assertions.assertCondition(mReactApplicationContext.equals(newRegister.getReactApplicationContext()),
+      "Extending native modules with non-matching application contexts.");
+
+    Map<Class<? extends NativeModule>, ModuleHolder> newModules = newRegister.getModuleMap();
+    ArrayList<ModuleHolder> batchCompleteListeners = newRegister.getBatchCompleteListenerModules();
+
+    for (Map.Entry<Class<? extends NativeModule>, ModuleHolder> entry : newModules.entrySet()) {
+      Class<? extends NativeModule> key = entry.getKey();
+      if (!mModules.containsKey(key)) {
+        ModuleHolder value = entry.getValue();
+        if (batchCompleteListeners.contains(value)) {
+          mBatchCompleteListenerModules.add(value);
+        }
+        mModules.put(key, value);
+      }
+    }
+  }
+
   /* package */ void notifyJSInstanceDestroy() {
     mReactApplicationContext.assertOnNativeModulesQueueThread();
     Systrace.beginSection(

ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp

@@ -97,6 +97,7 @@ void CatalystInstanceImpl::registerNatives() {
   registerHybrid({
     makeNativeMethod("initHybrid", CatalystInstanceImpl::initHybrid),
     makeNativeMethod("initializeBridge", CatalystInstanceImpl::initializeBridge),
+    makeNativeMethod("jniExtendNativeModules", CatalystInstanceImpl::extendNativeModules),
     makeNativeMethod("jniSetSourceURL", CatalystInstanceImpl::jniSetSourceURL),
     makeNativeMethod("jniLoadScriptFromAssets", CatalystInstanceImpl::jniLoadScriptFromAssets),
     makeNativeMethod("jniLoadScriptFromFile", CatalystInstanceImpl::jniLoadScriptFromFile),
@@ -147,18 +148,32 @@ void CatalystInstanceImpl::initializeBridge(
   // don't need jsModuleDescriptions any more, all the way up and down the
   // stack.
 
+  moduleRegistry_ = std::make_shared<ModuleRegistry>(
+    buildNativeModuleList(
+       std::weak_ptr<Instance>(instance_),
+       javaModules,
+       cxxModules,
+       moduleMessageQueue_,
+       uiBackgroundMessageQueue_));
+
   instance_->initializeBridge(
     folly::make_unique<JInstanceCallback>(
     callback,
     uiBackgroundMessageQueue_ != NULL ? uiBackgroundMessageQueue_ : moduleMessageQueue_),
     jseh->getExecutorFactory(),
     folly::make_unique<JMessageQueueThread>(jsQueue),
-    buildModuleRegistry(
-      std::weak_ptr<Instance>(instance_),
-      javaModules,
-      cxxModules,
-      moduleMessageQueue_,
-      uiBackgroundMessageQueue_));
+    moduleRegistry_);
+}
+
+void CatalystInstanceImpl::extendNativeModules(
+    jni::alias_ref<jni::JCollection<JavaModuleWrapper::javaobject>::javaobject> javaModules,
+    jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject> cxxModules) {
+  moduleRegistry_->registerModules(buildNativeModuleList(
+    std::weak_ptr<Instance>(instance_),
+    javaModules,
+    cxxModules,
+    moduleMessageQueue_,
+    uiBackgroundMessageQueue_));
 }
 
 void CatalystInstanceImpl::jniSetSourceURL(const std::string& sourceURL) {

ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h

@@ -52,6 +52,10 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
       jni::alias_ref<jni::JCollection<JavaModuleWrapper::javaobject>::javaobject> javaModules,
       jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject> cxxModules);
 
+  void extendNativeModules(
+    jni::alias_ref<jni::JCollection<JavaModuleWrapper::javaobject>::javaobject> javaModules,
+    jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject> cxxModules);
+
   /**
    * Sets the source URL of the underlying bridge without loading any JS code.
    */
@@ -74,6 +78,7 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
   // This should be the only long-lived strong reference, but every C++ class
   // will have a weak reference.
   std::shared_ptr<Instance> instance_;
+  std::shared_ptr<ModuleRegistry> moduleRegistry_;
   std::shared_ptr<JMessageQueueThread> moduleMessageQueue_;
   std::shared_ptr<JMessageQueueThread> uiBackgroundMessageQueue_;
 };

ReactAndroid/src/main/jni/react/jni/ModuleRegistryBuilder.cpp

@@ -29,7 +29,7 @@ xplat::module::CxxModule::Provider ModuleHolder::getProvider() const {
   };
 }
 
-std::unique_ptr<ModuleRegistry> buildModuleRegistry(
+std::vector<std::unique_ptr<NativeModule>> buildNativeModuleList(
     std::weak_ptr<Instance> winstance,
     jni::alias_ref<jni::JCollection<JavaModuleWrapper::javaobject>::javaobject> javaModules,
     jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject> cxxModules,
@@ -60,11 +60,7 @@ std::unique_ptr<ModuleRegistry> buildModuleRegistry(
                              winstance, cm->getName(), cm->getProvider(), moduleMessageQueue));
     }
   }
-  if (modules.empty()) {
-    return nullptr;
-  } else {
-    return folly::make_unique<ModuleRegistry>(std::move(modules));
-  }
+  return modules;
 }
 
 }}

ReactAndroid/src/main/jni/react/jni/ModuleRegistryBuilder.h

@@ -23,7 +23,7 @@ class ModuleHolder : public jni::JavaClass<ModuleHolder> {
   xplat::module::CxxModule::Provider getProvider() const;
 };
 
-std::unique_ptr<ModuleRegistry> buildModuleRegistry(
+std::vector<std::unique_ptr<NativeModule>> buildNativeModuleList(
   std::weak_ptr<Instance> winstance,
   jni::alias_ref<jni::JCollection<JavaModuleWrapper::javaobject>::javaobject> javaModules,
   jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject> cxxModules,

ReactCommon/cxxreact/Instance.cpp

@@ -32,11 +32,12 @@ void Instance::initializeBridge(
     std::shared_ptr<MessageQueueThread> jsQueue,
     std::shared_ptr<ModuleRegistry> moduleRegistry) {
   callback_ = std::move(callback);
+  moduleRegistry_ = std::move(moduleRegistry);
 
   jsQueue->runOnQueueSync(
-    [this, &jsef, moduleRegistry, jsQueue] () mutable {
+    [this, &jsef, jsQueue] () mutable {
       nativeToJsBridge_ = folly::make_unique<NativeToJsBridge>(
-          jsef.get(), moduleRegistry, jsQueue, callback_);
+          jsef.get(), moduleRegistry_, jsQueue, callback_);
 
       std::lock_guard<std::mutex> lock(m_syncMutex);
       m_syncReady = true;

ReactCommon/cxxreact/Instance.h

@@ -73,6 +73,7 @@ class Instance {
 
   std::shared_ptr<InstanceCallback> callback_;
   std::unique_ptr<NativeToJsBridge> nativeToJsBridge_;
+  std::shared_ptr<ModuleRegistry> moduleRegistry_;
 
   std::mutex m_syncMutex;
   std::condition_variable m_syncCV;

ReactCommon/cxxreact/ModuleRegistry.cpp

@@ -30,15 +30,36 @@ std::string normalizeName(std::string name) {
 ModuleRegistry::ModuleRegistry(std::vector<std::unique_ptr<NativeModule>> modules)
     : modules_(std::move(modules)) {}
 
-void ModuleRegistry::registerModules(std::vector<std::unique_ptr<NativeModule>> modules) {
-  // TODO: consider relaxing this restriction
-  CHECK(modulesByName_.empty()) << "Can only register additional modules before NativeModules have been accessed";
+void ModuleRegistry::updateModuleNamesFromIndex(size_t index) {
+  for (; index < modules_.size(); index++ ) {
+    std::string name = normalizeName(modules_[index]->getName());
+    modulesByName_[name] = index;
+  }
+}
 
-  if (modules_.empty()) {
+void ModuleRegistry::registerModules(std::vector<std::unique_ptr<NativeModule>> modules) {
+  if (modules_.empty() && unknownModules_.empty()) {
     modules_ = std::move(modules);
   } else {
-    modules_.reserve(modules_.size() + modules.size());
+    size_t modulesSize = modules_.size();
+    size_t addModulesSize = modules.size();
+    bool addToNames = !modulesByName_.empty();
+    modules_.reserve(modulesSize + addModulesSize);
     std::move(modules.begin(), modules.end(), std::back_inserter(modules_));
+    if (!unknownModules_.empty()) {
+      for (size_t index = modulesSize; index < modulesSize + addModulesSize; index++) {
+        std::string name = normalizeName(modules_[index]->getName());
+        auto it = unknownModules_.find(name);
+        if (it != unknownModules_.end()) {
+          throw std::runtime_error(
+            folly::to<std::string>("module ", name, " was required without being registered and is now being registered."));
+        } else if (addToNames) {
+          modulesByName_[name] = index;
+        }
+      }
+    } else if (addToNames) {
+      updateModuleNamesFromIndex(modulesSize);
+    }
   }
 }
 
@@ -62,6 +83,7 @@ folly::Optional<ModuleConfig> ModuleRegistry::getConfig(const std::string& name)
 
   auto it = modulesByName_.find(name);
   if (it == modulesByName_.end()) {
+    unknownModules_.insert(name);
     return nullptr;
   }
 

ReactCommon/cxxreact/ModuleRegistry.h

@@ -4,6 +4,7 @@
 
 #include <memory>
 #include <vector>
+#include <unordered_set>
 
 #include <cxxreact/NativeModule.h>
 #include <folly/Optional.h>
@@ -42,8 +43,15 @@ class ModuleRegistry {
   // This is always populated
   std::vector<std::unique_ptr<NativeModule>> modules_;
 
+  // This is used to extend the population of modulesByName_ if registerModules is called after moduleNames
+  void updateModuleNamesFromIndex(size_t size);
+
   // This is only populated if moduleNames() is called.  Values are indices into modules_.
   std::unordered_map<std::string, size_t> modulesByName_;
+
+  // This is populated with modules that are requested via getConfig but are unknown.
+  // An error will be thrown if they are subsquently added to the registry.
+  std::unordered_set<std::string> unknownModules_;
 };
 
 }