Support ModuleHolder-based lazy init of C++ modules with C++ bridge on android

mhorowitz · facebook-github-bot · commit e622d51e20ea · 2017-03-14T15:35:01.000-07:00
Reviewed By: AaaChiuuu

Differential Revision: D4614479

fbshipit-source-id: 109ac34b8688f0113675e4a4479d1ddcc6169ed4
diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm
@@ -553,7 +553,8 @@ - (BOOL)moduleIsInitialized:(Class)moduleClass
       }
       modules.emplace_back(
         new QueueNativeModule(self, std::make_unique<CxxNativeModule>(
-          _reactInstance, [(RCTCxxModule *)(moduleData.instance) move])));
+                                  _reactInstance, [moduleData.name UTF8String],
+                                  [moduleData] { return [(RCTCxxModule *)(moduleData.instance) move]; })));
     } else {
       modules.emplace_back(new RCTNativeModule(self, moduleData));
     }
diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/CatalystInstanceImpl.java
@@ -110,7 +110,7 @@ private CatalystInstanceImpl(
       final JavaScriptModuleRegistry jsModuleRegistry,
       final JSBundleLoader jsBundleLoader,
       NativeModuleCallExceptionHandler nativeModuleCallExceptionHandler) {
-    FLog.d(ReactConstants.TAG, "Initializing React Xplat Bridge.");
+    FLog.w(ReactConstants.TAG, "Initializing React Xplat Bridge.");
     mHybridData = initHybrid();
 
     mReactQueueConfiguration = ReactQueueConfigurationImpl.create(
@@ -123,13 +123,15 @@ private CatalystInstanceImpl(
     mNativeModuleCallExceptionHandler = nativeModuleCallExceptionHandler;
     mTraceListener = new JSProfilerTraceListener(this);
 
+    FLog.w(ReactConstants.TAG, "Initializing React Xplat Bridge before initializeBridge");
     initializeBridge(
       new BridgeCallback(this),
       jsExecutor,
       mReactQueueConfiguration.getJSQueueThread(),
       mReactQueueConfiguration.getNativeModulesQueueThread(),
       mJavaRegistry.getJavaModules(this),
       mJavaRegistry.getCxxModules());
+    FLog.w(ReactConstants.TAG, "Initializing React Xplat Bridge after initializeBridge");
     mMainExecutorToken = getMainExecutorToken();
   }
 
@@ -182,7 +184,7 @@ private native void initializeBridge(
       MessageQueueThread jsQueue,
       MessageQueueThread moduleQueue,
       Collection<JavaModuleWrapper> javaModules,
-      Collection<CxxModuleWrapper> cxxModules);
+      Collection<ModuleHolder> cxxModules);
 
   /**
    * This API is used in situations where the JS bundle is being executed not on
diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java
@@ -53,12 +53,12 @@ public NativeModuleRegistry(
     return javaModules;
   }
 
-  /* package */ Collection<CxxModuleWrapper> getCxxModules() {
-    ArrayList<CxxModuleWrapper> cxxModules = new ArrayList<>();
+  /* package */ Collection<ModuleHolder> getCxxModules() {
+    ArrayList<ModuleHolder> cxxModules = new ArrayList<>();
     for (Map.Entry<Class<? extends NativeModule>, ModuleHolder> entry : mModules.entrySet()) {
       Class<?> type = entry.getKey();
       if (CxxModuleWrapper.class.isAssignableFrom(type)) {
-        cxxModules.add((CxxModuleWrapper) entry.getValue().getModule());
+        cxxModules.add(entry.getValue());
       }
     }
     return cxxModules;
diff --git a/ReactAndroid/src/main/jni/xreact/jni/Android.mk b/ReactAndroid/src/main/jni/xreact/jni/Android.mk
@@ -15,6 +15,7 @@ LOCAL_SRC_FILES := \
   JSLogging.cpp \
   JniJSModulesUnbundle.cpp \
   MethodInvoker.cpp \
+  ModuleRegistryBuilder.cpp \
   NativeArray.cpp \
   NativeCommon.cpp \
   NativeMap.cpp \
diff --git a/ReactAndroid/src/main/jni/xreact/jni/BUCK b/ReactAndroid/src/main/jni/xreact/jni/BUCK
@@ -6,6 +6,7 @@ EXPORTED_HEADERS = [
     "JExecutorToken.h",
     "JSLoader.h",
     "MethodInvoker.h",
+    "ModuleRegistryBuilder.h",
     "NativeArray.h",
     "NativeCommon.h",
     "NativeMap.h",
diff --git a/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.cpp b/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.cpp
@@ -20,6 +20,7 @@
 #include <cxxreact/ModuleRegistry.h>
 #include <cxxreact/CxxNativeModule.h>
 
+#include "CxxModuleWrapper.h"
 #include "JavaScriptExecutorHolder.h"
 #include "JniJSModulesUnbundle.h"
 #include "JNativeRunnable.h"
@@ -128,21 +129,10 @@ void CatalystInstanceImpl::initializeBridge(
     jni::alias_ref<JavaMessageQueueThread::javaobject> jsQueue,
     jni::alias_ref<JavaMessageQueueThread::javaobject> moduleQueue,
     jni::alias_ref<jni::JCollection<JavaModuleWrapper::javaobject>::javaobject> javaModules,
-    jni::alias_ref<jni::JCollection<CxxModuleWrapper::javaobject>::javaobject> cxxModules) {
+    jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject> cxxModules) {
   // TODO mhorowitz: how to assert here?
   // Assertions.assertCondition(mBridge == null, "initializeBridge should be called once");
 
-  std::vector<std::unique_ptr<NativeModule>> modules;
-  std::weak_ptr<Instance> winstance(instance_);
-  for (const auto& jm : *javaModules) {
-    modules.emplace_back(folly::make_unique<JavaNativeModule>(winstance, jm));
-  }
-  for (const auto& cm : *cxxModules) {
-    modules.emplace_back(
-      folly::make_unique<CxxNativeModule>(winstance, std::move(cthis(cm)->getModule())));
-  }
-  auto moduleRegistry = std::make_shared<ModuleRegistry>(std::move(modules));
-
   // This used to be:
   //
   // Java CatalystInstanceImpl -> C++ CatalystInstanceImpl -> Bridge -> Bridge::Callback
@@ -163,7 +153,8 @@ void CatalystInstanceImpl::initializeBridge(
                               jseh->getExecutorFactory(),
                               folly::make_unique<JMessageQueueThread>(jsQueue),
                               folly::make_unique<JMessageQueueThread>(moduleQueue),
-                              moduleRegistry);
+                              buildModuleRegistry(std::weak_ptr<Instance>(instance_),
+                                                  javaModules, cxxModules));
 }
 
 void CatalystInstanceImpl::jniSetSourceURL(const std::string& sourceURL) {
diff --git a/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.h b/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.h
@@ -10,6 +10,7 @@
 #include "JMessageQueueThread.h"
 #include "JSLoader.h"
 #include "JavaModuleWrapper.h"
+#include "ModuleRegistryBuilder.h"
 
 namespace facebook {
 namespace react {
@@ -48,7 +49,7 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
       jni::alias_ref<JavaMessageQueueThread::javaobject> jsQueue,
       jni::alias_ref<JavaMessageQueueThread::javaobject> moduleQueue,
       jni::alias_ref<jni::JCollection<JavaModuleWrapper::javaobject>::javaobject> javaModules,
-      jni::alias_ref<jni::JCollection<CxxModuleWrapper::javaobject>::javaobject> cxxModules);
+      jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject> cxxModules);
 
   /**
    * Sets the source URL of the underlying bridge without loading any JS code.
diff --git a/ReactAndroid/src/main/jni/xreact/jni/CxxModuleWrapper.h b/ReactAndroid/src/main/jni/xreact/jni/CxxModuleWrapper.h
@@ -12,7 +12,13 @@
 namespace facebook {
 namespace react {
 
-class CxxModuleWrapper : public jni::HybridClass<CxxModuleWrapper> {
+struct JNativeModule : jni::JavaClass<JNativeModule> {
+  constexpr static const char *const kJavaDescriptor =
+    "Lcom/facebook/react/bridge/NativeModule;";
+};
+
+class CxxModuleWrapper :
+    public jni::HybridClass<CxxModuleWrapper, JNativeModule> {
 public:
   constexpr static const char *const kJavaDescriptor =
     "Lcom/facebook/react/cxxbridge/CxxModuleWrapper;";
diff --git a/ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.h b/ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.h
@@ -27,6 +27,8 @@ struct JavaModuleWrapper : jni::JavaClass<JavaModuleWrapper> {
   static constexpr auto kJavaDescriptor = "Lcom/facebook/react/cxxbridge/JavaModuleWrapper;";
 
   jni::local_ref<JBaseJavaModule::javaobject> getModule() {
+    // This is the call which causes a lazy Java module to actually be
+    // created.
     static auto getModule = javaClassStatic()->getMethod<JBaseJavaModule::javaobject()>("getModule");
     return getModule(self());
   }
diff --git a/ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryBuilder.cpp b/ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryBuilder.cpp
@@ -0,0 +1,51 @@
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+#include "ModuleRegistryBuilder.h"
+
+#include <cxxreact/CxxNativeModule.h>
+#include <folly/Memory.h>
+
+namespace facebook {
+namespace react {
+
+std::string ModuleHolder::getName() const {
+  static auto method = getClass()->getMethod<jstring()>("getName");
+  return method(self())->toStdString();
+}
+
+xplat::module::CxxModule::Provider ModuleHolder::getProvider() const {
+  return [self=jni::make_global(self())] {
+    static auto method =
+      ModuleHolder::javaClassStatic()->getMethod<JNativeModule::javaobject()>(
+        "getModule");
+    // This is the call which uses the lazy Java Provider to instantiate the
+    // Java CxxModuleWrapper which contains the CxxModule.
+    auto module = method(self);
+    CHECK(module->isInstanceOf(CxxModuleWrapper::javaClassStatic()))
+      << "module isn't a C++ module";
+    auto cxxModule = jni::static_ref_cast<CxxModuleWrapper::javaobject>(module);
+    // Then, we grab the CxxModule from the wrapper, which is no longer needed.
+    return cxxModule->cthis()->getModule();
+  };
+}
+
+std::unique_ptr<ModuleRegistry> buildModuleRegistry(
+    std::weak_ptr<Instance> winstance,
+    jni::alias_ref<jni::JCollection<JavaModuleWrapper::javaobject>::javaobject> javaModules,
+    jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject> cxxModules) {
+  std::vector<std::unique_ptr<NativeModule>> modules;
+  for (const auto& jm : *javaModules) {
+    modules.emplace_back(folly::make_unique<JavaNativeModule>(winstance, jm));
+  }
+  for (const auto& cm : *cxxModules) {
+    modules.emplace_back(
+      folly::make_unique<CxxNativeModule>(winstance, cm->getName(), cm->getProvider()));
+  }
+  if (modules.empty()) {
+    return nullptr;
+  } else {
+    return folly::make_unique<ModuleRegistry>(std::move(modules));
+  }
+}
+
+}}
diff --git a/ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryBuilder.h b/ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryBuilder.h
@@ -0,0 +1,30 @@
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+#include <string>
+
+#include <cxxreact/CxxModule.h>
+#include <cxxreact/ModuleRegistry.h>
+#include <fb/fbjni.h>
+
+#include "CxxModuleWrapper.h"
+#include "JavaModuleWrapper.h"
+
+namespace facebook {
+namespace react {
+
+class ModuleHolder : public jni::JavaClass<ModuleHolder> {
+ public:
+  static auto constexpr kJavaDescriptor =
+    "Lcom/facebook/react/cxxbridge/ModuleHolder;";
+
+  std::string getName() const;
+  xplat::module::CxxModule::Provider getProvider() const;
+};
+
+std::unique_ptr<ModuleRegistry> buildModuleRegistry(
+  std::weak_ptr<Instance> winstance,
+  jni::alias_ref<jni::JCollection<JavaModuleWrapper::javaobject>::javaobject> javaModules,
+  jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject> cxxModules);
+
+}
+}
diff --git a/ReactCommon/cxxreact/Android.mk b/ReactCommon/cxxreact/Android.mk
@@ -5,6 +5,7 @@ include $(CLEAR_VARS)
 LOCAL_MODULE := libreactnativefb
 
 LOCAL_SRC_FILES := \
+  CxxMessageQueue.cpp \
   CxxNativeModule.cpp \
   Instance.cpp \
   JSCExecutor.cpp \
diff --git a/ReactCommon/cxxreact/CxxModule.h b/ReactCommon/cxxreact/CxxModule.h
@@ -50,6 +50,8 @@ class CxxModule {
   class SyncTagType {};
 
 public:
+  typedef std::function<std::unique_ptr<CxxModule>()> Provider;
+
   typedef std::function<void(std::vector<folly::dynamic>)> Callback;
 
   constexpr static AsyncTagType AsyncTag = AsyncTagType();
diff --git a/ReactCommon/cxxreact/CxxNativeModule.cpp b/ReactCommon/cxxreact/CxxNativeModule.cpp
@@ -47,18 +47,19 @@ CxxModule::Callback convertCallback(
 }
 
 CxxNativeModule::CxxNativeModule(std::weak_ptr<Instance> instance,
-                                 std::unique_ptr<CxxModule> module)
+                                 std::string name,
+                                 CxxModule::Provider provider)
   : instance_(instance)
-  , module_(std::move(module))
-  , methods_(module_->getMethods()) {
-    module_->setInstance(instance);
-  }
+  , name_(std::move(name))
+  , provider_(provider) {}
 
 std::string CxxNativeModule::getName() {
-  return module_->getName();
+  return name_;
 }
 
 std::vector<MethodDescriptor> CxxNativeModule::getMethods() {
+  lazyInit();
+
   std::vector<MethodDescriptor> descs;
   for (auto& method : methods_) {
     assert(method.func || method.syncFunc);
@@ -68,6 +69,8 @@ std::vector<MethodDescriptor> CxxNativeModule::getMethods() {
 }
 
 folly::dynamic CxxNativeModule::getConstants() {
+  lazyInit();
+
   folly::dynamic constants = folly::dynamic::object();
   for (auto& pair : module_->getConstants()) {
     constants.insert(std::move(pair.first), std::move(pair.second));
@@ -168,5 +171,15 @@ MethodCallResult CxxNativeModule::callSerializableNativeHook(
   return method.syncFunc(std::move(args));
 }
 
+void CxxNativeModule::lazyInit() {
+  if (module_) {
+    return;
+  }
+
+  module_ = provider_();
+  methods_ = module_->getMethods();
+  module_->setInstance(instance_);
+}
+
 }
 }
diff --git a/ReactCommon/cxxreact/CxxNativeModule.h b/ReactCommon/cxxreact/CxxNativeModule.h
@@ -15,8 +15,8 @@ std::function<void(folly::dynamic)> makeCallback(
 
 class CxxNativeModule : public NativeModule {
 public:
-  CxxNativeModule(std::weak_ptr<Instance> instance,
-                  std::unique_ptr<xplat::module::CxxModule> module);
+  CxxNativeModule(std::weak_ptr<Instance> instance, std::string name,
+                  xplat::module::CxxModule::Provider provider);
 
   std::string getName() override;
   std::vector<MethodDescriptor> getMethods() override;
@@ -27,7 +27,11 @@ class CxxNativeModule : public NativeModule {
     ExecutorToken token, unsigned int hookId, folly::dynamic&& args) override;
 
 private:
+  void lazyInit();
+
   std::weak_ptr<Instance> instance_;
+  std::string name_;
+  xplat::module::CxxModule::Provider provider_;
   std::unique_ptr<xplat::module::CxxModule> module_;
   std::vector<xplat::module::CxxModule::Method> methods_;
 };
diff --git a/ReactCommon/cxxreact/Instance.cpp b/ReactCommon/cxxreact/Instance.cpp
@@ -40,7 +40,7 @@ void Instance::initializeBridge(
   if (!nativeQueue) {
     // TODO pass down a thread/queue from java, instead of creating our own.
 
-    auto queue = std::make_unique<CxxMessageQueue>();
+    auto queue = folly::make_unique<CxxMessageQueue>();
     std::thread t(queue->getUnregisteredRunLoop());
     t.detach();
     nativeQueue = std::move(queue);
diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp
diff --git a/ReactCommon/cxxreact/JSCNativeModules.cpp b/ReactCommon/cxxreact/JSCNativeModules.cpp
diff --git a/ReactCommon/cxxreact/NativeToJsBridge.cpp b/ReactCommon/cxxreact/NativeToJsBridge.cpp
diff --git a/ReactCommon/jschelpers/Value.h b/ReactCommon/jschelpers/Value.h

Original file line number	Diff line number	Diff line change
`@@ -553,7 +553,8 @@ - (BOOL)moduleIsInitialized:(Class)moduleClass`
`553`	`553`	`}`
`554`	`554`	`modules.emplace_back(`
`555`	`555`	`new QueueNativeModule(self, std::make_unique<CxxNativeModule>(`
`556`		`- _reactInstance, [(RCTCxxModule *)(moduleData.instance) move])));`
	`556`	`+ _reactInstance, [moduleData.name UTF8String],`
	`557`	`+ [moduleData] { return [(RCTCxxModule *)(moduleData.instance) move]; })));`
`557`	`558`	`} else {`
`558`	`559`	`modules.emplace_back(new RCTNativeModule(self, moduleData));`
`559`	`560`	`}`
Original file line number	Diff line number	Diff line change
`@@ -53,12 +53,12 @@ public NativeModuleRegistry(`
`53`	`53`	`return javaModules;`
`54`	`54`	`}`
`55`	`55`
`56`		`- /* package */ Collection<CxxModuleWrapper> getCxxModules() {`
`57`		`- ArrayList<CxxModuleWrapper> cxxModules = new ArrayList<>();`
	`56`	`+ /* package */ Collection<ModuleHolder> getCxxModules() {`
	`57`	`+ ArrayList<ModuleHolder> cxxModules = new ArrayList<>();`
`58`	`58`	`for (Map.Entry<Class<? extends NativeModule>, ModuleHolder> entry : mModules.entrySet()) {`
`59`	`59`	`Class<?> type = entry.getKey();`
`60`	`60`	`if (CxxModuleWrapper.class.isAssignableFrom(type)) {`
`61`		`- cxxModules.add((CxxModuleWrapper) entry.getValue().getModule());`
	`61`	`+ cxxModules.add(entry.getValue());`
`62`	`62`	`}`
`63`	`63`	`}`
`64`	`64`	`return cxxModules;`
Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,8 @@ struct JavaModuleWrapper : jni::JavaClass<JavaModuleWrapper> {`
`27`	`27`	`static constexpr auto kJavaDescriptor = "Lcom/facebook/react/cxxbridge/JavaModuleWrapper;";`
`28`	`28`
`29`	`29`	`jni::local_ref<JBaseJavaModule::javaobject> getModule() {`
	`30`	`+ // This is the call which causes a lazy Java module to actually be`
	`31`	`+ // created.`
`30`	`32`	`static auto getModule = javaClassStatic()->getMethod<JBaseJavaModule::javaobject()>("getModule");`
`31`	`33`	`return getModule(self());`
`32`	`34`	`}`