Skip to content

Commit

Permalink
Back out "Back out "[Venice][iOS] Fix: Install Fabric UIManager befor…
Browse files Browse the repository at this point in the history
…e main bundle execution""

Summary:
Changelog: [Internal][Bridgeless][iOS]

# Before diff
Be in Venice > run Metro > run FBiOS > navigate to any RN surface.

 `UIManagerBinding createAndInstallIfNeeded` happens After `ReactInstance loadScript -> evaluateJavaScript`: install Fabric UIManager before main bundle execution

Reviewed By: RSNara

Differential Revision: D39760231

fbshipit-source-id: f17bf02e9b1fb0f9b0ff24c86aa6dc9349c42192
  • Loading branch information
p-sun authored and facebook-github-bot committed Sep 24, 2022
1 parent 47548c1 commit 9a253d1
Show file tree
Hide file tree
Showing 6 changed files with 25 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
bridgelessBindingsExecutor:(std::optional<facebook::react::RuntimeExecutor>)bridgelessBindingsExecutor;

@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`.
std::optional<RuntimeExecutor> _bridgelessBindingsExecutor; // 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
bridgelessBindingsExecutor:(std::optional<RuntimeExecutor>)bridgelessBindingsExecutor
{
if (self = [super init]) {
assert(contextContainer && "RuntimeExecutor must be not null.");
_runtimeExecutor = runtimeExecutor;
_bridgelessBindingsExecutor = bridgelessBindingsExecutor;
_contextContainer = contextContainer;

_surfaceRegistry = [RCTSurfaceRegistry new];
Expand Down Expand Up @@ -295,6 +298,7 @@ - (RCTScheduler *)_createScheduler
}

toolbox.runtimeExecutor = runtimeExecutor;
toolbox.bridgelessBindingsExecutor = _bridgelessBindingsExecutor;

toolbox.mainRunLoopObserverFactory = [](RunLoopObserver::Activity activities,
RunLoopObserver::WeakOwner const &owner) {
Expand Down
3 changes: 2 additions & 1 deletion React/Fabric/RCTSurfacePresenterBridgeAdapter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ - (instancetype)initWithBridge:(RCTBridge *)bridge contextContainer:(ContextCont
if (self = [super init]) {
contextContainer->update(*RCTContextContainerFromBridge(bridge));
_surfacePresenter = [[RCTSurfacePresenter alloc] initWithContextContainer:contextContainer
runtimeExecutor:RCTRuntimeExecutorFromBridge(bridge)];
runtimeExecutor:RCTRuntimeExecutorFromBridge(bridge)
bridgelessBindingsExecutor:std::nullopt];

_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 @@ -468,7 +468,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.bridgelessBindingsExecutor = std::nullopt;
toolbox.runtimeExecutor = runtimeExecutor;

toolbox.synchronousEventBeatFactory = synchronousBeatFactory;
toolbox.asynchronousEventBeatFactory = asynchronousBeatFactory;

Expand Down
6 changes: 5 additions & 1 deletion ReactCommon/react/renderer/scheduler/Scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,11 @@ Scheduler::Scheduler(
uiManager->setDelegate(this);
uiManager->setComponentDescriptorRegistry(componentDescriptorRegistry_);

runtimeExecutor_([uiManager](jsi::Runtime &runtime) {
auto bindingsExecutor =
schedulerToolbox.bridgelessBindingsExecutor.has_value()
? schedulerToolbox.bridgelessBindingsExecutor.value()
: runtimeExecutor_;
bindingsExecutor([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.
*/
std::optional<RuntimeExecutor> bridgelessBindingsExecutor;

/*
* Represents running JavaScript VM and associated execution queue.
* Executes lambdas after main bundle has loaded.
*/
RuntimeExecutor runtimeExecutor;

Expand Down

0 comments on commit 9a253d1

Please sign in to comment.