Skip to content

Commit b0f823c

Browse files
rozelefacebook-github-bot
authored andcommitted
Keep subview-clipping bookkeeping in sync when addViewAt reparents a view
Summary: When the Fabric mounting layer inserts a view that unexpectedly still has a parent, `SurfaceMountingManager.addViewAt` takes evasive action and removes the view from its current parent before re-adding it to the new one. That recovery used a raw `ViewGroup.removeView`, which detaches the view from the Android view hierarchy but does not update a clipping-enabled parent's internal child bookkeeping (the `allChildren` array maintained while `removeClippedSubviews` is set). As a result, the old parent kept a stale reference to a view that had just been reparented elsewhere. A later clipping pass on that parent then asserted the child was still parented to it and threw an `IllegalStateException`, crashing the app. Route the evasive removal through the current parent's `ViewManager` (when its view state is known) so the clipping-aware removal path runs and the `allChildren` bookkeeping stays consistent. Behavior is unchanged for non-clipping parents, which still fall back to the raw `removeView`. Changelog: [Android][Fixed] - Fix rare crash in views using `removeClippedSubviews` when a child is reparented during mounting Differential Revision: D110059699
1 parent 3fcde34 commit b0f823c

1 file changed

Lines changed: 28 additions & 1 deletion

File tree

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ internal constructor(
360360
// thus prevent the RemoveDeleteTree worker from deleting this
361361
// View in the future.
362362
if (currParentView is ViewGroup) {
363-
currParentView.removeView(view)
363+
removeViewFromCurrentParent(currParentView, view)
364364
}
365365
erroneouslyReaddedReactTags.add(tag)
366366
}
@@ -395,6 +395,33 @@ internal constructor(
395395
}
396396
}
397397

398+
/**
399+
* Removes [view] from [currentParent] as part of [addViewAt]'s evasive recovery, keeping any
400+
* subview-clipping bookkeeping in sync.
401+
*
402+
* A raw [ViewGroup.removeView] only detaches the view from the Android hierarchy. When
403+
* [currentParent] manages subview clipping (`removeClippedSubviews`), it also keeps an internal
404+
* `allChildren` array that is only updated through its [ViewManager]'s removal path; a raw
405+
* removal leaves a stale entry there pointing at a view that has just been reparented elsewhere,
406+
* and a later clipping pass then trips the parent's `parent === this` invariant and crashes.
407+
* Routing the removal through the parent's [ViewManager] (when its [ViewState] is known) reuses
408+
* the clipping-aware removal and keeps the bookkeeping consistent; otherwise we fall back to the
409+
* raw removal.
410+
*/
411+
private fun removeViewFromCurrentParent(currentParent: ViewGroup, view: View) {
412+
val currentParentState = getNullableViewState(currentParent.id)
413+
if (currentParentState?.viewManager is IViewGroupManager<*>) {
414+
val manager = getViewGroupManager(currentParentState)
415+
for (i in 0 until manager.getChildCount(currentParent)) {
416+
if (manager.getChildAt(currentParent, i) === view) {
417+
manager.removeViewAt(currentParent, i)
418+
return
419+
}
420+
}
421+
}
422+
currentParent.removeView(view)
423+
}
424+
398425
@UiThread
399426
public fun removeViewAt(tag: Int, parentTag: Int, index: Int): Unit {
400427
UiThreadUtil.assertOnUiThread()

0 commit comments

Comments
 (0)