Add a soft cleanup stage for Grid Tab Switcher RecyclerView

Before the Grid Tab Switcher RecyclerView is fully cleared, add an
intermediate state where the thumbnails are released, but the Views are
kept.

Bug: 968822
Change-Id: I0f795773a73949daf7e04c8c6c51e682ed95cfe2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1643070
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666380}

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/GridTabSwitcherCoordinator.java

@@ -151,6 +151,11 @@ public class GridTabSwitcherCoordinator
         return mTabGridCoordinator.resetWithListOfTabs(tabs);
     }
 
+    @Override
+    public void softCleanup() {
+        mTabGridCoordinator.softCleanup();
+    }
+
     @Override
     public void destroy() {
         mTabGridCoordinator.destroy();

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/GridTabSwitcherMediator.java

@@ -13,6 +13,7 @@ import static org.chromium.chrome.browser.tasks.tab_management.TabListContainerP
 import static org.chromium.chrome.browser.tasks.tab_management.TabListContainerProperties.TOP_PADDING;
 import static org.chromium.chrome.browser.tasks.tab_management.TabListContainerProperties.VISIBILITY_LISTENER;
 
+import android.graphics.Bitmap;
 import android.os.Handler;
 import android.support.annotation.Nullable;
 
@@ -58,10 +59,14 @@ class GridTabSwitcherMediator
     private static final int DEFAULT_TOP_PADDING = 0;
 
     /** Field trial parameter for the {@link TabListRecyclerView} cleanup delay. */
+    private static final String SOFT_CLEANUP_DELAY_PARAM = "soft-cleanup-delay";
+    private static final int DEFAULT_SOFT_CLEANUP_DELAY_MS = 3_000;
     private static final String CLEANUP_DELAY_PARAM = "cleanup-delay";
-    private static final int DEFAULT_CLEANUP_DELAY_MS = 10_000;
+    private static final int DEFAULT_CLEANUP_DELAY_MS = 30_000;
+    private Integer mSoftCleanupDelayMsForTesting;
     private Integer mCleanupDelayMsForTesting;
     private final Handler mHandler;
+    private final Runnable mSoftClearTabListRunnable;
     private final Runnable mClearTabListRunnable;
 
     private final ResetHandler mResetHandler;
@@ -106,6 +111,11 @@ class GridTabSwitcherMediator
          * @return Whether the {@link TabListRecyclerView} can be shown quickly.
          */
         boolean resetWithTabList(TabList tabList);
+
+        /**
+         * Release the thumbnail {@link Bitmap} but keep the {@link TabGridViewHolder}.
+         */
+        void softCleanup();
     }
 
     /**
@@ -184,10 +194,23 @@ class GridTabSwitcherMediator
         mCompositorViewHolder = compositorViewHolder;
         mTabGridDialogResetHandler = tabGridDialogResetHandler;
 
+        mSoftClearTabListRunnable = mResetHandler::softCleanup;
         mClearTabListRunnable = () -> mResetHandler.resetWithTabList(null);
         mHandler = new Handler();
     }
 
+    private int getSoftCleanupDelay() {
+        if (mSoftCleanupDelayMsForTesting != null) return mSoftCleanupDelayMsForTesting;
+
+        String delay = ChromeFeatureList.getFieldTrialParamByFeature(
+                ChromeFeatureList.TAB_TO_GTS_ANIMATION, SOFT_CLEANUP_DELAY_PARAM);
+        try {
+            return Integer.valueOf(delay);
+        } catch (NumberFormatException e) {
+            return DEFAULT_SOFT_CLEANUP_DELAY_MS;
+        }
+    }
+
     private int getCleanupDelay() {
         if (mCleanupDelayMsForTesting != null) return mCleanupDelayMsForTesting;
 
@@ -237,6 +260,7 @@ class GridTabSwitcherMediator
     }
 
     boolean prepareOverview() {
+        mHandler.removeCallbacks(mSoftClearTabListRunnable);
         mHandler.removeCallbacks(mClearTabListRunnable);
         boolean quick = mResetHandler.resetWithTabList(
                 mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter());
@@ -290,9 +314,17 @@ class GridTabSwitcherMediator
      * @see GridTabSwitcher#postHiding
      */
     void postHiding() {
+        mHandler.postDelayed(mSoftClearTabListRunnable, getSoftCleanupDelay());
         mHandler.postDelayed(mClearTabListRunnable, getCleanupDelay());
     }
 
+    /**
+     * Set the delay for soft cleanup.
+     */
+    void setSoftCleanupDelayForTesting(int timeoutMs) {
+        mSoftCleanupDelayMsForTesting = timeoutMs;
+    }
+
     /**
      * Set the delay for lazy cleanup.
      */

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabGridViewBinder.java

@@ -58,19 +58,12 @@ class TabGridViewBinder {
                 holder.itemView.setForeground(
                         item.get(TabProperties.IS_SELECTED) ? drawable : null);
             }
+        } else if (TabProperties.IS_HIDDEN == propertyKey) {
+            updateThumbnail(holder, item);
         } else if (TabProperties.FAVICON == propertyKey) {
             holder.favicon.setImageDrawable(item.get(TabProperties.FAVICON));
         } else if (TabProperties.THUMBNAIL_FETCHER == propertyKey) {
-            TabListMediator.ThumbnailFetcher fetcher = item.get(TabProperties.THUMBNAIL_FETCHER);
-            if (fetcher == null) return;
-            Callback<Bitmap> callback = result -> {
-                if (result == null) {
-                    holder.resetThumbnail();
-                } else {
-                    holder.thumbnail.setImageBitmap(result);
-                }
-            };
-            fetcher.fetch(callback);
+            updateThumbnail(holder, item);
         } else if (TabProperties.TAB_ID == propertyKey) {
             holder.setTabId(item.get(TabProperties.TAB_ID));
         }
@@ -125,4 +118,23 @@ class TabGridViewBinder {
             TabGridViewHolder holder, PropertyModel item, PropertyKey propertyKey) {
         // TODO(meiliang): Bind SELECTABLE_TAB properties
     }
+
+    private static void updateThumbnail(TabGridViewHolder holder, PropertyModel item) {
+        if (item.get(TabProperties.IS_HIDDEN)) {
+            // Release the thumbnail to save memory.
+            holder.resetThumbnail();
+            return;
+        }
+
+        TabListMediator.ThumbnailFetcher fetcher = item.get(TabProperties.THUMBNAIL_FETCHER);
+        if (fetcher == null) return;
+        Callback<Bitmap> callback = result -> {
+            if (result == null) {
+                holder.resetThumbnail();
+            } else {
+                holder.thumbnail.setImageBitmap(result);
+            }
+        };
+        fetcher.fetch(callback);
+    }
 }

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListCoordinator.java

@@ -192,6 +192,10 @@ public class TabListCoordinator implements Destroyable {
         return mMediator.resetWithListOfTabs(tabs);
     }
 
+    void softCleanup() {
+        mMediator.softCleanup();
+    }
+
     void prepareOverview() {
         mRecyclerView.prepareOverview();
     }

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java

@@ -521,6 +521,8 @@ class TabListMediator {
             for (int i = 0; i < tabs.size(); i++) {
                 Tab tab = tabs.get(i);
 
+                mModel.get(i).set(TabProperties.IS_HIDDEN, false);
+
                 boolean isSelected = mTabModelSelector.getCurrentTab() == tab;
                 mModel.get(i).set(TabProperties.IS_SELECTED, isSelected);
 
@@ -547,6 +549,15 @@ class TabListMediator {
         return false;
     }
 
+    /**
+     * @see GridTabSwitcherMediator.ResetHandler#softCleanup
+     */
+    void softCleanup() {
+        for (int i = 0; i < mModel.size(); i++) {
+            mModel.get(i).set(TabProperties.IS_HIDDEN, true);
+        }
+    }
+
     /**
      * @return The callback that hosts the logic for swipe and drag related actions.
      */
@@ -698,6 +709,7 @@ class TabListMediator {
                         .with(TabProperties.FAVICON,
                                 mTabListFaviconProvider.getDefaultFaviconDrawable())
                         .with(TabProperties.IS_SELECTED, isSelected)
+                        .with(TabProperties.IS_HIDDEN, false)
                         .with(TabProperties.IPH_PROVIDER, showIPH ? mIphProvider : null)
                         .with(TabProperties.TAB_SELECTED_LISTENER, tabSelectedListener)
                         .with(TabProperties.TAB_CLOSED_LISTENER, mTabClosedListener)

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabProperties.java

@@ -37,6 +37,8 @@ public class TabProperties {
 
     public static final WritableBooleanPropertyKey IS_SELECTED = new WritableBooleanPropertyKey();
 
+    public static final WritableBooleanPropertyKey IS_HIDDEN = new WritableBooleanPropertyKey();
+
     public static final WritableObjectPropertyKey<TabListMediator.TabActionListener>
             CREATE_GROUP_LISTENER = new WritableObjectPropertyKey<>();
 
@@ -45,7 +47,7 @@ public class TabProperties {
 
     public static final PropertyKey[] ALL_KEYS_TAB_GRID = new PropertyKey[] {TAB_ID,
             TAB_SELECTED_LISTENER, TAB_CLOSED_LISTENER, FAVICON, THUMBNAIL_FETCHER, IPH_PROVIDER,
-            TITLE, IS_SELECTED, CREATE_GROUP_LISTENER, ALPHA};
+            TITLE, IS_SELECTED, IS_HIDDEN, CREATE_GROUP_LISTENER, ALPHA};
 
     public static final PropertyKey[] ALL_KEYS_TAB_STRIP = new PropertyKey[] {
             TAB_ID, TAB_SELECTED_LISTENER, TAB_CLOSED_LISTENER, FAVICON, IS_SELECTED, TITLE};

chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabListViewHolderTest.java

@@ -34,6 +34,7 @@ import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
 
 import java.lang.ref.WeakReference;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * Tests for the {@link android.support.v7.widget.RecyclerView.ViewHolder} classes for {@link
@@ -58,8 +59,10 @@ public class TabListViewHolderTest extends DummyUiActivityTestCase {
                             ? Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888)
                             : null;
                     callback.onResult(bitmap);
+                    mThumbnailFetchedCount.incrementAndGet();
                 }
             }, null, false);
+    private AtomicInteger mThumbnailFetchedCount = new AtomicInteger();
 
     private TabListMediator.TabActionListener mMockCloseListener =
             new TabListMediator.TabActionListener() {
@@ -160,6 +163,7 @@ public class TabListViewHolderTest extends DummyUiActivityTestCase {
         mShouldReturnBitmap = true;
         mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
         assertThat(mTabGridViewHolder.thumbnail.getDrawable(), instanceOf(BitmapDrawable.class));
+        Assert.assertEquals(2, mThumbnailFetchedCount.get());
     }
 
     @Test
@@ -179,6 +183,7 @@ public class TabListViewHolderTest extends DummyUiActivityTestCase {
         mShouldReturnBitmap = false;
         mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
         Assert.assertTrue(canBeGarbageCollected(ref));
+        Assert.assertEquals(2, mThumbnailFetchedCount.get());
     }
 
     @Test
@@ -197,6 +202,7 @@ public class TabListViewHolderTest extends DummyUiActivityTestCase {
 
         mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
         Assert.assertTrue(canBeGarbageCollected(ref));
+        Assert.assertEquals(2, mThumbnailFetchedCount.get());
     }
 
     @Test
@@ -216,6 +222,71 @@ public class TabListViewHolderTest extends DummyUiActivityTestCase {
         Assert.assertTrue(canBeGarbageCollected(ref));
     }
 
+    @Test
+    @MediumTest
+    @UiThreadTest
+    public void testHiddenGC() throws Exception {
+        mShouldReturnBitmap = true;
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
+        assertThat(mTabGridViewHolder.thumbnail.getDrawable(), instanceOf(BitmapDrawable.class));
+        Bitmap bitmap = ((BitmapDrawable) mTabGridViewHolder.thumbnail.getDrawable()).getBitmap();
+        WeakReference<Bitmap> ref = new WeakReference<>(bitmap);
+        bitmap = null;
+
+        Assert.assertFalse(canBeGarbageCollected(ref));
+
+        mGridModel.set(TabProperties.IS_HIDDEN, true);
+        Assert.assertTrue(canBeGarbageCollected(ref));
+        Assert.assertNull(mTabGridViewHolder.thumbnail.getDrawable());
+        Assert.assertEquals(1, mThumbnailFetchedCount.get());
+    }
+
+    @Test
+    @MediumTest
+    @UiThreadTest
+    public void testHiddenThenShow() throws Exception {
+        mShouldReturnBitmap = true;
+        mGridModel.set(TabProperties.IS_HIDDEN, false);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
+        assertThat(mTabGridViewHolder.thumbnail.getDrawable(), instanceOf(BitmapDrawable.class));
+        Assert.assertEquals(1, mThumbnailFetchedCount.get());
+
+        mGridModel.set(TabProperties.IS_HIDDEN, true);
+        Assert.assertNull(mTabGridViewHolder.thumbnail.getDrawable());
+        Assert.assertEquals(1, mThumbnailFetchedCount.get());
+
+        mGridModel.set(TabProperties.IS_HIDDEN, false);
+        assertThat(mTabGridViewHolder.thumbnail.getDrawable(), instanceOf(BitmapDrawable.class));
+        Assert.assertEquals(2, mThumbnailFetchedCount.get());
+    }
+
+    @Test
+    @MediumTest
+    @UiThreadTest
+    public void testSkipFetchingWhenHidden() throws Exception {
+        mShouldReturnBitmap = true;
+        mGridModel.set(TabProperties.IS_HIDDEN, true);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, null);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, null);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
+        Assert.assertNull(mTabGridViewHolder.thumbnail.getDrawable());
+        Assert.assertEquals(0, mThumbnailFetchedCount.get());
+
+        mGridModel.set(TabProperties.IS_HIDDEN, false);
+        assertThat(mTabGridViewHolder.thumbnail.getDrawable(), instanceOf(BitmapDrawable.class));
+        Assert.assertEquals(1, mThumbnailFetchedCount.get());
+
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, null);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, null);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, null);
+        mGridModel.set(TabProperties.THUMBNAIL_FETCHER, mMockThumbnailProvider);
+        Assert.assertEquals(4, mThumbnailFetchedCount.get());
+    }
+
     @Test
     @MediumTest
     @UiThreadTest

chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/tab_management/GridTabSwitcherMediatorUnitTest.java

@@ -282,8 +282,10 @@ public class GridTabSwitcherMediatorUnitTest {
     @Test
     public void resetsToNullAfterHidingFinishes() {
         initAndAssertAllProperties();
+        mMediator.setSoftCleanupDelayForTesting(0);
         mMediator.setCleanupDelayForTesting(0);
         mMediator.postHiding();
+        verify(mResetHandler).softCleanup();
         verify(mResetHandler).resetWithTabList(eq(null));
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/widget/GridTabSwitcherLayoutPerfTest.java

@@ -53,7 +53,7 @@ import java.util.List;
 @RunWith(ChromeJUnit4ClassRunner.class)
 @CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
         "enable-features=" + ChromeFeatureList.TAB_TO_GTS_ANIMATION + "<Study",
-        "force-fieldtrials=Study/Group", "force-fieldtrial-params=Study.Group:cleanup-delay/0"})
+        "force-fieldtrials=Study/Group"})
 @Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
 public class GridTabSwitcherLayoutPerfTest {
     private static final String TAG = "GTSLayoutTest";
@@ -84,7 +84,7 @@ public class GridTabSwitcherLayoutPerfTest {
         assertTrue(layout instanceof GridTabSwitcherLayout);
         mGtsLayout = (GridTabSwitcherLayout) layout;
         mUrl = mTestServer.getURL("/chrome/test/data/android/navigate/simple.html");
-        mRepeat = 2;
+        mRepeat = 3;
         mWaitingTime = 0;
         mTabNumCap = 3;
 
@@ -98,6 +98,8 @@ public class GridTabSwitcherLayoutPerfTest {
 
     @Test
     @MediumTest
+    @CommandLineFlags.
+    Add({"force-fieldtrial-params=Study.Group:soft-cleanup-delay/0/cleanup-delay/0"})
     public void testTabToGridFromLiveTab() throws InterruptedException {
         prepareTabs(1, NTP_URL);
         reportTabToGridPerf(mUrl, "Tab-to-Grid from live tab");
@@ -105,6 +107,8 @@ public class GridTabSwitcherLayoutPerfTest {
 
     @Test
     @MediumTest
+    @CommandLineFlags.
+    Add({"force-fieldtrial-params=Study.Group:soft-cleanup-delay/0/cleanup-delay/0"})
     public void testTabToGridFromLiveTabWith10Tabs() throws InterruptedException {
         prepareTabs(10, NTP_URL);
         reportTabToGridPerf(mUrl, "Tab-to-Grid from live tab with 10 tabs");
@@ -112,7 +116,8 @@ public class GridTabSwitcherLayoutPerfTest {
 
     @Test
     @MediumTest
-    @CommandLineFlags.Add({"force-fieldtrial-params=Study.Group:cleanup-delay/10000"})
+    @CommandLineFlags.
+    Add({"force-fieldtrial-params=Study.Group:soft-cleanup-delay/10000/cleanup-delay/10000"})
     public void testTabToGridFromLiveTabWith10TabsWarm() throws InterruptedException {
         prepareTabs(10, NTP_URL);
         reportTabToGridPerf(mUrl, "Tab-to-Grid from live tab with 10 tabs (warm)");
@@ -120,7 +125,17 @@ public class GridTabSwitcherLayoutPerfTest {
 
     @Test
     @MediumTest
-    @CommandLineFlags.Add({"force-fieldtrial-params=Study.Group:downsampling-scale/1"})
+    @CommandLineFlags.
+    Add({"force-fieldtrial-params=Study.Group:soft-cleanup-delay/0/cleanup-delay/10000"})
+    public void testTabToGridFromLiveTabWith10TabsSoft() throws InterruptedException {
+        prepareTabs(10, NTP_URL);
+        reportTabToGridPerf(mUrl, "Tab-to-Grid from live tab with 10 tabs (soft)");
+    }
+
+    @Test
+    @MediumTest
+    @CommandLineFlags.
+    Add({"force-fieldtrial-params=Study.Group:downsampling-scale/1/soft-cleanup-delay/0/cleanup-delay/0"})
     public void testTabToGridFromLiveTabWith10TabsNoDownsample() throws InterruptedException {
         prepareTabs(10, NTP_URL);
         reportTabToGridPerf(mUrl, "Tab-to-Grid from live tab with 10 tabs (no downsample)");
@@ -128,6 +143,8 @@ public class GridTabSwitcherLayoutPerfTest {
 
     @Test
     @MediumTest
+    @CommandLineFlags.
+    Add({"force-fieldtrial-params=Study.Group:soft-cleanup-delay/0/cleanup-delay/0"})
     public void testTabToGridFromLiveTabWith10TabsWithoutThumbnail() throws InterruptedException {
         // Note that most of the tabs won't have thumbnails.
         prepareTabs(10, null);
@@ -136,6 +153,8 @@ public class GridTabSwitcherLayoutPerfTest {
 
     @Test
     @LargeTest
+    @CommandLineFlags.
+    Add({"force-fieldtrial-params=Study.Group:soft-cleanup-delay/0/cleanup-delay/0"})
     public void testTabToGridFromLiveTabWith100Tabs() throws InterruptedException {
         // Skip waiting for loading. Otherwise it would take too long.
         // Note that most of the tabs won't have thumbnails.
@@ -145,6 +164,8 @@ public class GridTabSwitcherLayoutPerfTest {
 
     @Test
     @MediumTest
+    @CommandLineFlags.
+    Add({"force-fieldtrial-params=Study.Group:soft-cleanup-delay/0/cleanup-delay/0"})
     public void testTabToGridFromNtp() throws InterruptedException {
         prepareTabs(1, NTP_URL);
         reportTabToGridPerf(NTP_URL, "Tab-to-Grid from NTP");