Lazily clear Grid Tab Switcher RecyclerView

In the Tab-to-Grid transition animation, constructing the RecyclerView
is very time consuming. The RecyclerView was recycled after the
Grid-to-Tab animation, and this CL postpone it later. If the
RecyclerView is still there when the Tab-to-Grid animation starts,
there will be a lot less work to do, resulting in higher frame rate.

The delay can be configured by field trial flag "cleanup-delay", with
default of 10 seconds. Setting to 0 would fall back to old behavior.

We can also opportunistically skip the Tab-to-Grid transition animation
if we have to reconstruct the RecyclerView. This can be configured by
field trial flag "skip-slow-zooming", and default to false. If skipped,
we fall back to the fading animation.

This CL also stops force updating the thumbnail of the current tab
when entering GTS. This could lead to stale thumbnail, but runs much
faster.

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

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

@@ -44,8 +44,9 @@ public interface GridTabSwitcher {
      * Before calling {@link OverviewModeController#showOverview} to start showing the
      * GridTabSwitcher {@link TabListRecyclerView}, call this to populate it without making it
      * visible.
+     * @return Whether the {@link TabListRecyclerView} can be shown quickly.
      */
-    void prepareOverview();
+    boolean prepareOverview();
 
     /**
      * This is called after the compositor animation is done, for potential clean-up work.

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

@@ -106,9 +106,9 @@ public class GridTabSwitcherCoordinator
     }
 
     @Override
-    public void prepareOverview() {
+    public boolean prepareOverview() {
         mTabGridCoordinator.prepareOverview();
-        mMediator.prepareOverview();
+        return mMediator.prepareOverview();
     }
 
     @Override
@@ -138,7 +138,7 @@ public class GridTabSwitcherCoordinator
      * @param tabList The current {@link TabList} to show the tabs for in the grid.
      */
     @Override
-    public void resetWithTabList(TabList tabList) {
+    public boolean resetWithTabList(TabList tabList) {
         List<Tab> tabs = null;
         if (tabList != null) {
             tabs = new ArrayList<>();
@@ -146,7 +146,7 @@ public class GridTabSwitcherCoordinator
                 tabs.add(tabList.getTabAt(i));
             }
         }
-        mTabGridCoordinator.resetWithListOfTabs(tabs);
+        return mTabGridCoordinator.resetWithListOfTabs(tabs);
     }
 
     @Override

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.os.Handler;
 import android.support.annotation.Nullable;
 
 import org.chromium.base.ContextUtils;
@@ -56,6 +57,13 @@ class GridTabSwitcherMediator
 
     private static final int DEFAULT_TOP_PADDING = 0;
 
+    /** Field trial parameter for the {@link TabListRecyclerView} cleanup delay. */
+    private static final String CLEANUP_DELAY_PARAM = "cleanup-delay";
+    private static final int DEFAULT_CLEANUP_DELAY_MS = 10_000;
+    private Integer mCleanupDelayMsForTesting;
+    private final Handler mHandler;
+    private final Runnable mClearTabListRunnable;
+
     private final ResetHandler mResetHandler;
     private final PropertyModel mContainerViewModel;
     private final TabModelSelector mTabModelSelector;
@@ -94,7 +102,10 @@ class GridTabSwitcherMediator
      * Interface to delegate resetting the tab grid.
      */
     interface ResetHandler {
-        void resetWithTabList(TabList tabList);
+        /**
+         * @return Whether the {@link TabListRecyclerView} can be shown quickly.
+         */
+        boolean resetWithTabList(TabList tabList);
     }
 
     /**
@@ -172,6 +183,21 @@ class GridTabSwitcherMediator
 
         mCompositorViewHolder = compositorViewHolder;
         mTabGridDialogResetHandler = tabGridDialogResetHandler;
+
+        mClearTabListRunnable = () -> mResetHandler.resetWithTabList(null);
+        mHandler = new Handler();
+    }
+
+    private int getCleanupDelay() {
+        if (mCleanupDelayMsForTesting != null) return mCleanupDelayMsForTesting;
+
+        String delay = ChromeFeatureList.getFieldTrialParamByFeature(
+                ChromeFeatureList.TAB_TO_GTS_ANIMATION, CLEANUP_DELAY_PARAM);
+        try {
+            return Integer.valueOf(delay);
+        } catch (NumberFormatException e) {
+            return DEFAULT_CLEANUP_DELAY_MS;
+        }
     }
 
     private void setVisibility(boolean isVisible) {
@@ -210,14 +236,16 @@ class GridTabSwitcherMediator
         mContainerViewModel.set(ANIMATE_VISIBILITY_CHANGES, true);
     }
 
-    void prepareOverview() {
-        mResetHandler.resetWithTabList(
+    boolean prepareOverview() {
+        mHandler.removeCallbacks(mClearTabListRunnable);
+        boolean quick = mResetHandler.resetWithTabList(
                 mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter());
         int initialPosition = Math.max(
                 mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter().index()
                         - INITIAL_SCROLL_INDEX_OFFSET,
                 0);
         mContainerViewModel.set(INITIAL_SCROLL_INDEX, initialPosition);
+        return quick;
     }
 
     @Override
@@ -257,10 +285,19 @@ class GridTabSwitcherMediator
         }
     }
 
+    /**
+     * Do clean-up work after the overview hiding animation is finished.
+     * @see GridTabSwitcher#postHiding
+     */
     void postHiding() {
-        // TODO(crbug.com/964406): see if we can lazily release it.
-        mResetHandler.resetWithTabList(null);
-        mContainerViewModel.set(INITIAL_SCROLL_INDEX, 0);
+        mHandler.postDelayed(mClearTabListRunnable, getCleanupDelay());
+    }
+
+    /**
+     * Set the delay for lazy cleanup.
+     */
+    void setCleanupDelayForTesting(int timeoutMs) {
+        mCleanupDelayMsForTesting = timeoutMs;
     }
 
     /**

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

@@ -184,13 +184,12 @@ public class TabListCoordinator implements Destroyable {
      * Reset the tab grid with the given List of Tabs. Can be null.
      * @param tabs List of Tabs to show for in the UI.
      */
-    public void resetWithListOfTabs(@Nullable List<Tab> tabs) {
-        mMediator.resetWithListOfTabs(tabs);
-
+    public boolean resetWithListOfTabs(@Nullable List<Tab> tabs) {
         if (mMode == TabListMode.STRIP && tabs != null && tabs.size() > 1) {
             TabGroupUtils.maybeShowIPH(
                     FeatureConstants.TAB_GROUPS_TAP_TO_SEE_ANOTHER_TAB_FEATURE, mRecyclerView);
         }
+        return mMediator.resetWithListOfTabs(tabs);
     }
 
     void prepareOverview() {

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

@@ -495,21 +495,55 @@ class TabListMediator {
         return position != TabModel.INVALID_TAB_INDEX && position < mModel.size();
     }
 
+    private boolean areTabsUnchanged(@Nullable List<Tab> tabs) {
+        if (tabs == null) {
+            return mModel.size() == 0;
+        }
+        if (tabs.size() != mModel.size()) return false;
+        for (int i = 0; i < tabs.size(); i++) {
+            if (tabs.get(i).getId() != mModel.get(i).get(TabProperties.TAB_ID)) return false;
+        }
+        return true;
+    }
+
     /**
      * Initialize the component with a list of tabs to show in a grid.
      * @param tabs The list of tabs to be shown.
+     * @return Whether the {@link TabListRecyclerView} can be shown quickly.
      */
-    public void resetWithListOfTabs(@Nullable List<Tab> tabs) {
+    public boolean resetWithListOfTabs(@Nullable List<Tab> tabs) {
+        if (areTabsUnchanged(tabs)) {
+            if (tabs == null) return true;
+
+            for (int i = 0; i < tabs.size(); i++) {
+                Tab tab = tabs.get(i);
+
+                boolean isSelected = mTabModelSelector.getCurrentTab() == tab;
+                mModel.get(i).set(TabProperties.IS_SELECTED, isSelected);
+
+                if (mThumbnailProvider != null && isSelected) {
+                    // TODO(crbug.com/964406): should force update but it's too slow.
+                    ThumbnailFetcher callback =
+                            new ThumbnailFetcher(mThumbnailProvider, tab, false);
+                    mModel.get(i).set(TabProperties.THUMBNAIL_FETCHER, callback);
+                }
+
+                mModel.get(i).set(TabProperties.CREATE_GROUP_LISTENER,
+                        getCreateGroupButtonListener(tab, isSelected));
+            }
+            return true;
+        }
         mModel.set(new ArrayList<>());
         if (tabs == null) {
-            return;
+            return true;
         }
         Tab currentTab = mTabModelSelector.getCurrentTab();
-        if (currentTab == null) return;
+        if (currentTab == null) return false;
 
         for (int i = 0; i < tabs.size(); i++) {
             addTabInfoToModel(tabs.get(i), i, tabs.get(i).getId() == currentTab.getId());
         }
+        return false;
     }
 
     /**
@@ -644,11 +678,6 @@ class TabListMediator {
     }
 
     private void addTabInfoToModel(final Tab tab, int index, boolean isSelected) {
-        TabActionListener createGroupButtonOnClickListener = null;
-        if (isSelected && mCreateGroupButtonProvider != null) {
-            createGroupButtonOnClickListener =
-                    mCreateGroupButtonProvider.getCreateGroupButtonOnClickListener(tab);
-        }
         boolean showIPH = false;
         if (mCloseAllRelatedTabs && !mShownIPH) {
             showIPH = getRelatedTabsForId(tab.getId()).size() > 1;
@@ -671,7 +700,8 @@ class TabListMediator {
                         .with(TabProperties.IPH_PROVIDER, showIPH ? mIphProvider : null)
                         .with(TabProperties.TAB_SELECTED_LISTENER, tabSelectedListener)
                         .with(TabProperties.TAB_CLOSED_LISTENER, mTabClosedListener)
-                        .with(TabProperties.CREATE_GROUP_LISTENER, createGroupButtonOnClickListener)
+                        .with(TabProperties.CREATE_GROUP_LISTENER,
+                                getCreateGroupButtonListener(tab, isSelected))
                         .with(TabProperties.ALPHA, 1f)
                         .build();
 
@@ -696,4 +726,14 @@ class TabListMediator {
         }
         tab.addObserver(mTabObserver);
     }
+
+    @Nullable
+    private TabActionListener getCreateGroupButtonListener(Tab tab, boolean isSelected) {
+        TabActionListener createGroupButtonOnClickListener = null;
+        if (isSelected && mCreateGroupButtonProvider != null) {
+            createGroupButtonOnClickListener =
+                    mCreateGroupButtonProvider.getCreateGroupButtonOnClickListener(tab);
+        }
+        return createGroupButtonOnClickListener;
+    }
 }

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

@@ -285,9 +285,9 @@ public class GridTabSwitcherMediatorUnitTest {
     @Test
     public void resetsToNullAfterHidingFinishes() {
         initAndAssertAllProperties();
+        mMediator.setCleanupDelayForTesting(0);
         mMediator.postHiding();
         verify(mResetHandler).resetWithTabList(eq(null));
-        assertThat(mModel.get(TabListContainerProperties.INITIAL_SCROLL_INDEX), equalTo(0));
     }
 
     @Test

chrome/android/java/src/org/chromium/chrome/browser/widget/GridTabSwitcherLayout.java

@@ -51,6 +51,10 @@ public class GridTabSwitcherLayout
     @VisibleForTesting
     static final long ZOOMING_DURATION = 300;
 
+    /** Field trial parameter for whether skipping slow zooming animation */
+    private static final String SKIP_SLOW_ZOOMING_PARAM = "skip-slow-zooming";
+    private static final boolean DEFAULT_SKIP_SLOW_ZOOMING = false;
+
     // The transition animation from a tab to the tab switcher.
     private AnimatorSet mTabToSwitcherAnimation;
 
@@ -101,7 +105,10 @@ public class GridTabSwitcherLayout
 
         boolean showShrinkingAnimation =
                 animate && ChromeFeatureList.isEnabled(ChromeFeatureList.TAB_TO_GTS_ANIMATION);
-        mGridTabSwitcher.prepareOverview();
+        boolean quick = mGridTabSwitcher.prepareOverview();
+        if (getSkipSlowZooming()) {
+            showShrinkingAnimation &= quick;
+        }
 
         if (!showShrinkingAnimation) {
             mGridController.showOverview(animate);
@@ -165,6 +172,7 @@ public class GridTabSwitcherLayout
     @Override
     public void onOverviewModeFinishedShowing() {
         doneShowing();
+        mTabContentManager.cacheTabThumbnail(mTabModelSelector.getCurrentTab());
     }
 
     @Override
@@ -320,6 +328,16 @@ public class GridTabSwitcherLayout
         }
     }
 
+    private boolean getSkipSlowZooming() {
+        String skip = ChromeFeatureList.getFieldTrialParamByFeature(
+                ChromeFeatureList.TAB_TO_GTS_ANIMATION, SKIP_SLOW_ZOOMING_PARAM);
+        try {
+            return Boolean.valueOf(skip);
+        } catch (NumberFormatException e) {
+            return DEFAULT_SKIP_SLOW_ZOOMING;
+        }
+    }
+
     @Override
     protected void updateSceneLayer(RectF viewport, RectF contentViewport,
             LayerTitleCache layerTitleCache, TabContentManager tabContentManager,

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

@@ -51,8 +51,9 @@ import java.util.List;
 
 /** Tests for the {@link GridTabSwitcherLayout}, mainly for animation performance. */
 @RunWith(ChromeJUnit4ClassRunner.class)
-@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
-@Features.EnableFeatures(ChromeFeatureList.TAB_TO_GTS_ANIMATION)
+@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"})
 @Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
 public class GridTabSwitcherLayoutPerfTest {
     private static final String TAG = "GTSLayoutTest";
@@ -109,6 +110,14 @@ public class GridTabSwitcherLayoutPerfTest {
         reportTabToGridPerf(mUrl, "Tab-to-Grid from live tab with 10 tabs");
     }
 
+    @Test
+    @MediumTest
+    @CommandLineFlags.Add({"force-fieldtrial-params=Study.Group:cleanup-delay/10000"})
+    public void testTabToGridFromLiveTabWith10TabsWarm() throws InterruptedException {
+        prepareTabs(10, NTP_URL);
+        reportTabToGridPerf(mUrl, "Tab-to-Grid from live tab with 10 tabs (warm)");
+    }
+
     @Test
     @MediumTest
     public void testTabToGridFromLiveTabWith10TabsWithoutThumbnail() throws InterruptedException {
@@ -249,10 +258,8 @@ public class GridTabSwitcherLayoutPerfTest {
             frameRates.add(fps);
             frameInterval.add((float) maxFrameInterval);
         };
-
         Thread.sleep(mWaitingTime);
 
-        GridTabSwitcher gts = mGtsLayout.getGridTabSwitcherForTesting();
         for (int i = 0; i < mRepeat; i++) {
             mGtsLayout.setPerfListenerForTesting(null);
             TestThreadUtils.runOnUiThreadBlocking(