Repository navigation
Pull model: pull each surface's pending transaction at the start of the mount phase (#59029) - #59029
Open
bartlomiejbloniarz wants to merge 1 commit into
Open
bartlomiejbloniarz wants to merge 1 commit into
bartlomiejbloniarz wants to merge 1 commit into
Conversation
|
@bartlomiejbloniarz has imported this pull request. If you are a Meta employee, you can view this in D124966304. |
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 13, 2026
…he mount phase (#59029) Summary: With `enableMountingCoordinatorPullModelAndroid`, each commit notification queues a `PullTransactionMountItem` in `MountItemDispatcher`. `MountingCoordinator::pullTransaction` mounts the latest revision, so a surface's first queued item mounts every pending commit and its later items pull nothing. The item's queue position only reflects when the notification arrived. The queued item breaks root view attach. While the root view is detached, the item waits in the surface's attach queue, ahead of the preallocations for the revisions it pulls. #58152 handles this by re-queueing the pulled batch on the dispatcher. That puts the batch behind a later pull of the same surface, so the newer revision mounts first. This PR replaces the queued item with the set of surfaces that have a transaction available. A dispatch runs view commands, then pre-mount items, then one `FabricUIManagerBinding.pullAndExecuteTransaction` per pending surface whose root view is attached, then the queued mount items. Pulls share the mount items' "mount" trace, their handling of ignorable mounting exceptions, and `reportMount`. A surface waiting for its root view stays pending. Its attach already runs through `MountingManager.MountItemExecutor`, which now also schedules a frame, and that frame pulls the surface after its preallocations. This removes the #58152 re-queue and `PullTransactionMountItem`. With `disableIdleMountItemFrameCallbackRearmAndroid`, an attached pending surface keeps the frame callback armed, like a queued item. When a dispatch has both a pending surface and queued mount items, it now pulls the surface first. On main, an item queued before the surface's first pull notification ran before the pull. Push behaves like main here: its batch is queued at the end of the JS task, behind the items queued during it. An item that targets a view the commit creates, such as an accessibility event or a `SynchronousMountItem`, now finds it. An item that targets a view the commit deletes now misses it, although that view is gone after this frame either way. It logs a soft exception and is dropped, or for a view command, retries once first. With the default flags, view commands still run before the pulls, as on main. ## Changelog: [ANDROID] [FIXED] - Pull model: fix batches applied out of order when a surface's root view attaches Test Plan: `FabricUIManagerPullModelTest` models each surface's `MountingCoordinator`: a pull mounts every revision committed since the previous pull, as one batch. ``` ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest \ --tests 'com.facebook.react.fabric.*' ``` | Test | Before | After | | --- | --- | --- | | `pullDeferredByRootAttach_isAppliedBeforeLaterPulls` | fails: revisions mount as 3, 1, 2 | passes | | `onTransactionAvailable_pullsEachPendingSurfaceOnce` | fails: pulls `[1, 2, 1, 3]` | passes | | `mountItemQueuedBeforeCommit_runsAfterThePull` | fails: the item runs before the batch | passes | | `pendingPullOfStoppedSurface_runsOnceAndIsDropped` | fails: the stop clears the queued pull, nothing is pulled | passes | | `viewCommandsInMountItemOrder_runAfterThePull` | fails: the command dispatched before the commit runs before the batch | passes | - The new tests for pre-mount items, `reportMount` and the frame callback with `disableIdleMountItemFrameCallbackRearmAndroid` pass before and after. One of them, a commit notified from the commit thread while a frame pulls another surface, fails if `hasPendingItems` ignores pending surfaces. - `onTransactionAvailable_enqueuesOnePullPerNotification` is now `onTransactionAvailable_pullsEachPendingSurfaceOnce`. `synchronousBatch_waitingForRootAttach_isDeferred` is removed, because a synchronous batch can no longer run while its root view is detached. - All `ReactAndroid` JVM tests pass. - A few one-off checks, not committed, passed on main and with this PR: commits on two surfaces, an item queued after a commit, and a dispatch without a commit. A push-model check also passed, with the pull flag on: batches queued around a root view attach apply in commit order. - RNTester debug on an Android emulator with `enableMountingCoordinatorPullModelAndroid`, `enableAccumulatedUpdatesInRawPropsAndroid` and `enablePreallocatedPropsDiffOnInsertAndroid`: FlatList, Image, Animated and Accessibility examples render and update, including after leaving and reopening the app. There were no crashes or mounting exceptions. Differential Revision: D124966304 Pulled By: bartlomiejbloniarz
Closed
4 of 5 tasks
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 13, 2026
…he mount phase (#59029) Summary: With `enableMountingCoordinatorPullModelAndroid`, each commit notification queues a `PullTransactionMountItem` in `MountItemDispatcher`. `MountingCoordinator::pullTransaction` mounts the latest revision, so a surface's first queued item mounts every pending commit and its later items pull nothing. The item's queue position only reflects when the notification arrived. The queued item breaks root view attach. While the root view is detached, the item waits in the surface's attach queue, ahead of the preallocations for the revisions it pulls. #58152 handles this by re-queueing the pulled batch on the dispatcher. That puts the batch behind a later pull of the same surface, so the newer revision mounts first. This PR replaces the queued item with the set of surfaces that have a transaction available. A dispatch runs view commands, then pre-mount items, then one `FabricUIManagerBinding.pullAndExecuteTransaction` per pending surface whose root view is attached, then the queued mount items. Pulls share the mount items' "mount" trace, their handling of ignorable mounting exceptions, and `reportMount`. A surface waiting for its root view stays pending. Its attach already runs through `MountingManager.MountItemExecutor`, which now also schedules a frame, and that frame pulls the surface after its preallocations. This removes the #58152 re-queue and `PullTransactionMountItem`. With `disableIdleMountItemFrameCallbackRearmAndroid`, an attached pending surface keeps the frame callback armed, like a queued item. When a dispatch has both a pending surface and queued mount items, it now pulls the surface first. On main, an item queued before the surface's first pull notification ran before the pull. Push behaves like main here: its batch is queued at the end of the JS task, behind the items queued during it. An item that targets a view the commit creates, such as an accessibility event or a `SynchronousMountItem`, now finds it. An item that targets a view the commit deletes now misses it, although that view is gone after this frame either way. It logs a soft exception and is dropped, or for a view command, retries once first. With the default flags, view commands still run before the pulls, as on main. ## Changelog: [ANDROID] [FIXED] - Pull model: fix batches applied out of order when a surface's root view attaches Test Plan: `FabricUIManagerPullModelTest` models each surface's `MountingCoordinator`: a pull mounts every revision committed since the previous pull, as one batch. ``` ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest \ --tests 'com.facebook.react.fabric.*' ``` | Test | Before | After | | --- | --- | --- | | `pullDeferredByRootAttach_isAppliedBeforeLaterPulls` | fails: revisions mount as 3, 1, 2 | passes | | `onTransactionAvailable_pullsEachPendingSurfaceOnce` | fails: pulls `[1, 2, 1, 3]` | passes | | `mountItemQueuedBeforeCommit_runsAfterThePull` | fails: the item runs before the batch | passes | | `pendingPullOfStoppedSurface_runsOnceAndIsDropped` | fails: the stop clears the queued pull, nothing is pulled | passes | | `viewCommandsInMountItemOrder_runAfterThePull` | fails: the command dispatched before the commit runs before the batch | passes | - The new tests for pre-mount items, `reportMount` and the frame callback with `disableIdleMountItemFrameCallbackRearmAndroid` pass before and after. One of them, a commit notified from the commit thread while a frame pulls another surface, fails if `hasPendingItems` ignores pending surfaces. - `onTransactionAvailable_enqueuesOnePullPerNotification` is now `onTransactionAvailable_pullsEachPendingSurfaceOnce`. `synchronousBatch_waitingForRootAttach_isDeferred` is removed, because a synchronous batch can no longer run while its root view is detached. - All `ReactAndroid` JVM tests pass. - A few one-off checks, not committed, passed on main and with this PR: commits on two surfaces, an item queued after a commit, and a dispatch without a commit. A push-model check also passed, with the pull flag on: batches queued around a root view attach apply in commit order. - RNTester debug on an Android emulator with `enableMountingCoordinatorPullModelAndroid`, `enableAccumulatedUpdatesInRawPropsAndroid` and `enablePreallocatedPropsDiffOnInsertAndroid`: FlatList, Image, Animated and Accessibility examples render and update, including after leaving and reopening the app. There were no crashes or mounting exceptions. Differential Revision: D124966304 Pulled By: bartlomiejbloniarz
meta-codesync
Bot
force-pushed
the
bartlomiejbloniarz/pull-model-pending-surfaces
branch
from
October 13, 2026 10:11
efa6834 to
bab8ccd
Compare
|
@bartlomiejbloniarz has exported this pull request. If you are a Meta employee, you can view the originating Diff in D124966304. |
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 13, 2026
…he mount phase (#59029) Summary: With `enableMountingCoordinatorPullModelAndroid`, each commit notification queues a `PullTransactionMountItem` in `MountItemDispatcher`. `MountingCoordinator::pullTransaction` mounts the latest revision, so a surface's first queued item mounts every pending commit and its later items pull nothing. The item's queue position only reflects when the notification arrived. The queued item breaks root view attach. While the root view is detached, the item waits in the surface's attach queue, ahead of the preallocations for the revisions it pulls. #58152 handles this by re-queueing the pulled batch on the dispatcher. That puts the batch behind a later pull of the same surface, so the newer revision mounts first. This PR replaces the queued item with the set of surfaces that have a transaction available. A dispatch runs view commands, then pre-mount items, then one `FabricUIManagerBinding.pullAndExecuteTransaction` per pending surface whose root view is attached, then the queued mount items. Pulls share the mount items' "mount" trace, their handling of ignorable mounting exceptions, and `reportMount`. A surface waiting for its root view stays pending. Its attach already runs through `MountingManager.MountItemExecutor`, which now also schedules a frame, and that frame pulls the surface after its preallocations. This removes the #58152 re-queue and `PullTransactionMountItem`. With `disableIdleMountItemFrameCallbackRearmAndroid`, an attached pending surface keeps the frame callback armed, like a queued item. When a dispatch has both a pending surface and queued mount items, it now pulls the surface first. On main, an item queued before the surface's first pull notification ran before the pull. Push behaves like main here: its batch is queued at the end of the JS task, behind the items queued during it. An item that targets a view the commit creates, such as an accessibility event or a `SynchronousMountItem`, now finds it. An item that targets a view the commit deletes now misses it, although that view is gone after this frame either way. It logs a soft exception and is dropped, or for a view command, retries once first. With the default flags, view commands still run before the pulls, as on main. ## Changelog: [ANDROID] [FIXED] - Pull model: fix batches applied out of order when a surface's root view attaches Test Plan: `FabricUIManagerPullModelTest` models each surface's `MountingCoordinator`: a pull mounts every revision committed since the previous pull, as one batch. ``` ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest \ --tests 'com.facebook.react.fabric.*' ``` | Test | Before | After | | --- | --- | --- | | `pullDeferredByRootAttach_isAppliedBeforeLaterPulls` | fails: revisions mount as 3, 1, 2 | passes | | `onTransactionAvailable_pullsEachPendingSurfaceOnce` | fails: pulls `[1, 2, 1, 3]` | passes | | `mountItemQueuedBeforeCommit_runsAfterThePull` | fails: the item runs before the batch | passes | | `pendingPullOfStoppedSurface_runsOnceAndIsDropped` | fails: the stop clears the queued pull, nothing is pulled | passes | | `viewCommandsInMountItemOrder_runAfterThePull` | fails: the command dispatched before the commit runs before the batch | passes | - The new tests for pre-mount items, `reportMount` and the frame callback with `disableIdleMountItemFrameCallbackRearmAndroid` pass before and after. One of them, a commit notified from the commit thread while a frame pulls another surface, fails if `hasPendingItems` ignores pending surfaces. - `onTransactionAvailable_enqueuesOnePullPerNotification` is now `onTransactionAvailable_pullsEachPendingSurfaceOnce`. `synchronousBatch_waitingForRootAttach_isDeferred` is removed, because a synchronous batch can no longer run while its root view is detached. - All `ReactAndroid` JVM tests pass. - A few one-off checks, not committed, passed on main and with this PR: commits on two surfaces, an item queued after a commit, and a dispatch without a commit. A push-model check also passed, with the pull flag on: batches queued around a root view attach apply in commit order. - RNTester debug on an Android emulator with `enableMountingCoordinatorPullModelAndroid`, `enableAccumulatedUpdatesInRawPropsAndroid` and `enablePreallocatedPropsDiffOnInsertAndroid`: FlatList, Image, Animated and Accessibility examples render and update, including after leaving and reopening the app. There were no crashes or mounting exceptions. Differential Revision: D124966304 Pulled By: bartlomiejbloniarz
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 13, 2026
Summary: With `enableMountingCoordinatorPullModelAndroid`, a commit tells Java only its surface id. At the next mount dispatch, `FabricUIManagerBinding::pullAndExecuteTransaction` looks the surface's `MountingCoordinator` up in the binding's surface registry. A surface is not in the registry while it starts or stops: - The transaction that stops a surface is never pulled, because `stopSurfaceWithSurfaceHandler` removes the surface from the registry before `SurfaceHandler::stop()` commits the empty tree. Mounting override delegates such as LayoutAnimations and C++ Animated never see those Deletes. With `enableLayoutAnimationsOnAndroid`, a layout animation that runs when its surface stops stays in flight, and `FabricUIManager` calls `driveCxxAnimations` on every frame until another surface pulls a transaction. - A commit that a bridgeless surface makes before `startSurfaceWithSurfaceHandler` registers it is not mounted until the surface commits again. - The legacy `stopSurface(int)` holds the registry lock across `stop()`. On the UI thread, the stop commit pulls inline and blocks on the same lock, so the app freezes. iOS does not look the surface up. `RCTMountingManager` captures the `MountingCoordinator` that reported the commit in the block it dispatches to the main queue, and pulls from it. This PR does the same. `schedulerShouldRenderTransactions` keeps the reporting `MountingCoordinator` until the UI thread pulls, and `pullAndExecuteTransaction` pulls from it. Java still receives only the surface id. The surface registry, the start and stop functions, `reportMount` and the push model are unchanged. As on iOS, a surface stopped on the UI thread has its stop transaction pulled during the stop commit. When it stops on another thread, its shadow tree is destroyed and its coordinator revoked before the UI thread pulls, so that transaction is not mounted. A pull of a stopped surface can log "Executing commit after surface N was stopped!" when a mounting override delegate still returns a transaction; iOS pulls these the same way. Only one coordinator per surface id is kept. Stopping and starting the same `ReactSurface` on the same host reuses its id. If the stopped surface's commit has not been pulled yet, the restarted surface's coordinator replaces it and a warning is logged. The replaced coordinator is revoked and would pull nothing. Java does not mount such a restarted surface anyway: `MountingManager.startSurface` logs "Called startSurface more than once" and keeps the stopped surface's manager. ## Changelog: [ANDROID] [FIXED] - Pull model: mount the commits of starting and stopping surfaces, and fix a freeze when the legacy `stopSurface(int)` runs on the UI thread Test Plan: New `FabricMountingManagerInstrumentationTest` cases drive a real `FabricUIManagerBinding`. The test helper runs no JavaScript and commits views to the surface's shadow tree itself. The pull-model cases run with the Props 2.0 flags and `enablePreallocatedPropsDiffOnInsertAndroid`. | Test | Without the fix | With the fix | | --- | --- | --- | | `native_stopSurfaceWithSurfaceHandler_pullModel_pullsDeletes` | fails: no Deletes are pulled | passes | | `native_stopSurface_pullModel_pullsDeletes` | hangs | passes | | `native_stopSurfaceWithSurfaceHandler_pushModel_pullsDeletes` | passes | passes | The push case is a control. The hang stops the run, so the bridgeless stop test was also run without the legacy one. With the fix, the whole class passes. A one-off check, not committed, covered the start: a bridgeless surface started on a background thread commits a view, and the start is held before `startSurfaceWithSurfaceHandler` registers the surface. On #59029 alone the view is not mounted while the start is held. With this PR it is mounted by the next frame. The test class has no Gradle target in the open-source repository, so I ran it on an Android emulator through a local androidTest source set: ``` ./gradlew :packages:react-native:ReactAndroid:assembleReleaseAndroidTest -PreactNativeArchitectures=arm64-v8a adb install -r -t packages/react-native/ReactAndroid/build/outputs/apk/androidTest/release/ReactAndroid-release-androidTest.apk adb shell am instrument -w -e class com.facebook.react.fabric.FabricMountingManagerInstrumentationTest \ com.facebook.react.tests.gradle/androidx.test.runner.AndroidJUnitRunner ``` I also checked the effect in RNTester with a temporary hook, not part of this PR. The Playground starts a 60 s `LayoutAnimation`, then `ReactDelegate.unloadApp()` stops the surface on the UI thread while the activity stays resumed. Props 2.0 and `enableLayoutAnimationsOnAndroid` were on: | Run | After the stop | | --- | --- | | Pull, without this PR | no `onAllAnimationsComplete`; `driveCxxAnimations` keeps running on every frame | | Pull, with this PR | `onAllAnimationsComplete` right after the stop, then no more driven frames | | Push, with this PR | `onAllAnimationsComplete` right after the stop, as before | Pulled By: bartlomiejbloniarz bartlomiejbloniarz Differential Revision: D124966432
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 14, 2026
…he mount phase (#59029) Summary: With `enableMountingCoordinatorPullModelAndroid`, each commit notification queues a `PullTransactionMountItem` in `MountItemDispatcher`. `MountingCoordinator::pullTransaction` mounts the latest revision, so a surface's first queued item mounts every pending commit and its later items pull nothing. The item's queue position only reflects when the notification arrived. The queued item breaks root view attach. While the root view is detached, the item waits in the surface's attach queue, ahead of the preallocations for the revisions it pulls. #58152 handles this by re-queueing the pulled batch on the dispatcher. That puts the batch behind a later pull of the same surface, so the newer revision mounts first. This PR replaces the queued item with the set of surfaces that have a transaction available. A dispatch runs view commands, then pre-mount items, then one `FabricUIManagerBinding.pullAndExecuteTransaction` per pending surface whose root view is attached, then the queued mount items. Pulls share the mount items' "mount" trace, their handling of ignorable mounting exceptions, and `reportMount`. A surface waiting for its root view stays pending. Its attach already runs through `MountingManager.MountItemExecutor`, which now also schedules a frame, and that frame pulls the surface after its preallocations. This removes the #58152 re-queue and `PullTransactionMountItem`. With `disableIdleMountItemFrameCallbackRearmAndroid`, an attached pending surface keeps the frame callback armed, like a queued item. When a dispatch has both a pending surface and queued mount items, it now pulls the surface first. On main, an item queued before the surface's first pull notification ran before the pull. Push behaves like main here: its batch is queued at the end of the JS task, behind the items queued during it. An item that targets a view the commit creates, such as an accessibility event or a `SynchronousMountItem`, now finds it. An item that targets a view the commit deletes now misses it, although that view is gone after this frame either way. It logs a soft exception and is dropped, or for a view command, retries once first. With the default flags, view commands still run before the pulls, as on main. ## Changelog: [ANDROID] [FIXED] - Pull model: fix batches applied out of order when a surface's root view attaches Test Plan: `FabricUIManagerPullModelTest` models each surface's `MountingCoordinator`: a pull mounts every revision committed since the previous pull, as one batch. ``` ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest \ --tests 'com.facebook.react.fabric.*' ``` | Test | Before | After | | --- | --- | --- | | `pullDeferredByRootAttach_isAppliedBeforeLaterPulls` | fails: revisions mount as 3, 1, 2 | passes | | `onTransactionAvailable_pullsEachPendingSurfaceOnce` | fails: pulls `[1, 2, 1, 3]` | passes | | `mountItemQueuedBeforeCommit_runsAfterThePull` | fails: the item runs before the batch | passes | | `pendingPullOfStoppedSurface_runsOnceAndIsDropped` | fails: the stop clears the queued pull, nothing is pulled | passes | | `viewCommandsInMountItemOrder_runAfterThePull` | fails: the command dispatched before the commit runs before the batch | passes | - The new tests for pre-mount items, `reportMount` and the frame callback with `disableIdleMountItemFrameCallbackRearmAndroid` pass before and after. One of them, a commit notified from the commit thread while a frame pulls another surface, fails if `hasPendingItems` ignores pending surfaces. - `onTransactionAvailable_enqueuesOnePullPerNotification` is now `onTransactionAvailable_pullsEachPendingSurfaceOnce`. `synchronousBatch_waitingForRootAttach_isDeferred` is removed, because a synchronous batch can no longer run while its root view is detached. - All `ReactAndroid` JVM tests pass. - A few one-off checks, not committed, passed on main and with this PR: commits on two surfaces, an item queued after a commit, and a dispatch without a commit. A push-model check also passed, with the pull flag on: batches queued around a root view attach apply in commit order. - RNTester debug on an Android emulator with `enableMountingCoordinatorPullModelAndroid`, `enableAccumulatedUpdatesInRawPropsAndroid` and `enablePreallocatedPropsDiffOnInsertAndroid`: FlatList, Image, Animated and Accessibility examples render and update, including after leaving and reopening the app. There were no crashes or mounting exceptions. Differential Revision: D124966304 Pulled By: bartlomiejbloniarz
meta-codesync
Bot
force-pushed
the
bartlomiejbloniarz/pull-model-pending-surfaces
branch
from
October 14, 2026 09:38
bab8ccd to
b9f9945
Compare
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 14, 2026
Summary: With `enableMountingCoordinatorPullModelAndroid`, a commit tells Java only its surface id. At the next mount dispatch, `FabricUIManagerBinding::pullAndExecuteTransaction` looks the surface's `MountingCoordinator` up in the binding's surface registry. A surface is not in the registry while it starts or stops: - The transaction that stops a surface is never pulled, because `stopSurfaceWithSurfaceHandler` removes the surface from the registry before `SurfaceHandler::stop()` commits the empty tree. Mounting override delegates such as LayoutAnimations and C++ Animated never see those Deletes. With `enableLayoutAnimationsOnAndroid`, a layout animation that runs when its surface stops stays in flight, and `FabricUIManager` calls `driveCxxAnimations` on every frame until another surface pulls a transaction. - A commit that a bridgeless surface makes before `startSurfaceWithSurfaceHandler` registers it is not mounted until the surface commits again. - The legacy `stopSurface(int)` holds the registry lock across `stop()`. On the UI thread, the stop commit pulls inline and blocks on the same lock, so the app freezes. iOS does not look the surface up. `RCTMountingManager` captures the `MountingCoordinator` that reported the commit in the block it dispatches to the main queue, and pulls from it. This PR does the same. `schedulerShouldRenderTransactions` keeps the reporting `MountingCoordinator` until the UI thread pulls, and `pullAndExecuteTransaction` pulls from it. Java still receives only the surface id. The surface registry, the start and stop functions, `reportMount` and the push model are unchanged. As on iOS, a surface stopped on the UI thread has its stop transaction pulled during the stop commit. When it stops on another thread, its shadow tree is destroyed and its coordinator revoked before the UI thread pulls, so that transaction is not mounted. A pull of a stopped surface can log "Executing commit after surface N was stopped!" when a mounting override delegate still returns a transaction; iOS pulls these the same way. Only one coordinator per surface id is kept. Stopping and starting the same `ReactSurface` on the same host reuses its id. If the stopped surface's commit has not been pulled yet, the restarted surface's coordinator replaces it and a warning is logged. The replaced coordinator is revoked and would pull nothing. Java does not mount such a restarted surface anyway: `MountingManager.startSurface` logs "Called startSurface more than once" and keeps the stopped surface's manager. ## Changelog: [ANDROID] [FIXED] - Pull model: mount the commits of starting and stopping surfaces, and fix a freeze when the legacy `stopSurface(int)` runs on the UI thread Test Plan: New `FabricMountingManagerInstrumentationTest` cases drive a real `FabricUIManagerBinding`. The test helper runs no JavaScript and commits views to the surface's shadow tree itself. The pull-model cases run with the Props 2.0 flags and `enablePreallocatedPropsDiffOnInsertAndroid`. | Test | Without the fix | With the fix | | --- | --- | --- | | `native_stopSurfaceWithSurfaceHandler_pullModel_pullsDeletes` | fails: no Deletes are pulled | passes | | `native_stopSurface_pullModel_pullsDeletes` | hangs | passes | | `native_stopSurfaceWithSurfaceHandler_pushModel_pullsDeletes` | passes | passes | The push case is a control. The hang stops the run, so the bridgeless stop test was also run without the legacy one. With the fix, the whole class passes. A one-off check, not committed, covered the start: a bridgeless surface started on a background thread commits a view, and the start is held before `startSurfaceWithSurfaceHandler` registers the surface. On #59029 alone the view is not mounted while the start is held. With this PR it is mounted by the next frame. The test class has no Gradle target in the open-source repository, so I ran it on an Android emulator through a local androidTest source set: ``` ./gradlew :packages:react-native:ReactAndroid:assembleReleaseAndroidTest -PreactNativeArchitectures=arm64-v8a adb install -r -t packages/react-native/ReactAndroid/build/outputs/apk/androidTest/release/ReactAndroid-release-androidTest.apk adb shell am instrument -w -e class com.facebook.react.fabric.FabricMountingManagerInstrumentationTest \ com.facebook.react.tests.gradle/androidx.test.runner.AndroidJUnitRunner ``` I also checked the effect in RNTester with a temporary hook, not part of this PR. The Playground starts a 60 s `LayoutAnimation`, then `ReactDelegate.unloadApp()` stops the surface on the UI thread while the activity stays resumed. Props 2.0 and `enableLayoutAnimationsOnAndroid` were on: | Run | After the stop | | --- | --- | | Pull, without this PR | no `onAllAnimationsComplete`; `driveCxxAnimations` keeps running on every frame | | Pull, with this PR | `onAllAnimationsComplete` right after the stop, then no more driven frames | | Push, with this PR | `onAllAnimationsComplete` right after the stop, as before | Pulled By: bartlomiejbloniarz bartlomiejbloniarz Differential Revision: D124966432
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 14, 2026
…he mount phase (#59029) Summary: With `enableMountingCoordinatorPullModelAndroid`, each commit notification queues a `PullTransactionMountItem` in `MountItemDispatcher`. `MountingCoordinator::pullTransaction` mounts the latest revision, so a surface's first queued item mounts every pending commit and its later items pull nothing. The item's queue position only reflects when the notification arrived. The queued item breaks root view attach. While the root view is detached, the item waits in the surface's attach queue, ahead of the preallocations for the revisions it pulls. #58152 handles this by re-queueing the pulled batch on the dispatcher. That puts the batch behind a later pull of the same surface, so the newer revision mounts first. This PR replaces the queued item with the set of surfaces that have a transaction available. A dispatch runs view commands, then pre-mount items, then one `FabricUIManagerBinding.pullAndExecuteTransaction` per pending surface whose root view is attached, then the queued mount items. Pulls share the mount items' "mount" trace, their handling of ignorable mounting exceptions, and `reportMount`. A surface waiting for its root view stays pending. Its attach already runs through `MountingManager.MountItemExecutor`, which now also schedules a frame, and that frame pulls the surface after its preallocations. This removes the #58152 re-queue and `PullTransactionMountItem`. With `disableIdleMountItemFrameCallbackRearmAndroid`, an attached pending surface keeps the frame callback armed, like a queued item. When a dispatch has both a pending surface and queued mount items, it now pulls the surface first. On main, an item queued before the surface's first pull notification ran before the pull. Push behaves like main here: its batch is queued at the end of the JS task, behind the items queued during it. An item that targets a view the commit creates, such as an accessibility event or a `SynchronousMountItem`, now finds it. An item that targets a view the commit deletes now misses it, although that view is gone after this frame either way. It logs a soft exception and is dropped, or for a view command, retries once first. With the default flags, view commands still run before the pulls, as on main. ## Changelog: [ANDROID] [FIXED] - Pull model: fix batches applied out of order when a surface's root view attaches Test Plan: `FabricUIManagerPullModelTest` models each surface's `MountingCoordinator`: a pull mounts every revision committed since the previous pull, as one batch. ``` ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest \ --tests 'com.facebook.react.fabric.*' ``` | Test | Before | After | | --- | --- | --- | | `pullDeferredByRootAttach_isAppliedBeforeLaterPulls` | fails: revisions mount as 3, 1, 2 | passes | | `onTransactionAvailable_pullsEachPendingSurfaceOnce` | fails: pulls `[1, 2, 1, 3]` | passes | | `mountItemQueuedBeforeCommit_runsAfterThePull` | fails: the item runs before the batch | passes | | `pendingPullOfStoppedSurface_runsOnceAndIsDropped` | fails: the stop clears the queued pull, nothing is pulled | passes | | `viewCommandsInMountItemOrder_runAfterThePull` | fails: the command dispatched before the commit runs before the batch | passes | - The new tests for pre-mount items, `reportMount` and the frame callback with `disableIdleMountItemFrameCallbackRearmAndroid` pass before and after. One of them, a commit notified from the commit thread while a frame pulls another surface, fails if `hasPendingItems` ignores pending surfaces. - `onTransactionAvailable_enqueuesOnePullPerNotification` is now `onTransactionAvailable_pullsEachPendingSurfaceOnce`. `synchronousBatch_waitingForRootAttach_isDeferred` is removed, because a synchronous batch can no longer run while its root view is detached. - All `ReactAndroid` JVM tests pass. - A few one-off checks, not committed, passed on main and with this PR: commits on two surfaces, an item queued after a commit, and a dispatch without a commit. A push-model check also passed, with the pull flag on: batches queued around a root view attach apply in commit order. - RNTester debug on an Android emulator with `enableMountingCoordinatorPullModelAndroid`, `enableAccumulatedUpdatesInRawPropsAndroid` and `enablePreallocatedPropsDiffOnInsertAndroid`: FlatList, Image, Animated and Accessibility examples render and update, including after leaving and reopening the app. There were no crashes or mounting exceptions. Differential Revision: D124966304 Pulled By: bartlomiejbloniarz
meta-codesync
Bot
force-pushed
the
bartlomiejbloniarz/pull-model-pending-surfaces
branch
from
October 14, 2026 13:17
b9f9945 to
3babb3a
Compare
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 14, 2026
Summary: With `enableMountingCoordinatorPullModelAndroid`, a commit tells Java only its surface id. At the next mount dispatch, `FabricUIManagerBinding::pullAndExecuteTransaction` looks the surface's `MountingCoordinator` up in the binding's surface registry. A surface is not in the registry while it starts or stops: - The transaction that stops a surface is never pulled, because `stopSurfaceWithSurfaceHandler` removes the surface from the registry before `SurfaceHandler::stop()` commits the empty tree. Mounting override delegates such as LayoutAnimations and C++ Animated never see those Deletes. With `enableLayoutAnimationsOnAndroid`, a layout animation that runs when its surface stops stays in flight, and `FabricUIManager` calls `driveCxxAnimations` on every frame until another surface pulls a transaction. - A commit that a bridgeless surface makes before `startSurfaceWithSurfaceHandler` registers it is not mounted until the surface commits again. - The legacy `stopSurface(int)` holds the registry lock across `stop()`. On the UI thread, the stop commit pulls inline and blocks on the same lock, so the app freezes. iOS does not look the surface up. `RCTMountingManager` captures the `MountingCoordinator` that reported the commit in the block it dispatches to the main queue, and pulls from it. This PR does the same. `schedulerShouldRenderTransactions` keeps the reporting `MountingCoordinator` until the UI thread pulls, and `pullAndExecuteTransaction` pulls from it. Java still receives only the surface id. The surface registry, the start and stop functions, `reportMount` and the push model are unchanged. As on iOS, a surface stopped on the UI thread has its stop transaction pulled during the stop commit. When it stops on another thread, its shadow tree is destroyed and its coordinator revoked before the UI thread pulls, so that transaction is not mounted. A pull of a stopped surface can log "Executing commit after surface N was stopped!" when a mounting override delegate still returns a transaction; iOS pulls these the same way. Only one coordinator per surface id is kept. Stopping and starting the same `ReactSurface` on the same host reuses its id. If the stopped surface's commit has not been pulled yet, the restarted surface's coordinator replaces it and a warning is logged. The replaced coordinator is revoked and would pull nothing. Java does not mount such a restarted surface anyway: `MountingManager.startSurface` logs "Called startSurface more than once" and keeps the stopped surface's manager. ## Changelog: [ANDROID] [FIXED] - Pull model: mount the commits of starting and stopping surfaces, and fix a freeze when the legacy `stopSurface(int)` runs on the UI thread Test Plan: New `FabricMountingManagerInstrumentationTest` cases drive a real `FabricUIManagerBinding`. The test helper runs no JavaScript and commits views to the surface's shadow tree itself. The pull-model cases run with the Props 2.0 flags and `enablePreallocatedPropsDiffOnInsertAndroid`. | Test | Without the fix | With the fix | | --- | --- | --- | | `native_stopSurfaceWithSurfaceHandler_pullModel_pullsDeletes` | fails: no Deletes are pulled | passes | | `native_stopSurface_pullModel_pullsDeletes` | hangs | passes | | `native_stopSurfaceWithSurfaceHandler_pushModel_pullsDeletes` | passes | passes | The push case is a control. The hang stops the run, so the bridgeless stop test was also run without the legacy one. With the fix, the whole class passes. A one-off check, not committed, covered the start: a bridgeless surface started on a background thread commits a view, and the start is held before `startSurfaceWithSurfaceHandler` registers the surface. On #59029 alone the view is not mounted while the start is held. With this PR it is mounted by the next frame. The test class has no Gradle target in the open-source repository, so I ran it on an Android emulator through a local androidTest source set: ``` ./gradlew :packages:react-native:ReactAndroid:assembleReleaseAndroidTest -PreactNativeArchitectures=arm64-v8a adb install -r -t packages/react-native/ReactAndroid/build/outputs/apk/androidTest/release/ReactAndroid-release-androidTest.apk adb shell am instrument -w -e class com.facebook.react.fabric.FabricMountingManagerInstrumentationTest \ com.facebook.react.tests.gradle/androidx.test.runner.AndroidJUnitRunner ``` I also checked the effect in RNTester with a temporary hook, not part of this PR. The Playground starts a 60 s `LayoutAnimation`, then `ReactDelegate.unloadApp()` stops the surface on the UI thread while the activity stays resumed. Props 2.0 and `enableLayoutAnimationsOnAndroid` were on: | Run | After the stop | | --- | --- | | Pull, without this PR | no `onAllAnimationsComplete`; `driveCxxAnimations` keeps running on every frame | | Pull, with this PR | `onAllAnimationsComplete` right after the stop, then no more driven frames | | Push, with this PR | `onAllAnimationsComplete` right after the stop, as before | Pulled By: bartlomiejbloniarz bartlomiejbloniarz Differential Revision: D124966432
…he mount phase (#59029) Summary: With `enableMountingCoordinatorPullModelAndroid`, each commit notification queues a `PullTransactionMountItem` in `MountItemDispatcher`. `MountingCoordinator::pullTransaction` mounts the latest revision, so a surface's first queued item mounts every pending commit and its later items pull nothing. The item's queue position only reflects when the notification arrived. The queued item breaks root view attach. While the root view is detached, the item waits in the surface's attach queue, ahead of the preallocations for the revisions it pulls. #58152 handles this by re-queueing the pulled batch on the dispatcher. That puts the batch behind a later pull of the same surface, so the newer revision mounts first. This PR replaces the queued item with the set of surfaces that have a transaction available. A dispatch runs view commands, then pre-mount items, then one `FabricUIManagerBinding.pullAndExecuteTransaction` per pending surface whose root view is attached, then the queued mount items. Pulls share the mount items' "mount" trace, their handling of ignorable mounting exceptions, and `reportMount`. A surface waiting for its root view stays pending. Its attach already runs through `MountingManager.MountItemExecutor`, which now also schedules a frame, and that frame pulls the surface after its preallocations. This removes the #58152 re-queue and `PullTransactionMountItem`. With `disableIdleMountItemFrameCallbackRearmAndroid`, an attached pending surface keeps the frame callback armed, like a queued item. When a dispatch has both a pending surface and queued mount items, it now pulls the surface first. On main, an item queued before the surface's first pull notification ran before the pull. Push behaves like main here: its batch is queued at the end of the JS task, behind the items queued during it. An item that targets a view the commit creates, such as an accessibility event or a `SynchronousMountItem`, now finds it. An item that targets a view the commit deletes now misses it, although that view is gone after this frame either way. It logs a soft exception and is dropped, or for a view command, retries once first. With the default flags, view commands still run before the pulls, as on main. ## Changelog: [ANDROID] [FIXED] - Pull model: fix batches applied out of order when a surface's root view attaches Pull Request resolved: #59029 Test Plan: `FabricUIManagerPullModelTest` models each surface's `MountingCoordinator`: a pull mounts every revision committed since the previous pull, as one batch. ``` ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest \ --tests 'com.facebook.react.fabric.*' ``` | Test | Before | After | | --- | --- | --- | | `pullDeferredByRootAttach_isAppliedBeforeLaterPulls` | fails: revisions mount as 3, 1, 2 | passes | | `onTransactionAvailable_pullsEachPendingSurfaceOnce` | fails: pulls `[1, 2, 1, 3]` | passes | | `mountItemQueuedBeforeCommit_runsAfterThePull` | fails: the item runs before the batch | passes | | `pendingPullOfStoppedSurface_runsOnceAndIsDropped` | fails: the stop clears the queued pull, nothing is pulled | passes | | `viewCommandsInMountItemOrder_runAfterThePull` | fails: the command dispatched before the commit runs before the batch | passes | - The new tests for pre-mount items, `reportMount` and the frame callback with `disableIdleMountItemFrameCallbackRearmAndroid` pass before and after. One of them, a commit notified from the commit thread while a frame pulls another surface, fails if `hasPendingItems` ignores pending surfaces. - `onTransactionAvailable_enqueuesOnePullPerNotification` is now `onTransactionAvailable_pullsEachPendingSurfaceOnce`. `synchronousBatch_waitingForRootAttach_isDeferred` is removed, because a synchronous batch can no longer run while its root view is detached. - All `ReactAndroid` JVM tests pass. - A few one-off checks, not committed, passed on main and with this PR: commits on two surfaces, an item queued after a commit, and a dispatch without a commit. A push-model check also passed, with the pull flag on: batches queued around a root view attach apply in commit order. - RNTester debug on an Android emulator with `enableMountingCoordinatorPullModelAndroid`, `enableAccumulatedUpdatesInRawPropsAndroid` and `enablePreallocatedPropsDiffOnInsertAndroid`: FlatList, Image, Animated and Accessibility examples render and update, including after leaving and reopening the app. There were no crashes or mounting exceptions. Differential Revision: D124966304 Pulled By: bartlomiejbloniarz
meta-codesync
Bot
force-pushed
the
bartlomiejbloniarz/pull-model-pending-surfaces
branch
from
October 14, 2026 13:23
3babb3a to
0e03c6c
Compare
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 14, 2026
Summary: With `enableMountingCoordinatorPullModelAndroid`, a commit tells Java only its surface id. At the next mount dispatch, `FabricUIManagerBinding::pullAndExecuteTransaction` looks the surface's `MountingCoordinator` up in the binding's surface registry. A surface is not in the registry while it starts or stops: - The transaction that stops a surface is never pulled, because `stopSurfaceWithSurfaceHandler` removes the surface from the registry before `SurfaceHandler::stop()` commits the empty tree. Mounting override delegates such as LayoutAnimations and C++ Animated never see those Deletes. With `enableLayoutAnimationsOnAndroid`, a layout animation that runs when its surface stops stays in flight, and `FabricUIManager` calls `driveCxxAnimations` on every frame until another surface pulls a transaction. - A commit that a bridgeless surface makes before `startSurfaceWithSurfaceHandler` registers it is not mounted until the surface commits again. - The legacy `stopSurface(int)` holds the registry lock across `stop()`. On the UI thread, the stop commit pulls inline and blocks on the same lock, so the app freezes. iOS does not look the surface up. `RCTMountingManager` captures the `MountingCoordinator` that reported the commit in the block it dispatches to the main queue, and pulls from it. This PR does the same. `schedulerShouldRenderTransactions` keeps the reporting `MountingCoordinator` until the UI thread pulls, and `pullAndExecuteTransaction` pulls from it. Java still receives only the surface id. The surface registry, the start and stop functions, `reportMount` and the push model are unchanged. As on iOS, a surface stopped on the UI thread has its stop transaction pulled during the stop commit. When it stops on another thread, its shadow tree is destroyed and its coordinator revoked before the UI thread pulls, so that transaction is not mounted. A pull of a stopped surface can log "Executing commit after surface N was stopped!" when a mounting override delegate still returns a transaction; iOS pulls these the same way. Only one coordinator per surface id is kept. Stopping and starting the same `ReactSurface` on the same host reuses its id. If the stopped surface's commit has not been pulled yet, the restarted surface's coordinator replaces it and a warning is logged. The replaced coordinator is revoked and would pull nothing. Java does not mount such a restarted surface anyway: `MountingManager.startSurface` logs "Called startSurface more than once" and keeps the stopped surface's manager. ## Changelog: [ANDROID] [FIXED] - Pull model: mount the commits of starting and stopping surfaces, and fix a freeze when the legacy `stopSurface(int)` runs on the UI thread Pull Request resolved: #59030 Test Plan: New `FabricMountingManagerInstrumentationTest` cases drive a real `FabricUIManagerBinding`. The test helper runs no JavaScript and commits views to the surface's shadow tree itself. The pull-model cases run with the Props 2.0 flags and `enablePreallocatedPropsDiffOnInsertAndroid`. | Test | Without the fix | With the fix | | --- | --- | --- | | `native_stopSurfaceWithSurfaceHandler_pullModel_pullsDeletes` | fails: no Deletes are pulled | passes | | `native_stopSurface_pullModel_pullsDeletes` | hangs | passes | | `native_stopSurfaceWithSurfaceHandler_pushModel_pullsDeletes` | passes | passes | The push case is a control. The hang stops the run, so the bridgeless stop test was also run without the legacy one. With the fix, the whole class passes. A one-off check, not committed, covered the start: a bridgeless surface started on a background thread commits a view, and the start is held before `startSurfaceWithSurfaceHandler` registers the surface. On #59029 alone the view is not mounted while the start is held. With this PR it is mounted by the next frame. The test class has no Gradle target in the open-source repository, so I ran it on an Android emulator through a local androidTest source set: ``` ./gradlew :packages:react-native:ReactAndroid:assembleReleaseAndroidTest -PreactNativeArchitectures=arm64-v8a adb install -r -t packages/react-native/ReactAndroid/build/outputs/apk/androidTest/release/ReactAndroid-release-androidTest.apk adb shell am instrument -w -e class com.facebook.react.fabric.FabricMountingManagerInstrumentationTest \ com.facebook.react.tests.gradle/androidx.test.runner.AndroidJUnitRunner ``` I also checked the effect in RNTester with a temporary hook, not part of this PR. The Playground starts a 60 s `LayoutAnimation`, then `ReactDelegate.unloadApp()` stops the surface on the UI thread while the activity stays resumed. Props 2.0 and `enableLayoutAnimationsOnAndroid` were on: | Run | After the stop | | --- | --- | | Pull, without this PR | no `onAllAnimationsComplete`; `driveCxxAnimations` keeps running on every frame | | Pull, with this PR | `onAllAnimationsComplete` right after the stop, then no more driven frames | | Push, with this PR | `onAllAnimationsComplete` right after the stop, as before | Pulled By: bartlomiejbloniarz bartlomiejbloniarz Differential Revision: D124966432
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
With
enableMountingCoordinatorPullModelAndroid, each commit notification queues aPullTransactionMountIteminMountItemDispatcher.MountingCoordinator::pullTransactionmounts the latest revision, so a surface's first queued item mounts every pending commit and its later items pull nothing. The item's queue position only reflects when the notification arrived.The queued item breaks root view attach. While the root view is detached, the item waits in the surface's attach queue, ahead of the preallocations for the revisions it pulls. #58152 handles this by re-queueing the pulled batch on the dispatcher. That puts the batch behind a later pull of the same surface, so the newer revision mounts first.
This PR replaces the queued item with the set of surfaces that have a transaction available. A dispatch runs view commands, then pre-mount items, then one
FabricUIManagerBinding.pullAndExecuteTransactionper pending surface whose root view is attached, then the queued mount items. Pulls share the mount items' "mount" trace, their handling of ignorable mounting exceptions, andreportMount. A surface waiting for its root view stays pending. Its attach already runs throughMountingManager.MountItemExecutor, which now also schedules a frame, and that frame pulls the surface after its preallocations. This removes the #58152 re-queue andPullTransactionMountItem. WithdisableIdleMountItemFrameCallbackRearmAndroid, an attached pending surface keeps the frame callback armed, like a queued item.When a dispatch has both a pending surface and queued mount items, it now pulls the surface first. On main, an item queued before the surface's first pull notification ran before the pull. Push behaves like main here: its batch is queued at the end of the JS task, behind the items queued during it. An item that targets a view the commit creates, such as an accessibility event or a
SynchronousMountItem, now finds it. An item that targets a view the commit deletes now misses it, although that view is gone after this frame either way. It logs a soft exception and is dropped, or for a view command, retries once first. With the default flags, view commands still run before the pulls, as on main.Changelog:
[ANDROID] [FIXED] - Pull model: fix batches applied out of order when a surface's root view attaches
Test Plan:
FabricUIManagerPullModelTestmodels each surface'sMountingCoordinator: a pull mounts every revision committed since the previous pull, as one batch.pullDeferredByRootAttach_isAppliedBeforeLaterPullsonTransactionAvailable_pullsEachPendingSurfaceOnce[1, 2, 1, 3]mountItemQueuedBeforeCommit_runsAfterThePullpendingPullOfStoppedSurface_runsOnceAndIsDroppedviewCommandsInMountItemOrder_runAfterThePullreportMountand the frame callback withdisableIdleMountItemFrameCallbackRearmAndroidpass before and after. One of them, a commit notified from the commit thread while a frame pulls another surface, fails ifhasPendingItemsignores pending surfaces.onTransactionAvailable_enqueuesOnePullPerNotificationis nowonTransactionAvailable_pullsEachPendingSurfaceOnce.synchronousBatch_waitingForRootAttach_isDeferredis removed, because a synchronous batch can no longer run while its root view is detached.ReactAndroidJVM tests pass.enableMountingCoordinatorPullModelAndroid,enableAccumulatedUpdatesInRawPropsAndroidandenablePreallocatedPropsDiffOnInsertAndroid: FlatList, Image, Animated and Accessibility examples render and update, including after leaving and reopening the app. There were no crashes or mounting exceptions.Differential Revision: D124966304
Pulled By: bartlomiejbloniarz