From 447be629091579b8656e1b10e365f3429a61bbf3 Mon Sep 17 00:00:00 2001 From: Paige Sun Date: Wed, 14 Sep 2022 14:51:27 -0700 Subject: [PATCH] Fix: Install Fabric UIManager before main bundle execution Summary: Changelog: [Internal] Fix install Fabric UIManager before main bundle execution in Bridgeless In iOS, fixed Bridgeless so that [UIManagerBinding::createAndInstallIfNeeded](https://github.com/facebook/react-native/blob/ce50c43986bae05ad62552be46f4d5bb4a46f097/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp#L24-L41) happens BEFORE the JS main bundle is evaluated. Logic is unchanged in Android. # Before ``` > UI [FBReactModule.mm:325] Initializing FBReactModule: start. > UI [FBReactModule.mm:524] Initializing FBReactModule: end. > UI [FBReactModule.mm:1839] Initializing RCTHost: start. > UI [FBReactModule.mm:1891] Initializing RCTHost: end. > UI[-[FBNavigationControllerObserver navigationController:willShowViewController:animated:]] will show (animated: 1) > VJCPP ****** ReactInstance loadScript -> evaluateJavaScript start <--- loads Main Bundle > UI[-[FBNavigationControllerObserver navigationController:didShowViewController:animated:]] did show (animated: 1) > VJCPP ****** ReactInstance loadScript -> evaluateJavaScript end > VJCPP ****** UIManagerBinding createAndInstallIfNeeded <--- should happen BEFORE evaluateJavaScript ``` Reviewed By: RSNara Differential Revision: D39493654 fbshipit-source-id: 4491d6de110966b2eb4f554ff4db8548899020e3 --- React/Fabric/RCTSurfacePresenter.h | 3 ++- React/Fabric/RCTSurfacePresenter.mm | 4 ++++ React/Fabric/RCTSurfacePresenterBridgeAdapter.mm | 4 +++- ReactAndroid/src/main/jni/react/fabric/Binding.cpp | 5 +++++ ReactCommon/react/renderer/scheduler/Scheduler.cpp | 2 +- ReactCommon/react/renderer/scheduler/SchedulerToolbox.h | 7 +++++++ 6 files changed, 22 insertions(+), 3 deletions(-) diff --git a/React/Fabric/RCTSurfacePresenter.h b/React/Fabric/RCTSurfacePresenter.h index 8e645be4c413c5..35d1886b64c3e8 100644 --- a/React/Fabric/RCTSurfacePresenter.h +++ b/React/Fabric/RCTSurfacePresenter.h @@ -28,7 +28,8 @@ NS_ASSUME_NONNULL_BEGIN @interface RCTSurfacePresenter : NSObject - (instancetype)initWithContextContainer:(facebook::react::ContextContainer::Shared)contextContainer - runtimeExecutor:(facebook::react::RuntimeExecutor)runtimeExecutor; + runtimeExecutor:(facebook::react::RuntimeExecutor)runtimeExecutor + bindingsInstallExecutor:(facebook::react::RuntimeExecutor)bindingsInstallExecutor; @property (nonatomic) facebook::react::ContextContainer::Shared contextContainer; @property (nonatomic) facebook::react::RuntimeExecutor runtimeExecutor; diff --git a/React/Fabric/RCTSurfacePresenter.mm b/React/Fabric/RCTSurfacePresenter.mm index 53d4d0dbc75b6f..131596019f27f2 100644 --- a/React/Fabric/RCTSurfacePresenter.mm +++ b/React/Fabric/RCTSurfacePresenter.mm @@ -79,6 +79,7 @@ @implementation RCTSurfacePresenter { RCTScheduler *_Nullable _scheduler; // Thread-safe. Pointer is protected by `_schedulerAccessMutex`. ContextContainer::Shared _contextContainer; // Protected by `_schedulerLifeCycleMutex`. RuntimeExecutor _runtimeExecutor; // Protected by `_schedulerLifeCycleMutex`. + RuntimeExecutor _bindingsInstallExecutor; // Only used for installing bindings. butter::shared_mutex _observerListMutex; std::vector<__weak id> _observers; // Protected by `_observerListMutex`. @@ -86,10 +87,12 @@ @implementation RCTSurfacePresenter { - (instancetype)initWithContextContainer:(ContextContainer::Shared)contextContainer runtimeExecutor:(RuntimeExecutor)runtimeExecutor + bindingsInstallExecutor:(RuntimeExecutor)bindingsInstallExecutor { if (self = [super init]) { assert(contextContainer && "RuntimeExecutor must be not null."); _runtimeExecutor = runtimeExecutor; + _bindingsInstallExecutor = bindingsInstallExecutor; _contextContainer = contextContainer; _surfaceRegistry = [RCTSurfaceRegistry new]; @@ -295,6 +298,7 @@ - (RCTScheduler *)_createScheduler } toolbox.runtimeExecutor = runtimeExecutor; + toolbox.bindingsInstallExecutor = _bindingsInstallExecutor; toolbox.mainRunLoopObserverFactory = [](RunLoopObserver::Activity activities, RunLoopObserver::WeakOwner const &owner) { diff --git a/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm b/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm index 9e0866245ffebd..36276f5cbf45b9 100644 --- a/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm +++ b/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm @@ -89,8 +89,10 @@ - (instancetype)initWithBridge:(RCTBridge *)bridge contextContainer:(ContextCont { if (self = [super init]) { contextContainer->update(*RCTContextContainerFromBridge(bridge)); + auto runtimeExecuter = RCTRuntimeExecutorFromBridge(bridge); _surfacePresenter = [[RCTSurfacePresenter alloc] initWithContextContainer:contextContainer - runtimeExecutor:RCTRuntimeExecutorFromBridge(bridge)]; + runtimeExecutor:runtimeExecuter + bindingsInstallExecutor:runtimeExecuter]; _bridge = bridge; _batchedBridge = [_bridge batchedBridge] ?: _bridge; diff --git a/ReactAndroid/src/main/jni/react/fabric/Binding.cpp b/ReactAndroid/src/main/jni/react/fabric/Binding.cpp index 83f578955af0f6..827ce224549683 100644 --- a/ReactAndroid/src/main/jni/react/fabric/Binding.cpp +++ b/ReactAndroid/src/main/jni/react/fabric/Binding.cpp @@ -462,7 +462,12 @@ void Binding::installFabricUIManager( auto toolbox = SchedulerToolbox{}; toolbox.contextContainer = contextContainer; toolbox.componentRegistryFactory = componentsRegistry->buildRegistryFunction; + + // TODO: (T130208323) runtimeExecutor should execute lambdas after + // main bundle eval, and bindingsInstallExecutor should execute before. + toolbox.bindingsInstallExecutor = runtimeExecutor; toolbox.runtimeExecutor = runtimeExecutor; + toolbox.synchronousEventBeatFactory = synchronousBeatFactory; toolbox.asynchronousEventBeatFactory = asynchronousBeatFactory; diff --git a/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/ReactCommon/react/renderer/scheduler/Scheduler.cpp index 89fd0a5fb73251..d6ac113cb2238c 100644 --- a/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -95,7 +95,7 @@ Scheduler::Scheduler( uiManager->setDelegate(this); uiManager->setComponentDescriptorRegistry(componentDescriptorRegistry_); - runtimeExecutor_([uiManager](jsi::Runtime &runtime) { + schedulerToolbox.bindingsInstallExecutor([uiManager](jsi::Runtime &runtime) { UIManagerBinding::createAndInstallIfNeeded(runtime, uiManager); }); diff --git a/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h b/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h index 6903b86e4531f0..280fed01ff1acc 100644 --- a/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h +++ b/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h @@ -40,6 +40,13 @@ struct SchedulerToolbox final { /* * Represents running JavaScript VM and associated execution queue. + * Can execute lambdas before main bundle has loaded. + */ + RuntimeExecutor bindingsInstallExecutor; + + /* + * Represents running JavaScript VM and associated execution queue. + * Only executes lambdas after main bundle has loaded. */ RuntimeExecutor runtimeExecutor;