Skip to content

Commit 6a62c9e

Browse files
heecheolmanclaude
andcommitted
Fix addViewAt crash when reparenting a view that is being animated out
Summary: When a view is moved to a new parent (for example because an intermediate view is flattened or unflattened), Fabric emits a Remove from the previous parent followed by an Insert into the new one. If the view is being animated out of its previous parent at that moment (it is in that parent's transitioning views, e.g. via LayoutTransition or startViewTransition), ViewGroup removes it from its children but keeps its parent pointer until the transition ends. addViewAt already detects that the view still has a parent and calls removeView() on it, but removeView() looks the view up in the children array, finds nothing, and silently returns. The following addView() then throws "The specified child already has a parent", which is rethrown as "addViewAt: failed to insert view [...]" and crashes the app. If the view is still attached to the same parent after removeView(), end the view transition on that parent, which clears the parent pointer, so the view can be inserted into its new parent. Changelog: [Android] [Fixed] - Fix `addViewAt: failed to insert view` crash when a view is reparented while being animated out of its previous parent Test Plan: Added SurfaceMountingManagerTest.addViewAt_reparentsViewThatIsTransitioningOutOfPreviousParent, which starts a view transition on the previous parent, removes the child, and inserts it into another parent. Without this change it fails with the same exception seen in production; with it, it passes. ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests 'com.facebook.react.fabric.*' Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent cd500a7 commit 6a62c9e

2 files changed

Lines changed: 35 additions & 0 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,12 @@ internal constructor(
361361
// View in the future.
362362
if (currParentView is ViewGroup) {
363363
currParentView.removeView(view)
364+
// If the view is being animated out of its previous parent (e.g. by a LayoutTransition),
365+
// ViewGroup has already dropped it from its children but keeps its parent pointer until
366+
// the transition ends, so removeView above is a no-op. End the transition to detach it.
367+
if (view.parent === currParentView) {
368+
currParentView.endViewTransition(view)
369+
}
364370
}
365371
erroneouslyReaddedReactTags.add(tag)
366372
}

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
package com.facebook.react.fabric
1111

12+
import android.view.ViewGroup
1213
import com.facebook.react.ReactRootView
1314
import com.facebook.react.bridge.JavaOnlyMap
1415
import com.facebook.react.bridge.ReactTestHelper
@@ -192,4 +193,32 @@ class SurfaceMountingManagerTest {
192193
smm.preallocateView("RCTView", 42, JavaOnlyMap.of(), null, true)
193194
assertThat(smm.getViewExists(42)).isTrue()
194195
}
196+
197+
/**
198+
* When a child is moved between parents (e.g. because an intermediate view is flattened or
199+
* unflattened) while it is still being animated out of its previous parent, [ViewGroup] removes
200+
* it from its children but keeps its parent pointer until the transition ends. addViewAt must
201+
* still be able to insert it into the new parent.
202+
*/
203+
@Test
204+
fun addViewAt_reparentsViewThatIsTransitioningOutOfPreviousParent() {
205+
val smm = startSurface()
206+
smm.createView("RCTView", 10, JavaOnlyMap.of(), null, null, true)
207+
smm.createView("RCTView", 12, JavaOnlyMap.of(), null, null, true)
208+
smm.createView("RCTView", 42, JavaOnlyMap.of(), null, null, true)
209+
smm.addViewAt(surfaceId, 10, 0)
210+
smm.addViewAt(surfaceId, 12, 1)
211+
smm.addViewAt(10, 42, 0)
212+
213+
val previousParent = smm.getView(10) as ViewGroup
214+
val view = smm.getView(42)
215+
previousParent.startViewTransition(view)
216+
smm.removeViewAt(42, 10, 0)
217+
assertThat(previousParent.indexOfChild(view)).isEqualTo(-1)
218+
assertThat(view.parent).isSameAs(previousParent)
219+
220+
smm.addViewAt(12, 42, 0)
221+
222+
assertThat(view.parent).isSameAs(smm.getView(12))
223+
}
195224
}

0 commit comments

Comments
 (0)