Read tab thumbnails on demand in TabGridLayoutAndroid

When using two-stage thumbnail fetching with THUMBNAIL_PROVIDER and
THUMBNAIL_KEY, we populate the Java side cache with thumbnails from
all the tabs, and then consume them when scrolling the recycler view.

This causes a few issues. 1) Instead of reading what's visible first,
it reads all the tabs, causing more delay and wasting power. 2) If the
number of tabs exceeds the cache capacity, some of them would be
evicted, and missing from the UI.

This CL replaces THUMBNAIL_PROVIDER and THUMBNAIL_KEY with
THUMBNAIL_FETCHER, and removes the Java side cache. THUMBNAIL_FETCHER
fetches the thumbnail on demand when it is about to be visible.

Bug: 930929
Change-Id: I5ef2e3c87db99240bed76419fb8149ec4212cf6a
Reviewed-on: https://chromium-review.googlesource.com/c/1490440
Commit-Queue: Yusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636156}

chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java

@@ -878,8 +878,8 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
         TraceEvent.begin("ChromeActivity:CompositorInitialization");
         super.initializeCompositor();
 
-        setTabContentManager(new TabContentManager(this, getContentOffsetProvider(),
-                getChromeApplication().getReferencePool(), DeviceClassManager.enableSnapshots()));
+        setTabContentManager(new TabContentManager(
+                this, getContentOffsetProvider(), DeviceClassManager.enableSnapshots()));
         mCompositorViewHolder.onNativeLibraryReady(getWindowAndroid(), getTabContentManager());
 
         if (isContextualSearchAllowed() && ContextualSearchFieldTrial.isEnabled()) {

chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/content/TabContentManager.java

@@ -12,15 +12,12 @@ import android.view.ViewGroup.MarginLayoutParams;
 
 import org.chromium.base.Callback;
 import org.chromium.base.CommandLine;
-import org.chromium.base.DiscardableReferencePool;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.BitmapCache;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.native_page.NativePage;
 import org.chromium.chrome.browser.tab.Tab;
-import org.chromium.chrome.browser.util.ConversionUtils;
 import org.chromium.ui.base.DeviceFormFactor;
 import org.chromium.ui.display.DisplayAndroid;
 
@@ -33,16 +30,11 @@ import java.util.List;
  */
 @JNINamespace("android")
 public class TabContentManager {
-    // The default cache size is an upper limit to have at least 8 thumbnails
-    // (6 visible + 2 padding) at around 400Kb size.
-    private static final int DEFAULT_CACHE_SIZE = (int) (3.2 * ConversionUtils.BYTES_PER_MEGABYTE);
-
     private final float mThumbnailScale;
     private final int mFullResThumbnailsMaxSize;
     private final ContentOffsetProvider mContentOffsetProvider;
     private int[] mPriorityTabIds;
     private long mNativeTabContentManager;
-    private BitmapCache mThumbnailCache;
 
     private final ArrayList<ThumbnailChangeListener> mListeners =
             new ArrayList<ThumbnailChangeListener>();
@@ -82,7 +74,7 @@ public class TabContentManager {
      * @param contentOffsetProvider The provider of content parameter.
      */
     public TabContentManager(Context context, ContentOffsetProvider contentOffsetProvider,
-            DiscardableReferencePool referencePool, boolean snapshotsEnabled) {
+            boolean snapshotsEnabled) {
         mContentOffsetProvider = contentOffsetProvider;
         mSnapshotsEnabled = snapshotsEnabled;
 
@@ -123,9 +115,6 @@ public class TabContentManager {
 
         mPriorityTabIds = new int[mFullResThumbnailsMaxSize];
 
-        mThumbnailCache =
-                referencePool == null ? null : new BitmapCache(referencePool, DEFAULT_CACHE_SIZE);
-
         mNativeTabContentManager = nativeInit(defaultCacheSize,
                 approximationCacheSize, compressionQueueMaxSize, writeQueueMaxSize,
                 useApproximationThumbnails);
@@ -225,42 +214,20 @@ public class TabContentManager {
     }
 
     /**
-     * Call to get a thumbnail for a given tab ID from disk through a {@link Callback}. If there is
-     * no up to date thumbnail on the native cache for the given tab, callback returns null.
+     * Call to get a thumbnail for a given tab from disk through a {@link Callback}. If there is
+     * no up-to-date thumbnail on disk for the given tab, callback returns null.
      * Currently this reads a compressed file from disk and sends the Bitmap over the
-     * JNI boundary after decompressing, also relying on a Java side limited size cache for better
-     * user experience. In its current form, should be used for experimental
+     * JNI boundary after decompressing. In its current form, should be used for experimental
      * purposes only.
      * TODO(yusufo): Change the plumbing so that at the least a {@link android.net.Uri} is sent
      * over JNI of an uncompressed file on disk.
      * @param tab The tab to get the thumbnail for.
-     * @param callback The callback to send the {@link Bitmap} key with.
+     * @param callback The callback to send the {@link Bitmap} with.
      */
-    public void getTabThumbnailWithCallback(Tab tab, Callback<String> callback) {
+    public void getTabThumbnailWithCallback(Tab tab, Callback<Bitmap> callback) {
         if (mNativeTabContentManager == 0 || !mSnapshotsEnabled) return;
 
-        final String url = tab.getUrl();
-        if (mThumbnailCache.getBitmap(url) != null) {
-            callback.onResult(url);
-            return;
-        }
-
-        Callback<Bitmap> bitmapCallback = result -> {
-            if (result != null) mThumbnailCache.putBitmap(url, result);
-            callback.onResult(url);
-        };
-
-        nativeGetTabThumbnailWithCallback(mNativeTabContentManager, tab.getId(), bitmapCallback);
-    }
-
-    /**
-     * This is a syncronous API to get an already cached thumbnail if possible.
-     * See {@link TabContentManager#getTabThumbnailWithCallback(Tab, Callback)} for caching.
-     * @param  key The key to use for getting a cached thumbnail.
-     * @return The currently cached thumbnail under the given key.
-     */
-    public Bitmap provideCachedThumbnailForKey(String key) {
-        return mThumbnailCache.getBitmap(key);
+        nativeGetTabThumbnailWithCallback(mNativeTabContentManager, tab.getId(), callback);
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/tasks/tab_list_ui/TabGridViewBinder.java

@@ -57,10 +57,8 @@ class TabGridViewBinder {
             });
         } else if (TabProperties.FAVICON == propertyKey) {
             holder.favicon.setImageBitmap(item.get(TabProperties.FAVICON));
-        } else if (TabProperties.THUMBNAIL_KEY == propertyKey) {
-            holder.thumbnail.setImageBitmap(
-                    item.get(TabProperties.THUMBNAIL_PROVIDER)
-                            .provideCachedThumbnailForKey(item.get(TabProperties.THUMBNAIL_KEY)));
+        } else if (TabProperties.THUMBNAIL_FETCHER == propertyKey) {
+            item.get(TabProperties.THUMBNAIL_FETCHER).fetch(holder.thumbnail::setImageBitmap);
         } else if (TabProperties.TAB_ID == propertyKey) {
             holder.setTabId(item.get(TabProperties.TAB_ID));
         }

chrome/android/java/src/org/chromium/chrome/browser/tasks/tab_list_ui/TabListMediator.java

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.tasks.tab_list_ui;
 import android.content.Context;
 import android.graphics.Bitmap;
 
+import org.chromium.base.Callback;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
@@ -36,10 +37,25 @@ class TabListMediator {
      * An interface to get the thumbnails to be shown inside the tab grid cards.
      */
     public interface ThumbnailProvider {
-        /**
-         * @return The thumbnail for the given key synchronously.
-         */
-        Bitmap provideCachedThumbnailForKey(String key);
+        void getTabThumbnailWithCallback(Tab tab, Callback<Bitmap> callback);
+    }
+
+    /**
+     * The object to set to TabProperties.THUMBNAIL_FETCHER for the TabGridViewBinder to obtain
+     * the thumbnail asynchronously.
+     */
+    class ThumbnailFetcher {
+        private ThumbnailProvider mThumbnailProvider;
+        private Tab mTab;
+
+        ThumbnailFetcher(ThumbnailProvider provider, Tab tab) {
+            mThumbnailProvider = provider;
+            mTab = tab;
+        }
+
+        void fetch(Callback<Bitmap> callback) {
+            mThumbnailProvider.getTabThumbnailWithCallback(mTab, callback);
+        }
     }
     private final int mFaviconSize;
     private final FaviconHelper mFaviconHelper = new FaviconHelper();
@@ -174,9 +190,7 @@ class TabListMediator {
                         .with(TabProperties.TITLE, tab.getTitle())
                         .with(TabProperties.FAVICON, tab.getFavicon())
                         .with(TabProperties.IS_SELECTED, isSelected)
-                        .with(TabProperties.THUMBNAIL_PROVIDER,
-                                mTabContentManager::provideCachedThumbnailForKey)
-                        .with(TabProperties.THUMBNAIL_KEY, "")
+                        .with(TabProperties.THUMBNAIL_FETCHER, null)
                         .with(TabProperties.TAB_SELECTED_LISTENER, mTabSelectedListener)
                         .with(TabProperties.TAB_CLOSED_LISTENER, mTabClosedListener)
                         .build();
@@ -186,10 +200,9 @@ class TabListMediator {
                     if (mModel.indexFromId(tab.getId()) == Tab.INVALID_TAB_ID) return;
                     mModel.get(mModel.indexFromId(tab.getId())).set(TabProperties.FAVICON, image);
                 });
-        mTabContentManager.getTabThumbnailWithCallback(tab, result -> {
-            if (mModel.indexFromId(tab.getId()) == Tab.INVALID_TAB_ID) return;
-            mModel.get(mModel.indexFromId(tab.getId())).set(TabProperties.THUMBNAIL_KEY, result);
-        });
+        ThumbnailFetcher callback =
+                new ThumbnailFetcher(mTabContentManager::getTabThumbnailWithCallback, tab);
+        tabInfo.set(TabProperties.THUMBNAIL_FETCHER, callback);
         tab.addObserver(mTabObserver);
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/tasks/tab_list_ui/TabProperties.java

@@ -9,7 +9,6 @@ import android.graphics.Bitmap;
 import org.chromium.ui.modelutil.PropertyKey;
 import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.modelutil.PropertyModel.ReadableIntPropertyKey;
-import org.chromium.ui.modelutil.PropertyModel.ReadableObjectPropertyKey;
 import org.chromium.ui.modelutil.PropertyModel.WritableBooleanPropertyKey;
 import org.chromium.ui.modelutil.PropertyModel.WritableObjectPropertyKey;
 
@@ -28,11 +27,8 @@ public class TabProperties {
     public static final WritableObjectPropertyKey<Bitmap> FAVICON =
             new WritableObjectPropertyKey<>();
 
-    public static final WritableObjectPropertyKey<String> THUMBNAIL_KEY =
-            new WritableObjectPropertyKey<>();
-
-    public static final ReadableObjectPropertyKey<TabListMediator.ThumbnailProvider>
-            THUMBNAIL_PROVIDER = new ReadableObjectPropertyKey<>();
+    public static final WritableObjectPropertyKey<TabListMediator.ThumbnailFetcher>
+            THUMBNAIL_FETCHER = new WritableObjectPropertyKey<>();
 
     public static final WritableObjectPropertyKey<String> TITLE = new WritableObjectPropertyKey<>();
 
@@ -40,7 +36,7 @@ public class TabProperties {
 
     public static final PropertyKey[] ALL_KEYS_TAB_GRID =
             new PropertyKey[] {TAB_ID, TAB_SELECTED_LISTENER, TAB_CLOSED_LISTENER, FAVICON,
-                    THUMBNAIL_PROVIDER, THUMBNAIL_KEY, TITLE, IS_SELECTED};
+                    THUMBNAIL_FETCHER, TITLE, IS_SELECTED};
 
     public static final PropertyKey[] ALL_KEYS_TAB_STRIP = new PropertyKey[] {
             TAB_ID, TAB_SELECTED_LISTENER, TAB_CLOSED_LISTENER, FAVICON, IS_SELECTED};

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorObserverTestRule.java

@@ -81,8 +81,8 @@ public class TabModelSelectorObserverTestRule extends ChromeBrowserTestRule {
         };
 
         TabModelOrderController orderController = new TabModelOrderController(mSelector);
-        TabContentManager tabContentManager = new TabContentManager(
-                InstrumentationRegistry.getTargetContext(), null, null, false);
+        TabContentManager tabContentManager =
+                new TabContentManager(InstrumentationRegistry.getTargetContext(), null, false);
         TabPersistencePolicy persistencePolicy = new TabbedModeTabPersistencePolicy(0, false);
         TabPersistentStore tabPersistentStore =
                 new TabPersistentStore(persistencePolicy, mSelector, null, null);