Refactor suggestion processors to take in the ImageFetcher.

This unifies the ImageFetcher into the mediator (ensures we keep
only one copy) and unifies the cleanup logic.

This does decrease the cache size of the Entity images.  Previously,
entities used a 20mb limit, and answers 5mb.  This converges on the
answer limit.

BUG=1015997

Change-Id: I84f23c1b1150f9e651516b946ba2de265f2345f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879815
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Ender <ender@google.com>
Cr-Commit-Position: refs/heads/master@{#710178}

chrome/android/java/DEPS

@@ -6,9 +6,9 @@ include_rules = [
   "+chrome/browser/ui/android/styles",
   "+chrome/browser/ui/android/widget",
   "+chrome/browser/download/android/java",
+  "+chrome/browser/image_fetcher/android/java",
   "+chrome/browser/util/android/java",
   "+chrome/lib/lifecycle/public",
-  "+chrome/lib/image_fetcher/public",
   "+components/embedder_support/android",
   "+components/autofill/android/java/src/org/chromium/components/autofill",
   "+components/background_task_scheduler/android/java/src/org/chromium/components/background_task_scheduler",

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java

@@ -18,6 +18,7 @@ import androidx.annotation.Nullable;
 import androidx.annotation.StringRes;
 
 import org.chromium.base.Log;
+import org.chromium.base.Supplier;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
@@ -25,6 +26,10 @@ import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.ActivityTabProvider.ActivityTabTabObserver;
 import org.chromium.chrome.browser.ChromeFeatureList;
+import org.chromium.chrome.browser.GlobalDiscardableReferencePool;
+import org.chromium.chrome.browser.image_fetcher.ImageFetcher;
+import org.chromium.chrome.browser.image_fetcher.ImageFetcherConfig;
+import org.chromium.chrome.browser.image_fetcher.ImageFetcherFactory;
 import org.chromium.chrome.browser.init.AsyncInitializationActivity;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
 import org.chromium.chrome.browser.lifecycle.StartStopWithNativeObserver;
@@ -41,6 +46,7 @@ import org.chromium.chrome.browser.omnibox.suggestions.entity.EntitySuggestionPr
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.toolbar.ToolbarDataProvider;
+import org.chromium.chrome.browser.util.ConversionUtils;
 import org.chromium.chrome.browser.util.UrlConstants;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.ui.base.DeviceFormFactor;
@@ -85,6 +91,8 @@ class AutocompleteMediator
     private static final String TAG = "Autocomplete";
     private static final int SUGGESTION_NOT_FOUND = -1;
 
+    private static final int MAX_IMAGE_CACHE_SIZE = 500 * ConversionUtils.BYTES_PER_KILOBYTE;
+
     // Delay triggering the omnibox results upon key press to allow the location bar to repaint
     // with the new characters.
     private static final long OMNIBOX_SUGGESTION_START_DELAY_MS = 30;
@@ -151,6 +159,8 @@ class AutocompleteMediator
     private ActivityLifecycleDispatcher mLifecycleDispatcher;
     private ActivityTabTabObserver mTabObserver;
 
+    private ImageFetcher mImageFetcher;
+
     public AutocompleteMediator(Context context, AutocompleteDelegate delegate,
             UrlBarEditingTextStateProvider textProvider, PropertyModel listPropertyModel) {
         mContext = context;
@@ -160,19 +170,24 @@ class AutocompleteMediator
         mCurrentModels = new ArrayList<>();
         mAutocomplete = new AutocompleteController(this);
         mHandler = new Handler();
+        Supplier<ImageFetcher> imageFetcherSupplier = this::getImageFetcher;
         mBasicSuggestionProcessor = new BasicSuggestionProcessor(mContext, this, textProvider);
-        mAnswerSuggestionProcessor = new AnswerSuggestionProcessor(mContext, this, textProvider);
+        mAnswerSuggestionProcessor =
+                new AnswerSuggestionProcessor(mContext, this, textProvider, imageFetcherSupplier);
         mEditUrlProcessor = new EditUrlSuggestionProcessor(
                 mContext, this, delegate, (suggestion) -> onSelection(suggestion, 0));
-        mEntitySuggestionProcessor = new EntitySuggestionProcessor(mContext, this);
+        mEntitySuggestionProcessor =
+                new EntitySuggestionProcessor(mContext, this, imageFetcherSupplier);
     }
 
     public void destroy() {
-        mAnswerSuggestionProcessor.destroy();
-        mAnswerSuggestionProcessor = null;
         if (mTabObserver != null) {
             mTabObserver.destroy();
         }
+        if (mImageFetcher != null) {
+            mImageFetcher.destroy();
+            mImageFetcher = null;
+        }
     }
 
     @Override
@@ -192,6 +207,22 @@ class AutocompleteMediator
         notifyPropertyModelsChanged();
     }
 
+    private ImageFetcher getImageFetcher() {
+        if (getCurrentProfile() == null) {
+            return null;
+        }
+        if (mImageFetcher == null) {
+            mImageFetcher = ImageFetcherFactory.createImageFetcher(
+                    ImageFetcherConfig.IN_MEMORY_ONLY,
+                    GlobalDiscardableReferencePool.getReferencePool(), MAX_IMAGE_CACHE_SIZE);
+        }
+        return mImageFetcher;
+    }
+
+    private Profile getCurrentProfile() {
+        return mDataProvider != null ? mDataProvider.getProfile() : null;
+    }
+
     /**
      * Check if the suggestion is created from clipboard.
      *
@@ -254,11 +285,6 @@ class AutocompleteMediator
         }
     }
 
-    @Override
-    public Profile getCurrentProfile() {
-        return mDataProvider != null ? mDataProvider.getProfile() : null;
-    }
-
     /**
      * Sets the data provider for the toolbar.
      */
@@ -410,6 +436,8 @@ class AutocompleteMediator
             // Prevent any upcoming omnibox suggestions from showing once a URL is loaded (and as
             // a consequence the omnibox is unfocused).
             hideSuggestions();
+
+            if (mImageFetcher != null) mImageFetcher.clear();
         }
         if (mEditUrlProcessor != null) mEditUrlProcessor.onUrlFocusChange(hasFocus);
         mAnswerSuggestionProcessor.onUrlFocusChange(hasFocus);

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/answer/AnswerSuggestionProcessor.java

@@ -8,13 +8,11 @@ import android.content.Context;
 import android.graphics.Bitmap;
 import android.support.annotation.DrawableRes;
 
+import org.chromium.base.Supplier;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.GlobalDiscardableReferencePool;
 import org.chromium.chrome.browser.image_fetcher.ImageFetcher;
-import org.chromium.chrome.browser.image_fetcher.ImageFetcherConfig;
-import org.chromium.chrome.browser.image_fetcher.ImageFetcherFactory;
 import org.chromium.chrome.browser.omnibox.OmniboxSuggestionType;
 import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
 import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion;
@@ -22,7 +20,6 @@ import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestionUiType;
 import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewProcessor;
 import org.chromium.chrome.browser.omnibox.suggestions.base.SuggestionDrawableState;
 import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionHost;
-import org.chromium.chrome.browser.util.ConversionUtils;
 import org.chromium.components.omnibox.AnswerType;
 import org.chromium.components.omnibox.SuggestionAnswer;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -34,31 +31,25 @@ import java.util.Map;
 
 /** A class that handles model and view creation for the most commonly used omnibox suggestion. */
 public class AnswerSuggestionProcessor extends BaseSuggestionViewProcessor {
-    private static final int MAX_CACHE_SIZE = 500 * ConversionUtils.BYTES_PER_KILOBYTE;
     private final Map<String, List<PropertyModel>> mPendingAnswerRequestUrls;
     private final Context mContext;
     private final SuggestionHost mSuggestionHost;
     private final UrlBarEditingTextStateProvider mUrlBarEditingTextProvider;
-    private ImageFetcher mImageFetcher;
+    private final Supplier<ImageFetcher> mImageFetcherSupplier;
 
     /**
      * @param context An Android context.
      * @param suggestionHost A handle to the object using the suggestions.
      */
     public AnswerSuggestionProcessor(Context context, SuggestionHost suggestionHost,
-            UrlBarEditingTextStateProvider editingTextProvider) {
+            UrlBarEditingTextStateProvider editingTextProvider,
+            Supplier<ImageFetcher> imageFetcherSupplier) {
         super(context, suggestionHost);
         mContext = context;
         mSuggestionHost = suggestionHost;
         mPendingAnswerRequestUrls = new HashMap<>();
         mUrlBarEditingTextProvider = editingTextProvider;
-    }
-
-    public void destroy() {
-        if (mImageFetcher != null) {
-            mImageFetcher.destroy();
-            mImageFetcher = null;
-        }
+        mImageFetcherSupplier = imageFetcherSupplier;
     }
 
     @Override
@@ -89,10 +80,6 @@ public class AnswerSuggestionProcessor extends BaseSuggestionViewProcessor {
 
     @Override
     public void onUrlFocusChange(boolean hasFocus) {
-        // This clear is necessary for memory as well as clearing when switching to/from incognito.
-        if (!hasFocus && mImageFetcher != null) {
-            mImageFetcher.clear();
-        }
     }
 
     @Override
@@ -112,20 +99,12 @@ public class AnswerSuggestionProcessor extends BaseSuggestionViewProcessor {
         // https://cs.chromium.org/Omnibox.SuggestionUsed.AnswerInSuggest
     }
 
-    /**
-     * Specify ImageFetcher instance to be used for testing purposes.
-     * TODO(http://crbug.com/1015997): Create fetcher instance in AutocompleteMediator and pass it
-     * to the constructor.
-     */
-    void setImageFetcherForTesting(ImageFetcher fetcher) {
-        mImageFetcher = fetcher;
-    }
-
     private void maybeFetchAnswerIcon(PropertyModel model, OmniboxSuggestion suggestion) {
         ThreadUtils.assertOnUiThread();
 
-        // Attempting to fetch answer data before we have a profile to request it for.
-        if (mSuggestionHost.getCurrentProfile() == null) return;
+        // Ensure an image fetcher is available prior to requesting images.
+        ImageFetcher imageFetcher = mImageFetcherSupplier.get();
+        if (imageFetcher == null) return;
 
         // Note: we also handle calculations here, which do not have answer defined.
         if (!suggestion.hasAnswer()) return;
@@ -139,17 +118,11 @@ public class AnswerSuggestionProcessor extends BaseSuggestionViewProcessor {
             return;
         }
 
-        if (mImageFetcher == null) {
-            mImageFetcher =
-                    ImageFetcherFactory.createImageFetcher(ImageFetcherConfig.IN_MEMORY_ONLY,
-                            GlobalDiscardableReferencePool.getReferencePool(), MAX_CACHE_SIZE);
-        }
-
         List<PropertyModel> models = new ArrayList<>();
         models.add(model);
         mPendingAnswerRequestUrls.put(url, models);
 
-        mImageFetcher.fetchImage(
+        imageFetcher.fetchImage(
                 url, ImageFetcher.ANSWER_SUGGESTIONS_UMA_CLIENT_NAME, (Bitmap bitmap) -> {
                     ThreadUtils.assertOnUiThread();
                     // Remove models for the URL ahead of all the checks to ensure we
@@ -175,7 +148,6 @@ public class AnswerSuggestionProcessor extends BaseSuggestionViewProcessor {
      * Sets both lines of the Omnibox suggestion based on an Answers in Suggest result.
      */
     private void setStateForSuggestion(PropertyModel model, OmniboxSuggestion suggestion) {
-        SuggestionAnswer answer = suggestion.getAnswer();
         AnswerText[] details = AnswerTextNewLayout.from(
                 mContext, suggestion, mUrlBarEditingTextProvider.getTextWithoutAutocomplete());
 

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/basic/SuggestionHost.java

@@ -5,7 +5,6 @@
 package org.chromium.chrome.browser.omnibox.suggestions.basic;
 
 import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion;
-import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.ui.modelutil.PropertyModel;
 
 /** A mechanism for creating {@link SuggestionViewDelegate}s. */
@@ -27,9 +26,4 @@ public interface SuggestionHost {
      * Notify the host that the suggestion models have changed.
      */
     void notifyPropertyModelsChanged();
-
-    /**
-     * @return The browser's active profile.
-     */
-    Profile getCurrentProfile();
 }

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/entity/EntitySuggestionProcessor.java

@@ -12,14 +12,12 @@ import android.text.TextUtils;
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.Log;
+import org.chromium.base.Supplier;
 import org.chromium.base.SysUtils;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.GlobalDiscardableReferencePool;
 import org.chromium.chrome.browser.image_fetcher.ImageFetcher;
-import org.chromium.chrome.browser.image_fetcher.ImageFetcherConfig;
-import org.chromium.chrome.browser.image_fetcher.ImageFetcherFactory;
 import org.chromium.chrome.browser.omnibox.OmniboxSuggestionType;
 import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion;
 import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestionUiType;
@@ -41,7 +39,7 @@ public class EntitySuggestionProcessor extends BaseSuggestionViewProcessor {
     private final SuggestionHost mSuggestionHost;
     private final Map<String, List<PropertyModel>> mPendingImageRequests;
     private final int mEntityImageSizePx;
-    private ImageFetcher mImageFetcher;
+    private final Supplier<ImageFetcher> mImageFetcherSupplier;
     // Threshold for low RAM devices. We won't be showing entity suggestion images
     // on devices that have less RAM than this to avoid bloat and reduce user-visible
     // slowdown while spinning up an image decompression process.
@@ -63,13 +61,15 @@ public class EntitySuggestionProcessor extends BaseSuggestionViewProcessor {
      * @param context An Android context.
      * @param suggestionHost A handle to the object using the suggestions.
      */
-    public EntitySuggestionProcessor(Context context, SuggestionHost suggestionHost) {
+    public EntitySuggestionProcessor(Context context, SuggestionHost suggestionHost,
+            Supplier<ImageFetcher> imageFetcherSupplier) {
         super(context, suggestionHost);
         mContext = context;
         mSuggestionHost = suggestionHost;
         mPendingImageRequests = new HashMap<>();
         mEntityImageSizePx = context.getResources().getDimensionPixelSize(
                 R.dimen.omnibox_suggestion_entity_icon_size);
+        mImageFetcherSupplier = imageFetcherSupplier;
     }
 
     @Override
@@ -89,13 +89,10 @@ public class EntitySuggestionProcessor extends BaseSuggestionViewProcessor {
 
     @Override
     public void onNativeInitialized() {
-        mImageFetcher = ImageFetcherFactory.createImageFetcher(ImageFetcherConfig.IN_MEMORY_ONLY,
-                GlobalDiscardableReferencePool.getReferencePool());
     }
 
     @Override
     public void onUrlFocusChange(boolean hasFocus) {
-        if (mImageFetcher != null && !hasFocus) mImageFetcher.clear();
     }
 
     @Override
@@ -111,20 +108,15 @@ public class EntitySuggestionProcessor extends BaseSuggestionViewProcessor {
         // http://cs.chromium.org/Omnibox.SuggestionUsed.RichEntity
     }
 
-    /**
-     * Specify ImageFetcher instance to be used for testing purposes.
-     * TODO(http://crbug.com/1015997): Create fetcher instance in AutocompleteMediator and pass it
-     * to the constructor.
-     */
-    void setImageFetcherForTesting(ImageFetcher fetcher) {
-        mImageFetcher = fetcher;
-    }
-
     private void fetchEntityImage(OmniboxSuggestion suggestion, PropertyModel model) {
         ThreadUtils.assertOnUiThread();
         final String url = suggestion.getImageUrl();
         if (TextUtils.isEmpty(url)) return;
 
+        // Ensure an image fetcher is available prior to requesting images.
+        ImageFetcher imageFetcher = mImageFetcherSupplier.get();
+        if (imageFetcher == null) return;
+
         // Do not make duplicate answer image requests for the same URL (to avoid generating
         // duplicate bitmaps for the same image).
         if (mPendingImageRequests.containsKey(url)) {
@@ -136,7 +128,7 @@ public class EntitySuggestionProcessor extends BaseSuggestionViewProcessor {
         models.add(model);
         mPendingImageRequests.put(url, models);
 
-        mImageFetcher.fetchImage(url, ImageFetcher.ENTITY_SUGGESTIONS_UMA_CLIENT_NAME,
+        imageFetcher.fetchImage(url, ImageFetcher.ENTITY_SUGGESTIONS_UMA_CLIENT_NAME,
                 mEntityImageSizePx, mEntityImageSizePx, (Bitmap bitmap) -> {
                     ThreadUtils.assertOnUiThread();
 

chrome/android/junit/src/org/chromium/chrome/browser/omnibox/suggestions/answer/AnswerSuggestionProcessorUnitTest.java

@@ -37,7 +37,6 @@ import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion;
 import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewProperties;
 import org.chromium.chrome.browser.omnibox.suggestions.base.SuggestionDrawableState;
 import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionHost;
-import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.components.omnibox.AnswerTextStyle;
 import org.chromium.components.omnibox.AnswerTextType;
 import org.chromium.components.omnibox.AnswerType;
@@ -73,9 +72,6 @@ public class AnswerSuggestionProcessorUnitTest {
     @Mock
     Bitmap mFakeBitmap;
 
-    @Mock
-    Profile mProfile;
-
     private Activity mActivity;
     private AnswerSuggestionProcessor mProcessor;
 
@@ -191,10 +187,9 @@ public class AnswerSuggestionProcessorUnitTest {
     public void setUp() {
         MockitoAnnotations.initMocks(this);
         mActivity = Robolectric.buildActivity(Activity.class).setup().get();
-        when(mSuggestionHost.getCurrentProfile()).thenReturn(mProfile);
 
-        mProcessor = new AnswerSuggestionProcessor(mActivity, mSuggestionHost, mUrlStateProvider);
-        mProcessor.setImageFetcherForTesting(mImageFetcher);
+        mProcessor = new AnswerSuggestionProcessor(
+                mActivity, mSuggestionHost, mUrlStateProvider, () -> mImageFetcher);
     }
 
     /** Populate model for associated suggestion. */
@@ -418,14 +413,13 @@ public class AnswerSuggestionProcessorUnitTest {
     }
 
     @Test
-    public void answerImage_noImageFetchWhenProfileIsUnavailable() {
+    public void answerImage_noImageFetchWhenFetcherIsUnavailable() {
         final String url = "http://site.com";
-        when(mSuggestionHost.getCurrentProfile()).thenReturn(null);
+        mImageFetcher = null;
         final SuggestionTestHelper suggHelper =
                 createAnswerSuggestion(AnswerType.WEATHER, "", 1, "", 1, url);
         processSuggestion(suggHelper);
-
-        verify(mImageFetcher, times(0)).fetchImage(anyString(), anyString(), any());
+        Assert.assertNotNull(suggHelper.getIcon());
     }
 
     @Test

chrome/android/junit/src/org/chromium/chrome/browser/omnibox/suggestions/entity/EntitySuggestionProcessorUnitTest.java

@@ -130,8 +130,7 @@ public class EntitySuggestionProcessorUnitTest {
         MockitoAnnotations.initMocks(this);
         mActivity = Robolectric.buildActivity(Activity.class).setup().get();
 
-        mProcessor = new EntitySuggestionProcessor(mActivity, mSuggestionHost);
-        mProcessor.setImageFetcherForTesting(mImageFetcher);
+        mProcessor = new EntitySuggestionProcessor(mActivity, mSuggestionHost, () -> mImageFetcher);
     }
 
     @Test