Skip to content

Commit

Permalink
Fix crash in getChildDrawingOrder (#40859)
Browse files Browse the repository at this point in the history
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 #30785

Pull Request resolved: #40859

Reviewed By: NickGerleman

Differential Revision: D50321718

Pulled By: javache

fbshipit-source-id: 7fa7069937b8c2afb9f30dd10554370b1be5d515
  • Loading branch information
huangtaibin authored and facebook-github-bot committed Oct 17, 2023
1 parent 18c9797 commit b941831
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import android.view.View;
import android.view.ViewGroup;
import androidx.annotation.Nullable;
import com.facebook.common.logging.FLog;
import com.facebook.react.common.ReactConstants;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
Expand Down Expand Up @@ -65,6 +67,17 @@ public boolean shouldEnableCustomDrawingOrder() {
* ViewGroup#getChildDrawingOrder}.
*/
public int getChildDrawingOrder(int childCount, int index) {
if (mDrawingOrderIndices != null
&& (index >= mDrawingOrderIndices.length || mDrawingOrderIndices[index] >= childCount)) {
FLog.w(
ReactConstants.TAG,
"getChildDrawingOrder index out of bounds! Please check any custom view manipulations you"
+ " may have done. childCount = %d, index = %d",
childCount,
index);
update();
}

if (mDrawingOrderIndices == null) {
ArrayList<View> viewsToSort = new ArrayList<>();
for (int i = 0; i < childCount; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,10 +418,10 @@ private void updateSubviewClipStatus(Rect clippingRect, int idx, int clippedSoFa
if (!intersects && child.getParent() != null && !isAnimating) {
// We can try saving on invalidate call here as the view that we remove is out of visible area
// therefore invalidation is not necessary.
super.removeViewsInLayout(idx - clippedSoFar, 1);
removeViewsInLayout(idx - clippedSoFar, 1);
needUpdateClippingRecursive = true;
} else if (intersects && child.getParent() == null) {
super.addViewInLayout(child, idx - clippedSoFar, sDefaultLayoutParam, true);
addViewInLayout(child, idx - clippedSoFar, sDefaultLayoutParam, true);
invalidate();
needUpdateClippingRecursive = true;
} else if (intersects) {
Expand Down Expand Up @@ -499,23 +499,18 @@ private boolean customDrawOrderDisabled() {
return ViewUtil.getUIManagerType(getId()) == UIManagerType.FABRIC;
}

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

if (!customDrawOrderDisabled()) {
getDrawingOrderHelper().handleAddView(child);
getDrawingOrderHelper().handleAddView(view);
setChildrenDrawingOrderEnabled(getDrawingOrderHelper().shouldEnableCustomDrawingOrder());
} else {
setChildrenDrawingOrderEnabled(false);
}

super.addView(child, index, params);
}

@Override
public void removeView(View view) {
private void handleRemoveView(View view) {
UiThreadUtil.assertOnUiThread();

if (!customDrawOrderDisabled()) {
Expand All @@ -524,22 +519,60 @@ public void removeView(View view) {
} else {
setChildrenDrawingOrderEnabled(false);
}
}

private void handleRemoveViews(int start, int count) {
int endIndex = start + count;
for (int index = start; index < endIndex; index++) {
if (index < getChildCount()) {
handleRemoveView(getChildAt(index));
}
}
}

@Override
public void addView(View child, int index, ViewGroup.LayoutParams params) {
// This will get called for every overload of addView so there is not need to override every
// method.
handleAddView(child);
super.addView(child, index, params);
}

@Override
protected boolean addViewInLayout(
View child, int index, LayoutParams params, boolean preventRequestLayout) {
handleAddView(child);
return super.addViewInLayout(child, index, params, preventRequestLayout);
}

@Override
public void removeView(View view) {
handleRemoveView(view);
super.removeView(view);
}

@Override
public void removeViewAt(int index) {
UiThreadUtil.assertOnUiThread();
handleRemoveView(getChildAt(index));
super.removeViewAt(index);
}

if (!customDrawOrderDisabled()) {
getDrawingOrderHelper().handleRemoveView(getChildAt(index));
setChildrenDrawingOrderEnabled(getDrawingOrderHelper().shouldEnableCustomDrawingOrder());
} else {
setChildrenDrawingOrderEnabled(false);
}
@Override
public void removeViewInLayout(View view) {
handleRemoveView(view);
super.removeViewInLayout(view);
}

super.removeViewAt(index);
@Override
public void removeViewsInLayout(int start, int count) {
handleRemoveViews(start, count);
super.removeViewsInLayout(start, count);
}

@Override
public void removeViews(int start, int count) {
handleRemoveViews(start, count);
super.removeViews(start, count);
}

@Override
Expand Down Expand Up @@ -663,7 +696,7 @@ public void run() {
clippedSoFar++;
}
}
super.removeViewsInLayout(index - clippedSoFar, 1);
removeViewsInLayout(index - clippedSoFar, 1);
}
removeFromArray(index);
}
Expand Down

0 comments on commit b941831

Please sign in to comment.