Skip to content

Commit e0a6801

Browse files
j-piaseckifacebook-github-bot
authored andcommitted
Schedule React revision merge to happen before dispatching events
Summary: 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. Branching broke that in two ways: 1. Merge was always scheduled on the end of the event loop (addressed by the previous diff) 2. Merge was scheduled using `runOnUIThread`, which turned out to be after the `DISPATCH_UI` choreographer phase, thus after the dispatch of events. Differential Revision: D100966623
1 parent 3a019fe commit e0a6801

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+
{
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)