diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index 431c59f4f..16b8782ec 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -553,7 +553,8 @@ struct RCTInstanceCallback : public InstanceCallback { } modules.emplace_back( new QueueNativeModule(self, std::make_unique( - _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 index 4e9d9c088..38a9d5485 100644 --- 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 @@ public class CatalystInstanceImpl implements CatalystInstance { 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,6 +123,7 @@ public class CatalystInstanceImpl implements CatalystInstance { mNativeModuleCallExceptionHandler = nativeModuleCallExceptionHandler; mTraceListener = new JSProfilerTraceListener(this); + FLog.w(ReactConstants.TAG, "Initializing React Xplat Bridge before initializeBridge"); initializeBridge( new BridgeCallback(this), jsExecutor, @@ -130,6 +131,7 @@ public class CatalystInstanceImpl implements CatalystInstance { mReactQueueConfiguration.getNativeModulesQueueThread(), mJavaRegistry.getJavaModules(this), mJavaRegistry.getCxxModules()); + FLog.w(ReactConstants.TAG, "Initializing React Xplat Bridge after initializeBridge"); mMainExecutorToken = getMainExecutorToken(); } @@ -182,7 +184,7 @@ public class CatalystInstanceImpl implements CatalystInstance { MessageQueueThread jsQueue, MessageQueueThread moduleQueue, Collection javaModules, - Collection cxxModules); + Collection 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 index 2b0a57625..140c4efdb 100644 --- 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 class NativeModuleRegistry { return javaModules; } - /* package */ Collection getCxxModules() { - ArrayList cxxModules = new ArrayList<>(); + /* package */ Collection getCxxModules() { + ArrayList cxxModules = new ArrayList<>(); for (Map.Entry, 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 index ea700e03a..46cebd13e 100644 --- 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 index ef06da174..f58ec9e4b 100644 --- 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 index b4aa91b63..a38ed7713 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.cpp +++ b/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.cpp @@ -20,6 +20,7 @@ #include #include +#include "CxxModuleWrapper.h" #include "JavaScriptExecutorHolder.h" #include "JniJSModulesUnbundle.h" #include "JNativeRunnable.h" @@ -128,21 +129,10 @@ void CatalystInstanceImpl::initializeBridge( jni::alias_ref jsQueue, jni::alias_ref moduleQueue, jni::alias_ref::javaobject> javaModules, - jni::alias_ref::javaobject> cxxModules) { + jni::alias_ref::javaobject> cxxModules) { // TODO mhorowitz: how to assert here? // Assertions.assertCondition(mBridge == null, "initializeBridge should be called once"); - std::vector> modules; - std::weak_ptr winstance(instance_); - for (const auto& jm : *javaModules) { - modules.emplace_back(folly::make_unique(winstance, jm)); - } - for (const auto& cm : *cxxModules) { - modules.emplace_back( - folly::make_unique(winstance, std::move(cthis(cm)->getModule()))); - } - auto moduleRegistry = std::make_shared(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(jsQueue), folly::make_unique(moduleQueue), - moduleRegistry); + buildModuleRegistry(std::weak_ptr(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 index 61e07b9c6..804fc927c 100644 --- 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 { jni::alias_ref jsQueue, jni::alias_ref moduleQueue, jni::alias_ref::javaobject> javaModules, - jni::alias_ref::javaobject> cxxModules); + jni::alias_ref::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 index 7e8009464..1d5e8ed0c 100644 --- 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 { +struct JNativeModule : jni::JavaClass { + constexpr static const char *const kJavaDescriptor = + "Lcom/facebook/react/bridge/NativeModule;"; +}; + +class CxxModuleWrapper : + public jni::HybridClass { 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 index 9ef6c483b..544b6eb16 100644 --- 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 { static constexpr auto kJavaDescriptor = "Lcom/facebook/react/cxxbridge/JavaModuleWrapper;"; jni::local_ref getModule() { + // This is the call which causes a lazy Java module to actually be + // created. static auto getModule = javaClassStatic()->getMethod("getModule"); return getModule(self()); } diff --git a/ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryBuilder.cpp b/ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryBuilder.cpp new file mode 100644 index 000000000..108091a8d --- /dev/null +++ b/ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryBuilder.cpp @@ -0,0 +1,51 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#include "ModuleRegistryBuilder.h" + +#include +#include + +namespace facebook { +namespace react { + +std::string ModuleHolder::getName() const { + static auto method = getClass()->getMethod("getName"); + return method(self())->toStdString(); +} + +xplat::module::CxxModule::Provider ModuleHolder::getProvider() const { + return [self=jni::make_global(self())] { + static auto method = + ModuleHolder::javaClassStatic()->getMethod( + "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(module); + // Then, we grab the CxxModule from the wrapper, which is no longer needed. + return cxxModule->cthis()->getModule(); + }; +} + +std::unique_ptr buildModuleRegistry( + std::weak_ptr winstance, + jni::alias_ref::javaobject> javaModules, + jni::alias_ref::javaobject> cxxModules) { + std::vector> modules; + for (const auto& jm : *javaModules) { + modules.emplace_back(folly::make_unique(winstance, jm)); + } + for (const auto& cm : *cxxModules) { + modules.emplace_back( + folly::make_unique(winstance, cm->getName(), cm->getProvider())); + } + if (modules.empty()) { + return nullptr; + } else { + return folly::make_unique(std::move(modules)); + } +} + +}} diff --git a/ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryBuilder.h b/ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryBuilder.h new file mode 100644 index 000000000..554473601 --- /dev/null +++ b/ReactAndroid/src/main/jni/xreact/jni/ModuleRegistryBuilder.h @@ -0,0 +1,30 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#include + +#include +#include +#include + +#include "CxxModuleWrapper.h" +#include "JavaModuleWrapper.h" + +namespace facebook { +namespace react { + +class ModuleHolder : public jni::JavaClass { + public: + static auto constexpr kJavaDescriptor = + "Lcom/facebook/react/cxxbridge/ModuleHolder;"; + + std::string getName() const; + xplat::module::CxxModule::Provider getProvider() const; +}; + +std::unique_ptr buildModuleRegistry( + std::weak_ptr winstance, + jni::alias_ref::javaobject> javaModules, + jni::alias_ref::javaobject> cxxModules); + +} +} diff --git a/ReactCommon/cxxreact/Android.mk b/ReactCommon/cxxreact/Android.mk index f9cdb1976..6f33a13ef 100644 --- 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 index 0b420b01f..897123008 100644 --- a/ReactCommon/cxxreact/CxxModule.h +++ b/ReactCommon/cxxreact/CxxModule.h @@ -50,6 +50,8 @@ class CxxModule { class SyncTagType {}; public: + typedef std::function()> Provider; + typedef std::function)> Callback; constexpr static AsyncTagType AsyncTag = AsyncTagType(); diff --git a/ReactCommon/cxxreact/CxxNativeModule.cpp b/ReactCommon/cxxreact/CxxNativeModule.cpp index f73cd3000..c0cacd0b8 100644 --- a/ReactCommon/cxxreact/CxxNativeModule.cpp +++ b/ReactCommon/cxxreact/CxxNativeModule.cpp @@ -47,18 +47,19 @@ CxxModule::Callback convertCallback( } CxxNativeModule::CxxNativeModule(std::weak_ptr instance, - std::unique_ptr 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 CxxNativeModule::getMethods() { + lazyInit(); + std::vector descs; for (auto& method : methods_) { assert(method.func || method.syncFunc); @@ -68,6 +69,8 @@ std::vector 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 index 308d004bb..ab6f3df87 100644 --- a/ReactCommon/cxxreact/CxxNativeModule.h +++ b/ReactCommon/cxxreact/CxxNativeModule.h @@ -15,8 +15,8 @@ std::function makeCallback( class CxxNativeModule : public NativeModule { public: - CxxNativeModule(std::weak_ptr instance, - std::unique_ptr module); + CxxNativeModule(std::weak_ptr instance, std::string name, + xplat::module::CxxModule::Provider provider); std::string getName() override; std::vector getMethods() override; @@ -27,7 +27,11 @@ public: ExecutorToken token, unsigned int hookId, folly::dynamic&& args) override; private: + void lazyInit(); + std::weak_ptr instance_; + std::string name_; + xplat::module::CxxModule::Provider provider_; std::unique_ptr module_; std::vector methods_; }; diff --git a/ReactCommon/cxxreact/Instance.cpp b/ReactCommon/cxxreact/Instance.cpp index 12a9dbe1e..f7ad78ed4 100644 --- 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(); + auto queue = folly::make_unique(); std::thread t(queue->getUnregisteredRunLoop()); t.detach(); nativeQueue = std::move(queue); diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 606db4ab4..cee2f2ece 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/ReactCommon/cxxreact/JSCExecutor.cpp @@ -361,12 +361,8 @@ void JSCExecutor::loadApplicationScript(std::unique_ptr scrip evaluateSourceCode(m_context, bcSourceCode, jsSourceURL); - // TODO(luk): t13903306 Remove this check once we make native modules - // working for java2js - if (m_delegate) { - bindBridge(); - flush(); - } + bindBridge(); + flush(); ReactMarker::logMarker("CREATE_REACT_CONTEXT_END"); ReactMarker::logMarker("RUN_JS_BUNDLE_END"); @@ -416,11 +412,8 @@ void JSCExecutor::loadApplicationScript(std::unique_ptr scrip evaluateScript(m_context, jsScript, jsSourceURL); } - // TODO(luk): t13903306 Remove this check once we make native modules working for java2js - if (m_delegate) { - bindBridge(); - flush(); - } + bindBridge(); + flush(); ReactMarker::logMarker("CREATE_REACT_CONTEXT_END"); ReactMarker::logMarker("RUN_JS_BUNDLE_END"); @@ -435,6 +428,9 @@ void JSCExecutor::setJSModulesUnbundle(std::unique_ptr unbund void JSCExecutor::bindBridge() throw(JSException) { SystraceSection s("JSCExecutor::bindBridge"); + if (!m_delegate || !m_delegate->getModuleRegistry()) { + return; + } auto global = Object::getGlobalObject(m_context); auto batchedBridgeValue = global.getProperty("__fbBatchedBridge"); if (batchedBridgeValue.isUndefined()) { @@ -466,7 +462,18 @@ void JSCExecutor::callNativeModules(Value&& value) { void JSCExecutor::flush() { SystraceSection s("JSCExecutor::flush"); - callNativeModules(m_flushedQueueJS->callAsFunction({})); + if (!m_delegate) { + // do nothing + } else if (!m_delegate->getModuleRegistry()) { + callNativeModules(Value::makeNull(m_context)); + } else { + // If this is failing, chances are you have provided a delegate with a + // module registry, but haven't loaded the JS which enables native function + // queueing. Add BatchedBridge.js to your bundle, pass a nullptr delegate, + // or make delegate->getModuleRegistry() return nullptr. + CHECK(m_flushedQueueJS) << "Attempting to use native methods without loading BatchedBridge.js"; + callNativeModules(m_flushedQueueJS->callAsFunction({})); + } } void JSCExecutor::callFunction(const std::string& moduleId, const std::string& methodId, const folly::dynamic& arguments) { @@ -476,6 +483,9 @@ void JSCExecutor::callFunction(const std::string& moduleId, const std::string& m auto result = [&] { try { + // See flush() + CHECK(m_callFunctionReturnFlushedQueueJS) + << "Attempting to call native methods without loading BatchedBridge.js"; return m_callFunctionReturnFlushedQueueJS->callAsFunction({ Value(m_context, String::createExpectingAscii(m_context, moduleId)), Value(m_context, String::createExpectingAscii(m_context, methodId)), @@ -511,6 +521,8 @@ Value JSCExecutor::callFunctionSyncWithValue( const std::string& module, const std::string& method, Value args) { SystraceSection s("JSCExecutor::callFunction"); + // See flush() + CHECK(m_callFunctionReturnResultAndFlushedQueueJS); Object result = m_callFunctionReturnResultAndFlushedQueueJS->callAsFunction({ Value(m_context, String::createExpectingAscii(m_context, module)), Value(m_context, String::createExpectingAscii(m_context, method)), diff --git a/ReactCommon/cxxreact/JSCNativeModules.cpp b/ReactCommon/cxxreact/JSCNativeModules.cpp index b1036ba37..105873cfd 100644 --- a/ReactCommon/cxxreact/JSCNativeModules.cpp +++ b/ReactCommon/cxxreact/JSCNativeModules.cpp @@ -11,6 +11,10 @@ JSCNativeModules::JSCNativeModules(std::shared_ptr moduleRegistr m_moduleRegistry(std::move(moduleRegistry)) {} JSValueRef JSCNativeModules::getModule(JSContextRef context, JSStringRef jsName) { + if (!m_moduleRegistry) { + return Value::makeUndefined(context); + } + std::string moduleName = String::ref(context, jsName).str(); const auto it = m_objects.find(moduleName); diff --git a/ReactCommon/cxxreact/NativeToJsBridge.cpp b/ReactCommon/cxxreact/NativeToJsBridge.cpp index 56a71adfd..5c4399766 100644 --- a/ReactCommon/cxxreact/NativeToJsBridge.cpp +++ b/ReactCommon/cxxreact/NativeToJsBridge.cpp @@ -47,6 +47,9 @@ public: void callNativeModules( JSExecutor& executor, folly::dynamic&& calls, bool isEndOfBatch) override { + + CHECK(m_registry || calls.empty()) << + "native module calls cannot be completed with no native modules"; ExecutorToken token = m_nativeToJs->getTokenForExecutor(executor); m_nativeQueue->runOnQueue([this, token, calls=std::move(calls), isEndOfBatch] () mutable { // An exception anywhere in here stops processing of the batch. This @@ -95,13 +98,8 @@ NativeToJsBridge::NativeToJsBridge( std::shared_ptr callback) : m_destroyed(std::make_shared(false)) , m_mainExecutorToken(callback->createExecutorToken()) - , m_delegate(registry - ? std::make_shared( - this, registry, std::move(nativeQueue), callback) - : nullptr) { - if (!m_delegate) { - nativeQueue->quitSynchronous(); - } + , m_delegate(std::make_shared( + this, registry, std::move(nativeQueue), callback)) { std::unique_ptr mainExecutor = jsExecutorFactory->createJSExecutor(m_delegate, jsQueue); // cached to avoid locked map lookup in the common case @@ -318,9 +316,7 @@ ExecutorToken NativeToJsBridge::getTokenForExecutor(JSExecutor& executor) { } void NativeToJsBridge::destroy() { - if (m_delegate) { - m_delegate->quitQueueSynchronous(); - } + m_delegate->quitQueueSynchronous(); auto* executorMessageQueueThread = getMessageQueueThread(m_mainExecutorToken); // All calls made through runOnExecutorQueue have an early exit if // m_destroyed is true. Setting this before the runOnQueueSync will cause diff --git a/ReactCommon/jschelpers/Value.h b/ReactCommon/jschelpers/Value.h index f2879bd1d..b5d79c53d 100644 --- a/ReactCommon/jschelpers/Value.h +++ b/ReactCommon/jschelpers/Value.h @@ -305,6 +305,10 @@ public: return Value(ctx, JSC_JSValueMakeUndefined(ctx)); } + static Value makeNull(JSContextRef ctx) { + return Value(ctx, JSC_JSValueMakeNull(ctx)); + } + std::string toJSONString(unsigned indent = 0) const; static Value fromJSON(JSContextRef ctx, const String& json); static JSValueRef fromDynamic(JSContextRef ctx, const folly::dynamic& value);