View recycling - fix API access and disable when not possible (#45484)

Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/45484 Prior to Android SDK 28, there was no way to tell if a View's pivotX or pivotY were set. Unfortunately this breaks view recycling due to the default behavior of: - `getPivotX()` and `getPivotY()` [initialize as 0](https://android.googlesource.com/platform/frameworks/base/+/android-8.1.0_r81/libs/hwui/RenderProperties.h#643) - As long as they haven't been set, [they actually default to width/2 and height/2](https://android.googlesource.com/platform/frameworks/base/+/android-8.1.0_r81/libs/hwui/RenderProperties.cpp#195). Thus even if we were to check for `getPixotX() == 0`, we wouldn't know if it was specifically set to 0 (making the pivot actually 0), or still just the default value. We'd then need to reset the pivot any time the width or height changed. [`View.resetPivot()`](https://developer.android.com/reference/android/view/View#resetPivot%28%29) was presumably added to fix this in API 28. This diff adds nullability to `prepareToRecycleView()` so we can act accordingly - returning null if the view can't be recycled. Also added a version check for [`setAnimationMatrix()`](https://developer.android.com/reference/android/view/View#setAnimationMatrix%28android.graphics.Matrix%29), which is only available in 29+. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D59827328 fbshipit-source-id: d1729bba347e8af7fb2b57c95ed2e0b66a15d155
2025-11-01 09:14:26 +00:00 · 2024-07-17 18:00:48 -07:00
parent 17cbe7d974
commit b10724890e
10 changed files with 54 additions and 22 deletions
@@ -5286,7 +5286,7 @@ public abstract class com/facebook/react/uimanager/ViewManager : com/facebook/re
 	protected fun onAfterUpdateTransaction (Landroid/view/View;)V
 	public fun onDropViewInstance (Landroid/view/View;)V
 	public fun onSurfaceStopped (I)V
-	protected fun prepareToRecycleView (Lcom/facebook/react/uimanager/ThemedReactContext;Landroid/view/View;)Landroid/view/View;
+	protected abstract fun prepareToRecycleView (Lcom/facebook/react/uimanager/ThemedReactContext;Landroid/view/View;)Landroid/view/View;
 	public fun receiveCommand (Landroid/view/View;ILcom/facebook/react/bridge/ReadableArray;)V
 	public fun receiveCommand (Landroid/view/View;Ljava/lang/String;Lcom/facebook/react/bridge/ReadableArray;)V
 	protected fun recycleView (Lcom/facebook/react/uimanager/ThemedReactContext;Landroid/view/View;)Landroid/view/View;
@@ -7266,6 +7266,7 @@ public class com/facebook/react/views/text/ReactRawTextManager : com/facebook/re
 	public fun createViewInstance (Lcom/facebook/react/uimanager/ThemedReactContext;)Lcom/facebook/react/views/text/ReactTextView;
 	public fun getName ()Ljava/lang/String;
 	public fun getShadowNodeClass ()Ljava/lang/Class;
+	protected fun prepareToRecycleView (Lcom/facebook/react/uimanager/ThemedReactContext;Landroid/view/View;)Landroid/view/View;
 	public fun updateExtraData (Landroid/view/View;Ljava/lang/Object;)V
 }

@@ -68,7 +68,7 @@ public abstract class BaseViewManager<T extends View, C extends LayoutShadowNode
  }

  @Override
-  protected T prepareToRecycleView(@NonNull ThemedReactContext reactContext, T view) {
+  protected @Nullable T prepareToRecycleView(@NonNull ThemedReactContext reactContext, T view) {
    // Reset tags
    view.setTag(null);
    view.setTag(R.id.pointer_events, null);
@@ -95,13 +95,21 @@ public abstract class BaseViewManager<T extends View, C extends LayoutShadowNode
    setTransformProperty(view, null, null);

    // RenderNode params not covered by setTransformProperty above
-    view.resetPivot();
+    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
+      view.resetPivot();
+    } else {
+      // no way of resetting pivot, or knowing whether it is set
+      return null;
+    }
    view.setTop(0);
    view.setBottom(0);
    view.setLeft(0);
    view.setRight(0);
    view.setElevation(0);
-    view.setAnimationMatrix(null);
+    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
+      // failsafe - should already be set to null when animation finishes
+      view.setAnimationMatrix(null);
+    }

    view.setTag(R.id.transform, null);
    view.setTag(R.id.transform_origin, null);
@@ -226,16 +226,20 @@ public abstract class ViewManager<T extends View, C extends ReactShadowNode>
    int surfaceId = themedReactContext.getSurfaceId();
    @Nullable Stack<T> recyclableViews = getRecyclableViewStack(surfaceId);
    if (recyclableViews != null) {
-      recyclableViews.push(prepareToRecycleView(themedReactContext, view));
+      T recyclableView = prepareToRecycleView(themedReactContext, view);
+      if (recyclableView != null) {
+        recyclableViews.push(recyclableView);
+      }
    }
  }

  /**
   * Called when a View is removed from the hierarchy. This should be used to reset any properties.
+   *
+   * @return {@code view} if it was properly recycled, or {@code null} if it could not be recycled
   */
-  protected T prepareToRecycleView(@NonNull ThemedReactContext reactContext, @NonNull T view) {
-    return view;
-  }
+  protected abstract @Nullable T prepareToRecycleView(
+      @NonNull ThemedReactContext reactContext, @NonNull T view);

  /** Called when a View is going to be reused. */
  protected T recycleView(@NonNull ThemedReactContext reactContext, @NonNull T view) {
@@ -8,6 +8,8 @@
 package com.facebook.react.views.text;

 import android.view.View;
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import com.facebook.react.common.annotations.VisibleForTesting;
 import com.facebook.react.module.annotations.ReactModule;
 import com.facebook.react.uimanager.ThemedReactContext;
@@ -32,6 +34,12 @@ public class ReactRawTextManager extends ViewManager<View, ReactRawTextShadowNod
    throw new IllegalStateException("Attempt to create a native view for RCTRawText");
  }

+  @Override
+  protected @Nullable View prepareToRecycleView(
+      @NonNull ThemedReactContext reactContext, @NonNull View view) {
+    throw new IllegalStateException("Attempt to recycle a native view for RCTRawText");
+  }
+
  @Override
  public void updateExtraData(View view, Object extraData) {}

@@ -61,17 +61,16 @@ public class ReactTextViewManager
  }

  @Override
-  protected ReactTextView prepareToRecycleView(
+  protected @Nullable ReactTextView prepareToRecycleView(
      @NonNull ThemedReactContext reactContext, ReactTextView view) {
    // BaseViewManager
-    super.prepareToRecycleView(reactContext, view);
-
-    // Resets background and borders
-    view.recycleView();
-
-    // Defaults from ReactTextAnchorViewManager
-    setSelectionColor(view, null);
-
+    ReactTextView preparedView = super.prepareToRecycleView(reactContext, view);
+    if (preparedView != null) {
+      // Resets background and borders
+      preparedView.recycleView();
+      // Defaults from ReactTextAnchorViewManager
+      setSelectionColor(preparedView, null);
+    }
    return view;
  }

@@ -64,13 +64,13 @@ public class ReactViewManager extends ReactClippingViewManager<ReactViewGroup> {
  }

  @Override
-  protected ReactViewGroup prepareToRecycleView(
+  protected @Nullable ReactViewGroup prepareToRecycleView(
      @NonNull ThemedReactContext reactContext, ReactViewGroup view) {
    // BaseViewManager
-    super.prepareToRecycleView(reactContext, view);
-
-    view.recycleView();
-
+    ReactViewGroup preparedView = super.prepareToRecycleView(reactContext, view);
+    if (preparedView != null) {
+      preparedView.recycleView();
+    }
    return view;
  }

@@ -29,6 +29,8 @@ class ReactPropAnnotationSetterSpecTest {
    override fun createViewInstance(reactContext: ThemedReactContext): View =
        createViewInstance(reactContext)

+    override fun prepareToRecycleView(reactContext: ThemedReactContext, view: View): View? = null
+
    override fun updateExtraData(root: View, extraData: Any) = Unit
  }

@@ -65,6 +65,10 @@ class ReactPropAnnotationSetterTest {
    override fun createViewInstance(reactContext: ThemedReactContext): View =
        error("This method should not be executed as a part of this test")

+    override fun prepareToRecycleView(reactContext: ThemedReactContext, view: View): View? {
+      error("This method should not be executed as a part of this test")
+    }
+
    override fun updateExtraData(root: View, extraData: Any) =
        error("This method should not be executed as a part of this test")

@@ -37,6 +37,10 @@ class ReactPropConstantsTest {
      error("This method should not be executed as a part of this test")
    }

+    override fun prepareToRecycleView(reactContext: ThemedReactContext, view: View): View? {
+      error("This method should not be executed as a part of this test")
+    }
+
    override fun getShadowNodeClass(): Class<out ReactShadowNode<*>> {
      return ReactShadowNode::class.java
    }
@@ -110,6 +110,8 @@ class ReactPropForShadowNodeSpecTest {

      override fun createViewInstance(reactContext: ThemedReactContext): View = View(null)

+      override fun prepareToRecycleView(reactContext: ThemedReactContext, view: View): View? = null
+
      override fun updateExtraData(root: View, extraData: Any?) = Unit
    }
  }