diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp index 95ddd80e6d9c..0cdf3513c3b6 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp @@ -1026,6 +1026,20 @@ void FabricMountingManager::destroyUnmountedShadowNode( auto tag = family.getTag(); auto surfaceId = family.getSurfaceId(); + // Remove from allocatedViewRegistry so that executeMount does not skip + // the Create mount item for this tag. Without this, if the view was + // preallocated and then destroyed (e.g. due to a superseded concurrent + // render), executeMount would skip the Create because allocatedViewTags + // still contains the tag, but the Java side no longer has the view in + // tagToViewState (it was deleted by destroyUnmountedView below). + { + std::lock_guard allocatedViewsLock(allocatedViewsMutex_); + auto allocatedViewsIterator = allocatedViewRegistry_.find(surfaceId); + if (allocatedViewsIterator != allocatedViewRegistry_.end()) { + allocatedViewsIterator->second.erase(tag); + } + } + // ThreadScope::WithClassLoader is necessary because // destroyUnmountedShadowNode is being called from a destructor thread jni::ThreadScope::WithClassLoader([&]() { diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/SurfaceMountingManagerTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/SurfaceMountingManagerTest.kt new file mode 100644 index 000000000000..be7b6f0c9d22 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/SurfaceMountingManagerTest.kt @@ -0,0 +1,118 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +@file:Suppress("DEPRECATION") + +package com.facebook.react.fabric + +import com.facebook.react.ReactRootView +import com.facebook.react.bridge.JavaOnlyMap +import com.facebook.react.bridge.ReactTestHelper +import com.facebook.react.fabric.mounting.MountingManager +import com.facebook.react.fabric.mounting.MountingManager.MountItemExecutor +import com.facebook.react.fabric.mounting.SurfaceMountingManager +import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests +import com.facebook.react.uimanager.ThemedReactContext +import com.facebook.react.uimanager.ViewManager +import com.facebook.react.uimanager.ViewManagerRegistry +import com.facebook.react.views.view.ReactViewManager +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Tests for [SurfaceMountingManager] view lifecycle around preallocateView, deleteView, and + * createView. + * + * The key scenario is the interaction between view preallocation and destroyUnmountedShadowNode + * (D98729251): when a preallocated view is destroyed (e.g. from a superseded concurrent render), + * and then a new Create mount item is emitted for the same tag, the view must be recreated. + * + * NOTE: These tests verify Java-side behavior only. The C++ fix in FabricMountingManager (erasing + * from allocatedViewRegistry_ in destroyUnmountedShadowNode) ensures the Create mount item is + * emitted in the first place. That behavior requires native code and is not testable via + * Robolectric. + * + * See T223288217. + */ +@RunWith(RobolectricTestRunner::class) +class SurfaceMountingManagerTest { + private lateinit var mountingManager: MountingManager + private lateinit var themedReactContext: ThemedReactContext + private val surfaceId = 1 + + @Before + fun setUp() { + ReactNativeFeatureFlagsForTests.setUp() + val reactContext = ReactTestHelper.createCatalystContextForTest() + themedReactContext = ThemedReactContext(reactContext, reactContext, null, -1) + mountingManager = + MountingManager( + ViewManagerRegistry(listOf>(ReactViewManager())), + MountItemExecutor {}, + ) + } + + private fun startSurface(): SurfaceMountingManager { + val rootView = ReactRootView(themedReactContext) + mountingManager.startSurface(surfaceId, themedReactContext, rootView) + return mountingManager.getSurfaceManagerEnforced(surfaceId, "test") + } + + @Test + fun preallocateView_createsView() { + val smm = startSurface() + smm.preallocateView("RCTView", 42, JavaOnlyMap.of(), null, true) + assertThat(smm.getViewExists(42)).isTrue() + } + + @Test + fun deleteView_removesPreallocatedView() { + val smm = startSurface() + smm.preallocateView("RCTView", 42, JavaOnlyMap.of(), null, true) + smm.deleteView(42) + assertThat(smm.getViewExists(42)).isFalse() + } + + /** + * Verifies the Java side of the scenario fixed by D98729251: + * 1. preallocateView creates the view + * 2. deleteView removes it (simulating destroyUnmountedShadowNode) + * 3. createView recreates it (the C++ fix ensures this Create is actually emitted) + * 4. addViewAt succeeds with the recreated view + */ + @Test + fun createView_succeedsAfterPreallocateAndDelete() { + val smm = startSurface() + + smm.preallocateView("RCTView", 42, JavaOnlyMap.of(), null, true) + assertThat(smm.getViewExists(42)).isTrue() + + smm.deleteView(42) + assertThat(smm.getViewExists(42)).isFalse() + + smm.createView("RCTView", 42, JavaOnlyMap.of(), null, null, true) + assertThat(smm.getViewExists(42)).isTrue() + + smm.addViewAt(surfaceId, 42, 0) + assertThat(smm.getView(42)).isNotNull() + } + + /** createView is a no-op when the view already exists from preallocate (normal case). */ + @Test + fun createView_isIdempotentWhenPreallocated() { + val smm = startSurface() + + smm.preallocateView("RCTView", 42, JavaOnlyMap.of(), null, true) + val viewBefore = smm.getView(42) + + smm.createView("RCTView", 42, JavaOnlyMap.of(), null, null, true) + assertThat(smm.getView(42)).isSameAs(viewBefore) + } +}