From 0b266458eacbf0910b7081fcb2a3d657e09a2f02 Mon Sep 17 00:00:00 2001 From: Emily Janzer Date: Tue, 12 Mar 2019 10:54:32 -0700 Subject: [PATCH] Change EventBeatManager and AsyncEventBeat to use RuntimeExecutor Summary: Right now we have a raw pointer to the js runtime in Java and we pass that to EventBeatManager's constructor; this diff adds a setter for the runtime ptr and removes it from the member initializer list, so that you can set it later from cpp instead. Q: Should I just rewrite this to use RuntimeExecutor instead? Reviewed By: shergin Differential Revision: D14318893 fbshipit-source-id: 1221dd5959927967bad870f15c901c15e5455874 --- .../react/fabric/FabricJSIModuleProvider.java | 2 +- .../react/fabric/jsi/EventBeatManager.java | 7 +++---- .../react/fabric/jsi/jni/AsyncEventBeat.h | 18 +++++++++++------- .../facebook/react/fabric/jsi/jni/Binding.cpp | 14 ++++++++------ .../react/fabric/jsi/jni/EventBeatManager.cpp | 18 +++++++++++------- .../react/fabric/jsi/jni/EventBeatManager.h | 12 ++++++------ 6 files changed, 40 insertions(+), 31 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java index c3cde5478ef..f095c318730 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java @@ -52,7 +52,7 @@ public class FabricJSIModuleProvider implements JSIModuleProvider { @Override public UIManager get() { final EventBeatManager eventBeatManager = - new EventBeatManager(mJSContext, mReactApplicationContext); + new EventBeatManager(mReactApplicationContext); final FabricUIManager uiManager = createUIManager(eventBeatManager); Systrace.beginSection( Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "FabricJSIModuleProvider.registerBinding"); diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/EventBeatManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/EventBeatManager.java index c6b1c333622..285a24f8946 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/EventBeatManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/EventBeatManager.java @@ -27,13 +27,12 @@ public class EventBeatManager implements BatchEventDispatchedListener { @DoNotStrip private final HybridData mHybridData; private final ReactApplicationContext mReactApplicationContext; - private static native HybridData initHybrid(long jsContext); + private static native HybridData initHybrid(); private native void beat(); - public EventBeatManager( - JavaScriptContextHolder jsContext, ReactApplicationContext reactApplicationContext) { - mHybridData = initHybrid(jsContext.get()); + public EventBeatManager(ReactApplicationContext reactApplicationContext) { + mHybridData = initHybrid(); mReactApplicationContext = reactApplicationContext; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/AsyncEventBeat.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/AsyncEventBeat.h index a87d53608b8..cdd78cfb2e1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/AsyncEventBeat.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/AsyncEventBeat.h @@ -6,6 +6,8 @@ #include #include +#include + #include "EventBeatManager.h" namespace facebook { @@ -16,7 +18,7 @@ namespace { class AsyncEventBeat : public EventBeat { private: EventBeatManager* eventBeatManager_; - jsi::Runtime* runtime_; + RuntimeExecutor runtimeExecutor_; jni::global_ref javaUIManager_; public: @@ -24,11 +26,11 @@ class AsyncEventBeat : public EventBeat { AsyncEventBeat( EventBeatManager* eventBeatManager, - jsi::Runtime* runtime, - jni::global_ref javaUIManager) { - eventBeatManager_ = eventBeatManager; - runtime_ = runtime; - javaUIManager_ = javaUIManager; + RuntimeExecutor runtimeExecutor, + jni::global_ref javaUIManager) : + eventBeatManager_(eventBeatManager), + runtimeExecutor_(std::move(runtimeExecutor)), + javaUIManager_(javaUIManager) { eventBeatManager->registerEventBeat(this); } @@ -37,7 +39,9 @@ class AsyncEventBeat : public EventBeat { } void induce() const override { - beat(*runtime_); + runtimeExecutor_([=](jsi::Runtime &runtime) { + this->beat(runtime); + }); } void request() const override { diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp index 41a64771cd7..81a4ceb0572 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp @@ -94,8 +94,6 @@ void Binding::installFabricUIManager( jni::alias_ref jsMessageQueueThread, ComponentFactoryDelegate* componentsRegistry, jni::alias_ref reactNativeConfig) { - Runtime* runtime = (Runtime*)jsContextNativePointer; - javaUIManager_ = make_global(javaUIManager); SharedContextContainer contextContainer = @@ -103,6 +101,8 @@ void Binding::installFabricUIManager( auto sharedJSMessageQueueThread = std::make_shared(jsMessageQueueThread); + + Runtime* runtime = (Runtime*)jsContextNativePointer; RuntimeExecutor runtimeExecutor = [runtime, sharedJSMessageQueueThread]( std::function&& callback) { @@ -112,18 +112,20 @@ void Binding::installFabricUIManager( }); }; + eventBeatManager->setRuntimeExecutor(runtimeExecutor); + // TODO: T31905686 Create synchronous Event Beat jni::global_ref localJavaUIManager = javaUIManager_; EventBeatFactory synchronousBeatFactory = - [eventBeatManager, runtime, localJavaUIManager]() mutable { + [eventBeatManager, runtimeExecutor, localJavaUIManager]() { return std::make_unique( - eventBeatManager, runtime, localJavaUIManager); + eventBeatManager, runtimeExecutor, localJavaUIManager); }; EventBeatFactory asynchronousBeatFactory = - [eventBeatManager, runtime, localJavaUIManager]() mutable { + [eventBeatManager, runtimeExecutor, localJavaUIManager]() { return std::make_unique( - eventBeatManager, runtime, localJavaUIManager); + eventBeatManager, runtimeExecutor, localJavaUIManager); }; std::shared_ptr config = std::make_shared(reactNativeConfig); diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/EventBeatManager.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/EventBeatManager.cpp index 19e79e9d843..bbacceb3329 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/EventBeatManager.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/EventBeatManager.cpp @@ -10,14 +10,16 @@ namespace facebook { namespace react { EventBeatManager::EventBeatManager( - Runtime* runtime, - jni::alias_ref jhybridobject) - : runtime_(runtime), jhybridobject_(jhybridobject) {} + jni::alias_ref jhybridobject) + : jhybridobject_(jhybridobject) {} jni::local_ref EventBeatManager::initHybrid( - jni::alias_ref jhybridobject, - jlong jsContext) { - return makeCxxInstance((Runtime*)jsContext, jhybridobject); + jni::alias_ref jhybridobject) { + return makeCxxInstance(jhybridobject); +} + +void EventBeatManager::setRuntimeExecutor(RuntimeExecutor runtimeExecutor) { + runtimeExecutor_ = runtimeExecutor; } void EventBeatManager::registerEventBeat(EventBeat* eventBeat) const { @@ -36,7 +38,9 @@ void EventBeatManager::beat() { std::lock_guard lock(mutex_); for (const auto eventBeat : registeredEventBeats_) { - eventBeat->beat(*runtime_); + runtimeExecutor_([=](jsi::Runtime &runtime) mutable { + eventBeat->beat(runtime); + }); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/EventBeatManager.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/EventBeatManager.h index ad6839864c6..4425bc386f6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/EventBeatManager.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/EventBeatManager.h @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -24,18 +25,18 @@ class EventBeatManager : public jni::HybridClass { static void registerNatives(); + void setRuntimeExecutor(RuntimeExecutor runtimeExecutor); + void registerEventBeat(EventBeat* eventBeat) const; void unregisterEventBeat(EventBeat* eventBeat) const; void beat(); - EventBeatManager( - Runtime* runtime, - jni::alias_ref jhybridobject); + EventBeatManager(jni::alias_ref jhybridobject); private: - Runtime* runtime_; + RuntimeExecutor runtimeExecutor_; jni::alias_ref jhybridobject_; @@ -45,8 +46,7 @@ class EventBeatManager : public jni::HybridClass { mutable std::mutex mutex_; static jni::local_ref initHybrid( - jni::alias_ref jhybridobject, - jlong jsContext); + jni::alias_ref jhybridobject); }; } // namespace react