Keep subview-clipping bookkeeping in sync when a tracked child is reparented or recycled (#57371)

Summary:

When a view tracked by a clipping-enabled parent (`removeClippedSubviews`) is detached from that parent with a raw `ViewGroup.removeView`, the parent's internal `allChildren` bookkeeping is not updated — that array is only kept in sync through the parent's clipping-aware removal path. The parent is then left holding a stale reference to a view that has been reparented elsewhere, and a later clipping pass asserts the child is still parented to it and throws an `IllegalStateException`, crashing the app.

This change introduces a small shared helper, `ClippingAwareViewRemover.removeFromParent(view)`, which removes a view from its current parent through the clipping-aware path when that parent is a clipping-enabled `ReactViewGroup`, and otherwise falls back to a plain `removeView`. All recycle/reparent sites that previously did a raw self-detach now route through it:

- `ReactViewGroup.recycleView`
- `ReactScrollView.recycleView` / `ReactHorizontalScrollView.recycleView` / `ReactNestedScrollView.recycleView` (the nested variant is generated from `ReactScrollView` via `generate-nested-scroll-view.js`; the generated file is regenerated here)
- `ReactTextView.recycleView`

Also fixes the Fabric mounting layer's reparent recovery in `SurfaceMountingManager.addViewAt` (the "View already has a parent" evasive path) to remove the view via the parent's `ViewManager` rather than a raw `removeView`, and hardens `ReactViewGroup.removeViewWithSubviewClippingEnabled` to no-op safely when the view is not tracked in `allChildren` (it previously indexed `allChildren[-1]`).

Behavior is unchanged for non-clipping parents, which still use the plain `removeView`.

Changelog:
[Android][Fixed] - Fix rare crash in views using `removeClippedSubviews` when a tracked child is reparented or recycled

Differential Revision: D110059699

Author: rozele

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactSoftExceptionLogger.kt

@@ -14,6 +14,7 @@ import com.facebook.react.bridge.ReactSoftExceptionLogger.Categories.CLIPPING_PR
 import com.facebook.react.bridge.ReactSoftExceptionLogger.Categories.RVG_ADD_CHILDREN_FOR_ACCESSIBILITY
 import com.facebook.react.bridge.ReactSoftExceptionLogger.Categories.RVG_IS_VIEW_CLIPPED
 import com.facebook.react.bridge.ReactSoftExceptionLogger.Categories.RVG_ON_VIEW_REMOVED
+import com.facebook.react.bridge.ReactSoftExceptionLogger.Categories.RVG_REMOVE_VIEW_NOT_IN_ALL_CHILDREN
 import com.facebook.react.bridge.ReactSoftExceptionLogger.Categories.SOFT_ASSERTIONS
 import com.facebook.react.bridge.ReactSoftExceptionLogger.Categories.SURFACE_MOUNTING_MANAGER_MISSING_VIEWSTATE
 import java.util.concurrent.CopyOnWriteArrayList
@@ -25,6 +26,7 @@ internal object ReactSoftExceptionLogger {
       RVG_ADD_CHILDREN_FOR_ACCESSIBILITY,
       RVG_IS_VIEW_CLIPPED,
       RVG_ON_VIEW_REMOVED,
+      RVG_REMOVE_VIEW_NOT_IN_ALL_CHILDREN,
       CLIPPING_PROHIBITED_VIEW,
       SOFT_ASSERTIONS,
       SURFACE_MOUNTING_MANAGER_MISSING_VIEWSTATE,
@@ -37,6 +39,8 @@ internal object ReactSoftExceptionLogger {
         "ReactViewGroup.addChildrenForAccessibility"
     const val RVG_IS_VIEW_CLIPPED: String = "ReactViewGroup.isViewClipped"
     const val RVG_ON_VIEW_REMOVED: String = "ReactViewGroup.onViewRemoved"
+    const val RVG_REMOVE_VIEW_NOT_IN_ALL_CHILDREN: String =
+        "ReactViewGroup.removeViewWithSubviewClippingEnabled:NotInAllChildren"
     const val CLIPPING_PROHIBITED_VIEW: String = "ReactClippingProhibitedView"
     const val SOFT_ASSERTIONS: String = "SoftAssertions"
     const val SURFACE_MOUNTING_MANAGER_MISSING_VIEWSTATE: String =

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt

@@ -360,7 +360,7 @@ internal constructor(
       // thus prevent the RemoveDeleteTree worker from deleting this
       // View in the future.
       if (currParentView is ViewGroup) {
-        currParentView.removeView(view)
+        removeViewFromCurrentParent(currParentView, view)
       }
       erroneouslyReaddedReactTags.add(tag)
     }
@@ -395,6 +395,33 @@ internal constructor(
     }
   }
 
+  /**
+   * Removes [view] from [currentParent] as part of [addViewAt]'s evasive recovery, keeping any
+   * subview-clipping bookkeeping in sync.
+   *
+   * A raw [ViewGroup.removeView] only detaches the view from the Android hierarchy. When
+   * [currentParent] manages subview clipping (`removeClippedSubviews`), it also keeps an internal
+   * `allChildren` array that is only updated through its [ViewManager]'s removal path; a raw
+   * removal leaves a stale entry there pointing at a view that has just been reparented elsewhere,
+   * and a later clipping pass then trips the parent's `parent === this` invariant and crashes.
+   * Routing the removal through the parent's [ViewManager] (when its [ViewState] is known) reuses
+   * the clipping-aware removal and keeps the bookkeeping consistent; otherwise we fall back to the
+   * raw removal.
+   */
+  private fun removeViewFromCurrentParent(currentParent: ViewGroup, view: View) {
+    val currentParentState = getNullableViewState(currentParent.id)
+    if (currentParentState?.viewManager is IViewGroupManager<*>) {
+      val manager = getViewGroupManager(currentParentState)
+      for (i in 0 until manager.getChildCount(currentParent)) {
+        if (manager.getChildAt(currentParent, i) === view) {
+          manager.removeViewAt(currentParent, i)
+          return
+        }
+      }
+    }
+    currentParent.removeView(view)
+  }
+
   @UiThread
   public fun removeViewAt(tag: Int, parentTag: Int, index: Int): Unit {
     UiThreadUtil.assertOnUiThread()

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.kt

@@ -33,6 +33,7 @@ import androidx.core.view.isNotEmpty
 import com.facebook.common.logging.FLog
 import com.facebook.react.R
 import com.facebook.react.common.ReactConstants
+import com.facebook.react.common.annotations.UnstableReactNativeAPI
 import com.facebook.react.common.build.ReactBuildConfig
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
 import com.facebook.react.uimanager.BackgroundStyleApplicator
@@ -61,6 +62,7 @@ import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_DISA
 import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_END
 import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_START
 import com.facebook.react.views.scroll.ReactScrollViewHelper.findNextFocusableView
+import com.facebook.react.views.view.ClippingAwareViewRemover
 import com.facebook.systrace.Systrace
 import kotlin.math.abs
 import kotlin.math.ceil
@@ -287,11 +289,12 @@ constructor(context: Context, private val fpsListener: FpsListener? = null) :
     scrollsChildToFocus = true
   }
 
+  @OptIn(UnstableReactNativeAPI::class)
   internal fun recycleView() {
     initView()
-    if (parent != null) {
-      (parent as ViewGroup).removeView(this)
-    }
+    // Route through the clipping-aware helper so a clipping parent does not retain a stale
+    // `allChildren` reference to this recycled view.
+    ClippingAwareViewRemover.removeFromParent(this)
     updateView()
   }
 

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactNestedScrollView.kt

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<2b0cbc5249ac34ae7f030a9c0fffd1f3>>
+ * @generated SignedSource<<83e5136ed4b37922bd298d3036fae133>>
  */
 
 /**
@@ -36,6 +36,7 @@ import com.facebook.common.logging.FLog
 import com.facebook.react.R
 import com.facebook.react.bridge.ReadableMap
 import com.facebook.react.common.ReactConstants
+import com.facebook.react.common.annotations.UnstableReactNativeAPI
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
 import com.facebook.react.uimanager.BackgroundStyleApplicator
 import com.facebook.react.uimanager.LengthPercentage
@@ -63,6 +64,7 @@ import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_DISA
 import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_END
 import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_START
 import com.facebook.react.views.scroll.ReactScrollViewHelper.findNextFocusableView
+import com.facebook.react.views.view.ClippingAwareViewRemover
 import com.facebook.systrace.Systrace
 import kotlin.math.abs
 import kotlin.math.ceil
@@ -263,11 +265,12 @@ constructor(context: Context, private val fpsListener: FpsListener? = null) :
     scrollsChildToFocus = true
   }
 
+  @OptIn(UnstableReactNativeAPI::class)
   internal fun recycleView() {
     initView()
-    if (parent != null) {
-      (parent as ViewGroup).removeView(this)
-    }
+    // Route through the clipping-aware helper so a clipping parent does not retain a stale
+    // `allChildren` reference to this recycled view.
+    ClippingAwareViewRemover.removeFromParent(this)
     updateView()
   }
 

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.kt

@@ -28,6 +28,7 @@ import com.facebook.common.logging.FLog
 import com.facebook.react.R
 import com.facebook.react.bridge.ReadableMap
 import com.facebook.react.common.ReactConstants
+import com.facebook.react.common.annotations.UnstableReactNativeAPI
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
 import com.facebook.react.uimanager.BackgroundStyleApplicator
 import com.facebook.react.uimanager.LengthPercentage
@@ -55,6 +56,7 @@ import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_DISA
 import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_END
 import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_START
 import com.facebook.react.views.scroll.ReactScrollViewHelper.findNextFocusableView
+import com.facebook.react.views.view.ClippingAwareViewRemover
 import com.facebook.systrace.Systrace
 import kotlin.math.abs
 import kotlin.math.ceil
@@ -255,11 +257,12 @@ constructor(context: Context, private val fpsListener: FpsListener? = null) :
     scrollsChildToFocus = true
   }
 
+  @OptIn(UnstableReactNativeAPI::class)
   internal fun recycleView() {
     initView()
-    if (parent != null) {
-      (parent as ViewGroup).removeView(this)
-    }
+    // Route through the clipping-aware helper so a clipping parent does not retain a stale
+    // `allChildren` reference to this recycled view.
+    ClippingAwareViewRemover.removeFromParent(this)
     updateView()
   }
 

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java

@@ -49,6 +49,7 @@
 import com.facebook.react.views.text.internal.span.CanvasEffectSpan;
 import com.facebook.react.views.text.internal.span.ReactFragmentIndexSpan;
 import com.facebook.react.views.text.internal.span.ReactTagSpan;
+import com.facebook.react.views.view.ClippingAwareViewRemover;
 import com.facebook.yoga.YogaMeasureMode;
 
 @Nullsafe(Nullsafe.Mode.LOCAL)
@@ -104,10 +105,9 @@ private void initView() {
     initView();
 
     // If the view is still attached to a parent, we need to remove it from the parent
-    // before we can recycle it.
-    if (getParent() != null) {
-      ((ViewGroup) getParent()).removeView(this);
-    }
+    // before we can recycle it. Route through the clipping-aware helper so a clipping parent
+    // does not retain a stale `allChildren` reference to this recycled view.
+    ClippingAwareViewRemover.removeFromParent(this);
 
     BackgroundStyleApplicator.reset(this);
 

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ClippingAwareViewRemover.kt

@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+
+package com.facebook.react.views.view
+
+import android.view.View
+import android.view.ViewGroup
+import com.facebook.react.common.annotations.UnstableReactNativeAPI
+
+/**
+ * Helper for detaching a [View] from its current parent while keeping subview-clipping bookkeeping
+ * consistent.
+ *
+ * When a parent [ReactViewGroup] has `removeClippedSubviews` enabled it maintains an internal
+ * `allChildren` array that is only kept in sync through its clipping-aware removal path. A raw
+ * [ViewGroup.removeView] detaches the child from the Android hierarchy but leaves a stale entry in
+ * that array pointing at a view that may then be re-mounted under another parent, which later trips
+ * the parent's `parent === this` clipping invariant and crashes. Recycling and reparenting paths
+ * that detach a view from an arbitrary parent should route through here instead of calling
+ * [ViewGroup.removeView] directly.
+ */
+@UnstableReactNativeAPI
+public object ClippingAwareViewRemover {
+  /**
+   * Removes [view] from its current parent. If the parent is a clipping-enabled [ReactViewGroup],
+   * the removal first goes through the clipping-aware path so `allChildren` stays in sync. Then, if
+   * the view is still attached — a non-clipping parent, or a child not tracked in the clipping
+   * bookkeeping — it is detached with a plain [ViewGroup.removeView]. No-op if [view] has no
+   * [ViewGroup] parent.
+   */
+  @JvmStatic
+  public fun removeFromParent(view: View) {
+    val parent = view.parent
+    if (parent is ReactViewGroup && parent.removeClippedSubviews) {
+      parent.removeViewWithSubviewClippingEnabled(view)
+    }
+    // `removeViewWithSubviewClippingEnabled` detaches tracked children itself; this handles the
+    // non-clipping parent and the untracked-child cases (and is a no-op once already detached).
+    (view.parent as? ViewGroup)?.removeView(view)
+  }
+}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.kt

@@ -33,6 +33,7 @@ import com.facebook.react.bridge.ReadableMap
 import com.facebook.react.bridge.UiThreadUtil.assertOnUiThread
 import com.facebook.react.bridge.UiThreadUtil.runOnUiThread
 import com.facebook.react.common.ReactConstants.TAG
+import com.facebook.react.common.annotations.UnstableReactNativeAPI
 import com.facebook.react.config.ReactFeatureFlags
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
 import com.facebook.react.touch.OnInterceptTouchEventListener
@@ -188,6 +189,7 @@ public open class ReactViewGroup public constructor(context: Context?) :
     nativeForegroundMap = null
   }
 
+  @OptIn(UnstableReactNativeAPI::class)
   internal open fun recycleView() {
     recycleCount++
 
@@ -208,10 +210,9 @@ public open class ReactViewGroup public constructor(context: Context?) :
     removeAllViews()
 
     // If the view is still attached to a parent, we need to remove it from the parent
-    // before we can recycle it.
-    if (parent != null) {
-      (parent as ViewGroup).removeView(this)
-    }
+    // before we can recycle it. Route through the clipping-aware helper so a clipping parent
+    // does not retain a stale `allChildren` reference to this recycled view.
+    ClippingAwareViewRemover.removeFromParent(this)
 
     // Reset background, borders
     updateBackgroundDrawable(null)
@@ -712,6 +713,20 @@ public open class ReactViewGroup public constructor(context: Context?) :
     val allChildren = checkNotNull(allChildren)
     view.removeOnLayoutChangeListener(childrenLayoutChangeListener)
     val index = indexOfChildInAllChildren(view)
+    if (index < 0) {
+      // The view is not tracked in allChildren (e.g. it was added before clipping was enabled or
+      // never routed through the clipping-aware add). Return instead of indexing allChildren[-1];
+      // there is nothing to remove from the clipping bookkeeping and detaching the view is the
+      // caller's responsibility. Logged because we are unsure how often this happens in practice
+      // and want to measure its frequency.
+      logSoftException(
+          ReactSoftExceptionLogger.Categories.RVG_REMOVE_VIEW_NOT_IN_ALL_CHILDREN,
+          ReactNoCrashSoftException(
+              "removeViewWithSubviewClippingEnabled: view not in allChildren. parentThis=${view.parent === this} allChildrenCount=$allChildrenCount recycleCount=$recycleCount"
+          ),
+      )
+      return
+    }
     if (!isViewClipped(allChildren[index], index)) {
       var clippedSoFar = 0
       for (i in 0..<index) {

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/view/ReactViewGroupTest.kt

@@ -12,6 +12,7 @@ import android.content.Context
 import android.view.View
 import android.view.ViewGroup
 import android.widget.FrameLayout
+import com.facebook.react.common.annotations.UnstableReactNativeAPI
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Before
@@ -20,6 +21,7 @@ import org.junit.runner.RunWith
 import org.robolectric.Robolectric
 import org.robolectric.RobolectricTestRunner
 
+@OptIn(UnstableReactNativeAPI::class)
 @RunWith(RobolectricTestRunner::class)
 class ReactViewGroupTest {
 
@@ -63,6 +65,83 @@ class ReactViewGroupTest {
     rvg.updateClippingRect()
     assertThat(rvg.childCount).isEqualTo(20)
   }
+
+  @Test
+  fun `ClippingAwareViewRemover - removes a tracked child from a clipping parent without leaving a stale entry`() {
+    val rvg = ReactViewGroup(context)
+    rvg.left = 0
+    rvg.right = 100
+    rvg.top = 0
+    rvg.bottom = 100
+    FrameLayout(context).addView(rvg)
+    rvg.removeClippedSubviews = true
+    val child = TestView(context, 0)
+    rvg.addViewWithSubviewClippingEnabled(child, 0)
+    rvg.updateClippingRect()
+    assertThat(rvg.allChildrenCount).isEqualTo(1)
+    assertThat(child.parent).isSameAs(rvg)
+
+    ClippingAwareViewRemover.removeFromParent(child)
+
+    // The clipping bookkeeping stays in sync (no stale allChildren entry) ...
+    assertThat(rvg.allChildrenCount).isEqualTo(0)
+    assertThat(child.parent).isNull()
+    // ... so a subsequent clipping pass does not trip the invalid-clipping-state invariant.
+    rvg.updateClippingRect()
+  }
+
+  @Test
+  fun `ClippingAwareViewRemover - falls back to a plain removeView for a non-clipping parent`() {
+    val parent = ReactViewGroup(context)
+    val child = TestView(context, 0)
+    parent.addView(child)
+    assertThat(child.parent).isSameAs(parent)
+
+    ClippingAwareViewRemover.removeFromParent(child)
+
+    assertThat(child.parent).isNull()
+    assertThat(parent.childCount).isEqualTo(0)
+  }
+
+  @Test
+  fun `removeViewWithSubviewClippingEnabled - is a no-op for a view not tracked in allChildren`() {
+    val rvg = ReactViewGroup(context)
+    rvg.left = 0
+    rvg.right = 100
+    rvg.top = 0
+    rvg.bottom = 100
+    FrameLayout(context).addView(rvg)
+    rvg.removeClippedSubviews = true
+
+    // A view that was never added through the clipping-aware path is not in allChildren; removing
+    // it must not index allChildren[-1] / throw (it logs a soft exception and returns instead).
+    rvg.removeViewWithSubviewClippingEnabled(TestView(context, 0))
+
+    assertThat(rvg.allChildrenCount).isEqualTo(0)
+  }
+
+  @Test
+  fun `ClippingAwareViewRemover - detaches an attached child that is not tracked in allChildren`() {
+    val rvg = ReactViewGroup(context)
+    rvg.left = 0
+    rvg.right = 100
+    rvg.top = 0
+    rvg.bottom = 100
+    FrameLayout(context).addView(rvg)
+    rvg.removeClippedSubviews = true
+    // Add through the raw ViewGroup API so the child is a real child but is NOT tracked in
+    // allChildren (the clipping-aware removal can't find it).
+    val child = TestView(context, 0)
+    rvg.addView(child)
+    assertThat(child.parent).isSameAs(rvg)
+    assertThat(rvg.allChildrenCount).isEqualTo(0)
+
+    ClippingAwareViewRemover.removeFromParent(child)
+
+    // The clipping-aware removal returns early (untracked); the helper's removeView fallback still
+    // detaches the child.
+    assertThat(child.parent).isNull()
+  }
 }
 
 class TestView(context: Context, yPos: Int) : View(context) {