Skip to content

Commit 3a019fe

Browse files
j-piaseckifacebook-github-bot
authored andcommitted
Merge promoted React revision immediately when on the main thread (#56448)
Summary: Changelog: [GENERAL][FIXED] When a React revision is being promoted on the main thread, merge it immediately instead of scheduling a rendering update In the current implementation, when React is committing on the main thread, the merge is scheduled at the end of the event loop anyway. This change breaks synchronous events if they cause another render. Instead of merging the new tree immediately, it was scheduled, resulting in a visible flash of an incorrect state. Differential Revision: D100942119
1 parent 33ad61b commit 3a019fe

16 files changed

Lines changed: 95 additions & 15 deletions

File tree

packages/react-native/React/Fabric/RCTScheduler.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ NS_ASSUME_NONNULL_BEGIN
3333

3434
- (void)schedulerShouldMergeReactRevision:(facebook::react::SurfaceId)surfaceId;
3535

36+
- (BOOL)schedulerShouldPromoteReactRevision:(facebook::react::SurfaceId)surfaceId;
37+
3638
- (void)schedulerDidDispatchCommand:(const facebook::react::ShadowView &)shadowView
3739
commandName:(const std::string &)commandName
3840
args:(const folly::dynamic &)args;

packages/react-native/React/Fabric/RCTScheduler.mm

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ void schedulerShouldMergeReactRevision(SurfaceId surfaceId) override
4343
[scheduler.delegate schedulerShouldMergeReactRevision:surfaceId];
4444
}
4545

46+
bool schedulerShouldPromoteReactRevision(SurfaceId surfaceId) override
47+
{
48+
RCTScheduler *scheduler = (__bridge RCTScheduler *)scheduler_;
49+
return [scheduler.delegate schedulerShouldPromoteReactRevision:surfaceId];
50+
}
51+
4652
void schedulerDidRequestPreliminaryViewAllocation(const ShadowNode &shadowNode) override
4753
{
4854
// Does nothing.

packages/react-native/React/Fabric/RCTSurfacePresenter.mm

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,20 @@ - (void)schedulerShouldMergeReactRevision:(SurfaceId)surfaceId
316316
});
317317
}
318318

319+
- (BOOL)schedulerShouldPromoteReactRevision:(SurfaceId)surfaceId
320+
{
321+
if (!RCTIsMainQueue()) {
322+
return NO;
323+
}
324+
325+
auto scheduler = [self scheduler];
326+
scheduler.uiManager->getShadowTreeRegistry().visit(surfaceId, [](const ShadowTree &shadowTree) {
327+
shadowTree.promoteReactRevision();
328+
shadowTree.mergeReactRevision();
329+
});
330+
return YES;
331+
}
332+
319333
- (void)schedulerDidDispatchCommand:(const ShadowView &)shadowView
320334
commandName:(const std::string &)commandName
321335
args:(const folly::dynamic &)args

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ class FabricMountingManager final {
7070

7171
void scheduleReactRevisionMerge(SurfaceId surfaceId);
7272

73-
private:
7473
bool isOnMainThread();
7574

75+
private:
7676
jni::global_ref<JFabricUIManager::javaobject> javaUIManager_;
7777

7878
std::recursive_mutex commitMutex_;

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,23 @@ void FabricUIManagerBinding::schedulerShouldMergeReactRevision(
711711
}
712712
}
713713

714+
bool FabricUIManagerBinding::schedulerShouldPromoteReactRevision(
715+
SurfaceId surfaceId) {
716+
std::shared_lock lock(installMutex_);
717+
auto mountingManager =
718+
getMountingManager("schedulerShouldPromoteReactRevision");
719+
if (!mountingManager || !mountingManager->isOnMainThread()) {
720+
return false;
721+
}
722+
723+
scheduler_->getUIManager()->getShadowTreeRegistry().visit(
724+
surfaceId, [](const ShadowTree& shadowTree) {
725+
shadowTree.promoteReactRevision();
726+
shadowTree.mergeReactRevision();
727+
});
728+
return true;
729+
}
730+
714731
void FabricUIManagerBinding::mergeReactRevision(SurfaceId surfaceId) {
715732
std::shared_lock lock(installMutex_);
716733
scheduler_->getUIManager()->getShadowTreeRegistry().visit(

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ class FabricUIManagerBinding : public jni::HybridClass<FabricUIManagerBinding>,
9999

100100
void schedulerShouldMergeReactRevision(SurfaceId surfaceId) override;
101101

102+
bool schedulerShouldPromoteReactRevision(SurfaceId surfaceId) override;
103+
102104
void mergeReactRevision(SurfaceId surfaceId);
103105

104106
void schedulerDidRequestPreliminaryViewAllocation(const ShadowNode &shadowNode) override;

packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,15 @@ void Scheduler::uiManagerShouldRemoveEventListener(
377377

378378
void Scheduler::uiManagerDidFinishReactCommit(const ShadowTree& shadowTree) {
379379
auto surfaceId = shadowTree.getSurfaceId();
380+
381+
// If the commit is happening on the main thread, it should be merged
382+
// immediately instead of being scheduled. This allows for synchronous
383+
// events to be handled correctly.
384+
if (delegate_ != nullptr &&
385+
delegate_->schedulerShouldPromoteReactRevision(surfaceId)) {
386+
return;
387+
}
388+
380389
runtimeScheduler_->scheduleRenderingUpdate(
381390
surfaceId, [surfaceId, uiManager = uiManager_]() {
382391
uiManager->getShadowTreeRegistry().visit(

packages/react-native/ReactCommon/react/renderer/scheduler/SchedulerDelegate.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ class SchedulerDelegate {
4444
*/
4545
virtual void schedulerShouldMergeReactRevision(SurfaceId surfaceId) = 0;
4646

47+
/*
48+
* Called when a React commit finishes and the JS revision needs to be
49+
* promoted. If the platform is currently on the main thread, it should
50+
* promote and merge the revision immediately, returning true.
51+
* If not on the main thread, it should return false and the caller will
52+
* schedule the promotion at the end of the event loop.
53+
*/
54+
virtual bool schedulerShouldPromoteReactRevision(SurfaceId surfaceId) = 0;
55+
4756
/*
4857
* Called right after a new ShadowNode was created.
4958
*/

packages/react-native/ReactCxxPlatform/react/renderer/scheduler/SchedulerDelegateImpl.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ void SchedulerDelegateImpl::schedulerShouldMergeReactRevision(
4343
[](const ShadowTree& shadowTree) { shadowTree.mergeReactRevision(); });
4444
}
4545

46+
bool SchedulerDelegateImpl::schedulerShouldPromoteReactRevision(
47+
SurfaceId /*surfaceId*/) {
48+
return false;
49+
}
50+
4651
void SchedulerDelegateImpl::schedulerDidRequestPreliminaryViewAllocation(
4752
const ShadowNode& shadowNode) {}
4853

packages/react-native/ReactCxxPlatform/react/renderer/scheduler/SchedulerDelegateImpl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ class SchedulerDelegateImpl : public SchedulerDelegate {
3535

3636
void schedulerShouldMergeReactRevision(SurfaceId surfaceId) override;
3737

38+
bool schedulerShouldPromoteReactRevision(SurfaceId surfaceId) override;
39+
3840
void schedulerDidRequestPreliminaryViewAllocation(const ShadowNode &shadowNode) override;
3941

4042
void schedulerDidDispatchCommand(

0 commit comments

Comments
 (0)