[IC] Add Java in-memory cache

Since the CachedImageFetcher uses a disk cache, there's potential for
flickering. Particularly for low-end devices. This adds an in-memory
caching layer that clients can use to prevent flickering problems
for latency-sensitive features.

Bug: 889234
Change-Id: I2da010aabeb7619b01f55f6305a8a0670d2831cf
Reviewed-on: https://chromium-review.googlesource.com/c/1343042Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611549}

base/android/java/src/org/chromium/base/DiscardableReferencePool.java

@@ -77,6 +77,18 @@ public class DiscardableReferencePool {
         return reference;
     }
 
+    /**
+     * Remove this reference from the pool, allowing garbage collection to pick it up.
+     *
+     * @param ref The discardable reference to remove.
+     */
+    public void remove(DiscardableReference<?> ref) {
+        assert ref != null;
+        assert mPool.contains(ref);
+        mPool.remove(ref);
+        ref.discard();
+    }
+
     /**
      * Drains the pool, removing all references to objects in the pool and therefore allowing them
      * to be garbage collected.

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedImageLoader.java

@@ -18,11 +18,13 @@ import com.google.android.libraries.feed.host.imageloader.BundledAssets;
 import com.google.android.libraries.feed.host.imageloader.ImageLoaderApi;
 
 import org.chromium.base.Callback;
+import org.chromium.base.DiscardableReferencePool;
+import org.chromium.base.SysUtils;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.cached_image_fetcher.CachedImageFetcher;
-import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.cached_image_fetcher.InMemoryCachedImageFetcher;
 import org.chromium.chrome.browser.suggestions.ThumbnailGradient;
 
 import java.util.Iterator;
@@ -41,15 +43,24 @@ public class FeedImageLoader implements ImageLoaderApi {
     private static final String OVERLAY_IMAGE_DIRECTION_END = "end";
 
     private Context mActivityContext;
+    private CachedImageFetcher mCachedImageFetcher;
 
     /**
      * Creates a FeedImageLoader for fetching image for the current user.
      *
-     * @param profile Profile of the user we are rendering the Feed for.
      * @param activityContext Context of the user we are rendering the Feed for.
      */
-    public FeedImageLoader(Profile profile, Context activityContext) {
+    public FeedImageLoader(Context activityContext, DiscardableReferencePool referencePool) {
         mActivityContext = activityContext;
+        if (SysUtils.isLowEndDevice()) {
+            mCachedImageFetcher = CachedImageFetcher.getInstance();
+        } else {
+            mCachedImageFetcher = new InMemoryCachedImageFetcher(referencePool);
+        }
+    }
+
+    public void destroy() {
+        mCachedImageFetcher.destroy();
     }
 
     @Override
@@ -156,6 +167,12 @@ public class FeedImageLoader implements ImageLoaderApi {
 
     @VisibleForTesting
     protected void fetchImage(String url, int width, int height, Callback<Bitmap> callback) {
-        CachedImageFetcher.getInstance().fetchImage(url, width, height, callback);
+        mCachedImageFetcher.fetchImage(url, width, height, callback);
+    }
+
+    @VisibleForTesting
+    FeedImageLoader(Context activityContext, CachedImageFetcher cachedImageFetcher) {
+        mActivityContext = activityContext;
+        mCachedImageFetcher = cachedImageFetcher;
     }
 }

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedNewTabPage.java

@@ -4,7 +4,6 @@
 
 package org.chromium.chrome.browser.feed;
 
-import android.app.Activity;
 import android.content.Context;
 import android.content.res.Configuration;
 import android.content.res.Resources;
@@ -257,6 +256,8 @@ public class FeedNewTabPage extends NewTabPage {
         mMediator.destroy();
         if (mStreamLifecycleManager != null) mStreamLifecycleManager.destroy();
         mTab.getWindowAndroid().removeContextMenuCloseListener(mContextMenuManager);
+        if (mImageLoader != null) mImageLoader.destroy();
+        mImageLoader = null;
     }
 
     @Override
@@ -301,10 +302,11 @@ public class FeedNewTabPage extends NewTabPage {
         FeedAppLifecycle appLifecycle = FeedProcessScopeFactory.getFeedAppLifecycle();
         appLifecycle.onNTPOpened();
 
-        Activity activity = mTab.getActivity();
+        ChromeActivity chromeActivity = mTab.getActivity();
         Profile profile = mTab.getProfile();
 
-        mImageLoader = new FeedImageLoader(profile, activity);
+        mImageLoader = new FeedImageLoader(
+                chromeActivity, chromeActivity.getChromeApplication().getReferencePool());
         FeedLoggingBridge loggingBridge = FeedProcessScopeFactory.getFeedLoggingBridge();
         FeedOfflineIndicator offlineIndicator = FeedProcessScopeFactory.getFeedOfflineIndicator();
         Runnable consumptionObserver =
@@ -315,17 +317,18 @@ public class FeedNewTabPage extends NewTabPage {
 
         FeedStreamScope streamScope =
                 feedProcessScope
-                        .createFeedStreamScopeBuilder(activity, mImageLoader, actionApi,
+                        .createFeedStreamScopeBuilder(chromeActivity, mImageLoader, actionApi,
                                 new BasicStreamConfiguration(),
-                                new BasicCardConfiguration(activity.getResources(), mUiConfig),
+                                new BasicCardConfiguration(
+                                        chromeActivity.getResources(), mUiConfig),
                                 new BasicSnackbarApi(mNewTabPageManager.getSnackbarManager()),
                                 loggingBridge, offlineIndicator)
                         .build();
 
         mStream = streamScope.getStream();
-        mStreamLifecycleManager = new StreamLifecycleManager(mStream, activity, mTab);
+        mStreamLifecycleManager = new StreamLifecycleManager(mStream, chromeActivity, mTab);
 
-        LayoutInflater inflater = LayoutInflater.from(activity);
+        LayoutInflater inflater = LayoutInflater.from(chromeActivity);
         mSectionHeaderView = (SectionHeaderView) inflater.inflate(
                 R.layout.new_tab_page_snippets_expandable_header, mRootView, false);
         mSectionHeaderViewMarginResizer = MarginResizer.createAndAttach(
@@ -363,13 +366,18 @@ public class FeedNewTabPage extends NewTabPage {
             mStreamLifecycleManager = null;
             // Do not call mStream.onDestroy(), the mStreamLifecycleManager has done that for us.
             mStream = null;
-            mImageLoader = null;
             mSectionHeaderView = null;
             mSectionHeaderViewMarginResizer.detach();
             mSectionHeaderViewMarginResizer = null;
             mSigninPromoView = null;
-            if (mSignInPromoViewMarginResizer != null) mSignInPromoViewMarginResizer.detach();
-            mSignInPromoViewMarginResizer = null;
+            if (mSignInPromoViewMarginResizer != null) {
+                mSignInPromoViewMarginResizer.detach();
+                mSignInPromoViewMarginResizer = null;
+            }
+            if (mImageLoader != null) {
+                mImageLoader.destroy();
+                mImageLoader = null;
+            }
         }
 
         mScrollViewForPolicy = new ScrollView(mTab.getActivity());

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

@@ -92,6 +92,16 @@ public class BitmapCache {
         mBitmapCache = referencePool.put(new RecentlyUsedCache(mCacheSize));
     }
 
+    /**
+     * Manually destroy the BitmapCache.
+     */
+    public void destroy() {
+        assert mReferencePool != null;
+        assert mBitmapCache != null;
+        mReferencePool.remove(mBitmapCache);
+        mBitmapCache = null;
+    }
+
     public Bitmap getBitmap(String key) {
         ThreadUtils.assertOnUiThread();
         Bitmap cachedBitmap = getBitmapCache().get(key);

chrome/android/java/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcher.java

@@ -5,49 +5,18 @@
 package org.chromium.chrome.browser.cached_image_fetcher;
 
 import android.graphics.Bitmap;
-import android.graphics.BitmapFactory;
 
 import org.chromium.base.Callback;
 import org.chromium.base.ThreadUtils;
-import org.chromium.base.VisibleForTesting;
-import org.chromium.base.task.AsyncTask;
-import org.chromium.chrome.browser.profiles.Profile;
+
 /**
  * Provides cached image fetching capabilities. Uses getLastUsedProfile, which
  * will need to be changed when supporting multi-profile.
  */
-public class CachedImageFetcher {
-    private static CachedImageFetcher sInstance;
-
-    public static CachedImageFetcher getInstance() {
+public interface CachedImageFetcher {
+    static CachedImageFetcher getInstance() {
         ThreadUtils.assertOnUiThread();
-
-        if (sInstance == null) {
-            sInstance = new CachedImageFetcher(Profile.getLastUsedProfile());
-        }
-
-        return sInstance;
-    }
-
-    private CachedImageFetcherBridge mCachedImageFetcherBridge;
-
-    /**
-     * Creates a CachedImageFetcher for the current user.
-     *
-     * @param profile Profile of the user we are fetching for.
-     */
-    private CachedImageFetcher(Profile profile) {
-        this(new CachedImageFetcherBridge(profile));
-    }
-
-    /**
-     * Creates a CachedImageFetcher for testing.
-     *
-     * @param bridge Mock bridge to use.
-     */
-    @VisibleForTesting
-    CachedImageFetcher(CachedImageFetcherBridge bridge) {
-        mCachedImageFetcherBridge = bridge;
+        return CachedImageFetcherImpl.getInstance();
     }
 
     /**
@@ -59,13 +28,7 @@ public class CachedImageFetcher {
      * @param height The new bitmap's desired height (in pixels).
      * @param callback The function which will be called when the image is ready.
      */
-    public void fetchImage(String url, int width, int height, Callback<Bitmap> callback) {
-        BitmapFactory.Options options = new BitmapFactory.Options();
-        options.outWidth = width;
-        options.outHeight = height;
-
-        fetchImage(url, options, callback);
-    }
+    void fetchImage(String url, int width, int height, Callback<Bitmap> callback);
 
     /**
      * Alias of fetchImage that ignores scaling.
@@ -73,45 +36,10 @@ public class CachedImageFetcher {
      * @param url The url to fetch the image from.
      * @param callback The function which will be called when the image is ready.
      */
-    public void fetchImage(String url, Callback<Bitmap> callback) {
-        fetchImage(url, new BitmapFactory.Options(), callback);
-    }
+    void fetchImage(String url, Callback<Bitmap> callback);
 
     /**
-     * Starts an AsyncTask to first check the disk for the desired image, then
-     * fetches from the network if it isn't found.
-     *
-     * @param url The url to fetch the image from.
-     * @param options Settings when loading the image.
-     * @param callback The function which will be called when the image is ready.
+     * Destroy method, called to clear resources to prevent leakage.
      */
-    @VisibleForTesting
-    protected void fetchImage(
-            String url, BitmapFactory.Options options, Callback<Bitmap> callback) {
+    void destroy();
+}
\ No newline at end of file
-        String filePath = mCachedImageFetcherBridge.getFilePath(url);
-        new AsyncTask<Bitmap>() {
-            @Override
-            protected Bitmap doInBackground() {
-                return tryToLoadImageFromDisk(filePath, options);
-            }
-
-            @Override
-            protected void onPostExecute(Bitmap bitmap) {
-                if (bitmap != null) {
-                    callback.onResult(bitmap);
-                    return;
-                }
-
-                mCachedImageFetcherBridge.fetchImage(
-                        url, options.outWidth, options.outHeight, callback);
-            }
-        }
-                .executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
-    }
-
-    /** Wrapper function to decode a file for disk, useful for testing. */
-    @VisibleForTesting
-    Bitmap tryToLoadImageFromDisk(String filePath, BitmapFactory.Options options) {
-        return BitmapFactory.decodeFile(filePath, options);
-    }
-}

chrome/android/java/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherImpl.java

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.cached_image_fetcher;
+
+import android.graphics.Bitmap;
+import android.graphics.BitmapFactory;
+
+import org.chromium.base.Callback;
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.VisibleForTesting;
+import org.chromium.base.task.AsyncTask;
+import org.chromium.chrome.browser.profiles.Profile;
+
+/**
+ * Implementation that uses a disk cache.
+ */
+public class CachedImageFetcherImpl implements CachedImageFetcher {
+    private static CachedImageFetcherImpl sInstance;
+
+    public static CachedImageFetcher getInstance() {
+        ThreadUtils.assertOnUiThread();
+
+        if (sInstance == null) {
+            sInstance = new CachedImageFetcherImpl(Profile.getLastUsedProfile());
+        }
+
+        return sInstance;
+    }
+
+    // The native bridge.
+    private CachedImageFetcherBridge mCachedImageFetcherBridge;
+
+    /**
+     * Creates a CachedImageFetcher for the current user.
+     *
+     * @param profile Profile of the user we are fetching for.
+     */
+    private CachedImageFetcherImpl(Profile profile) {
+        this(new CachedImageFetcherBridge(profile));
+    }
+
+    /**
+     * Creates a CachedImageFetcher for testing.
+     *
+     * @param bridge Mock bridge to use.
+     */
+    @VisibleForTesting
+    CachedImageFetcherImpl(CachedImageFetcherBridge bridge) {
+        mCachedImageFetcherBridge = bridge;
+    }
+
+    @Override
+    public void destroy() {
+        // Do nothing, this lives for the lifetime of the application.
+    }
+
+    @Override
+    public void fetchImage(String url, int width, int height, Callback<Bitmap> callback) {
+        fetchImageImpl(url, width, height, callback);
+    }
+
+    @Override
+    public void fetchImage(String url, Callback<Bitmap> callback) {
+        fetchImageImpl(url, 0, 0, callback);
+    }
+
+    /**
+     * Starts an AsyncTask to first check the disk for the desired image, then
+     * fetches from the network if it isn't found.
+     *
+     * @param url The url to fetch the image from.
+     * @param width The new bitmap's desired width (in pixels).
+     * @param height The new bitmap's desired height (in pixels).
+     * @param callback The function which will be called when the image is ready.
+     */
+    @VisibleForTesting
+    void fetchImageImpl(String url, int width, int height, Callback<Bitmap> callback) {
+        String filePath = mCachedImageFetcherBridge.getFilePath(url);
+        new AsyncTask<Bitmap>() {
+            @Override
+            protected Bitmap doInBackground() {
+                return tryToLoadImageFromDisk(filePath);
+            }
+
+            @Override
+            protected void onPostExecute(Bitmap bitmap) {
+                if (bitmap != null) {
+                    callback.onResult(bitmap);
+                    return;
+                }
+
+                mCachedImageFetcherBridge.fetchImage(url, width, height, callback);
+            }
+        }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
+    }
+
+    /** Wrapper function to decode a file for disk, useful for testing. */
+    @VisibleForTesting
+    Bitmap tryToLoadImageFromDisk(String filePath) {
+        return BitmapFactory.decodeFile(filePath, null);
+    }
+}

chrome/android/java/src/org/chromium/chrome/browser/cached_image_fetcher/InMemoryCachedImageFetcher.java

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.cached_image_fetcher;
+
+import android.graphics.Bitmap;
+
+import org.chromium.base.Callback;
+import org.chromium.base.DiscardableReferencePool;
+import org.chromium.base.VisibleForTesting;
+import org.chromium.chrome.browser.BitmapCache;
+import org.chromium.chrome.browser.util.ConversionUtils;
+
+/**
+ * A wrapper around the static CachedImageFetcher that also provides in-memory cahcing.
+ */
+public class InMemoryCachedImageFetcher implements CachedImageFetcher {
+    private static final int DEFAULT_CACHE_SIZE = 20 * ConversionUtils.BYTES_PER_MEGABYTE; // 20mb
+    private static final float PORTION_OF_AVAILABLE_MEMORY = 1.f / 8.f;
+
+    private CachedImageFetcher mCachedImageFetcher;
+    private BitmapCache mBitmapCache;
+
+    /**
+     * Create an instance with the default max cache size.
+     *
+     * @param referencePool Pool used to discard references when under memory pressure.
+     */
+    public InMemoryCachedImageFetcher(DiscardableReferencePool referencePool) {
+        this(referencePool, DEFAULT_CACHE_SIZE);
+    }
+
+    /**
+     * Create an instance with a custom max cache size.
+     *
+     * @param referencePool Pool used to discard references when under memory pressure.
+     * @param cacheSize The cache size to use (in bytes), may be smaller depending on the device's
+     *         memory.
+     */
+    public InMemoryCachedImageFetcher(DiscardableReferencePool referencePool, int cacheSize) {
+        int actualCacheSize = determineCacheSize(cacheSize);
+        mBitmapCache = new BitmapCache(referencePool, actualCacheSize);
+        mCachedImageFetcher = CachedImageFetcher.getInstance();
+    }
+
+    @Override
+    public void destroy() {
+        mCachedImageFetcher.destroy();
+        mBitmapCache.destroy();
+    }
+
+    @Override
+    public void fetchImage(String url, int width, int height, Callback<Bitmap> callback) {
+        Bitmap cachedBitmap = tryToGetBitmap(url, width, height);
+        if (cachedBitmap == null) {
+            mCachedImageFetcher.fetchImage(url, width, height, (Bitmap bitmap) -> {
+                storeBitmap(bitmap, url, width, height);
+                callback.onResult(bitmap);
+            });
+        } else {
+            callback.onResult(cachedBitmap);
+        }
+    }
+
+    @Override
+    public void fetchImage(String url, Callback<Bitmap> callback) {
+        fetchImage(url, 0, 0, callback);
+    }
+
+    /**
+     * Try to get a bitmap from the in-memory cache.
+     *
+     * @param url The url of the image.
+     * @param width The width (in pixels) of the image.
+     * @param height The height (in pixels) of the image.
+     * @return The Bitmap stored in memory or null.
+     */
+    private Bitmap tryToGetBitmap(String url, int width, int height) {
+        String key = encodeCacheKey(url, width, height);
+        return mBitmapCache.getBitmap(key);
+    }
+
+    /**
+     * Store the bitmap in memory.
+     *
+     * @param url The url of the image.
+     * @param width The width (in pixels) of the image.
+     * @param height The height (in pixels) of the image.
+     */
+    private void storeBitmap(Bitmap bitmap, String url, int width, int height) {
+        if (bitmap == null) return;
+
+        String key = encodeCacheKey(url, width, height);
+        mBitmapCache.putBitmap(key, bitmap);
+    }
+
+    /**
+     * Use the given parameters to encode a key used in the String -> Bitmap mapping.
+     *
+     * @param url The url of the image.
+     * @param width The width (in pixels) of the image.
+     * @param height The height (in pixels) of the image.
+     * @return The key for the BitmapCache.
+     */
+    private String encodeCacheKey(String url, int width, int height) {
+        // Encoding for cache key is:
+        // <url>/<width>/<height>.
+        return url + "/" + width + "/" + height;
+    }
+
+    /**
+     * Size the cache size depending on available memory and the client's preferred cache size.
+     *
+     * @param preferredCacheSize The preferred cache size (in bytes).
+     * @return The actual size of the cache (in bytes).
+     */
+    private int determineCacheSize(int preferredCacheSize) {
+        final Runtime runtime = Runtime.getRuntime();
+        final long usedMem = runtime.totalMemory() - runtime.freeMemory();
+        final long maxHeapSize = runtime.maxMemory();
+        final long availableMemory = maxHeapSize - usedMem;
+
+        // Try to size the cache according to client's wishes. If there's not enough space, then
+        // take a portion of available memory.
+        final int maxCacheUsage = (int) (availableMemory * PORTION_OF_AVAILABLE_MEMORY);
+        return Math.min(maxCacheUsage, preferredCacheSize);
+    }
+
+    /** Test constructor. */
+    @VisibleForTesting
+    InMemoryCachedImageFetcher(BitmapCache bitmapCache, CachedImageFetcher cachedImageFetcher) {
+        mBitmapCache = bitmapCache;
+        mCachedImageFetcher = cachedImageFetcher;
+    }
+}
\ No newline at end of file

chrome/android/java_sources.gni

@@ -1385,7 +1385,9 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/printing/PrintShareActivity.java",
   "java/src/org/chromium/chrome/browser/printing/TabPrinter.java",
   "java/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcher.java",
+  "java/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherImpl.java",
   "java/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherBridge.java",
+  "java/src/org/chromium/chrome/browser/cached_image_fetcher/InMemoryCachedImageFetcher.java",
   "java/src/org/chromium/chrome/browser/profiles/Profile.java",
   "java/src/org/chromium/chrome/browser/profiles/ProfileDownloader.java",
   "java/src/org/chromium/chrome/browser/profiles/ProfileManagerUtils.java",
@@ -2320,7 +2322,8 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/browserservices/TrustedWebActivityClientTest.java",
   "junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/ClientAppDataRecorderTest.java",
   "junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityDisclosureControllerTest.java",
-  "junit/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherTest.java",
+  "junit/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherImplTest.java",
+  "junit/src/org/chromium/chrome/browser/cached_image_fetcher/InMemoryCachedImageFetcherTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimatorTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManagerImplTest.java",

chrome/android/junit/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherTest.java → chrome/android/junit/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherImplTest.java

@@ -9,11 +9,11 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyObject;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
 import android.graphics.Bitmap;
-import android.graphics.BitmapFactory;
 import android.support.test.filters.SmallTest;
 
 import org.junit.Before;
@@ -26,115 +26,100 @@ import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 import org.mockito.invocation.InvocationOnMock;
 import org.robolectric.annotation.Config;
+import org.robolectric.shadows.ShadowLooper;
 
 import org.chromium.base.Callback;
-import org.chromium.base.task.test.CustomShadowAsyncTask;
+import org.chromium.base.task.test.BackgroundShadowAsyncTask;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.browser.contextmenu.ChromeContextMenuPopulatorTest.ShadowUrlUtilities;
 
 /**
- * Unit tests for CachedImageFetcher.
+ * Unit tests for CachedImageFetcherImpl.
  */
 @RunWith(BaseRobolectricTestRunner.class)
-@Config(manifest = Config.NONE, shadows = {ShadowUrlUtilities.class, CustomShadowAsyncTask.class})
-public class CachedImageFetcherTest {
-    private static String sDummyUrl = "foo.bar";
+@Config(manifest = Config.NONE,
+        shadows = {ShadowUrlUtilities.class, BackgroundShadowAsyncTask.class})
+public class CachedImageFetcherImplTest {
+    private static final String URL = "http://foo.bar";
+    private static final int WIDTH_PX = 100;
+    private static final int HEIGHT_PX = 200;
 
-    CachedImageFetcher mCachedImageFetcher;
+    CachedImageFetcherImpl mCachedImageFetcher;
 
     @Mock
     CachedImageFetcherBridge mCachedImageFetcherBridge;
-
     @Mock
     Bitmap mBitmap;
 
     @Captor
-    ArgumentCaptor<Callback<Bitmap>> mCallbackArgument;
+    ArgumentCaptor<Integer> mWidthCaptor;
+    @Captor
+    ArgumentCaptor<Integer> mHeightCaptor;
+    @Captor
+    ArgumentCaptor<Callback<Bitmap>> mCallbackCaptor;
 
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
-        mCachedImageFetcher = Mockito.spy(new CachedImageFetcher(mCachedImageFetcherBridge));
-        Mockito.doReturn(sDummyUrl).when(mCachedImageFetcherBridge).getFilePath(anyObject());
+        mCachedImageFetcher = Mockito.spy(new CachedImageFetcherImpl(mCachedImageFetcherBridge));
+        Mockito.doReturn(URL).when(mCachedImageFetcherBridge).getFilePath(anyObject());
+        doAnswer((InvocationOnMock invocation) -> {
+            mCallbackCaptor.getValue().onResult(mBitmap);
+            return null;
+        })
+                .when(mCachedImageFetcherBridge)
+                .fetchImage(eq(URL), mWidthCaptor.capture(), mHeightCaptor.capture(),
+                        mCallbackCaptor.capture());
     }
 
-    private void answerCallback() {
-        Mockito.doAnswer((InvocationOnMock invocation) -> {
-                   mCallbackArgument.getValue().onResult(null);
-                   return null;
-               })
-                .when(mCachedImageFetcherBridge)
-                .fetchImage(anyObject(), anyInt(), anyInt(), mCallbackArgument.capture());
+    @Test
+    @SmallTest
+    public void testFetchImageWithDimensionsFoundOnDisk() throws Exception {
+        Mockito.doReturn(mBitmap).when(mCachedImageFetcher).tryToLoadImageFromDisk(anyObject());
+        mCachedImageFetcher.fetchImage(
+                URL, WIDTH_PX, HEIGHT_PX, (Bitmap bitmap) -> { assertEquals(bitmap, mBitmap); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+
+        verify(mCachedImageFetcher).fetchImageImpl(eq(URL), eq(WIDTH_PX), eq(HEIGHT_PX), any());
+        verify(mCachedImageFetcherBridge, never()) // Should never make it to native.
+                .fetchImage(eq(URL), anyInt(), anyInt(), any());
     }
 
     @Test
     @SmallTest
-    public void testFetchImageWithDimensionsFoundOnDisk() {
-        answerCallback();
-        Mockito.doReturn(mBitmap)
-                .when(mCachedImageFetcher)
-                .tryToLoadImageFromDisk(anyObject(), anyObject());
-        mCachedImageFetcher.fetchImage(sDummyUrl, 100, 100,
-                (Bitmap bitmap)
-                        -> {
-                                // Do nothing.
-                        });
-        ArgumentCaptor<BitmapFactory.Options> optionsCaptor =
-                ArgumentCaptor.forClass(BitmapFactory.Options.class);
-        verify(mCachedImageFetcher).fetchImage(eq(sDummyUrl), optionsCaptor.capture(), any());
-
-        BitmapFactory.Options opts = optionsCaptor.getValue();
-        assertEquals(opts.outWidth, 100);
-        assertEquals(opts.outHeight, 100);
-
-        verify(mCachedImageFetcherBridge, never())
-                .fetchImage(eq(sDummyUrl), anyInt(), anyInt(), any());
+    public void testFetchImageWithDimensionsCallToNative() throws Exception {
+        Mockito.doReturn(null).when(mCachedImageFetcher).tryToLoadImageFromDisk(anyObject());
+        mCachedImageFetcher.fetchImage(
+                URL, WIDTH_PX, HEIGHT_PX, (Bitmap bitmap) -> { assertEquals(bitmap, mBitmap); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+
+        verify(mCachedImageFetcher).fetchImageImpl(eq(URL), eq(WIDTH_PX), eq(HEIGHT_PX), any());
+        verify(mCachedImageFetcherBridge).fetchImage(eq(URL), anyInt(), anyInt(), any());
     }
 
     @Test
     @SmallTest
-    public void testFetchImageWithNoDimensionsFoundOnDisk() {
-        answerCallback();
-        Mockito.doReturn(mBitmap)
-                .when(mCachedImageFetcher)
-                .tryToLoadImageFromDisk(anyObject(), anyObject());
-        mCachedImageFetcher.fetchImage(sDummyUrl,
-                (Bitmap bitmap)
-                        -> {
-                                // Do nothing.
-                        });
-        ArgumentCaptor<BitmapFactory.Options> optionsCaptor =
-                ArgumentCaptor.forClass(BitmapFactory.Options.class);
-        verify(mCachedImageFetcher).fetchImage(eq(sDummyUrl), optionsCaptor.capture(), any());
-
-        BitmapFactory.Options opts = optionsCaptor.getValue();
-        assertEquals(opts.outWidth, 0);
-        assertEquals(opts.outHeight, 0);
-
-        verify(mCachedImageFetcherBridge, never())
-                .fetchImage(eq(sDummyUrl), anyInt(), anyInt(), any());
+    public void testFetchImageWithNoDimensionsFoundOnDisk() throws Exception {
+        Mockito.doReturn(mBitmap).when(mCachedImageFetcher).tryToLoadImageFromDisk(anyObject());
+        mCachedImageFetcher.fetchImage(URL, (Bitmap bitmap) -> { assertEquals(bitmap, mBitmap); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+
+        verify(mCachedImageFetcher).fetchImageImpl(eq(URL), eq(0), eq(0), any());
+        verify(mCachedImageFetcherBridge, never()).fetchImage(eq(URL), anyInt(), anyInt(), any());
     }
 
     @Test
     @SmallTest
-    public void testFetchImageWithNoDimensionsCallToNative() {
-        answerCallback();
-        Mockito.doReturn(null)
-                .when(mCachedImageFetcher)
-                .tryToLoadImageFromDisk(anyObject(), anyObject());
-        mCachedImageFetcher.fetchImage(sDummyUrl,
-                (Bitmap bitmap)
-                        -> {
-                                // Do nothing.
-                        });
-        ArgumentCaptor<BitmapFactory.Options> optionsCaptor =
-                ArgumentCaptor.forClass(BitmapFactory.Options.class);
-        verify(mCachedImageFetcher).fetchImage(eq(sDummyUrl), optionsCaptor.capture(), any());
-
-        BitmapFactory.Options opts = optionsCaptor.getValue();
-        assertEquals(opts.outWidth, 0);
-        assertEquals(opts.outHeight, 0);
-
-        verify(mCachedImageFetcherBridge).fetchImage(eq(sDummyUrl), anyInt(), anyInt(), any());
+    public void testFetchImageWithNoDimensionsCallToNative() throws Exception {
+        Mockito.doReturn(null).when(mCachedImageFetcher).tryToLoadImageFromDisk(anyObject());
+        mCachedImageFetcher.fetchImage(URL, (Bitmap bitmap) -> { assertEquals(bitmap, mBitmap); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+
+        verify(mCachedImageFetcher).fetchImageImpl(eq(URL), eq(0), eq(0), any());
+        verify(mCachedImageFetcherBridge).fetchImage(eq(URL), anyInt(), anyInt(), any());
     }
 }
\ No newline at end of file

chrome/android/junit/src/org/chromium/chrome/browser/cached_image_fetcher/InMemoryCachedImageFetcherTest.java

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.cached_image_fetcher;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import android.graphics.Bitmap;
+import android.support.test.filters.SmallTest;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.mockito.invocation.InvocationOnMock;
+import org.robolectric.annotation.Config;
+import org.robolectric.shadows.ShadowLooper;
+
+import org.chromium.base.Callback;
+import org.chromium.base.DiscardableReferencePool;
+import org.chromium.base.task.test.BackgroundShadowAsyncTask;
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.chrome.browser.BitmapCache;
+import org.chromium.chrome.browser.util.test.ShadowUrlUtilities;
+
+/**
+ * Unit tests for InMemoryCachedImageFetcher.
+ */
+@RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE,
+        shadows = {ShadowUrlUtilities.class, BackgroundShadowAsyncTask.class})
+public class InMemoryCachedImageFetcherTest {
+    private static final String URL = "http://foo.bar";
+    private static final int WIDTH_PX = 100;
+    private static final int HEIGHT_PX = 200;
+    private static final int DEFAULT_CACHE_SIZE = 100;
+
+    private final DiscardableReferencePool mReferencePool = new DiscardableReferencePool();
+    private final Bitmap mBitmap =
+            Bitmap.createBitmap(WIDTH_PX, HEIGHT_PX, Bitmap.Config.ARGB_8888);
+
+    private InMemoryCachedImageFetcher mInMemoryCachedImageFetcher;
+    private BitmapCache mBitmapCache;
+
+    @Mock
+    private CachedImageFetcherImpl mCachedImageFetcherImpl;
+
+    @Captor
+    private ArgumentCaptor<Integer> mWidthCaptor;
+    @Captor
+    private ArgumentCaptor<Integer> mHeightCaptor;
+    @Captor
+    private ArgumentCaptor<Callback<Bitmap>> mCallbackCaptor;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+
+        mBitmapCache = new BitmapCache(mReferencePool, DEFAULT_CACHE_SIZE);
+        mInMemoryCachedImageFetcher =
+                new InMemoryCachedImageFetcher(mBitmapCache, mCachedImageFetcherImpl);
+        // clang-format off
+        doAnswer((InvocationOnMock invocation) -> {
+            mCallbackCaptor.getValue().onResult(mBitmap);
+            return null;
+        }).when(mCachedImageFetcherImpl)
+          .fetchImage(eq(URL), mWidthCaptor.capture(), mHeightCaptor.capture(),
+                      mCallbackCaptor.capture());
+        // clang-format on
+    }
+
+    @Test
+    @SmallTest
+    public void testFetchImageCachesFirstCall() throws Exception {
+        mInMemoryCachedImageFetcher.fetchImage(
+                URL, WIDTH_PX, HEIGHT_PX, (Bitmap bitmap) -> { assertEquals(bitmap, mBitmap); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+        mInMemoryCachedImageFetcher.fetchImage(
+                URL, WIDTH_PX, HEIGHT_PX, (Bitmap bitmap) -> { assertEquals(bitmap, mBitmap); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+        verify(mCachedImageFetcherImpl, /* Should only go to native the first time. */ times(1))
+                .fetchImage(eq(URL), eq(WIDTH_PX), eq(HEIGHT_PX), any());
+    }
+}
\ No newline at end of file

chrome/android/junit/src/org/chromium/chrome/browser/feed/FeedImageLoaderTest.java

@@ -35,7 +35,7 @@ import org.robolectric.annotation.Config;
 import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.test.BaseRobolectricTestRunner;
-import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.cached_image_fetcher.CachedImageFetcher;
 import org.chromium.chrome.test.support.DisableHistogramsRule;
 
 import java.util.Arrays;
@@ -71,9 +71,9 @@ public class FeedImageLoaderTest {
     public DisableHistogramsRule mDisableHistogramsRule = new DisableHistogramsRule();
 
     @Mock
-    private Consumer<Drawable> mConsumer;
+    CachedImageFetcher mCachedImageFetcher;
     @Mock
-    private Profile mProfile;
+    private Consumer<Drawable> mConsumer;
     @Mock
     private Bitmap mBitmap;
     @Captor
@@ -88,8 +88,8 @@ public class FeedImageLoaderTest {
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
-        mImageLoader =
-                Mockito.spy(new FeedImageLoader(mProfile, ContextUtils.getApplicationContext()));
+        mImageLoader = Mockito.spy(
+                new FeedImageLoader(ContextUtils.getApplicationContext(), mCachedImageFetcher));
     }
 
     private void answerFetchImage(String url, Bitmap bitmap) {