From eb4e2baaa91f29e34369105ab493ce41ff0274e4 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Fri, 3 Apr 2020 09:44:40 -0700 Subject: [PATCH] Rename Instance::getNativeCallinvoker to Instance::getDecoratedNativeCallInvoker Summary: Now, instead of accepting a `std::function` that schedules work, and returning a `CallInvoker`, `Instance::getDecoratedNativeCallInvoker` will accept a `CallInvoker` that schedules work, and return a decorated `CallInvoker`. I think this change will help with readability. It also clarifies that the bridge is adding additional behaviour to the native `CallInvoker`. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D20826885 fbshipit-source-id: a2c5681d10a4544ee3d2a0d1f1cbd386ef06d0e6 --- .../jni/react/jni/CatalystInstanceImpl.cpp | 25 +++++++++++++++---- ReactCommon/cxxreact/Instance.cpp | 6 ++--- ReactCommon/cxxreact/Instance.h | 13 +++++----- ReactCommon/cxxreact/NativeToJsBridge.cpp | 15 ++++++----- ReactCommon/cxxreact/NativeToJsBridge.h | 4 +-- 5 files changed, 38 insertions(+), 25 deletions(-) diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp index 3b8bfc201c0..062068b375f 100644 --- a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp +++ b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp @@ -297,12 +297,27 @@ CatalystInstanceImpl::getJSCallInvokerHolder() { jni::alias_ref CatalystInstanceImpl::getNativeCallInvokerHolder() { if (!nativeCallInvokerHolder_) { + class NativeThreadCallInvoker : public CallInvoker { + private: + std::shared_ptr messageQueueThread_; + + public: + NativeThreadCallInvoker( + std::shared_ptr messageQueueThread) + : messageQueueThread_(messageQueueThread) {} + void invokeAsync(std::function &&work) override { + messageQueueThread_->runOnQueue(std::move(work)); + } + }; + + std::shared_ptr nativeInvoker = + std::make_shared(moduleMessageQueue_); + + std::shared_ptr decoratedNativeInvoker = + instance_->getDecoratedNativeCallInvoker(nativeInvoker); + nativeCallInvokerHolder_ = jni::make_global( - CallInvokerHolder::newObjectCxxArgs(instance_->getNativeCallInvoker( - [moduleMessageQueue = - moduleMessageQueue_](std::function &&work) { - moduleMessageQueue->runOnQueue(std::move(work)); - }))); + CallInvokerHolder::newObjectCxxArgs(decoratedNativeInvoker)); } return nativeCallInvokerHolder_; diff --git a/ReactCommon/cxxreact/Instance.cpp b/ReactCommon/cxxreact/Instance.cpp index 7c87c26f5a1..bd41b933239 100644 --- a/ReactCommon/cxxreact/Instance.cpp +++ b/ReactCommon/cxxreact/Instance.cpp @@ -228,9 +228,9 @@ std::shared_ptr Instance::getJSCallInvoker() { return std::static_pointer_cast(jsCallInvoker_); } -std::shared_ptr Instance::getNativeCallInvoker( - std::function &&work)> &&scheduleWork) { - return nativeToJsBridge_->getNativeCallInvoker(std::move(scheduleWork)); +std::shared_ptr Instance::getDecoratedNativeCallInvoker( + std::shared_ptr nativeInvoker) { + return nativeToJsBridge_->getDecoratedNativeCallInvoker(nativeInvoker); } void Instance::JSCallInvoker::setNativeToJsBridgeAndFlushCalls( diff --git a/ReactCommon/cxxreact/Instance.h b/ReactCommon/cxxreact/Instance.h index 4a76d5e9d65..c6e1278d038 100644 --- a/ReactCommon/cxxreact/Instance.h +++ b/ReactCommon/cxxreact/Instance.h @@ -106,7 +106,7 @@ class RN_EXPORT Instance { * Native CallInvoker is used by TurboModules to schedule work on the * NativeModule thread(s). * - * Why is the bridge creating JS CallInvoker? + * Why is the bridge decorating native CallInvoker? * * - The bridge must be informed of all TurboModule async method calls. Why? * When all queued NativeModule method calls are flushed by a call from @@ -118,17 +118,16 @@ class RN_EXPORT Instance { * since the last time the bridge was flushed. If this number is non-zero, * we fire onBatchComplete. * - * Why must we pass in a scheduleWork function? + * Why can't we just create and return a new native CallInvoker? * * - On Android, we have one NativeModule thread. That thread is created and * managed outisde of NativeToJsBridge. On iOS, we have one MethodQueue per * module. Those MethodQueues are also created and managed outside of - * NativeToJsBridge. Therefore, we need to pass in a function that schedules - * work on the respective thread. - * + * NativeToJsBridge. Therefore, we need to pass in a CallInvoker that + * schedules work on the respective thread. */ - std::shared_ptr getNativeCallInvoker( - std::function &&work)> &&scheduleWork); + std::shared_ptr getDecoratedNativeCallInvoker( + std::shared_ptr nativeInvoker); private: void callNativeModules(folly::dynamic &&calls, bool isEndOfBatch); diff --git a/ReactCommon/cxxreact/NativeToJsBridge.cpp b/ReactCommon/cxxreact/NativeToJsBridge.cpp index 83ceb0c1a77..cc0889db534 100644 --- a/ReactCommon/cxxreact/NativeToJsBridge.cpp +++ b/ReactCommon/cxxreact/NativeToJsBridge.cpp @@ -302,30 +302,29 @@ void NativeToJsBridge::runOnExecutorQueue( }); } -std::shared_ptr NativeToJsBridge::getNativeCallInvoker( - std::function &&work)> &&scheduleWork) { +std::shared_ptr NativeToJsBridge::getDecoratedNativeCallInvoker( + std::shared_ptr nativeInvoker) { class NativeCallInvoker : public CallInvoker { private: std::weak_ptr m_jsToNativeBridge; - std::function &&work)> m_scheduleWork; + std::shared_ptr m_nativeInvoker; public: NativeCallInvoker( std::weak_ptr jsToNativeBridge, - std::function &&work)> &&scheduleWork) + std::shared_ptr nativeInvoker) : m_jsToNativeBridge(jsToNativeBridge), - m_scheduleWork(std::move(scheduleWork)) {} + m_nativeInvoker(nativeInvoker) {} void invokeAsync(std::function &&func) override { if (auto strongJsToNativeBridge = m_jsToNativeBridge.lock()) { strongJsToNativeBridge->recordTurboModuleAsyncMethodCall(); } - m_scheduleWork(std::move(func)); + m_nativeInvoker->invokeAsync(std::move(func)); } }; - return std::make_shared( - m_delegate, std::move(scheduleWork)); + return std::make_shared(m_delegate, nativeInvoker); } } // namespace react diff --git a/ReactCommon/cxxreact/NativeToJsBridge.h b/ReactCommon/cxxreact/NativeToJsBridge.h index e6d6dc643c2..9cc9176b093 100644 --- a/ReactCommon/cxxreact/NativeToJsBridge.h +++ b/ReactCommon/cxxreact/NativeToJsBridge.h @@ -103,8 +103,8 @@ class NativeToJsBridge { * Native CallInvoker is used by TurboModules to schedule work on the * NativeModule thread(s). */ - std::shared_ptr getNativeCallInvoker( - std::function &&work)> &&scheduleWork); + std::shared_ptr getDecoratedNativeCallInvoker( + std::shared_ptr nativeInvoker); private: // This is used to avoid a race condition where a proxyCallback gets queued