[Offline indicator v2] Optimize memory by inflating view when needed

With this CL, we inflate the view right before showing it and destroy it
when it's hidden. Animations are also set to null when they end.

Bug: 1005843
Change-Id: I2fd05939bd81eb7155a48744cfca33a1bc319e9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2355005Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Sinan Sahin <sinansahin@google.com>
Cr-Commit-Position: refs/heads/master@{#801059}

chrome/android/java/src/org/chromium/chrome/browser/status_indicator/StatusIndicatorCoordinator.java

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.status_indicator;
 import android.app.Activity;
 import android.graphics.drawable.Drawable;
 import android.view.View;
+import android.view.ViewGroup;
 import android.view.ViewStub;
 
 import androidx.annotation.ColorInt;
@@ -42,6 +43,7 @@ public class StatusIndicatorCoordinator {
     }
 
     private StatusIndicatorMediator mMediator;
+    private PropertyModelChangeProcessor mMCP;
     private StatusIndicatorSceneLayer mSceneLayer;
     private boolean mIsShowing;
     private Runnable mRemoveOnLayoutChangeListener;
@@ -65,42 +67,74 @@ public class StatusIndicatorCoordinator {
             BrowserControlsStateProvider browserControlsStateProvider,
             Supplier<Integer> statusBarColorWithoutStatusIndicatorSupplier,
             Supplier<Boolean> canAnimateNativeBrowserControls, Callback<Runnable> requestRender) {
-        // TODO(crbug.com/1005843): Create this view lazily if/when we need it. This is a task for
-        // when we have the public API figured out. First, we should avoid inflating the view here
-        // in case it's never used.
-        final ViewStub stub = activity.findViewById(R.id.status_indicator_stub);
+        mSceneLayer = new StatusIndicatorSceneLayer(browserControlsStateProvider);
+
+        // This will create the view before showing it.
+        Runnable inflateView = () -> {
+            ViewStub stub = activity.findViewById(R.id.status_indicator_stub);
             ViewResourceFrameLayout root = (ViewResourceFrameLayout) stub.inflate();
-        mSceneLayer = new StatusIndicatorSceneLayer(root, () -> browserControlsStateProvider);
+            resourceManager.getDynamicResourceLoader().registerResource(
+                    root.getId(), root.getResourceAdapter());
+            mSceneLayer.setResourceId(root.getId());
             PropertyModel model =
                     new PropertyModel.Builder(StatusIndicatorProperties.ALL_KEYS)
                             .with(StatusIndicatorProperties.ANDROID_VIEW_VISIBILITY, View.GONE)
                             .with(StatusIndicatorProperties.COMPOSITED_VIEW_VISIBLE, false)
                             .build();
-        PropertyModelChangeProcessor.create(model,
+            mMCP = PropertyModelChangeProcessor.create(model,
                     new StatusIndicatorViewBinder.ViewHolder(root, mSceneLayer),
                     StatusIndicatorViewBinder::bind);
+            mMediator.setModel(model);
+            root.addOnLayoutChangeListener(mMediator);
+            mRemoveOnLayoutChangeListener = () -> root.removeOnLayoutChangeListener(mMediator);
+        };
+
+        // This will run to destroy the view once it's hidden.
+        Runnable destroyView = () -> {
+            mRemoveOnLayoutChangeListener.run();
+            mRemoveOnLayoutChangeListener = null;
+            ViewResourceFrameLayout root = activity.findViewById(R.id.status_indicator);
+            mSceneLayer.setResourceId(0);
+            resourceManager.getDynamicResourceLoader().unregisterResource(root.getId());
+            mMediator.setModel(null);
+            mMCP.destroy();
+            mMCP = null;
+
+            // Remove the view and add a ViewStub in its place. This is basically returning the
+            // view tree to its initial condition and will make it possible to inflate the view
+            // easily the next time it's needed, i.e. next #show() call.
+            final ViewGroup parent = ((ViewGroup) root.getParent());
+            final int index = parent.indexOfChild(root);
+            parent.removeViewAt(index);
+            final ViewStub stub = new ViewStub(activity);
+            stub.setLayoutParams(new ViewGroup.LayoutParams(
+                    ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.WRAP_CONTENT));
+            stub.setId(R.id.status_indicator_stub);
+            stub.setInflatedId(R.id.status_indicator);
+            stub.setLayoutResource(R.layout.status_indicator_container);
+            parent.addView(stub, index);
+        };
         Callback<Runnable> invalidateCompositorView = callback -> {
+            ViewResourceFrameLayout root = activity.findViewById(R.id.status_indicator);
             root.getResourceAdapter().invalidate(null);
             requestRender.onResult(callback);
         };
-        Runnable requestLayout = () -> root.requestLayout();
-
-        mMediator = new StatusIndicatorMediator(model, browserControlsStateProvider,
-                statusBarColorWithoutStatusIndicatorSupplier, canAnimateNativeBrowserControls,
-                invalidateCompositorView, requestLayout);
-        resourceManager.getDynamicResourceLoader().registerResource(
-                root.getId(), root.getResourceAdapter());
-        root.addOnLayoutChangeListener(mMediator);
-        mRemoveOnLayoutChangeListener = () -> root.removeOnLayoutChangeListener(mMediator);
+        Runnable requestLayout = () -> {
+            ViewResourceFrameLayout root = activity.findViewById(R.id.status_indicator);
+            root.requestLayout();
+        };
+        mMediator = new StatusIndicatorMediator(browserControlsStateProvider,
+                statusBarColorWithoutStatusIndicatorSupplier, inflateView, destroyView,
+                canAnimateNativeBrowserControls, invalidateCompositorView, requestLayout);
     }
 
     public void destroy() {
+        if (mRemoveOnLayoutChangeListener != null) {
             mRemoveOnLayoutChangeListener.run();
+        }
         mMediator.destroy();
     }
 
-    // TODO(sinansahin): Destroy the view when not needed.
-
     /**
      * Show the status indicator with the initial properties with animations.
      *

chrome/android/java/src/org/chromium/chrome/browser/status_indicator/StatusIndicatorMediator.java

@@ -37,6 +37,8 @@ class StatusIndicatorMediator
             new HashSet<>();
     private Supplier<Integer> mStatusBarWithoutIndicatorColorSupplier;
     private Runnable mOnShowAnimationEnd;
+    private Runnable mInflateView;
+    private Runnable mDestroyView;
     private Supplier<Boolean> mCanAnimateNativeBrowserControls;
     private Callback<Runnable> mInvalidateCompositorView;
     private Runnable mRequestLayout;
@@ -52,12 +54,13 @@ class StatusIndicatorMediator
 
     /**
      * Constructs the status indicator mediator.
-     * @param model The {@link PropertyModel} for the status indicator.
      * @param browserControlsStateProvider The {@link BrowserControlsStateProvider} to listen to
      *                                     for the changes in controls offsets.
      * @param statusBarWithoutIndicatorColorSupplier A supplier that will get the status bar color
      *                                               without taking the status indicator into
      *                                               account.
+     * @param inflateView A {@link Runnable} to inflate the status indicator view before showing.
+     * @param destroyView A {@link Runnable} to destroy the status indicator view once hidden.
      * @param canAnimateNativeBrowserControls Will supply a boolean denoting whether the native
      *                                        browser controls can be animated. This will be false
      *                                        where we can't have a reliable cc::BCOM instance, e.g.
@@ -65,14 +68,14 @@ class StatusIndicatorMediator
      * @param invalidateCompositorView Callback to invalidate the compositor texture.
      * @param requestLayout Runnable to request layout for the view.
      */
-    StatusIndicatorMediator(PropertyModel model,
-            BrowserControlsStateProvider browserControlsStateProvider,
-            Supplier<Integer> statusBarWithoutIndicatorColorSupplier,
-            Supplier<Boolean> canAnimateNativeBrowserControls,
+    StatusIndicatorMediator(BrowserControlsStateProvider browserControlsStateProvider,
+            Supplier<Integer> statusBarWithoutIndicatorColorSupplier, Runnable inflateView,
+            Runnable destroyView, Supplier<Boolean> canAnimateNativeBrowserControls,
             Callback<Runnable> invalidateCompositorView, Runnable requestLayout) {
-        mModel = model;
         mBrowserControlsStateProvider = browserControlsStateProvider;
         mStatusBarWithoutIndicatorColorSupplier = statusBarWithoutIndicatorColorSupplier;
+        mInflateView = inflateView;
+        mDestroyView = destroyView;
         mCanAnimateNativeBrowserControls = canAnimateNativeBrowserControls;
         mInvalidateCompositorView = invalidateCompositorView;
         mRequestLayout = requestLayout;
@@ -110,6 +113,10 @@ class StatusIndicatorMediator
         mObservers.remove(observer);
     }
 
+    void setModel(PropertyModel model) {
+        mModel = model;
+    }
+
     // Animations
 
     // TODO(sinansahin): We might want to end the running animations if we need to hide before we're
@@ -137,6 +144,8 @@ class StatusIndicatorMediator
      */
     void animateShow(@NonNull String statusText, Drawable statusIcon, @ColorInt int backgroundColor,
             @ColorInt int textColor, @ColorInt int iconTint) {
+        mInflateView.run();
+
         Runnable initializeProperties = () -> {
             mModel.set(StatusIndicatorProperties.STATUS_TEXT, statusText);
             mModel.set(StatusIndicatorProperties.STATUS_ICON, statusIcon);
@@ -169,13 +178,13 @@ class StatusIndicatorMediator
             @Override
             public void onEnd(Animator animation) {
                 initializeProperties.run();
+                mStatusBarAnimation = null;
             }
         });
         mStatusBarAnimation.start();
     }
 
     private void animateTextFadeIn() {
-        if (mTextFadeInAnimation == null) {
         mTextFadeInAnimation = ValueAnimator.ofFloat(0.f, 1.f);
         mTextFadeInAnimation.setInterpolator(Interpolators.FAST_OUT_SLOW_IN_INTERPOLATOR);
         mTextFadeInAnimation.setDuration(FADE_TEXT_DURATION_MS);
@@ -188,8 +197,12 @@ class StatusIndicatorMediator
             public void onStart(Animator animation) {
                 mRequestLayout.run();
             }
-            });
+
+            @Override
+            public void onEnd(Animator animator) {
+                mTextFadeInAnimation = null;
             }
+        });
         mTextFadeInAnimation.start();
     }
 
@@ -274,6 +287,7 @@ class StatusIndicatorMediator
             @Override
             public void onEnd(Animator animation) {
                 animationCompleteCallback.run();
+                mUpdateAnimatorSet = null;
             }
         });
         mUpdateAnimatorSet.start();
@@ -328,6 +342,7 @@ class StatusIndicatorMediator
                 } else {
                     updateVisibility(true);
                 }
+                mHideAnimatorSet = null;
             }
         });
         mHideAnimatorSet.start();
@@ -394,6 +409,7 @@ class StatusIndicatorMediator
             mBrowserControlsStateProvider.removeObserver(this);
             mIsHiding = false;
             mJavaLayoutHeight = 0;
+            mDestroyView.run();
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/status_indicator/StatusIndicatorSceneLayer.java

@@ -8,7 +8,6 @@ import android.graphics.RectF;
 
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
-import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
 import org.chromium.chrome.browser.compositor.LayerTitleCache;
 import org.chromium.chrome.browser.compositor.layouts.components.VirtualView;
@@ -16,7 +15,6 @@ import org.chromium.chrome.browser.compositor.layouts.eventfilter.EventFilter;
 import org.chromium.chrome.browser.compositor.overlays.SceneOverlay;
 import org.chromium.chrome.browser.compositor.scene_layer.SceneLayer;
 import org.chromium.chrome.browser.compositor.scene_layer.SceneOverlayLayer;
-import org.chromium.components.browser_ui.widget.ViewResourceFrameLayout;
 import org.chromium.ui.resources.ResourceManager;
 
 import java.util.List;
@@ -33,23 +31,18 @@ class StatusIndicatorSceneLayer extends SceneOverlayLayer implements SceneOverla
     /** The resource ID used to reference the view bitmap in native. */
     private int mResourceId;
 
-    /** The {@link ViewResourceFrameLayout} that this scene layer represents. */
-    private ViewResourceFrameLayout mStatusIndicator;
-
-    /** A {@link Supplier} for accessing the {@link BrowserControlsStateProvider}. */
-    private Supplier<BrowserControlsStateProvider> mBrowserControlsStateProviderSupplier;
+    /** The {@link BrowserControlsStateProvider} to access browser controls offsets. */
+    private BrowserControlsStateProvider mBrowserControlsStateProvider;
 
     private boolean mIsVisible;
 
     /**
      * Build a composited status view layer.
-     * @param statusIndicator The view used to generate the composited version.
+     * @param browserControlsStateProvider {@link BrowserControlsStateProvider} to access browser
+     *                                     controls offsets.
      */
-    public StatusIndicatorSceneLayer(ViewResourceFrameLayout statusIndicator,
-            Supplier<BrowserControlsStateProvider> browserControlsStateProviderSupplier) {
-        mStatusIndicator = statusIndicator;
-        mResourceId = mStatusIndicator.getId();
-        mBrowserControlsStateProviderSupplier = browserControlsStateProviderSupplier;
+    public StatusIndicatorSceneLayer(BrowserControlsStateProvider browserControlsStateProvider) {
+        mBrowserControlsStateProvider = browserControlsStateProvider;
     }
 
     /**
@@ -60,6 +53,14 @@ class StatusIndicatorSceneLayer extends SceneOverlayLayer implements SceneOverla
         mIsVisible = visible;
     }
 
+    /**
+     * Set the resource ID.
+     * @param id Resource view ID.
+     */
+    public void setResourceId(int id) {
+        mResourceId = id;
+    }
+
     @Override
     protected void initializeNative() {
         if (mNativePtr == 0) {
@@ -77,11 +78,7 @@ class StatusIndicatorSceneLayer extends SceneOverlayLayer implements SceneOverla
     @Override
     public SceneOverlayLayer getUpdatedSceneOverlayTree(RectF viewport, RectF visibleViewport,
             LayerTitleCache layerTitleCache, ResourceManager resourceManager, float yOffset) {
-        final BrowserControlsStateProvider browserControlsStateProvider =
-                mBrowserControlsStateProviderSupplier.get();
-        final int offset = browserControlsStateProvider != null
-                ? browserControlsStateProvider.getTopControlsMinHeightOffset()
-                : 0;
+        final int offset = mBrowserControlsStateProvider.getTopControlsMinHeightOffset();
         StatusIndicatorSceneLayerJni.get().updateStatusIndicatorLayer(
                 mNativePtr, StatusIndicatorSceneLayer.this, resourceManager, mResourceId, offset);
         return this;

chrome/android/javatests/src/org/chromium/chrome/browser/status_indicator/StatusIndicatorViewBinderTest.java

@@ -64,7 +64,7 @@ public class StatusIndicatorViewBinderTest extends DummyUiActivityTestCase {
             mContainer = getActivity().findViewById(R.id.status_indicator);
             mStatusTextView = mContainer.findViewById(R.id.status_text);
 
-            mSceneLayer = new MockStatusIndicatorSceneLayer(mContainer);
+            mSceneLayer = new MockStatusIndicatorSceneLayer();
             mModel = new PropertyModel.Builder(StatusIndicatorProperties.ALL_KEYS)
                              .with(StatusIndicatorProperties.STATUS_TEXT, "")
                              .with(StatusIndicatorProperties.STATUS_ICON, null)
@@ -197,8 +197,8 @@ public class StatusIndicatorViewBinderTest extends DummyUiActivityTestCase {
 
     /** Mock {@link StatusIndicatorSceneLayer} class to avoid native initialization. */
     private class MockStatusIndicatorSceneLayer extends StatusIndicatorSceneLayer {
-        MockStatusIndicatorSceneLayer(ViewResourceFrameLayout statusIndicator) {
-            super(statusIndicator, null);
+        MockStatusIndicatorSceneLayer() {
+            super(null);
         }
 
         @Override

chrome/android/junit/src/org/chromium/chrome/browser/status_indicator/StatusIndicatorMediatorTest.java

@@ -46,6 +46,10 @@ public class StatusIndicatorMediatorTest {
     @Mock
     StatusIndicatorCoordinator.StatusIndicatorObserver mObserver;
     @Mock
+    Runnable mInflateView;
+    @Mock
+    Runnable mDestroyView;
+    @Mock
     Supplier<Boolean> mCanAnimateNativeBrowserControls;
     @Mock
     Callback<Runnable> mInvalidateCompositorView;
@@ -58,6 +62,8 @@ public class StatusIndicatorMediatorTest {
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
+        doNothing().when(mInflateView).run();
+        doNothing().when(mDestroyView).run();
         when(mCanAnimateNativeBrowserControls.get()).thenReturn(true);
         doNothing().when(mInvalidateCompositorView).onResult(any(Runnable.class));
         doNothing().when(mRequestLayout).run();
@@ -65,9 +71,10 @@ public class StatusIndicatorMediatorTest {
                          .with(StatusIndicatorProperties.ANDROID_VIEW_VISIBILITY, View.GONE)
                          .with(StatusIndicatorProperties.COMPOSITED_VIEW_VISIBLE, false)
                          .build();
-        mMediator = new StatusIndicatorMediator(mModel, mBrowserControlsStateProvider,
-                () -> Color.WHITE, mCanAnimateNativeBrowserControls, mInvalidateCompositorView,
-                mRequestLayout);
+        mMediator = new StatusIndicatorMediator(mBrowserControlsStateProvider, () -> Color.WHITE,
+                mInflateView, mDestroyView, mCanAnimateNativeBrowserControls,
+                mInvalidateCompositorView, mRequestLayout);
+        mMediator.setModel(mModel);
     }
 
     @Test