From 62591ac840840e48b4bb1e700ec58f1f497cf828 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 29 Jul 2019 10:38:10 -0700 Subject: [PATCH] Set scheduler delegate during construction Summary: I think it's possible that there's a race condition between creating the scheduler and setting the delegate leading to bugs like T47272192. Reviewed By: mdvacca Differential Revision: D16537737 fbshipit-source-id: 9c579537658be5a9aeed37c0e4935c997cabb6aa --- React/Fabric/RCTScheduler.mm | 3 +-- .../src/main/java/com/facebook/react/fabric/jni/Binding.cpp | 3 +-- ReactCommon/fabric/uimanager/Scheduler.cpp | 6 +++++- ReactCommon/fabric/uimanager/Scheduler.h | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/React/Fabric/RCTScheduler.mm b/React/Fabric/RCTScheduler.mm index 07d0872a0de..7e00d2af022 100644 --- a/React/Fabric/RCTScheduler.mm +++ b/React/Fabric/RCTScheduler.mm @@ -56,8 +56,7 @@ class SchedulerDelegateProxy : public SchedulerDelegate { { if (self = [super init]) { _delegateProxy = std::make_shared((__bridge void *)self); - _scheduler = std::make_shared(toolbox); - _scheduler->setDelegate(_delegateProxy.get()); + _scheduler = std::make_shared(toolbox, _delegateProxy.get()); } return self; diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp index b4c7d31ef44..811a6d90950 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp @@ -229,8 +229,7 @@ void Binding::installFabricUIManager( toolbox.runtimeExecutor = runtimeExecutor; toolbox.synchronousEventBeatFactory = synchronousBeatFactory; toolbox.asynchronousEventBeatFactory = asynchronousBeatFactory; - scheduler_ = std::make_shared(toolbox); - scheduler_->setDelegate(this); + scheduler_ = std::make_shared(toolbox, this); } void Binding::uninstallFabricUIManager() { diff --git a/ReactCommon/fabric/uimanager/Scheduler.cpp b/ReactCommon/fabric/uimanager/Scheduler.cpp index af6e080f0cf..681d147a8d7 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.cpp +++ b/ReactCommon/fabric/uimanager/Scheduler.cpp @@ -18,7 +18,9 @@ namespace facebook { namespace react { -Scheduler::Scheduler(SchedulerToolbox schedulerToolbox) { +Scheduler::Scheduler( + SchedulerToolbox schedulerToolbox, + SchedulerDelegate *delegate) { runtimeExecutor_ = schedulerToolbox.runtimeExecutor; reactNativeConfig_ = @@ -56,6 +58,8 @@ Scheduler::Scheduler(SchedulerToolbox schedulerToolbox) { rootComponentDescriptor_ = std::make_unique(eventDispatcher); + delegate_ = delegate; + uiManagerRef.setDelegate(this); uiManagerRef.setShadowTreeRegistry(&shadowTreeRegistry_); uiManagerRef.setComponentDescriptorRegistry(componentDescriptorRegistry_); diff --git a/ReactCommon/fabric/uimanager/Scheduler.h b/ReactCommon/fabric/uimanager/Scheduler.h index 199a7a61ea7..cdeaf237194 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.h +++ b/ReactCommon/fabric/uimanager/Scheduler.h @@ -32,7 +32,7 @@ namespace react { */ class Scheduler final : public UIManagerDelegate, public ShadowTreeDelegate { public: - Scheduler(SchedulerToolbox schedulerToolbox); + Scheduler(SchedulerToolbox schedulerToolbox, SchedulerDelegate *delegate); ~Scheduler(); #pragma mark - Surface Management