Skip to content

Commit b63efbf

Browse files
j-piaseckimeta-codesync[bot]
authored andcommitted
Schedule React revision merge to happen before dispatching events (#56449)
Summary: Pull Request resolved: #56449 Changelog: [ANDROID][FIXED] Schedule React revision merge to happen on `DISPATCH_UI` choreographer queue, before dispatching events Before the introduction of branching, it was possible to handle synchronous events on the same frame they were dispatched. Introduction of branching broke that because merge was scheduled using `runOnUIThread`, which turned out to be after the `DISPATCH_UI` choreographer phase, thus after the dispatch of events. Reviewed By: rubennorte Differential Revision: D100966623 fbshipit-source-id: 16925a5df129a23597b3a9cea93c5eb85b380ec3
1 parent 4b954e1 commit b63efbf

1 file changed

Lines changed: 21 additions & 7 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@
101101
import java.util.Map;
102102
import java.util.Queue;
103103
import java.util.Set;
104+
import java.util.concurrent.ConcurrentLinkedQueue;
104105
import java.util.concurrent.CopyOnWriteArrayList;
105106

106107
/**
@@ -192,6 +193,13 @@ public class FabricUIManager
192193
@ThreadConfined(UI)
193194
private final Set<SynchronousEvent> mSynchronousEvents = new HashSet<>();
194195

196+
/**
197+
* Queue of surface IDs that need their React revision merged. Drained during doFrame so that
198+
* synchronous events dispatched by the merge are processed in the same frame.
199+
*/
200+
private final ConcurrentLinkedQueue<Integer> mPendingReactRevisionMerges =
201+
new ConcurrentLinkedQueue<>();
202+
195203
/**
196204
* This is used to keep track of whether or not the FabricUIManager has been destroyed. Once the
197205
* Catalyst instance is being destroyed, we should cease all operation here.
@@ -956,13 +964,7 @@ private void scheduleReactRevisionMerge(int surfaceId) {
956964
mBinding.mergeReactRevision(surfaceId);
957965
}
958966
} else {
959-
UiThreadUtil.runOnUiThread(
960-
() -> {
961-
FabricUIManagerBinding binding = mBinding;
962-
if (binding != null) {
963-
binding.mergeReactRevision(surfaceId);
964-
}
965-
});
967+
mPendingReactRevisionMerges.add(surfaceId);
966968
}
967969
}
968970

@@ -1558,6 +1560,18 @@ public void doFrameGuarded(long frameTimeNanos) {
15581560
return;
15591561
}
15601562

1563+
// Drain pending React revision merges first so that animations,
1564+
// preallocation, and mount items operate against the latest revision.
1565+
if (ReactNativeFeatureFlags.enableFabricCommitBranching()) {
1566+
FabricUIManagerBinding binding = mBinding;
1567+
if (binding != null) {
1568+
Integer mergeSurfaceId;
1569+
while ((mergeSurfaceId = mPendingReactRevisionMerges.poll()) != null) {
1570+
binding.mergeReactRevision(mergeSurfaceId);
1571+
}
1572+
}
1573+
}
1574+
15611575
// Drive any animations from C++.
15621576
// There is a race condition here between getting/setting
15631577
// `mDriveCxxAnimations` which shouldn't matter; it's safe to call

0 commit comments

Comments
 (0)