From cf7a97cc0f44853b7ad17a5fc619a64c994b6cbc Mon Sep 17 00:00:00 2001 From: Andy Street Date: Fri, 12 Feb 2016 09:09:41 -0800 Subject: [PATCH] WebWorkers: Pass Bridge to JSExecutors Reviewed By: cjhopman Differential Revision: D2921840 fb-gh-sync-id: f89f9ffe2c3bb0dabce7e8cbb9c7dc8b5062e267 shipit-source-id: f89f9ffe2c3bb0dabce7e8cbb9c7dc8b5062e267 --- ReactAndroid/src/main/jni/react/Bridge.cpp | 34 ++++++--------- ReactAndroid/src/main/jni/react/Bridge.h | 13 +++--- ReactAndroid/src/main/jni/react/Executor.h | 24 +++++------ .../src/main/jni/react/JSCExecutor.cpp | 43 +++++++++---------- ReactAndroid/src/main/jni/react/JSCExecutor.h | 35 +++++++++------ .../src/main/jni/react/jni/OnLoad.cpp | 9 +--- .../src/main/jni/react/jni/ProxyExecutor.cpp | 19 ++++---- .../src/main/jni/react/jni/ProxyExecutor.h | 13 +++--- 8 files changed, 89 insertions(+), 101 deletions(-) diff --git a/ReactAndroid/src/main/jni/react/Bridge.cpp b/ReactAndroid/src/main/jni/react/Bridge.cpp index a4bb6887e..821975bff 100644 --- a/ReactAndroid/src/main/jni/react/Bridge.cpp +++ b/ReactAndroid/src/main/jni/react/Bridge.cpp @@ -11,16 +11,9 @@ namespace facebook { namespace react { Bridge::Bridge(JSExecutorFactory* jsExecutorFactory, Callback callback) : - m_callback(std::move(callback)), - m_destroyed(std::shared_ptr(new bool(false))) -{ - auto destroyed = m_destroyed; - m_jsExecutor = jsExecutorFactory->createJSExecutor([this, destroyed] (std::string queueJSON, bool isEndOfBatch) { - if (*destroyed) { - return; - } - m_callback(parseMethodCalls(queueJSON), isEndOfBatch); - }); + m_callback(std::move(callback)), + m_destroyed(std::shared_ptr(new bool(false))) { + m_jsExecutor = jsExecutorFactory->createJSExecutor(this); } // This must be called on the same thread on which the constructor was called. @@ -40,14 +33,6 @@ void Bridge::loadApplicationUnbundle( m_jsExecutor->loadApplicationUnbundle(std::move(unbundle), startupCode, sourceURL); } -void Bridge::flush() { - if (*m_destroyed) { - return; - } - auto returnedJSON = m_jsExecutor->flush(); - m_callback(parseMethodCalls(returnedJSON), true /* = isEndOfBatch */); -} - void Bridge::callFunction(const double moduleId, const double methodId, const folly::dynamic& arguments) { if (*m_destroyed) { return; @@ -55,8 +40,7 @@ void Bridge::callFunction(const double moduleId, const double methodId, const fo #ifdef WITH_FBSYSTRACE FbSystraceSection s(TRACE_TAG_REACT_CXX_BRIDGE, "Bridge.callFunction"); #endif - auto returnedJSON = m_jsExecutor->callFunction(moduleId, methodId, arguments); - m_callback(parseMethodCalls(returnedJSON), true /* = isEndOfBatch */); + m_jsExecutor->callFunction(moduleId, methodId, arguments); } void Bridge::invokeCallback(const double callbackId, const folly::dynamic& arguments) { @@ -66,8 +50,7 @@ void Bridge::invokeCallback(const double callbackId, const folly::dynamic& argum #ifdef WITH_FBSYSTRACE FbSystraceSection s(TRACE_TAG_REACT_CXX_BRIDGE, "Bridge.invokeCallback"); #endif - auto returnedJSON = m_jsExecutor->invokeCallback(callbackId, arguments); - m_callback(parseMethodCalls(returnedJSON), true /* = isEndOfBatch */); + m_jsExecutor->invokeCallback(callbackId, arguments); } void Bridge::setGlobalVariable(const std::string& propName, const std::string& jsonValue) { @@ -94,4 +77,11 @@ void Bridge::handleMemoryPressureCritical() { m_jsExecutor->handleMemoryPressureCritical(); } +void Bridge::callNativeModules(const std::string& callJSON, bool isEndOfBatch) { + if (*m_destroyed) { + return; + } + m_callback(parseMethodCalls(callJSON), isEndOfBatch); +} + } } diff --git a/ReactAndroid/src/main/jni/react/Bridge.h b/ReactAndroid/src/main/jni/react/Bridge.h index 26b1aa8ef..adfff7ae0 100644 --- a/ReactAndroid/src/main/jni/react/Bridge.h +++ b/ReactAndroid/src/main/jni/react/Bridge.h @@ -5,10 +5,11 @@ #include #include #include -#include "Value.h" + #include "Executor.h" #include "MethodCall.h" #include "JSModulesUnbundle.h" +#include "Value.h" namespace folly { @@ -26,11 +27,6 @@ public: Bridge(JSExecutorFactory* jsExecutorFactory, Callback callback); virtual ~Bridge(); - /** - * Flush get the next queue of changes. - */ - void flush(); - /** * Executes a function with the module ID and method ID and any additional * arguments in JS. @@ -62,6 +58,11 @@ public: void stopProfiler(const std::string& title, const std::string& filename); void handleMemoryPressureModerate(); void handleMemoryPressureCritical(); + + /** + * TODO: get rid of isEndOfBatch + */ + void callNativeModules(const std::string& callJSON, bool isEndOfBatch); private: Callback m_callback; // This is used to avoid a race condition where a proxyCallback gets queued after ~Bridge(), diff --git a/ReactAndroid/src/main/jni/react/Executor.h b/ReactAndroid/src/main/jni/react/Executor.h index fb740d9f9..beace69a4 100644 --- a/ReactAndroid/src/main/jni/react/Executor.h +++ b/ReactAndroid/src/main/jni/react/Executor.h @@ -5,6 +5,7 @@ #include #include #include + #include "JSModulesUnbundle.h" namespace folly { @@ -16,13 +17,11 @@ struct dynamic; namespace facebook { namespace react { +class Bridge; class JSExecutor; - -typedef std::function FlushImmediateCallback; - class JSExecutorFactory { public: - virtual std::unique_ptr createJSExecutor(FlushImmediateCallback cb) = 0; + virtual std::unique_ptr createJSExecutor(Bridge *bridge) = 0; virtual ~JSExecutorFactory() {}; }; @@ -43,23 +42,20 @@ public: const std::string& startupCode, const std::string& sourceURL) = 0; - /** - * Executes BatchedBridge.flushedQueue in JS to get the next queue of changes. - */ - virtual std::string flush() = 0; - /** * Executes BatchedBridge.callFunctionReturnFlushedQueue with the module ID, - * method ID and optional additional arguments in JS, and returns the next - * queue. + * method ID and optional additional arguments in JS. The executor is responsible + * for using Bridge->callNativeModules to invoke any necessary native modules methods. */ - virtual std::string callFunction(const double moduleId, const double methodId, const folly::dynamic& arguments) = 0; + virtual void callFunction(const double moduleId, const double methodId, const folly::dynamic& arguments) = 0; /** * Executes BatchedBridge.invokeCallbackAndReturnFlushedQueue with the cbID, - * and optional additional arguments in JS and returns the next queue. + * and optional additional arguments in JS and returns the next queue. The executor + * is responsible for using Bridge->callNativeModules to invoke any necessary + * native modules methods. */ - virtual std::string invokeCallback(const double callbackId, const folly::dynamic& arguments) = 0; + virtual void invokeCallback(const double callbackId, const folly::dynamic& arguments) = 0; virtual void setGlobalVariable( const std::string& propName, diff --git a/ReactAndroid/src/main/jni/react/JSCExecutor.cpp b/ReactAndroid/src/main/jni/react/JSCExecutor.cpp index 03d28562c..7815aae25 100644 --- a/ReactAndroid/src/main/jni/react/JSCExecutor.cpp +++ b/ReactAndroid/src/main/jni/react/JSCExecutor.cpp @@ -10,9 +10,10 @@ #include #include +#include "Bridge.h" #include "JSCHelpers.h" -#include "Value.h" #include "Platform.h" +#include "Value.h" #ifdef WITH_JSC_EXTRA_TRACING #include "JSCTracing.h" @@ -45,13 +46,6 @@ namespace react { static std::unordered_map s_globalContextRefToJSCExecutor; -static JSValueRef nativeFlushQueueImmediate( - JSContextRef ctx, - JSObjectRef function, - JSObjectRef thisObject, - size_t argumentCount, - const JSValueRef arguments[], - JSValueRef *exception); static JSValueRef nativePerformanceNow( JSContextRef ctx, JSObjectRef function, @@ -86,14 +80,15 @@ static std::string executeJSCallWithJSC( return Value(ctx, result).toJSONString(); } -std::unique_ptr JSCExecutorFactory::createJSExecutor(FlushImmediateCallback cb) { - return std::unique_ptr(new JSCExecutor(cb, cacheDir_)); +std::unique_ptr JSCExecutorFactory::createJSExecutor(Bridge *bridge) { + return std::unique_ptr(new JSCExecutor(bridge, cacheDir_)); } -JSCExecutor::JSCExecutor(FlushImmediateCallback cb, const std::string& cacheDir) : - m_flushImmediateCallback(cb), m_deviceCacheDir(cacheDir) { +JSCExecutor::JSCExecutor(Bridge *bridge, const std::string& cacheDir) : + m_bridge(bridge), + m_deviceCacheDir(cacheDir), + m_messageQueueThread(MessageQueues::getCurrentMessageQueueThread()) { m_context = JSGlobalContextCreateInGroup(nullptr, nullptr); - m_messageQueueThread = MessageQueues::getCurrentMessageQueueThread(); s_globalContextRefToJSCExecutor[m_context] = this; installGlobalFunction(m_context, "nativeFlushQueueImmediate", nativeFlushQueueImmediate); installGlobalFunction(m_context, "nativePerformanceNow", nativePerformanceNow); @@ -152,6 +147,7 @@ void JSCExecutor::executeApplicationScript( // in which a cache file for that script will be stored. evaluateScript(m_context, jsScript, jsSourceURL, m_deviceCacheDir.c_str()); } + flush(); } void JSCExecutor::loadApplicationUnbundle( @@ -165,28 +161,31 @@ void JSCExecutor::loadApplicationUnbundle( executeApplicationScript(startupCode, sourceURL); } -std::string JSCExecutor::flush() { +void JSCExecutor::flush() { // TODO: Make this a first class function instead of evaling. #9317773 - return executeJSCallWithJSC(m_context, "flushedQueue", std::vector()); + std::string calls = executeJSCallWithJSC(m_context, "flushedQueue", std::vector()); + m_bridge->callNativeModules(calls, true); } -std::string JSCExecutor::callFunction(const double moduleId, const double methodId, const folly::dynamic& arguments) { +void JSCExecutor::callFunction(const double moduleId, const double methodId, const folly::dynamic& arguments) { // TODO: Make this a first class function instead of evaling. #9317773 std::vector call{ (double) moduleId, (double) methodId, std::move(arguments), }; - return executeJSCallWithJSC(m_context, "callFunctionReturnFlushedQueue", std::move(call)); + std::string calls = executeJSCallWithJSC(m_context, "callFunctionReturnFlushedQueue", std::move(call)); + m_bridge->callNativeModules(calls, true); } -std::string JSCExecutor::invokeCallback(const double callbackId, const folly::dynamic& arguments) { +void JSCExecutor::invokeCallback(const double callbackId, const folly::dynamic& arguments) { // TODO: Make this a first class function instead of evaling. #9317773 std::vector call{ (double) callbackId, std::move(arguments) }; - return executeJSCallWithJSC(m_context, "invokeCallbackAndReturnFlushedQueue", std::move(call)); + std::string calls = executeJSCallWithJSC(m_context, "invokeCallbackAndReturnFlushedQueue", std::move(call)); + m_bridge->callNativeModules(calls, true); } void JSCExecutor::setGlobalVariable(const std::string& propName, const std::string& jsonValue) { @@ -240,7 +239,7 @@ void JSCExecutor::handleMemoryPressureCritical() { } void JSCExecutor::flushQueueImmediate(std::string queueJSON) { - m_flushImmediateCallback(queueJSON, false); + m_bridge->callNativeModules(queueJSON, false); } void JSCExecutor::loadModule(uint32_t moduleId) { @@ -271,7 +270,7 @@ void JSCExecutor::onMessageReceived(int workerId, const std::string& json) { JSValueRef args[] = { JSCWebWorker::createMessageObject(m_context, json) }; onmessageValue.asObject().callAsFunction(1, args); - m_flushImmediateCallback(flush(), true); + flush(); } int JSCExecutor::addWebWorker(const std::string& script, JSValueRef workerRef) { @@ -348,7 +347,7 @@ static JSValueRef createErrorString(JSContextRef ctx, const char *msg) { return JSValueMakeString(ctx, String(msg)); } -static JSValueRef nativeFlushQueueImmediate( +JSValueRef JSCExecutor::nativeFlushQueueImmediate( JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, diff --git a/ReactAndroid/src/main/jni/react/JSCExecutor.h b/ReactAndroid/src/main/jni/react/JSCExecutor.h index d06071a2a..4db73b058 100644 --- a/ReactAndroid/src/main/jni/react/JSCExecutor.h +++ b/ReactAndroid/src/main/jni/react/JSCExecutor.h @@ -18,7 +18,7 @@ class MessageQueueThread; class JSCExecutorFactory : public JSExecutorFactory { public: JSCExecutorFactory(const std::string& cacheDir) : cacheDir_(cacheDir) {} - virtual std::unique_ptr createJSExecutor(FlushImmediateCallback cb) override; + virtual std::unique_ptr createJSExecutor(Bridge *bridge) override; private: std::string cacheDir_; }; @@ -28,7 +28,7 @@ public: /** * Should be invoked from the JS thread. */ - explicit JSCExecutor(FlushImmediateCallback flushImmediateCallback, const std::string& cacheDir); + explicit JSCExecutor(Bridge *bridge, const std::string& cacheDir); ~JSCExecutor() override; virtual void executeApplicationScript( @@ -38,12 +38,11 @@ public: std::unique_ptr unbundle, const std::string& startupCode, const std::string& sourceURL) override; - virtual std::string flush() override; - virtual std::string callFunction( + virtual void callFunction( const double moduleId, const double methodId, const folly::dynamic& arguments) override; - virtual std::string invokeCallback( + virtual void invokeCallback( const double callbackId, const folly::dynamic& arguments) override; virtual void setGlobalVariable( @@ -55,7 +54,6 @@ public: virtual void handleMemoryPressureModerate() override; virtual void handleMemoryPressureCritical() override; - void flushQueueImmediate(std::string queueJSON); void installNativeHook(const char *name, JSObjectCallAsFunctionCallback callback); virtual void onMessageReceived(int workerId, const std::string& message) override; virtual JSGlobalContextRef getContext() override; @@ -63,17 +61,19 @@ public: private: JSGlobalContextRef m_context; - FlushImmediateCallback m_flushImmediateCallback; std::unordered_map m_webWorkers; std::unordered_map m_webWorkerJSObjs; - std::shared_ptr m_messageQueueThread; + Bridge *m_bridge; std::string m_deviceCacheDir; + std::shared_ptr m_messageQueueThread; std::unique_ptr m_unbundle; int addWebWorker(const std::string& script, JSValueRef workerRef); void postMessageToWebWorker(int worker, JSValueRef message, JSValueRef *exn); + void flush(); void terminateWebWorker(int worker); void loadModule(uint32_t moduleId); + void flushQueueImmediate(std::string queueJSON); static JSValueRef nativeStartWorker( JSContextRef ctx, @@ -97,12 +97,19 @@ private: const JSValueRef arguments[], JSValueRef *exception); static JSValueRef nativeRequire( - JSContextRef ctx, - JSObjectRef function, - JSObjectRef thisObject, - size_t argumentCount, - const JSValueRef arguments[], - JSValueRef *exception); + JSContextRef ctx, + JSObjectRef function, + JSObjectRef thisObject, + size_t argumentCount, + const JSValueRef arguments[], + JSValueRef *exception); + static JSValueRef nativeFlushQueueImmediate( + JSContextRef ctx, + JSObjectRef function, + JSObjectRef thisObject, + size_t argumentCount, + const JSValueRef arguments[], + JSValueRef *exception); }; } } diff --git a/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp b/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp index 783b45dd9..a8dd4a0b5 100644 --- a/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp +++ b/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp @@ -656,9 +656,7 @@ static void executeApplicationScript( const std::string& script, const std::string& sourceUri) { try { - // Execute the application script and collect/dispatch any native calls that might have occured bridge->executeApplicationScript(script, sourceUri); - bridge->flush(); } catch (...) { translatePendingCppExceptionToJavaException(); } @@ -669,15 +667,12 @@ static void loadApplicationUnbundle( AAssetManager *assetManager, const std::string& startupCode, const std::string& startupFileName) { - try { - // Load the application unbundle and collect/dispatch any native calls that might have occured bridge->loadApplicationUnbundle( std::unique_ptr( new JniJSModulesUnbundle(assetManager, startupFileName)), startupCode, startupFileName); - bridge->flush(); } catch (...) { translatePendingCppExceptionToJavaException(); } @@ -818,8 +813,8 @@ std::string getDeviceCacheDir() { } struct CountableJSCExecutorFactory : CountableJSExecutorFactory { - virtual std::unique_ptr createJSExecutor(FlushImmediateCallback cb) override { - return JSCExecutorFactory(getDeviceCacheDir()).createJSExecutor(cb); + virtual std::unique_ptr createJSExecutor(Bridge *bridge) override { + return JSCExecutorFactory(getDeviceCacheDir()).createJSExecutor(bridge); } }; diff --git a/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.cpp b/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.cpp index e1025b06b..d43553a1d 100644 --- a/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.cpp +++ b/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.cpp @@ -8,6 +8,7 @@ #include #include +#include namespace facebook { namespace react { @@ -27,12 +28,12 @@ static std::string executeJSCallWithProxy( return result->toString(); } -std::unique_ptr ProxyExecutorOneTimeFactory::createJSExecutor(FlushImmediateCallback ignoredCallback) { +std::unique_ptr ProxyExecutorOneTimeFactory::createJSExecutor(Bridge *bridge) { FBASSERTMSGF( m_executor.get() != nullptr, "Proxy instance should not be null. Did you attempt to call createJSExecutor() on this factory " "instance more than once?"); - return std::unique_ptr(new ProxyExecutor(std::move(m_executor))); + return std::unique_ptr(new ProxyExecutor(std::move(m_executor), bridge)); } ProxyExecutor::~ProxyExecutor() { @@ -57,25 +58,23 @@ void ProxyExecutor::loadApplicationUnbundle(std::unique_ptr, "Loading application unbundles is not supported for proxy executors"); } -std::string ProxyExecutor::flush() { - return executeJSCallWithProxy(m_executor.get(), "flushedQueue", std::vector()); -} - -std::string ProxyExecutor::callFunction(const double moduleId, const double methodId, const folly::dynamic& arguments) { +void ProxyExecutor::callFunction(const double moduleId, const double methodId, const folly::dynamic& arguments) { std::vector call{ (double) moduleId, (double) methodId, std::move(arguments), }; - return executeJSCallWithProxy(m_executor.get(), "callFunctionReturnFlushedQueue", std::move(call)); + std::string result = executeJSCallWithProxy(m_executor.get(), "callFunctionReturnFlushedQueue", std::move(call)); + m_bridge->callNativeModules(result, true); } -std::string ProxyExecutor::invokeCallback(const double callbackId, const folly::dynamic& arguments) { +void ProxyExecutor::invokeCallback(const double callbackId, const folly::dynamic& arguments) { std::vector call{ (double) callbackId, std::move(arguments) }; - return executeJSCallWithProxy(m_executor.get(), "invokeCallbackAndReturnFlushedQueue", std::move(call)); + std::string result = executeJSCallWithProxy(m_executor.get(), "invokeCallbackAndReturnFlushedQueue", std::move(call)); + m_bridge->callNativeModules(result, true); } void ProxyExecutor::setGlobalVariable(const std::string& propName, const std::string& jsonValue) { diff --git a/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.h b/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.h index 09e7778b0..1d9b8aebd 100644 --- a/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.h +++ b/ReactAndroid/src/main/jni/react/jni/ProxyExecutor.h @@ -19,7 +19,7 @@ class ProxyExecutorOneTimeFactory : public CountableJSExecutorFactory { public: ProxyExecutorOneTimeFactory(jni::global_ref&& executorInstance) : m_executor(std::move(executorInstance)) {} - virtual std::unique_ptr createJSExecutor(FlushImmediateCallback ignoredCallback) override; + virtual std::unique_ptr createJSExecutor(Bridge *bridge) override; private: jni::global_ref m_executor; @@ -27,8 +27,9 @@ private: class ProxyExecutor : public JSExecutor { public: - ProxyExecutor(jni::global_ref&& executorInstance) : - m_executor(std::move(executorInstance)) {} + ProxyExecutor(jni::global_ref&& executorInstance, Bridge *bridge) : + m_executor(std::move(executorInstance)), + m_bridge(bridge) {} virtual ~ProxyExecutor() override; virtual void executeApplicationScript( const std::string& script, @@ -37,12 +38,11 @@ public: std::unique_ptr bundle, const std::string& startupCode, const std::string& sourceURL) override; - virtual std::string flush() override; - virtual std::string callFunction( + virtual void callFunction( const double moduleId, const double methodId, const folly::dynamic& arguments) override; - virtual std::string invokeCallback( + virtual void invokeCallback( const double callbackId, const folly::dynamic& arguments) override; virtual void setGlobalVariable( @@ -51,6 +51,7 @@ public: private: jni::global_ref m_executor; + Bridge *m_bridge; }; } }