Defer the pull model's synchronous mount batch until the root view is attached (#58152)

Summary:
Pull Request resolved: #58152

Two crashes can occur when a synchronous mount batch runs before its root view is attached. Both need the same bad state: a `ViewState` for a tag is present in `tagToViewState`, but its `view` field is null.

## Cause

A surface can render before its root view is attached. While the root view is not attached, `MountItemDispatcher.executeOrEnqueue` defers every mount item into `SurfaceMountingManager.onViewAttachMountItems`. The pull model does not defer one of them: `FabricUIManager.scheduleMountItem(synchronous = true)` calls `mountItem.execute()` directly.

That gives this sequence for a tag `T`:

1. **C++ claims the tag first.** `preallocateShadowView` puts `T` into `allocatedViewRegistry_`. It does this before it calls Java.
2. **Java does not create the view.** The `PreAllocateViewMountItem` for `T` is deferred, because `isWaitingForViewAttach` is true. No `ViewState` exists for `T`.
3. **C++ omits the Create instruction.** `executeMount` finds `T` in `allocatedViewTags`, so it does not add a Create for `T`.
4. **The mount batch runs too early.** The batch is not deferred, so it runs while the root view is still not attached. It has no Create for `T`, but it has an `UpdateEventEmitter`. `updateEventEmitter` calls `tagToViewState.getOrPut(T) { ViewState(T) }`, which makes a `ViewState` with a null `view`.
5. **The preallocation is cancelled.** The root view attaches and the deferred `PreAllocateViewMountItem` runs. `preallocateView` finds a `ViewState` for `T` and returns. `T` now has no view, and no Create will come.

The next `updateState` or `updateOverflowInset` for `T` throws.

## Fix

Apply the same attach barrier to the synchronous batch that every other mount item already obeys. If the root view is not attached, put the batch in the dispatcher queue instead of running it inline. The preallocations then run first, and the batch runs after the root view is attached.

Only `pullAndExecuteTransaction` passes `synchronous = true`, so the push model never reaches this path.

## Changelog:

[Android] [Fixed] - pull model mounting is now deferred until the root attaches

Reviewed By: zeyap

Differential Revision: D117519782

fbshipit-source-id: 5d5104eccbdd382b5be06417d6d31450ef86b697

Author: Bartlomiej Bloniarz

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

@@ -971,9 +971,15 @@ private void scheduleMountItem(
     if (shouldSchedule) {
       Assertions.assertNotNull(mountItem, "MountItem is null");
       if (synchronous) {
-        // Pull model: we are already on the UI thread, inside the dispatcher's loop executing
-        // a PullTransactionMountItem. We don't schedule the item, we execute it directly.
-        mountItem.execute(mMountingManager);
+        if (mMountingManager.isWaitingForViewAttach(mountItem.getSurfaceId())) {
+          // Regular mount items are still being deferred into the surface's attach queue.
+          // Executing this batch inline would run it ahead of the preallocations queued there.
+          mMountItemDispatcher.addMountItem(mountItem);
+        } else {
+          // Pull model: we are already on the UI thread, inside the dispatcher's loop executing
+          // a PullTransactionMountItem. We don't schedule the item, we execute it directly.
+          mountItem.execute(mMountingManager);
+        }
       } else {
         mMountItemDispatcher.addMountItem(mountItem);
         if (UiThreadUtil.isOnUiThread()) {

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerPullModelTest.kt

@@ -9,22 +9,30 @@
 
 package com.facebook.react.fabric
 
+import com.facebook.react.ReactRootView
 import com.facebook.react.bridge.ReactApplicationContext
 import com.facebook.react.bridge.ReactTestHelper
+import com.facebook.react.fabric.mounting.MountingManager
+import com.facebook.react.fabric.mounting.mountitems.MountItem
+import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
+import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsDefaults
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
+import com.facebook.react.uimanager.ThemedReactContext
 import com.facebook.react.uimanager.ViewManagerRegistry
 import com.facebook.testutils.fakes.FakeBatchEventDispatchedListener
 import com.facebook.testutils.shadows.ShadowFabricUIManagerBinding
 import com.facebook.testutils.shadows.ShadowNativeLoader
 import com.facebook.testutils.shadows.ShadowPerformanceTracer
 import com.facebook.testutils.shadows.ShadowSoLoader
 import org.assertj.core.api.Assertions.assertThat
+import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.annotation.Config
 import org.robolectric.shadow.api.Shadow
+import org.robolectric.util.ReflectionHelpers
 
 /**
  * Tests for the pull-model notification path: [FabricUIManager.onTransactionAvailable] enqueues a
@@ -55,6 +63,11 @@ class FabricUIManagerPullModelTest {
   @Before
   fun setup() {
     ReactNativeFeatureFlagsForTests.setUp()
+    ReactNativeFeatureFlags.override(
+        object : ReactNativeFeatureFlagsDefaults() {
+          override fun enableMountingCoordinatorPullModelAndroid(): Boolean = true
+        },
+    )
     reactContext = ReactTestHelper.createCatalystContextForTest()
     underTest =
         FabricUIManager(
@@ -67,6 +80,11 @@ class FabricUIManagerPullModelTest {
     underTest.setBinding(binding)
   }
 
+  @After
+  fun tearDown() {
+    ReactNativeFeatureFlags.dangerouslyReset()
+  }
+
   private fun runOnBackgroundThread(block: () -> Unit) {
     var error: Throwable? = null
     val thread = Thread {
@@ -81,6 +99,28 @@ class FabricUIManagerPullModelTest {
     error?.let { throw it }
   }
 
+  private fun scheduleSynchronously(mountItem: MountItem) {
+    val method =
+        FabricUIManager::class
+            .java
+            .getDeclaredMethod(
+                "scheduleMountItem",
+                MountItem::class.java,
+                Integer.TYPE,
+                java.lang.Long.TYPE,
+                java.lang.Long.TYPE,
+                java.lang.Long.TYPE,
+                java.lang.Long.TYPE,
+                java.lang.Long.TYPE,
+                java.lang.Long.TYPE,
+                java.lang.Long.TYPE,
+                Integer.TYPE,
+                java.lang.Boolean.TYPE,
+            )
+    method.isAccessible = true
+    method.invoke(underTest, mountItem, 0, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0, true)
+  }
+
   @Test
   fun onTransactionAvailable_onUiThread_pullsSynchronously() {
     underTest.onTransactionAvailable(1)
@@ -105,4 +145,31 @@ class FabricUIManagerPullModelTest {
 
     assertThat(shadowBinding.pulledSurfaceIds).containsExactly(1, 2, 1, 3)
   }
+
+  @Test
+  fun synchronousBatch_waitingForRootAttach_isDeferred() {
+    val surfaceId = 12
+    val themedReactContext = ThemedReactContext(reactContext, reactContext, "TestModule", surfaceId)
+    val mountingManager = ReflectionHelpers.getField<MountingManager>(underTest, "mMountingManager")
+    mountingManager.startSurface(surfaceId, themedReactContext, null)
+
+    var executionCount = 0
+    val mountItem =
+        object : MountItem {
+          override fun execute(mountingManager: MountingManager) {
+            executionCount++
+          }
+
+          override fun getSurfaceId(): Int = surfaceId
+        }
+
+    scheduleSynchronously(mountItem)
+
+    assertThat(executionCount).isZero()
+
+    mountingManager.attachRootView(surfaceId, ReactRootView(reactContext), themedReactContext)
+    underTest.onTransactionAvailable(surfaceId)
+
+    assertThat(executionCount).isEqualTo(1)
+  }
 }