[IC] Convert ExploreOnContent to use CachedImageFetcher

Adding method to native to get image/favicon urls so they can be
fetched in native.

Bug: 831629
Change-Id: I4ec1be9f1de41471940ec63ce1830eabd11bb356
Reviewed-on: https://chromium-review.googlesource.com/c/1374958
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619507}

chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsBridge.java

@@ -4,8 +4,6 @@
 
 package org.chromium.chrome.browser.contextual_suggestions;
 
-import android.graphics.Bitmap;
-
 import org.chromium.base.Callback;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
@@ -88,18 +86,27 @@ public class ContextualSuggestionsBridge {
         nativeFetchSuggestions(mNativeContextualSuggestionsBridge, url, callback);
     }
 
-    /** Fetches a thumbnail for the suggestion. */
-    void fetchSuggestionImage(SnippetArticle suggestion, Callback<Bitmap> callback) {
+    /**
+     * Get the url of the image used for the given suggestion.
+     *
+     * @param suggestion The suggestion that the image url is for.
+     * @return The url for the image or null if one can't be found.
+     */
+    String getImageUrl(SnippetArticle suggestion) {
         assert mNativeContextualSuggestionsBridge != 0;
-        nativeFetchSuggestionImage(
-                mNativeContextualSuggestionsBridge, suggestion.mIdWithinCategory, callback);
+        return nativeGetImageUrl(mNativeContextualSuggestionsBridge, suggestion.mIdWithinCategory);
     }
 
-    /** Fetches a favicon for the suggestion. */
-    void fetchSuggestionFavicon(SnippetArticle suggestion, Callback<Bitmap> callback) {
+    /**
+     * Get the url of the favicon used for the given suggestion.
+     *
+     * @param suggestion The suggestion that the favicon url is for.
+     * @return The url for the image or null if one can't be found.
+     */
+    String getFaviconUrl(SnippetArticle suggestion) {
         assert mNativeContextualSuggestionsBridge != 0;
-        nativeFetchSuggestionFavicon(
-                mNativeContextualSuggestionsBridge, suggestion.mIdWithinCategory, callback);
+        return nativeGetFaviconUrl(
+                mNativeContextualSuggestionsBridge, suggestion.mIdWithinCategory);
     }
 
     /** Requests the backend to clear state related to this bridge. */
@@ -157,10 +164,10 @@ public class ContextualSuggestionsBridge {
     private native void nativeDestroy(long nativeContextualSuggestionsBridge);
     private native void nativeFetchSuggestions(long nativeContextualSuggestionsBridge, String url,
             Callback<ContextualSuggestionsResult> callback);
-    private native void nativeFetchSuggestionImage(
-            long nativeContextualSuggestionsBridge, String suggestionId, Callback<Bitmap> callback);
-    private native void nativeFetchSuggestionFavicon(
-            long nativeContextualSuggestionsBridge, String suggestionId, Callback<Bitmap> callback);
+    private native String nativeGetImageUrl(
+            long nativeContextualSuggestionsBridge, String suggestionId);
+    private native String nativeGetFaviconUrl(
+            long nativeContextualSuggestionsBridge, String suggestionId);
     private native void nativeClearState(long nativeContextualSuggestionsBridge);
     private native void nativeReportEvent(
             long nativeContextualSuggestionsBridge, WebContents webContents, int eventId);

chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsSourceImpl.java

@@ -7,6 +7,10 @@ package org.chromium.chrome.browser.contextual_suggestions;
 import android.graphics.Bitmap;
 
 import org.chromium.base.Callback;
+import org.chromium.base.VisibleForTesting;
+import org.chromium.base.task.PostTask;
+import org.chromium.base.task.TaskTraits;
+import org.chromium.chrome.browser.cached_image_fetcher.CachedImageFetcher;
 import org.chromium.chrome.browser.contextual_suggestions.ContextualSuggestionsBridge.ContextualSuggestionsResult;
 import org.chromium.chrome.browser.ntp.snippets.EmptySuggestionsSource;
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
@@ -19,6 +23,7 @@ import org.chromium.content_public.browser.WebContents;
 class ContextualSuggestionsSourceImpl
         extends EmptySuggestionsSource implements ContextualSuggestionsSource {
     private ContextualSuggestionsBridge mBridge;
+    private CachedImageFetcher mCachedImageFetcher;
 
     /**
      * Creates a ContextualSuggestionsSource for getting contextual suggestions for the current
@@ -28,6 +33,7 @@ class ContextualSuggestionsSourceImpl
      */
     public ContextualSuggestionsSourceImpl(Profile profile) {
         mBridge = new ContextualSuggestionsBridge(profile);
+        mCachedImageFetcher = CachedImageFetcher.getInstance();
     }
 
     @Override
@@ -37,19 +43,32 @@ class ContextualSuggestionsSourceImpl
 
     @Override
     public void fetchSuggestionImage(SnippetArticle suggestion, Callback<Bitmap> callback) {
-        mBridge.fetchSuggestionImage(suggestion, callback);
+        String url = mBridge.getImageUrl(suggestion);
+        mCachedImageFetcher.fetchImage(url, callback);
     }
 
     @Override
     public void fetchContextualSuggestionImage(
             SnippetArticle suggestion, Callback<Bitmap> callback) {
-        mBridge.fetchSuggestionImage(suggestion, callback);
+        String url = mBridge.getImageUrl(suggestion);
+        if (url == null) {
+            PostTask.postTask(new TaskTraits(), () -> callback.onResult(null));
+            return;
+        }
+
+        mCachedImageFetcher.fetchImage(url, callback);
     }
 
     @Override
     public void fetchSuggestionFavicon(SnippetArticle suggestion, int minimumSizePx,
             int desiredSizePx, Callback<Bitmap> callback) {
-        mBridge.fetchSuggestionFavicon(suggestion, callback);
+        String url = mBridge.getFaviconUrl(suggestion);
+        if (url == null) {
+            PostTask.postTask(new TaskTraits(), () -> callback.onResult(null));
+            return;
+        }
+
+        mCachedImageFetcher.fetchImage(url, desiredSizePx, desiredSizePx, callback);
     }
 
     @Override
@@ -66,4 +85,11 @@ class ContextualSuggestionsSourceImpl
     public void clearState() {
         mBridge.clearState();
     }
+
+    @VisibleForTesting
+    ContextualSuggestionsSourceImpl(
+            ContextualSuggestionsBridge bridge, CachedImageFetcher cachedImageFethcer) {
+        mBridge = bridge;
+        mCachedImageFetcher = cachedImageFethcer;
+    }
 }

chrome/android/java_sources.gni

@@ -2385,6 +2385,7 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityDisclosureControllerTest.java",
   "junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityOpenTimeRecorderTest.java",
   "junit/src/org/chromium/chrome/browser/cached_image_fetcher/CachedImageFetcherImplTest.java",
+  "junit/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsSourceImplTest.java",
   "junit/src/org/chromium/chrome/browser/cached_image_fetcher/InMemoryCachedImageFetcherTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManagerImplTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/EventOffsetHandlerTest.java",

chrome/android/junit/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsSourceImplTest.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.contextual_suggestions;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+
+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.task.test.BackgroundShadowAsyncTask;
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.chrome.browser.cached_image_fetcher.CachedImageFetcher;
+import org.chromium.chrome.browser.contextmenu.ChromeContextMenuPopulatorTest.ShadowUrlUtilities;
+import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
+
+/**
+ * Unit tests for CachedImageFetcherImpl.
+ */
+@RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE,
+        shadows = {ShadowUrlUtilities.class, BackgroundShadowAsyncTask.class})
+public class ContextualSuggestionsSourceImplTest {
+    private static final String IMAGE_URL = "http://foo.com/bar.png";
+    private static final int FAVICON_DIMENSION = 100;
+
+    ContextualSuggestionsSourceImpl mContextualSuggestionsSourceImpl;
+
+    @Mock
+    ContextualSuggestionsBridge mBridge;
+
+    @Mock
+    CachedImageFetcher mCachedImageFetcher;
+
+    @Mock
+    Bitmap mBitmap;
+
+    @Captor
+    ArgumentCaptor<Callback<Bitmap>> mCallbackCaptor;
+
+    @Before
+    public void setUp() throws Exception {
+        MockitoAnnotations.initMocks(this);
+        mContextualSuggestionsSourceImpl =
+                new ContextualSuggestionsSourceImpl(mBridge, mCachedImageFetcher);
+        // Answer with or without width/height.
+        doAnswer((InvocationOnMock invocation) -> {
+            mCallbackCaptor.getValue().onResult(mBitmap);
+            return null;
+        })
+                .when(mCachedImageFetcher)
+                .fetchImage(any(), mCallbackCaptor.capture());
+    }
+
+    @Test
+    @SmallTest
+    public void testFetchContextualSuggestionImage() throws Exception {
+        doReturn(IMAGE_URL).when(mBridge).getImageUrl(any());
+
+        SnippetArticle article = mock(SnippetArticle.class);
+        mContextualSuggestionsSourceImpl.fetchContextualSuggestionImage(
+                article, (Bitmap bitmap) -> { assertEquals(bitmap, mBitmap); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+    }
+
+    @Test
+    @SmallTest
+    public void testFetchContextualSuggestionImageWhenNullUrlIsReturned() throws Exception {
+        doReturn(null).when(mBridge).getImageUrl(any());
+
+        SnippetArticle article = mock(SnippetArticle.class);
+        mContextualSuggestionsSourceImpl.fetchContextualSuggestionImage(
+                article, (Bitmap bitmap) -> { assertEquals(bitmap, null); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+    }
+
+    @Test
+    @SmallTest
+    public void testFetchContextualSuggestionFavicon() throws Exception {
+        doReturn(IMAGE_URL).when(mBridge).getFaviconUrl(any());
+
+        SnippetArticle article = mock(SnippetArticle.class);
+        mContextualSuggestionsSourceImpl.fetchSuggestionFavicon(article, FAVICON_DIMENSION,
+                FAVICON_DIMENSION, (Bitmap bitmap) -> { assertEquals(bitmap, mBitmap); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+    }
+
+    @Test
+    @SmallTest
+    public void testFetchContextualSuggestionFaviconNullUrlIsReturned() throws Exception {
+        doReturn(null).when(mBridge).getFaviconUrl(any());
+
+        SnippetArticle article = mock(SnippetArticle.class);
+        mContextualSuggestionsSourceImpl.fetchSuggestionFavicon(article, FAVICON_DIMENSION,
+                FAVICON_DIMENSION, (Bitmap bitmap) -> { assertEquals(bitmap, null); });
+        BackgroundShadowAsyncTask.runBackgroundTasks();
+        ShadowLooper.runUiThreadTasks();
+    }
+}
\ No newline at end of file

chrome/browser/android/contextual_suggestions/contextual_suggestions_bridge.cc

@@ -114,30 +114,27 @@ void ContextualSuggestionsBridge::FetchSuggestions(
                           ScopedJavaGlobalRef<jobject>(j_callback)));
 }
 
-void ContextualSuggestionsBridge::FetchSuggestionImage(
+base::android::ScopedJavaLocalRef<jstring>
+ContextualSuggestionsBridge::GetImageUrl(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
-    const JavaParamRef<jstring>& j_suggestion_id,
-    const JavaParamRef<jobject>& j_callback) {
+    const JavaParamRef<jstring>& j_suggestion_id) {
   std::string suggestion_id(ConvertJavaStringToUTF8(env, j_suggestion_id));
-  service_proxy_->FetchContextualSuggestionImage(
-      suggestion_id,
-      base::BindOnce(&ContextualSuggestionsBridge::OnImageFetched,
-                     weak_ptr_factory_.GetWeakPtr(),
-                     ScopedJavaGlobalRef<jobject>(j_callback)));
+  std::string image_url =
+      service_proxy_->GetContextualSuggestionImageUrl(suggestion_id);
+  return image_url.empty() ? nullptr : ConvertUTF8ToJavaString(env, image_url);
 }
 
-void ContextualSuggestionsBridge::FetchSuggestionFavicon(
+base::android::ScopedJavaLocalRef<jstring>
+ContextualSuggestionsBridge::GetFaviconUrl(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
-    const JavaParamRef<jstring>& j_suggestion_id,
-    const JavaParamRef<jobject>& j_callback) {
+    const JavaParamRef<jstring>& j_suggestion_id) {
   std::string suggestion_id(ConvertJavaStringToUTF8(env, j_suggestion_id));
-  service_proxy_->FetchContextualSuggestionFavicon(
-      suggestion_id,
-      base::BindOnce(&ContextualSuggestionsBridge::OnImageFetched,
-                     weak_ptr_factory_.GetWeakPtr(),
-                     ScopedJavaGlobalRef<jobject>(j_callback)));
+  std::string favicon_url =
+      service_proxy_->GetContextualSuggestionFaviconUrl(suggestion_id);
+  return favicon_url.empty() ? nullptr
+                             : ConvertUTF8ToJavaString(env, favicon_url);
 }
 
 void ContextualSuggestionsBridge::ClearState(JNIEnv* env,

chrome/browser/android/contextual_suggestions/contextual_suggestions_bridge.h

@@ -44,13 +44,17 @@ class ContextualSuggestionsBridge {
       const base::android::JavaParamRef<jstring>& j_suggestion_id,
       const base::android::JavaParamRef<jobject>& j_callback);
 
-  // Fetches a favicon corresponding to suggestion with |j_suggestion_id| and
-  // passes results to Java side using |j_callback|.
-  void FetchSuggestionFavicon(
+  // Gets the image URL for the given |suggestion_id|.
+  base::android::ScopedJavaLocalRef<jstring> GetImageUrl(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj,
-      const base::android::JavaParamRef<jstring>& j_suggestion_id,
-      const base::android::JavaParamRef<jobject>& j_callback);
+      const base::android::JavaParamRef<jstring>& j_suggestion_id);
+
+  // Gets the favicon URL for the given |suggestion_id|.
+  base::android::ScopedJavaLocalRef<jstring> GetFaviconUrl(
+      JNIEnv* env,
+      const base::android::JavaParamRef<jobject>& obj,
+      const base::android::JavaParamRef<jstring>& j_suggestion_id);
 
   // Requests the backend to clear state related to this bridge.
   void ClearState(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);

chrome/browser/ntp_snippets/contextual_content_suggestions_service_factory.cc

@@ -12,20 +12,13 @@
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/search/suggestions/image_decoder_impl.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
-#include "components/image_fetcher/core/image_decoder.h"
-#include "components/image_fetcher/core/image_fetcher.h"
-#include "components/image_fetcher/core/image_fetcher_impl.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/ntp_snippets/contextual/contextual_content_suggestions_service.h"
 #include "components/ntp_snippets/contextual/contextual_suggestions_features.h"
 #include "components/ntp_snippets/contextual/contextual_suggestions_fetcher_impl.h"
 #include "components/ntp_snippets/contextual/reporting/contextual_suggestions_debugging_reporter.h"
 #include "components/ntp_snippets/contextual/reporting/contextual_suggestions_reporter.h"
-#include "components/ntp_snippets/remote/cached_image_fetcher.h"
-#include "components/ntp_snippets/remote/remote_suggestions_database.h"
-#include "components/prefs/pref_service.h"
 #include "components/unified_consent/feature.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/storage_partition.h"
@@ -39,9 +32,6 @@
 using contextual_suggestions::ContextualSuggestionsFetcherImpl;
 using contextual_suggestions::ContextualContentSuggestionsService;
 
-using ntp_snippets::CachedImageFetcher;
-using ntp_snippets::RemoteSuggestionsDatabase;
-
 namespace {
 
 bool AreContextualContentSuggestionsEnabled() {
@@ -96,7 +86,6 @@ ContextualContentSuggestionsServiceFactory::BuildServiceInstanceFor(
     return nullptr;
   }
 
-  PrefService* pref_service = profile->GetPrefs();
   content::StoragePartition* storage_partition =
       content::BrowserContext::GetDefaultStoragePartition(context);
   std::unique_ptr<unified_consent::UrlKeyedDataCollectionConsentHelper>
@@ -112,26 +101,12 @@ ContextualContentSuggestionsServiceFactory::BuildServiceInstanceFor(
       std::make_unique<ContextualSuggestionsFetcherImpl>(
           storage_partition->GetURLLoaderFactoryForBrowserProcess(),
           std::move(consent_helper), g_browser_process->GetApplicationLocale());
-  const base::FilePath::CharType kDatabaseFolder[] =
-      FILE_PATH_LITERAL("contextualSuggestionsDatabase");
-  base::FilePath database_dir(profile->GetPath().Append(kDatabaseFolder));
-  auto contextual_suggestions_database =
-      std::make_unique<RemoteSuggestionsDatabase>(database_dir);
-  auto cached_image_fetcher =
-      std::make_unique<ntp_snippets::CachedImageFetcher>(
-          std::make_unique<image_fetcher::ImageFetcherImpl>(
-              std::make_unique<suggestions::ImageDecoderImpl>(),
-              content::BrowserContext::GetDefaultStoragePartition(profile)
-                  ->GetURLLoaderFactoryForBrowserProcess()),
-          pref_service, contextual_suggestions_database.get());
   auto reporter_provider = std::make_unique<
       contextual_suggestions::ContextualSuggestionsReporterProvider>(
       std::make_unique<
           contextual_suggestions::ContextualSuggestionsDebuggingReporter>());
   auto* service = new ContextualContentSuggestionsService(
-      std::move(contextual_suggestions_fetcher),
-      std::move(cached_image_fetcher),
-      std::move(contextual_suggestions_database), std::move(reporter_provider));
+      std::move(contextual_suggestions_fetcher), std::move(reporter_provider));
 
   return service;
 }

components/ntp_snippets/contextual/contextual_content_suggestions_service.cc

@@ -14,17 +14,11 @@
 #include "components/ntp_snippets/contextual/contextual_content_suggestions_service_proxy.h"
 #include "components/ntp_snippets/contextual/contextual_suggestions_features.h"
 #include "components/ntp_snippets/contextual/contextual_suggestions_result.h"
-#include "components/ntp_snippets/remote/cached_image_fetcher.h"
-#include "components/ntp_snippets/remote/remote_suggestions_database.h"
 #include "components/ntp_snippets/remote/remote_suggestions_provider_impl.h"
-#include "ui/gfx/image/image.h"
 
 namespace contextual_suggestions {
 
 using ntp_snippets::ContentSuggestion;
-using ntp_snippets::ImageDataFetchedCallback;
-using ntp_snippets::ImageFetchedCallback;
-using ntp_snippets::CachedImageFetcher;
 using ntp_snippets::RemoteSuggestionsDatabase;
 
 namespace {
@@ -37,15 +31,10 @@ bool IsEligibleURL(const GURL& url) {
 ContextualContentSuggestionsService::ContextualContentSuggestionsService(
     std::unique_ptr<ContextualSuggestionsFetcher>
         contextual_suggestions_fetcher,
-    std::unique_ptr<CachedImageFetcher> image_fetcher,
-    std::unique_ptr<RemoteSuggestionsDatabase> contextual_suggestions_database,
     std::unique_ptr<ContextualSuggestionsReporterProvider> reporter_provider)
-    : contextual_suggestions_database_(
-          std::move(contextual_suggestions_database)),
-      fetch_cache_(kFetchCacheCapacity),
+    : fetch_cache_(kFetchCacheCapacity),
       contextual_suggestions_fetcher_(
           std::move(contextual_suggestions_fetcher)),
-      image_fetcher_(std::move(image_fetcher)),
       reporter_provider_(std::move(reporter_provider)) {}
 
 ContextualContentSuggestionsService::~ContextualContentSuggestionsService() =
@@ -70,15 +59,6 @@ void ContextualContentSuggestionsService::FetchContextualSuggestionClusters(
   }
 }
 
-void ContextualContentSuggestionsService::FetchContextualSuggestionImage(
-    const ContentSuggestion::ID& suggestion_id,
-    const GURL& image_url,
-    ImageFetchedCallback callback) {
-  image_fetcher_->FetchSuggestionImage(suggestion_id, image_url,
-                                       ImageDataFetchedCallback(),
-                                       std::move(callback));
-}
-
 void ContextualContentSuggestionsService::FetchDone(
     const GURL& url,
     FetchClustersCallback callback,

components/ntp_snippets/contextual/contextual_content_suggestions_service.h

@@ -12,7 +12,6 @@
 
 #include "base/callback.h"
 #include "base/optional.h"
-#include "components/image_fetcher/core/image_fetcher.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/ntp_snippets/callbacks.h"
 #include "components/ntp_snippets/content_suggestion.h"
@@ -22,27 +21,18 @@
 #include "components/ntp_snippets/contextual/reporting/contextual_suggestions_reporter.h"
 #include "services/metrics/public/cpp/ukm_source_id.h"
 
-namespace ntp_snippets {
-class CachedImageFetcher;
-class RemoteSuggestionsDatabase;
-}  // namespace ntp_snippets
-
 namespace contextual_suggestions {
 
 static constexpr int kFetchCacheCapacity = 10;
 
 class ContextualContentSuggestionsServiceProxy;
 
-// Retrieves contextual suggestions for a given URL and fetches images
-// for contextual suggestion, using caching.
+// Retrieves contextual suggestions for a given URL using caching.
 class ContextualContentSuggestionsService : public KeyedService {
  public:
   ContextualContentSuggestionsService(
       std::unique_ptr<ContextualSuggestionsFetcher>
           contextual_suggestions_fetcher,
-      std::unique_ptr<ntp_snippets::CachedImageFetcher> image_fetcher,
-      std::unique_ptr<ntp_snippets::RemoteSuggestionsDatabase>
-          contextual_suggestions_database,
       std::unique_ptr<
           contextual_suggestions::ContextualSuggestionsReporterProvider>
           reporter_provider);
@@ -54,13 +44,6 @@ class ContextualContentSuggestionsService : public KeyedService {
       FetchClustersCallback callback,
       ReportFetchMetricsCallback metrics_callback);
 
-  // Fetches an image pointed to by |url| and internally caches it using
-  // |suggestion_id|. Asynchronous if cache or network is queried.
-  virtual void FetchContextualSuggestionImage(
-      const ntp_snippets::ContentSuggestion::ID& suggestion_id,
-      const GURL& url,
-      ntp_snippets::ImageFetchedCallback callback);
-
   void FetchDone(const GURL& url,
                  FetchClustersCallback callback,
                  ReportFetchMetricsCallback metrics_callback,
@@ -82,9 +65,6 @@ class ContextualContentSuggestionsService : public KeyedService {
   void BelowConfidenceThresholdFetchDone(
       FetchClustersCallback callback,
       ReportFetchMetricsCallback metrics_callback);
-  // Cache for images of contextual suggestions, needed by CachedImageFetcher.
-  std::unique_ptr<ntp_snippets::RemoteSuggestionsDatabase>
-      contextual_suggestions_database_;
 
   // Cache of contextual suggestions fetch results, keyed by the context url.
   ContextualSuggestionsCache fetch_cache_;
@@ -92,15 +72,9 @@ class ContextualContentSuggestionsService : public KeyedService {
   // Performs actual network request to fetch contextual suggestions.
   std::unique_ptr<ContextualSuggestionsFetcher> contextual_suggestions_fetcher_;
 
-  std::unique_ptr<ntp_snippets::CachedImageFetcher> image_fetcher_;
-
   std::unique_ptr<contextual_suggestions::ContextualSuggestionsReporterProvider>
       reporter_provider_;
 
-  // Look up by ContentSuggestion::ID::id_within_category() aka std::string to
-  // get image URL.
-  std::map<std::string, GURL> image_url_by_id_;
-
   DISALLOW_COPY_AND_ASSIGN(ContextualContentSuggestionsService);
 };
 

components/ntp_snippets/contextual/contextual_content_suggestions_service_proxy.cc

@@ -49,36 +49,29 @@ void ContextualContentSuggestionsServiceProxy::FetchContextualSuggestions(
           weak_ptr_factory_.GetWeakPtr(), last_ukm_source_id_, url.spec()));
 }
 
-void ContextualContentSuggestionsServiceProxy::FetchContextualSuggestionImage(
-    const std::string& suggestion_id,
-    ntp_snippets::ImageFetchedCallback callback) {
+std::string
+ContextualContentSuggestionsServiceProxy::GetContextualSuggestionImageUrl(
+    const std::string& suggestion_id) {
   auto suggestion_iter = suggestions_.find(suggestion_id);
   if (suggestion_iter == suggestions_.end()) {
     DVLOG(1) << "Unkown suggestion ID: " << suggestion_id;
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(std::move(callback), gfx::Image()));
-    return;
+    return "";
   }
 
   std::string& image_id = suggestion_iter->second.image_id;
-  GURL image_url = ImageUrlFromId(image_id);
-
-  FetchImageImpl(image_url, image_id, std::move(callback));
+  return ImageUrlFromId(image_id).spec();
 }
 
-void ContextualContentSuggestionsServiceProxy::FetchContextualSuggestionFavicon(
-    const std::string& suggestion_id,
-    ntp_snippets::ImageFetchedCallback callback) {
+std::string
+ContextualContentSuggestionsServiceProxy::GetContextualSuggestionFaviconUrl(
+    const std::string& suggestion_id) {
   auto suggestion_iter = suggestions_.find(suggestion_id);
   if (suggestion_iter == suggestions_.end()) {
     DVLOG(1) << "Unkown suggestion ID: " << suggestion_id;
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(std::move(callback), gfx::Image()));
-    return;
+    return "";
   }
 
-  FetchImageImpl(GURL(suggestion_iter->second.favicon_image_url),
-                 suggestion_iter->second.favicon_image_id, std::move(callback));
+  return suggestion_iter->second.favicon_image_url;
 }
 
 void ContextualContentSuggestionsServiceProxy::ClearState() {
@@ -115,19 +108,6 @@ void ContextualContentSuggestionsServiceProxy::FlushMetrics() {
   last_ukm_source_id_ = ukm::kInvalidSourceId;
 }
 
-void ContextualContentSuggestionsServiceProxy::FetchImageImpl(
-    const GURL& image_url,
-    const std::string& image_id,
-    ntp_snippets::ImageFetchedCallback callback) {
-  ntp_snippets::ContentSuggestion::ID synthetic_cache_id(
-      ntp_snippets::Category::FromKnownCategory(
-          ntp_snippets::KnownCategories::CONTEXTUAL),
-      image_id);
-
-  service_->FetchContextualSuggestionImage(synthetic_cache_id, image_url,
-                                           std::move(callback));
-}
-
 void ContextualContentSuggestionsServiceProxy::CacheSuggestions(
     FetchClustersCallback callback,
     ContextualSuggestionsResult result) {

components/ntp_snippets/contextual/contextual_content_suggestions_service_proxy.h

@@ -34,17 +34,12 @@ class ContextualContentSuggestionsServiceProxy {
   void FetchContextualSuggestions(const GURL& url,
                                   FetchClustersCallback callback);
 
-  // Fetches an image for a contextual suggestion with specified
-  // |suggestion_id|.
-  void FetchContextualSuggestionImage(
-      const std::string& suggestion_id,
-      ntp_snippets::ImageFetchedCallback callback);
-
-  // Fetches a favicon for a contextual suggestion with specified
-  // |suggestion_id|.
-  void FetchContextualSuggestionFavicon(
-      const std::string& suggestion_id,
-      ntp_snippets::ImageFetchedCallback callback);
+  // Get the URL for the given suggestion id.
+  std::string GetContextualSuggestionImageUrl(const std::string& suggestion_id);
+
+  // Get the URL for the given suggestion id.
+  std::string GetContextualSuggestionFaviconUrl(
+      const std::string& suggestion_id);
 
   // Clears the state of the proxy.
   void ClearState();
@@ -58,10 +53,6 @@ class ContextualContentSuggestionsServiceProxy {
   void FlushMetrics();
 
  private:
-  void FetchImageImpl(const GURL& image_url,
-                      const std::string& image_id,
-                      ntp_snippets::ImageFetchedCallback callback);
-
   void CacheSuggestions(FetchClustersCallback callback,
                         ContextualSuggestionsResult result);
   // Pointer to the service.

components/ntp_snippets/contextual/contextual_content_suggestions_service_proxy_unittest.cc

@@ -12,7 +12,6 @@
 #include "components/ntp_snippets/contextual/contextual_content_suggestions_service.h"
 #include "components/ntp_snippets/contextual/contextual_suggestions_test_utils.h"
 #include "components/ntp_snippets/contextual/reporting/contextual_suggestions_metrics_reporter.h"
-#include "components/ntp_snippets/remote/cached_image_fetcher.h"
 #include "components/ntp_snippets/remote/remote_suggestions_database.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -27,6 +26,11 @@ namespace {
 static constexpr char kTestPeekText[] = "Test peek test";
 static constexpr char kValidFromUrl[] = "http://some.url";
 
+static constexpr char kImageId[] = "id";
+// Keep the string after tbn: in sync with the above constant for image id.
+static constexpr char kImageUrl[] = "http://www.google.com/images?q=tbn:id";
+static constexpr char kFaviconUrl[] = "http://google.com/cats.fav";
+
 class FakeContextualContentSuggestionsService
     : public ContextualContentSuggestionsService {
  public:
@@ -50,7 +54,7 @@ class FakeContextualContentSuggestionsService
 
 FakeContextualContentSuggestionsService::
     FakeContextualContentSuggestionsService()
-    : ContextualContentSuggestionsService(nullptr, nullptr, nullptr, nullptr) {}
+    : ContextualContentSuggestionsService(nullptr, nullptr) {}
 
 FakeContextualContentSuggestionsService::
     ~FakeContextualContentSuggestionsService() {}
@@ -90,6 +94,30 @@ TEST_F(ContextualContentSuggestionsServiceProxyTest,
   EXPECT_TRUE(mock_cluster_callback.has_run);
 }
 
+TEST_F(ContextualContentSuggestionsServiceProxyTest, GetImageUrl) {
+  auto suggestion = SuggestionBuilder(GURL(kValidFromUrl))
+                        .ImageId(kImageId)
+                        .FaviconImageUrl(kFaviconUrl)
+                        .Build();
+  auto cluster =
+      ClusterBuilder(kValidFromUrl).AddSuggestion(suggestion).Build();
+  MockClustersCallback mock_cluster_callback;
+
+  proxy()->FetchContextualSuggestions(GURL(kValidFromUrl),
+                                      mock_cluster_callback.ToOnceCallback());
+  service()->RunClustersCallback(ContextualSuggestionsResult(
+      kTestPeekText, std::vector<Cluster>({cluster}), PeekConditions(),
+      ServerExperimentInfos()));
+
+  EXPECT_EQ(kImageUrl, proxy()->GetContextualSuggestionImageUrl(suggestion.id));
+  EXPECT_EQ(kFaviconUrl,
+            proxy()->GetContextualSuggestionFaviconUrl(suggestion.id));
+
+  // Id not found.
+  EXPECT_EQ("", proxy()->GetContextualSuggestionImageUrl(""));
+  EXPECT_EQ("", proxy()->GetContextualSuggestionFaviconUrl(""));
+}
+
 // TODO(fgorski): More tests will be added, once we have the suggestions
 // restructured.
 

components/ntp_snippets/contextual/contextual_content_suggestions_service_unittest.cc

@@ -15,7 +15,6 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/test/mock_callback.h"
 #include "base/test/scoped_task_environment.h"
-#include "components/image_fetcher/core/image_fetcher_impl.h"
 #include "components/ntp_snippets/category_info.h"
 #include "components/ntp_snippets/content_suggestion.h"
 #include "components/ntp_snippets/contextual/contextual_suggestion.h"
@@ -23,23 +22,16 @@
 #include "components/ntp_snippets/contextual/contextual_suggestions_test_utils.h"
 #include "components/ntp_snippets/contextual/reporting/contextual_suggestions_debugging_reporter.h"
 #include "components/ntp_snippets/contextual/reporting/contextual_suggestions_reporter.h"
-#include "components/ntp_snippets/remote/cached_image_fetcher.h"
 #include "components/ntp_snippets/remote/json_to_categories.h"
-#include "components/ntp_snippets/remote/remote_suggestions_database.h"
+#include "components/ntp_snippets/remote/request_throttler.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "ui/gfx/image/image.h"
-#include "ui/gfx/image/image_unittest_util.h"
 
-using ntp_snippets::CachedImageFetcher;
 using ntp_snippets::Category;
 using ntp_snippets::ContentSuggestion;
 using ntp_snippets::KnownCategories;
-using ntp_snippets::ImageFetchedCallback;
-using ntp_snippets::ImageDataFetchedCallback;
-using ntp_snippets::RemoteSuggestionsDatabase;
 using ntp_snippets::RequestThrottler;
 
 using testing::_;
@@ -80,26 +72,6 @@ class FakeContextualSuggestionsFetcher : public ContextualSuggestionsFetcher {
   PeekConditions peek_conditions_;
 };
 
-// Always fetches a fake image if the given URL is valid.
-class FakeCachedImageFetcher : public CachedImageFetcher {
- public:
-  explicit FakeCachedImageFetcher(PrefService* pref_service)
-      : CachedImageFetcher(std::unique_ptr<image_fetcher::ImageFetcher>(),
-                           pref_service,
-                           nullptr) {}
-
-  void FetchSuggestionImage(const ContentSuggestion::ID&,
-                            const GURL& image_url,
-                            ImageDataFetchedCallback image_data_callback,
-                            ImageFetchedCallback callback) override {
-    gfx::Image image;
-    if (image_url.is_valid()) {
-      image = gfx::test::CreateImage();
-    }
-    std::move(callback).Run(image);
-  }
-};
-
 }  // namespace
 
 class ContextualContentSuggestionsServiceTest : public testing::Test {
@@ -116,8 +88,6 @@ class ContextualContentSuggestionsServiceTest : public testing::Test {
         std::move(debugging_reporter));
     source_ = std::make_unique<ContextualContentSuggestionsService>(
         std::move(fetcher),
-        std::make_unique<FakeCachedImageFetcher>(&pref_service_),
-        std::unique_ptr<RemoteSuggestionsDatabase>(),
         std::move(reporter_provider));
   }