Skip to content

Commit 11895bc

Browse files
hannojghannojg
committed
fix(android): fix ViewGroupManager addView crash if in transition
1 parent 5873cd0 commit 11895bc

File tree

2 files changed

+102
-93
lines changed

2 files changed

+102
-93
lines changed

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.kt‎

Lines changed: 96 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -9,79 +9,119 @@ package com.facebook.react.uimanager
99

1010
import android.view.View
1111
import android.view.ViewGroup
12+
import androidx.core.view.doOnDetach
13+
import com.facebook.common.logging.FLog
1214
import com.facebook.react.bridge.ReactApplicationContext
1315
import com.facebook.react.bridge.UiThreadUtil
1416
import java.util.WeakHashMap
1517

18+
@Suppress("DEPRECATION")
1619
public abstract class ViewGroupManager<T : ViewGroup>
1720
@JvmOverloads
1821
constructor(reactContext: ReactApplicationContext? = null) :
1922
BaseViewManager<T, LayoutShadowNode>(reactContext), IViewGroupManager<T> {
2023

21-
public override fun createShadowNodeInstance(): LayoutShadowNode = LayoutShadowNode()
22-
23-
public override fun getShadowNodeClass(): Class<out LayoutShadowNode> =
24-
LayoutShadowNode::class.java
24+
public override fun createShadowNodeInstance(): LayoutShadowNode = LayoutShadowNode()
25+
26+
public override fun getShadowNodeClass(): Class<out LayoutShadowNode> =
27+
LayoutShadowNode::class.java
28+
29+
public override fun updateExtraData(root: T, extraData: Any): Unit = Unit
30+
31+
// parent: childIndex[] - Used when we can't immediately add a view
32+
protected val operationsMap: WeakHashMap<T, MutableMap<Int, Boolean>> = WeakHashMap()
33+
34+
public fun addViewSafely(parent: T, child: View, index: Int, callback: () -> Unit) {
35+
UiThreadUtil.assertOnUiThread()
36+
37+
if (child.parent == null) {
38+
callback()
39+
return
40+
}
41+
42+
operationsMap.getOrPut(parent) {
43+
mutableMapOf()
44+
}[index] = true
45+
46+
// When the child-parent relation is removed, onDetachedFromWindow will be called.
47+
// Its important to wait for detaching as the view might be in a transition, and isn't removed immediately.
48+
child.doOnDetach {
49+
// Looking at how endViewTransition is implemented, dispatchDetachedFromWindow
50+
// gets called _before_ the parent relation is removed, so we need to post this to the end of the frame:
51+
child.post {
52+
if(operationsMap.remove(parent) == null) {
53+
// The addView operation was already countered by a removeView operation while we were waiting
54+
FLog.w("ReactClippingViewManager", "Tried to add a view to a parent after the child was detached, but a remove operation was already enqueued")
55+
return@post
56+
}
57+
FLog.w("ReactClippingViewManager", "addView(): ${child::class.java.simpleName} had a parent, removed from previous parent and after onDetach adding to new parent $parent")
58+
callback()
59+
}
60+
}
61+
62+
// With the detach listener in place, we can now remove the view from the previous parent:
63+
// Note: This call here is potentially redundant, as SurfaceMountingManager.kt is already removing it
64+
(child.parent as? ViewGroup)?.removeView(child)
65+
}
2566

26-
public override fun updateExtraData(root: T, extraData: Any): Unit = Unit
67+
public override fun addView(parent: T, child: View, index: Int): Unit =
68+
addViewSafely(parent, child, index) { parent.addView(child, index) }
69+
70+
/**
71+
* Convenience method for batching a set of addView calls Note that this adds the views to the
72+
* beginning of the ViewGroup
73+
*
74+
* @param parent the parent ViewGroup
75+
* @param views the set of views to add
76+
*/
77+
public fun addViews(parent: T, views: List<View>) {
78+
UiThreadUtil.assertOnUiThread()
79+
views.forEachIndexed { i, view -> addView(parent, view, i) }
80+
}
2781

28-
public override fun addView(parent: T, child: View, index: Int): Unit =
29-
parent.addView(child, index)
82+
public override fun getChildCount(parent: T): Int = parent.childCount
3083

31-
/**
32-
* Convenience method for batching a set of addView calls Note that this adds the views to the
33-
* beginning of the ViewGroup
34-
*
35-
* @param parent the parent ViewGroup
36-
* @param views the set of views to add
37-
*/
38-
public fun addViews(parent: T, views: List<View>) {
39-
UiThreadUtil.assertOnUiThread()
40-
views.forEachIndexed { i, view -> addView(parent, view, i) }
41-
}
84+
public override fun getChildAt(parent: T, index: Int): View? = parent.getChildAt(index)
4285

43-
public override fun getChildCount(parent: T): Int = parent.childCount
86+
public override fun removeViewAt(parent: T, index: Int) {
87+
UiThreadUtil.assertOnUiThread()
88+
parent.removeViewAt(index)
89+
operationsMap[parent]?.remove(index)
90+
}
4491

45-
public override fun getChildAt(parent: T, index: Int): View? = parent.getChildAt(index)
92+
/**
93+
* Expo overrides this function GroupViewManagerWrapper.kt`, which is a replacement view manager
94+
* adding support for delegates receiving callbacks whenever one of the methods in the view
95+
* manager are called.
96+
*/
97+
public open fun removeView(parent: T, view: View) {
98+
UiThreadUtil.assertOnUiThread()
99+
100+
for (i in 0 until getChildCount(parent)) {
101+
if (getChildAt(parent, i) === view) {
102+
103+
removeViewAt(parent, i)
104+
break
105+
}
106+
}
107+
}
46108

47-
public override fun removeViewAt(parent: T, index: Int) {
48-
UiThreadUtil.assertOnUiThread()
49-
parent.removeViewAt(index)
50-
}
109+
/**
110+
* Returns whether this View type needs to handle laying out its own children instead of deferring
111+
* to the standard css-layout algorithm. Returns true for the layout to *not* be automatically
112+
* invoked. Instead onLayout will be invoked as normal and it is the View instance's
113+
* responsibility to properly call layout on its children. Returns false for the default behavior
114+
* of automatically laying out children without going through the ViewGroup's onLayout method. In
115+
* that case, onLayout for this View type must *not* call layout on its children.
116+
*/
117+
public override fun needsCustomLayoutForChildren(): Boolean = false
51118

52-
/**
53-
* Expo overrides this function GroupViewManagerWrapper.kt`, which is a replacement view manager
54-
* adding support for delegates receiving callbacks whenever one of the methods in the view
55-
* manager are called.
56-
*/
57-
public open fun removeView(parent: T, view: View) {
58-
UiThreadUtil.assertOnUiThread()
119+
public companion object {
120+
private val zIndexHash: WeakHashMap<View, Int> = WeakHashMap()
59121

60-
for (i in 0 until getChildCount(parent)) {
61-
if (getChildAt(parent, i) === view) {
122+
@JvmStatic
123+
public fun setViewZIndex(view: View, zIndex: Int): Unit = zIndexHash.set(view, zIndex)
62124

63-
removeViewAt(parent, i)
64-
break
65-
}
125+
@JvmStatic public fun getViewZIndex(view: View?): Int? = zIndexHash[view]
66126
}
67-
}
68-
69-
/**
70-
* Returns whether this View type needs to handle laying out its own children instead of deferring
71-
* to the standard css-layout algorithm. Returns true for the layout to *not* be automatically
72-
* invoked. Instead onLayout will be invoked as normal and it is the View instance's
73-
* responsibility to properly call layout on its children. Returns false for the default behavior
74-
* of automatically laying out children without going through the ViewGroup's onLayout method. In
75-
* that case, onLayout for this View type must *not* call layout on its children.
76-
*/
77-
public override fun needsCustomLayoutForChildren(): Boolean = false
78-
79-
public companion object {
80-
private val zIndexHash: WeakHashMap<View, Int> = WeakHashMap()
81-
82-
@JvmStatic
83-
public fun setViewZIndex(view: View, zIndex: Int): Unit = zIndexHash.set(view, zIndex)
84-
85-
@JvmStatic public fun getViewZIndex(view: View?): Int? = zIndexHash[view]
86-
}
87127
}

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

Lines changed: 6 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -31,47 +31,16 @@ public abstract class ReactClippingViewManager<T : ReactViewGroup> : ViewGroupMa
3131
view.removeClippedSubviews = removeClippedSubviews
3232
}
3333

34-
// parent: childIndex[] - Used when we can't immediately add a view
35-
private val operationsMap = WeakHashMap<T, MutableMap<Int, Boolean>>()
36-
3734
override fun addView(parent: T, child: View, index: Int) {
3835
UiThreadUtil.assertOnUiThread()
3936

40-
if (child.parent != null) {
41-
operationsMap.getOrPut(parent) {
42-
mutableMapOf()
43-
}[index] = true
44-
45-
// When the child-parent relation is removed, onDetachedFromWindow will be called.
46-
// Its important to wait for detaching as the view might be in a transition, and isn't removed immediately.
47-
child.doOnDetach {
48-
// Looking at how endViewTransition is implemented, dispatchDetachedFromWindow
49-
// gets called _before_ the parent relation is removed, so we need to post this to the end of the frame:
50-
child.post {
51-
if(operationsMap.remove(parent) == null) {
52-
// The addView operation was already countered by a removeView operation while we were waiting
53-
FLog.w("ReactClippingViewManager", "Tried to add a view to a parent after the child was detached, but a remove operation was already enqueued")
54-
return@post
55-
}
56-
FLog.w("ReactClippingViewManager", "addView(): ${child::class.java.simpleName} had a parent, removed from previous parent and after onDetach adding to new parent $parent")
57-
addViewInternal(parent, child, index)
58-
}
37+
addViewSafely(parent, child, index) {
38+
val removeClippedSubviews = parent.removeClippedSubviews
39+
if (removeClippedSubviews) {
40+
parent.addViewWithSubviewClippingEnabled(child, index)
41+
} else {
42+
parent.addView(child, index)
5943
}
60-
61-
// With the detach listener in place, we can now remove the view from the previous parent:
62-
// Note: This call here is potentially redundant, as SurfaceMountingManager.kt is already removing it
63-
(child.parent as? ViewGroup)?.removeView(child)
64-
} else {
65-
addViewInternal(parent, child, index)
66-
}
67-
}
68-
69-
private fun addViewInternal(parent: T, child: View, index: Int) {
70-
val removeClippedSubviews = parent.removeClippedSubviews
71-
if (removeClippedSubviews) {
72-
parent.addViewWithSubviewClippingEnabled(child, index)
73-
} else {
74-
parent.addView(child, index)
7544
}
7645
}
7746

0 commit comments

Comments
 (0)