Skip to content

Fix addViewAt crash when reparenting a view that is being animated out #59364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
heecheolman wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix addViewAt crash when reparenting a view that is being animated out #59364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ...e/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,12 @@ internal constructor(
// View in the future.
if (currParentView is ViewGroup) {
currParentView.removeView(view)
// If the view is being animated out of its previous parent (e.g. by a LayoutTransition),
// ViewGroup has already dropped it from its children but keeps its parent pointer until
// the transition ends, so removeView above is a no-op. End the transition to detach it.
if (view.parent === currParentView) {
currParentView.endViewTransition(view)
}
}
erroneouslyReaddedReactTags.add(tag)
}
Expand Down
29 changes: 29 additions & 0 deletions ...native/ReactAndroid/src/test/java/com/facebook/react/fabric/SurfaceMountingManagerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

package com.facebook.react.fabric

import android.view.ViewGroup
import com.facebook.react.ReactRootView
import com.facebook.react.bridge.JavaOnlyMap
import com.facebook.react.bridge.ReactTestHelper
Expand Down Expand Up @@ -192,4 +193,32 @@ class SurfaceMountingManagerTest {
smm.preallocateView("RCTView", 42, JavaOnlyMap.of(), null, true)
assertThat(smm.getViewExists(42)).isTrue()
}

/**
* When a child is moved between parents (e.g. because an intermediate view is flattened or
* unflattened) while it is still being animated out of its previous parent, [ViewGroup] removes
* it from its children but keeps its parent pointer until the transition ends. addViewAt must
* still be able to insert it into the new parent.
*/
@Test
fun addViewAt_reparentsViewThatIsTransitioningOutOfPreviousParent() {
val smm = startSurface()
smm.createView("RCTView", 10, JavaOnlyMap.of(), null, null, true)
smm.createView("RCTView", 12, JavaOnlyMap.of(), null, null, true)
smm.createView("RCTView", 42, JavaOnlyMap.of(), null, null, true)
smm.addViewAt(surfaceId, 10, 0)
smm.addViewAt(surfaceId, 12, 1)
smm.addViewAt(10, 42, 0)

val previousParent = smm.getView(10) as ViewGroup
val view = smm.getView(42)
previousParent.startViewTransition(view)
smm.removeViewAt(42, 10, 0)
assertThat(previousParent.indexOfChild(view)).isEqualTo(-1)
assertThat(view.parent).isSameAs(previousParent)

smm.addViewAt(12, 42, 0)

assertThat(view.parent).isSameAs(smm.getView(12))
}
}
Loading