From 342ce6115b744cd2fcfd80effb609b37dc9d5b37 Mon Sep 17 00:00:00 2001 From: huangtaibin Date: Tue, 17 Oct 2023 05:30:54 -0700 Subject: [PATCH] Fix crash in getChildDrawingOrder (#40859) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 https://github.com/facebook/react-native/issues/30785 Pull Request resolved: https://github.com/facebook/react-native/pull/40859 Reviewed By: NickGerleman Differential Revision: D50321718 Pulled By: javache fbshipit-source-id: 7fa7069937b8c2afb9f30dd10554370b1be5d515 --- .../ViewGroupDrawingOrderHelper.java | 13 ++++ .../react/views/view/ReactViewGroup.java | 75 +++++++++++++------ 2 files changed, 67 insertions(+), 21 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupDrawingOrderHelper.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupDrawingOrderHelper.java index 96448f6c01a..2bd1d754f3a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupDrawingOrderHelper.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupDrawingOrderHelper.java @@ -10,6 +10,8 @@ package com.facebook.react.uimanager; 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; @@ -65,6 +67,17 @@ public class ViewGroupDrawingOrderHelper { * 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 viewsToSort = new ArrayList<>(); for (int i = 0; i < childCount; i++) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java index d21362d48fb..6bd724ecb34 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java @@ -418,10 +418,10 @@ public class ReactViewGroup extends ViewGroup 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) { @@ -499,23 +499,18 @@ public class ReactViewGroup extends ViewGroup 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()) { @@ -524,24 +519,62 @@ public class ReactViewGroup extends ViewGroup } 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(); - - if (!customDrawOrderDisabled()) { - getDrawingOrderHelper().handleRemoveView(getChildAt(index)); - setChildrenDrawingOrderEnabled(getDrawingOrderHelper().shouldEnableCustomDrawingOrder()); - } else { - setChildrenDrawingOrderEnabled(false); - } - + handleRemoveView(getChildAt(index)); super.removeViewAt(index); } + @Override + public void removeViewInLayout(View view) { + handleRemoveView(view); + super.removeViewInLayout(view); + } + + @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 protected int getChildDrawingOrder(int childCount, int index) { UiThreadUtil.assertOnUiThread(); @@ -663,7 +696,7 @@ public class ReactViewGroup extends ViewGroup clippedSoFar++; } } - super.removeViewsInLayout(index - clippedSoFar, 1); + removeViewsInLayout(index - clippedSoFar, 1); } removeFromArray(index); }