Facebook/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2025-11-01 09:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix RemoveDeleteTree and subview clipping compatibility (#42355)

Browse Source 
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42355

`RemoveDeleteTreeUIFrameCallback` operated directly on the view to clean up its children, which does not correctly account for subviews which have been clipped because they're outside the visible frame.

Changelog: [Android][Added] Added `removeAllViews` to IViewGroupManager.

Reviewed By: jehartzog, sammy-SC

Differential Revision: D52834835

fbshipit-source-id: fb7f07a17d07467eecd3ce9721afc2f3abcc0caa
This commit is contained in:
Pieter De Baets
2024-01-18 07:58:49 -08:00
committed by Facebook GitHub Bot
parent ca2dde5906
commit 3cd85dc933
4 changed files with 31 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/react-native/ReactAndroid/api/ReactAndroid.api
+2 -2
View File
@@ -3987,6 +3987,7 @@ public abstract interface class com/facebook/react/uimanager/IViewGroupManager :
public abstract fun addView (Landroid/view/View;Landroid/view/View;I)V
public abstract fun getChildAt (Landroid/view/View;I)Landroid/view/View;
public abstract fun getChildCount (Landroid/view/View;)I
public fun removeAllViews (Landroid/view/View;)V
public abstract fun removeViewAt (Landroid/view/View;I)V
}
@@ -5061,7 +5062,6 @@ public abstract class com/facebook/react/uimanager/ViewGroupManager : com/facebo
public fun getShadowNodeClass ()Ljava/lang/Class;
public static fun getViewZIndex (Landroid/view/View;)Ljava/lang/Integer;
public fun needsCustomLayoutForChildren ()Z
public fun removeAllViews (Landroid/view/ViewGroup;)V
public fun removeView (Landroid/view/ViewGroup;Landroid/view/View;)V
public synthetic fun removeViewAt (Landroid/view/View;I)V
public fun removeViewAt (Landroid/view/ViewGroup;I)V
@@ -7519,7 +7519,7 @@ public abstract class com/facebook/react/views/view/ReactClippingViewManager : c
public synthetic fun getChildCount (Landroid/view/View;)I
public synthetic fun getChildCount (Landroid/view/ViewGroup;)I
public fun getChildCount (Lcom/facebook/react/views/view/ReactViewGroup;)I
public synthetic fun removeAllViews (Landroid/view/ViewGroup;)V
public synthetic fun removeAllViews (Landroid/view/View;)V
public fun removeAllViews (Lcom/facebook/react/views/view/ReactViewGroup;)V
public synthetic fun removeViewAt (Landroid/view/View;I)V
public synthetic fun removeViewAt (Landroid/view/ViewGroup;I)V
packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java
+19 -24
View File
@@ -90,7 +90,7 @@ public class SurfaceMountingManager {
private final Set<Integer> mErroneouslyReaddedReactTags = new HashSet<>();
@ThreadConfined(UI)
private RemoveDeleteTreeUIFrameCallback mRemoveDeleteTreeUIFrameCallback;
private @Nullable RemoveDeleteTreeUIFrameCallback mRemoveDeleteTreeUIFrameCallback;
// This is null *until* StopSurface is called.
private Set<Integer> mTagSetForStoppedSurfaceLegacy;
@@ -773,17 +773,9 @@ public class SurfaceMountingManager {
// The View has been removed from the View hierarchy; now it
// and all of its children, if any, need to be deleted, recursively.
// We want to maintain the legacy ordering: delete (and call onViewStateDeleted)
// for leaf nodes, and then parents, recursively.
// Schedule the Runnable first, to detect if we need to schedule a Runnable at all.
// Since this current function and the Runnable both run on the UI thread, there is
// no race condition here.
runDeferredTagRemovalAndDeletion();
mReactTagsToRemove.push(tag);
}
@UiThread
private void runDeferredTagRemovalAndDeletion() {
if (mReactTagsToRemove.empty()) {
if (mRemoveDeleteTreeUIFrameCallback == null) {
mRemoveDeleteTreeUIFrameCallback = new RemoveDeleteTreeUIFrameCallback(mThemedReactContext);
@@ -792,6 +784,7 @@ public class SurfaceMountingManager {
.postFrameCallback(
ReactChoreographer.CallbackType.IDLE_EVENT, mRemoveDeleteTreeUIFrameCallback);
}
mReactTagsToRemove.push(tag);
}
@UiThread
@@ -1455,19 +1448,20 @@ public class SurfaceMountingManager {
ViewState thisViewState = getNullableViewState(reactTag);
if (thisViewState != null) {
View thisView = thisViewState.mView;
int numChildren = 0;
// Children are managed by React Native if both of the following are true:
// 1) There are 1 or more children of this View, which must be a ViewGroup
// 2) Those children are managed by RN (this is not the case for certain native
// components, like embedded Litho hierarchies)
boolean childrenAreManaged = false;
if (thisView instanceof ViewGroup) {
View nextChild = null;
IViewGroupManager viewManager = getViewGroupManager(thisViewState);
// Children are managed by React Native if both of the following are true:
// 1) There are 1 or more children of this View, which must be a ViewGroup
// 2) Those children are managed by RN (this is not the case for certain native
// components, like embedded Litho hierarchies)
boolean childrenAreManaged = false;
// For reasons documented elsewhere in this class, getChildCount is not
// necessarily reliable, and so we rely instead on requesting children directly.
while ((nextChild = ((ViewGroup) thisView).getChildAt(numChildren)) != null) {
View nextChild = null;
int numChildren = 0;
while ((nextChild = viewManager.getChildAt(thisView, numChildren)) != null) {
int childId = nextChild.getId();
childrenAreManaged = childrenAreManaged || getNullableViewState(childId) != null;
localChildren.push(nextChild.getId());
@@ -1487,16 +1481,17 @@ public class SurfaceMountingManager {
// In debug mode, the SoftException will cause a crash. In production it
// will not. This should give good visibility into whether or not this is
// a problem without causing user-facing errors.
((ViewGroup) thisView).removeAllViews();
viewManager.removeAllViews(thisView);
} catch (RuntimeException e) {
childrenAreManaged = false;
ReactSoftExceptionLogger.logSoftException(TAG, e);
}
}
}
if (childrenAreManaged) {
// Push tags onto the stack so we process all children
mReactTagsToRemove.addAll(localChildren);
if (childrenAreManaged) {
// Push tags onto the stack so we process all children
mReactTagsToRemove.addAll(localChildren);
}
}
// Immediately remove tag and notify listeners.
packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/IViewGroupManager.java
+10
View File
@@ -8,6 +8,7 @@
package com.facebook.react.uimanager;
import android.view.View;
import com.facebook.react.bridge.UiThreadUtil;
/** Interface providing children management API for view managers of classes extending ViewGroup. */
public interface IViewGroupManager<T extends View> extends IViewManagerWithChildren {
@@ -21,6 +22,15 @@ public interface IViewGroupManager<T extends View> extends IViewManagerWithChild
/** Removes View from the parent View at the index specified as a parameter. */
void removeViewAt(T parent, int index);
/** Remove all child views from the parent View. */
default void removeAllViews(T parent) {
UiThreadUtil.assertOnUiThread();
for (int i = getChildCount(parent) - 1; i >= 0; i--) {
removeViewAt(parent, i);
}
}
/** Return the amount of children contained by the view specified as a parameter. */
int getChildCount(T parent);
}
packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java
-8
View File
@@ -89,14 +89,6 @@ public abstract class ViewGroupManager<T extends ViewGroup>
}
}
public void removeAllViews(T parent) {
UiThreadUtil.assertOnUiThread();
for (int i = getChildCount(parent) - 1; i >= 0; i--) {
removeViewAt(parent, i);
}
}
/**
* Returns whether this View type needs to handle laying out its own children instead of deferring
* to the standard css-layout algorithm. Returns true for the layout to *not* be automatically