Skip to content

Commit f34677f

Browse files
huangtaibinOthinn
authored andcommitted
Fix crash in getChildDrawingOrder (facebook#40859)
Summary: Problem Causes: In ReactViewGroup, there is a conflict between the zIndex attribute and the removeClippedSubviews optimization attribute. When both are used at the same time, the array mDrawingOrderIndices in ViewGroupDrawingOrderHelper that records the rendering order of subviews is not reset when super is called in the updateSubviewClipStatus method to add and remove subviews. Solution:�Because there are many third-party components that inherit from or depend on ReactViewGroup, all methods for adding and removing subviews in ViewGroup need to be override in ReactViewGroup, and ViewGroupDrawingOrderHelper corresponding to handleAddView and handleRemoveView needs to be called in these methods. And all the precautions for directly calling super to add and remove subviews are changed to calling the overridden method by ReactViewGroup. Special Note: All addView related methods in ViewGroup will eventually be called to the addView(View child, int index, LayoutParams params) method, except addViewInLayout. Regarding the method of adding subviews, we only need to override addView(View child, int index, LayoutParams params) and addViewInLayout(View child, int index, LayoutParams params,boolean preventRequestLayout) in ReactViewGroup. ## Changelog: [Android] [Fixed] - Fix the crash in ReactViewGroup of facebook#30785 Pull Request resolved: facebook#40859 Reviewed By: NickGerleman Differential Revision: D50321718 Pulled By: javache fbshipit-source-id: 7fa7069937b8c2afb9f30dd10554370b1be5d515
1 parent 7cfd6d4 commit f34677f

File tree

2 files changed

+66
-20
lines changed

2 files changed

+66
-20
lines changed

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupDrawingOrderHelper.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import android.view.View;
1111
import android.view.ViewGroup;
1212
import androidx.annotation.Nullable;
13+
import com.facebook.common.logging.FLog;
14+
import com.facebook.react.common.ReactConstants;
1315
import java.util.ArrayList;
1416
import java.util.Collections;
1517
import java.util.Comparator;
@@ -65,6 +67,17 @@ public boolean shouldEnableCustomDrawingOrder() {
6567
* ViewGroup#getChildDrawingOrder}.
6668
*/
6769
public int getChildDrawingOrder(int childCount, int index) {
70+
if (mDrawingOrderIndices != null
71+
&& (index >= mDrawingOrderIndices.length || mDrawingOrderIndices[index] >= childCount)) {
72+
FLog.w(
73+
ReactConstants.TAG,
74+
"getChildDrawingOrder index out of bounds! Please check any custom view manipulations you"
75+
+ " may have done. childCount = %d, index = %d",
76+
childCount,
77+
index);
78+
update();
79+
}
80+
6881
if (mDrawingOrderIndices == null) {
6982
ArrayList<View> viewsToSort = new ArrayList<>();
7083
for (int i = 0; i < childCount; i++) {

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

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -418,10 +418,10 @@ private void updateSubviewClipStatus(Rect clippingRect, int idx, int clippedSoFa
418418
if (!intersects && child.getParent() != null && !isAnimating) {
419419
// We can try saving on invalidate call here as the view that we remove is out of visible area
420420
// therefore invalidation is not necessary.
421-
super.removeViewsInLayout(idx - clippedSoFar, 1);
421+
removeViewsInLayout(idx - clippedSoFar, 1);
422422
needUpdateClippingRecursive = true;
423423
} else if (intersects && child.getParent() == null) {
424-
super.addViewInLayout(child, idx - clippedSoFar, sDefaultLayoutParam, true);
424+
addViewInLayout(child, idx - clippedSoFar, sDefaultLayoutParam, true);
425425
invalidate();
426426
needUpdateClippingRecursive = true;
427427
} else if (intersects) {
@@ -499,23 +499,18 @@ private boolean customDrawOrderDisabled() {
499499
return ViewUtil.getUIManagerType(getId()) == UIManagerType.FABRIC;
500500
}
501501

502-
@Override
503-
public void addView(View child, int index, ViewGroup.LayoutParams params) {
504-
// This will get called for every overload of addView so there is not need to override every
505-
// method.
502+
private void handleAddView(View view) {
503+
UiThreadUtil.assertOnUiThread();
506504

507505
if (!customDrawOrderDisabled()) {
508-
getDrawingOrderHelper().handleAddView(child);
506+
getDrawingOrderHelper().handleAddView(view);
509507
setChildrenDrawingOrderEnabled(getDrawingOrderHelper().shouldEnableCustomDrawingOrder());
510508
} else {
511509
setChildrenDrawingOrderEnabled(false);
512510
}
513-
514-
super.addView(child, index, params);
515511
}
516512

517-
@Override
518-
public void removeView(View view) {
513+
private void handleRemoveView(View view) {
519514
UiThreadUtil.assertOnUiThread();
520515

521516
if (!customDrawOrderDisabled()) {
@@ -524,22 +519,60 @@ public void removeView(View view) {
524519
} else {
525520
setChildrenDrawingOrderEnabled(false);
526521
}
522+
}
523+
524+
private void handleRemoveViews(int start, int count) {
525+
int endIndex = start + count;
526+
for (int index = start; index < endIndex; index++) {
527+
if (index < getChildCount()) {
528+
handleRemoveView(getChildAt(index));
529+
}
530+
}
531+
}
532+
533+
@Override
534+
public void addView(View child, int index, ViewGroup.LayoutParams params) {
535+
// This will get called for every overload of addView so there is not need to override every
536+
// method.
537+
handleAddView(child);
538+
super.addView(child, index, params);
539+
}
527540

541+
@Override
542+
protected boolean addViewInLayout(
543+
View child, int index, LayoutParams params, boolean preventRequestLayout) {
544+
handleAddView(child);
545+
return super.addViewInLayout(child, index, params, preventRequestLayout);
546+
}
547+
548+
@Override
549+
public void removeView(View view) {
550+
handleRemoveView(view);
528551
super.removeView(view);
529552
}
530553

531554
@Override
532555
public void removeViewAt(int index) {
533-
UiThreadUtil.assertOnUiThread();
556+
handleRemoveView(getChildAt(index));
557+
super.removeViewAt(index);
558+
}
534559

535-
if (!customDrawOrderDisabled()) {
536-
getDrawingOrderHelper().handleRemoveView(getChildAt(index));
537-
setChildrenDrawingOrderEnabled(getDrawingOrderHelper().shouldEnableCustomDrawingOrder());
538-
} else {
539-
setChildrenDrawingOrderEnabled(false);
540-
}
560+
@Override
561+
public void removeViewInLayout(View view) {
562+
handleRemoveView(view);
563+
super.removeViewInLayout(view);
564+
}
541565

542-
super.removeViewAt(index);
566+
@Override
567+
public void removeViewsInLayout(int start, int count) {
568+
handleRemoveViews(start, count);
569+
super.removeViewsInLayout(start, count);
570+
}
571+
572+
@Override
573+
public void removeViews(int start, int count) {
574+
handleRemoveViews(start, count);
575+
super.removeViews(start, count);
543576
}
544577

545578
@Override
@@ -663,7 +696,7 @@ public void run() {
663696
clippedSoFar++;
664697
}
665698
}
666-
super.removeViewsInLayout(index - clippedSoFar, 1);
699+
removeViewsInLayout(index - clippedSoFar, 1);
667700
}
668701
removeFromArray(index);
669702
}

0 commit comments

Comments
 (0)