Skip to content

Commit

Permalink
Fix: Install Fabric UIManager before main bundle execution
Browse files Browse the repository at this point in the history
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:]] <FBNewNavigationController: 0x7fd0e7859400> will show <FBReactRootViewController: 0x7fd0e7161a00; react_GemstoneHomeRoute> (animated: 1)
>   VJCPP ****** ReactInstance loadScript ->  evaluateJavaScript start      <--- loads Main Bundle
> UI[-[FBNavigationControllerObserver navigationController:didShowViewController:animated:]] <FBNewNavigationController: 0x7fd0e7859400> did show <FBReactRootViewController: 0x7fd0e7161a00; react_GemstoneHomeRoute> (animated: 1)
>   VJCPP ****** ReactInstance loadScript ->  evaluateJavaScript end
>   VJCPP ****** UIManagerBinding createAndInstallIfNeeded      <--- should happen BEFORE evaluateJavaScript

```

Reviewed By: RSNara

Differential Revision: D39493654

fbshipit-source-id: 4491d6de110966b2eb4f554ff4db8548899020e3
  • Loading branch information
p-sun authored and facebook-github-bot committed Sep 14, 2022
1 parent e2028a8 commit 447be62
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 3 deletions.
3 changes: 2 additions & 1 deletion React/Fabric/RCTSurfacePresenter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions React/Fabric/RCTSurfacePresenter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,20 @@ @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<RCTSurfacePresenterObserver>> _observers; // Protected by `_observerListMutex`.
}

- (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];
Expand Down Expand Up @@ -295,6 +298,7 @@ - (RCTScheduler *)_createScheduler
}

toolbox.runtimeExecutor = runtimeExecutor;
toolbox.bindingsInstallExecutor = _bindingsInstallExecutor;

toolbox.mainRunLoopObserverFactory = [](RunLoopObserver::Activity activities,
RunLoopObserver::WeakOwner const &owner) {
Expand Down
4 changes: 3 additions & 1 deletion React/Fabric/RCTSurfacePresenterBridgeAdapter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions ReactAndroid/src/main/jni/react/fabric/Binding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion ReactCommon/react/renderer/scheduler/Scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down
7 changes: 7 additions & 0 deletions ReactCommon/react/renderer/scheduler/SchedulerToolbox.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 447be62

Please sign in to comment.