Clean up View Recycling

Summary:
Followups to View Recycling diffs to improve things / clean up things a bit. This also fixes memory warnings which were not hooked up before.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D36707792

fbshipit-source-id: 410e70bf0eeec5569566138af547e1601394d0e6

ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

@@ -166,6 +166,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
   @NonNull private final MountingManager mMountingManager;
   @NonNull private final EventDispatcher mEventDispatcher;
   @NonNull private final MountItemDispatcher mMountItemDispatcher;
+  @NonNull private final ViewManagerRegistry mViewManagerRegistry;
 
   @NonNull private final EventBeatManager mEventBeatManager;
 
@@ -226,6 +227,9 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
     mShouldDeallocateEventDispatcher = false;
     mEventBeatManager = eventBeatManager;
     mReactApplicationContext.addLifecycleEventListener(this);
+
+    mViewManagerRegistry = viewManagerRegistry;
+    mReactApplicationContext.registerComponentCallbacks(viewManagerRegistry);
   }
 
   public FabricUIManager(
@@ -244,6 +248,9 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
     mShouldDeallocateEventDispatcher = true;
     mEventBeatManager = eventBeatManager;
     mReactApplicationContext.addLifecycleEventListener(this);
+
+    mViewManagerRegistry = viewManagerRegistry;
+    mReactApplicationContext.registerComponentCallbacks(viewManagerRegistry);
   }
 
   // TODO (T47819352): Rename this to startSurface for consistency with xplat/iOS
@@ -451,6 +458,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
     mEventDispatcher.removeBatchEventDispatchedListener(mEventBeatManager);
     mEventDispatcher.unregisterEventEmitter(FABRIC);
 
+    mReactApplicationContext.unregisterComponentCallbacks(mViewManagerRegistry);
+
     // Remove lifecycle listeners (onHostResume, onHostPause) since the FabricUIManager is going
     // away. Then stop the mDispatchUIFrameCallback false will cause the choreographer
     // callbacks to stop firing.

ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java

@@ -105,6 +105,36 @@ public abstract class BaseViewManager<T extends View, C extends LayoutShadowNode
     view.setOutlineAmbientShadowColor(Color.BLACK);
     view.setOutlineSpotShadowColor(Color.BLACK);
 
+    // Focus IDs
+    // Also see in AOSP source:
+    // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#4493
+    view.setNextFocusDownId(View.NO_ID);
+    view.setNextFocusForwardId(View.NO_ID);
+    view.setNextFocusRightId(View.NO_ID);
+    view.setNextFocusUpId(View.NO_ID);
+
+    // This is possibly subject to change and overrideable per-platform, but these
+    // are the default view flags in View.java:
+    // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#2712
+    // `mViewFlags = SOUND_EFFECTS_ENABLED | HAPTIC_FEEDBACK_ENABLED | LAYOUT_DIRECTION_INHERIT`
+    // Therefore we set the following options as such:
+    view.setFocusable(false);
+    view.setFocusableInTouchMode(false);
+
+    // https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-mainline-12.0.0_r96/core/java/android/view/View.java#5491
+    view.setElevation(0);
+
+    // Predictably, alpha defaults to 1:
+    // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#2186
+    // This accounts for resetting mBackfaceOpacity and mBackfaceVisibility
+    view.setAlpha(1);
+
+    // setPadding is a noop for most View types, but it is not for Text
+    setPadding(view, 0, 0, 0, 0);
+
+    // Other stuff
+    view.setForeground(null);
+
     return view;
   }
 

ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java

@@ -207,6 +207,7 @@ public class UIManagerModule extends ReactContextBaseJavaModule
   @Override
   public void initialize() {
     getReactApplicationContext().registerComponentCallbacks(mMemoryTrimCallback);
+    getReactApplicationContext().registerComponentCallbacks(mViewManagerRegistry);
     mEventDispatcher.registerEventEmitter(
         DEFAULT, getReactApplicationContext().getJSModule(RCTEventEmitter.class));
   }
@@ -234,6 +235,7 @@ public class UIManagerModule extends ReactContextBaseJavaModule
 
     ReactApplicationContext reactApplicationContext = getReactApplicationContext();
     reactApplicationContext.unregisterComponentCallbacks(mMemoryTrimCallback);
+    reactApplicationContext.unregisterComponentCallbacks(mViewManagerRegistry);
     YogaNodePool.get().clear();
     ViewManagerPropertyUpdater.clear();
   }

ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java

@@ -41,15 +41,23 @@ public abstract class ViewManager<T extends View, C extends ReactShadowNode>
    * null signals that View Recycling is disabled. `enableViewRecycling` must be explicitly called
    * in a concrete constructor to enable View Recycling per ViewManager.
    */
-  private HashMap<Integer, Stack<T>> mRecyclableViews = null;
+  @Nullable private HashMap<Integer, Stack<T>> mRecyclableViews = null;
+
+  private int mRecyclableViewsBufferSize = 1024;
 
   /** Call in constructor of concrete ViewManager class to enable. */
-  protected void enableViewRecycling() {
+  protected void setupViewRecycling() {
     if (ReactFeatureFlags.enableViewRecycling) {
       mRecyclableViews = new HashMap<>();
     }
   }
 
+  /** Call in constructor of concrete ViewManager class to enable. */
+  protected void setupViewRecycling(int bufferSize) {
+    mRecyclableViewsBufferSize = bufferSize;
+    setupViewRecycling();
+  }
+
   private @Nullable Stack<T> getRecyclableViewStack(int surfaceId) {
     if (mRecyclableViews == null) {
       return null;
@@ -191,12 +199,16 @@ public abstract class ViewManager<T extends View, C extends ReactShadowNode>
    * {@link ViewManager} subclass.
    */
   public void onDropViewInstance(@NonNull T view) {
-    @Nullable
-    Stack<T> recyclableViews =
-        getRecyclableViewStack(((ThemedReactContext) view.getContext()).getSurfaceId());
-    // By default we treat views as recyclable
-    if (recyclableViews != null) {
-      recyclableViews.push(prepareToRecycleView((ThemedReactContext) view.getContext(), view));
+    // View recycling
+    ThemedReactContext themedReactContext = (ThemedReactContext) view.getContext();
+    int surfaceId = themedReactContext.getSurfaceId();
+    @Nullable Stack<T> recyclableViews = getRecyclableViewStack(surfaceId);
+
+    // Any max buffer size <0 results in an infinite buffer size
+    if (recyclableViews != null
+        && (mRecyclableViewsBufferSize < 0
+            || recyclableViews.size() < mRecyclableViewsBufferSize)) {
+      recyclableViews.push(prepareToRecycleView(themedReactContext, view));
     }
   }
 

ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerRegistry.java

@@ -19,11 +19,10 @@ import java.util.Map;
  * Class that stores the mapping between native view name used in JS and the corresponding instance
  * of {@link ViewManager}.
  */
-public final class ViewManagerRegistry {
+public final class ViewManagerRegistry implements ComponentCallbacks2 {
 
   private final Map<String, ViewManager> mViewManagers;
   private final @Nullable ViewManagerResolver mViewManagerResolver;
-  private final MemoryTrimCallback mMemoryTrimCallback = new MemoryTrimCallback();
 
   public ViewManagerRegistry(ViewManagerResolver viewManagerResolver) {
     mViewManagers = MapBuilder.newHashMap();
@@ -46,40 +45,6 @@ public final class ViewManagerRegistry {
     mViewManagerResolver = null;
   }
 
-  /**
-   * Trim View Recycling memory aggressively. Whenever the system is running even slightly low on
-   * memory or is backgrounded, we immediately flush all recyclable Views. GC and memory swaps cause
-   * intense CPU pressure, so we always favor low memory usage over View recycling, even if there is
-   * only "moderate" pressure.
-   */
-  private class MemoryTrimCallback implements ComponentCallbacks2 {
-    @Override
-    public void onTrimMemory(int level) {
-      Runnable runnable =
-          new Runnable() {
-            @Override
-            public void run() {
-              for (Map.Entry<String, ViewManager> entry : mViewManagers.entrySet()) {
-                entry.getValue().trimMemory();
-              }
-            }
-          };
-      if (UiThreadUtil.isOnUiThread()) {
-        runnable.run();
-      } else {
-        UiThreadUtil.runOnUiThread(runnable);
-      }
-    }
-
-    @Override
-    public void onConfigurationChanged(Configuration newConfig) {}
-
-    @Override
-    public void onLowMemory() {
-      this.onTrimMemory(0);
-    }
-  }
-
   /**
    * @param className {@link String} that identifies the {@link ViewManager} inside the {@link
    *     ViewManagerRegistry}. This methods {@throws IllegalViewOperationException} if there is no
@@ -147,4 +112,33 @@ public final class ViewManagerRegistry {
       UiThreadUtil.runOnUiThread(runnable);
     }
   }
+
+  /** ComponentCallbacks2 method. */
+  @Override
+  public void onTrimMemory(int level) {
+    Runnable runnable =
+        new Runnable() {
+          @Override
+          public void run() {
+            for (Map.Entry<String, ViewManager> entry : mViewManagers.entrySet()) {
+              entry.getValue().trimMemory();
+            }
+          }
+        };
+    if (UiThreadUtil.isOnUiThread()) {
+      runnable.run();
+    } else {
+      UiThreadUtil.runOnUiThread(runnable);
+    }
+  }
+
+  /** ComponentCallbacks2 method. */
+  @Override
+  public void onConfigurationChanged(Configuration newConfig) {}
+
+  /** ComponentCallbacks2 method. */
+  @Override
+  public void onLowMemory() {
+    this.onTrimMemory(0);
+  }
 }

ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java

@@ -96,16 +96,18 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie
     mSpanned = null;
   }
 
-  /* package */ void recycleView(ReactTextView defaultView) {
+  /* package */ void recycleView() {
     // Set default field values
     initView();
 
-    setForeground(null);
+    // Defaults for these fields:
+    // https://github.com/aosp-mirror/platform_frameworks_base/blob/master/core/java/android/widget/TextView.java#L1061
+    setBreakStrategy(Layout.BREAK_STRATEGY_SIMPLE);
+    setMovementMethod(getDefaultMovementMethod());
+    setJustificationMode(Layout.JUSTIFICATION_MODE_NONE);
 
     // reset text
     setLayoutParams(EMPTY_LAYOUT_PARAMS);
-    setMovementMethod(defaultView.getMovementMethod());
-    setBreakStrategy(defaultView.getBreakStrategy());
     super.setText(null);
 
     // Call setters to ensure that any super setters are called
@@ -124,21 +126,8 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie
     // reset data detectors
     setLinkifyMask(0);
 
-    setJustificationMode(defaultView.getJustificationMode());
-
     setEllipsizeLocation(mEllipsizeLocation);
 
-    // Focus IDs
-    // Also see in AOSP source:
-    // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#4493
-    setNextFocusDownId(View.NO_ID);
-    setNextFocusForwardId(View.NO_ID);
-    setNextFocusRightId(View.NO_ID);
-    setNextFocusUpId(View.NO_ID);
-
-    // https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-mainline-12.0.0_r96/core/java/android/view/View.java#5491
-    setElevation(0);
-
     // View flags - defaults are here:
     // https://android.googlesource.com/platform/frameworks/base/+/98e54bb941cb6feb07127b75da37833281951d52/core/java/android/view/View.java#5311
     //         mViewFlags = SOUND_EFFECTS_ENABLED | HAPTIC_FEEDBACK_ENABLED |
@@ -146,8 +135,7 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie
     setEnabled(true);
     setFocusable(View.FOCUSABLE_AUTO);
 
-    // Things that could be set as a result of updateText/setText
-    setPadding(0, 0, 0, 0);
+    setHyphenationFrequency(Layout.HYPHENATION_FREQUENCY_NONE);
 
     updateView(); // call after changing ellipsizeLocation in particular
   }

ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java

@@ -45,33 +45,25 @@ public class ReactTextViewManager
 
   @VisibleForTesting public static final String REACT_CLASS = "RCTText";
 
-  private ReactTextView mDefaultViewForRecycling = null;
-
   protected @Nullable ReactTextViewManagerCallback mReactTextViewManagerCallback;
 
   public ReactTextViewManager() {
     super();
 
-    enableViewRecycling();
+    setupViewRecycling();
   }
 
   @Override
   protected ReactTextView prepareToRecycleView(
       @NonNull ThemedReactContext reactContext, ReactTextView view) {
-    // TODO: use context as key
-    if (mDefaultViewForRecycling == null) {
-      mDefaultViewForRecycling = createViewInstance(reactContext);
-    }
-
     // BaseViewManager
     super.prepareToRecycleView(reactContext, view);
 
     // Resets background and borders
-    view.recycleView(mDefaultViewForRecycling);
+    view.recycleView();
 
     // Defaults from ReactTextAnchorViewManager
     setSelectionColor(view, null);
-    setAndroidHyphenationFrequency(view, null);
 
     return view;
   }

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

@@ -176,32 +176,6 @@ public class ReactViewGroup extends ViewGroup
     // Reset background, borders
     updateBackgroundDrawable(null);
 
-    setForeground(null);
-
-    // This is possibly subject to change and overrideable per-platform, but these
-    // are the default view flags in View.java:
-    // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#2712
-    // `mViewFlags = SOUND_EFFECTS_ENABLED | HAPTIC_FEEDBACK_ENABLED | LAYOUT_DIRECTION_INHERIT`
-    // Therefore we set the following options as such:
-    setFocusable(false);
-    setFocusableInTouchMode(false);
-
-    // Focus IDs
-    // Also see in AOSP source:
-    // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#4493
-    setNextFocusDownId(View.NO_ID);
-    setNextFocusForwardId(View.NO_ID);
-    setNextFocusRightId(View.NO_ID);
-    setNextFocusUpId(View.NO_ID);
-
-    // Predictable, alpha defaults to 1:
-    // https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#2186
-    // This accounts for resetting mBackfaceOpacity and mBackfaceVisibility
-    setAlpha(1);
-
-    // https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-mainline-12.0.0_r96/core/java/android/view/View.java#5491
-    setElevation(0);
-
     resetPointerEvents();
   }
 
@@ -813,11 +787,6 @@ public class ReactViewGroup extends ViewGroup
     mOverflowInset.set(left, top, right, bottom);
   }
 
-  public void setOverflowInset(Rect overflowInset) {
-    mOverflowInset.set(
-        overflowInset.left, overflowInset.top, overflowInset.right, overflowInset.bottom);
-  }
-
   @Override
   public Rect getOverflowInset() {
     return mOverflowInset;

ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java

@@ -52,12 +52,10 @@ public class ReactViewManager extends ReactClippingViewManager<ReactViewGroup> {
   private static final int CMD_SET_PRESSED = 2;
   private static final String HOTSPOT_UPDATE_KEY = "hotspotUpdate";
 
-  private ReactViewGroup mDefaultViewForRecycling = null;
-
   public ReactViewManager() {
     super();
 
-    enableViewRecycling();
+    setupViewRecycling();
   }
 
   @Override