[Content suggestions favicons] Let the clients rescale the gfx::Image

components/ntp_snippets/content_suggestions_service.cc

 // Copyright 2016 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.
 
 #include "components/ntp_snippets/content_suggestions_service.h"
 
 #include <algorithm>
 #include <iterator>
 #include <set>
 #include <utility>
@@ skipping 152 matching lines @@
     int desired_size_in_pixel,
     const ImageFetchedCallback& callback) {
   const GURL& domain_with_favicon = GetFaviconDomain(suggestion_id);
   if (!domain_with_favicon.is_valid() || !large_icon_service_) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(callback, gfx::Image()));
     RecordFaviconFetchResult(FaviconFetchResult::FAILURE);
     return;
   }
 
-  // TODO(jkrcal): Create a general wrapper function in LargeIconService that
-  // does handle the get-from-cache-and-fallback-to-google-server functionality
-  // in one shot (for all clients that do not need to react in between).
-  large_icon_service_->GetLargeIconImageOrFallbackStyle(
-      domain_with_favicon, minimum_size_in_pixel, desired_size_in_pixel,
-      base::Bind(&ContentSuggestionsService::OnGetFaviconFromCacheFinished,
-                 base::Unretained(this), domain_with_favicon,
-                 minimum_size_in_pixel, desired_size_in_pixel, callback,
-                 /*continue_to_google_server=*/true),
-      &favicons_task_tracker_);
+  GetFaviconFromCache(domain_with_favicon, minimum_size_in_pixel,
+                      desired_size_in_pixel, callback,
+                      /*continue_to_google_server=*/true);
 }
 
 GURL ContentSuggestionsService::GetFaviconDomain(
     const ContentSuggestion::ID& suggestion_id) {
   const std::vector<ContentSuggestion>& suggestions =
       suggestions_by_category_[suggestion_id.category()];
   auto position =
       std::find_if(suggestions.begin(), suggestions.end(),
                    [&suggestion_id](const ContentSuggestion& suggestion) {
                      return suggestion_id == suggestion.id();
                    });
   if (position != suggestions.end()) {
     return position->url_with_favicon();
   }
 
   // Look up the URL in the archive of |remote_suggestions_provider_|.
   // TODO(jkrcal): Fix how Fetch more works or find other ways to remove this
   // hack. crbug.com/714031
   if (providers_by_category_[suggestion_id.category()] ==
       remote_suggestions_provider_) {
     return remote_suggestions_provider_->GetUrlWithFavicon(suggestion_id);
   }
   return GURL();
 }
 
+void ContentSuggestionsService::GetFaviconFromCache(
+    const GURL& publisher_url,
+    int minimum_size_in_pixel,
+    int desired_size_in_pixel,
+    const ImageFetchedCallback& callback,
+    bool continue_to_google_server) {
+  // TODO(jkrcal): Create a general wrapper function in LargeIconService that

[fhorschig, 2017/05/19 08:17:45]

Isn't that what you did? Why is this TODO still here?

[jkrcal, 2017/05/19 11:36:39]

No, this TODO means something different (basically, avoiding the sequence of
functions that are implemented in this file).

+  // does handle the get-from-cache-and-fallback-to-google-server functionality
+  // in one shot (for all clients that do not need to react in between).
+  large_icon_service_->GetLargeIconImageOrFallbackStyle(
+      publisher_url, minimum_size_in_pixel, desired_size_in_pixel,
+      base::Bind(&ContentSuggestionsService::OnGetFaviconFromCacheFinished,
+                 base::Unretained(this), publisher_url, minimum_size_in_pixel,
+                 /*desired_size_in_pixel=*/0, callback,
+                 continue_to_google_server),
+      &favicons_task_tracker_);
+}
+
 void ContentSuggestionsService::OnGetFaviconFromCacheFinished(
     const GURL& publisher_url,
     int minimum_size_in_pixel,
     int desired_size_in_pixel,
     const ImageFetchedCallback& callback,
     bool continue_to_google_server,
     const favicon_base::LargeIconImageResult& result) {
   if (!result.image.IsEmpty()) {
     callback.Run(result.image);
     // The icon is from cache if we haven't gone to Google server yet. The icon
@@ skipping 30 matching lines @@
     int minimum_size_in_pixel,
     int desired_size_in_pixel,
     const ImageFetchedCallback& callback,
     bool success) {
   if (!success) {
     callback.Run(gfx::Image());
     RecordFaviconFetchResult(FaviconFetchResult::FAILURE);
     return;
   }
 
-  // Get the freshly downloaded icon from the cache.
-  large_icon_service_->GetLargeIconImageOrFallbackStyle(
-      publisher_url, minimum_size_in_pixel, desired_size_in_pixel,
-      base::Bind(&ContentSuggestionsService::OnGetFaviconFromCacheFinished,
-                 base::Unretained(this), publisher_url, minimum_size_in_pixel,
-                 desired_size_in_pixel, callback,
-                 /*continue_to_google_server=*/false),
-      &favicons_task_tracker_);
+  GetFaviconFromCache(publisher_url, minimum_size_in_pixel,
+                      desired_size_in_pixel, callback,
+                      /*continue_to_google_server=*/false);
 }
 
 void ContentSuggestionsService::ClearHistory(
     base::Time begin,
     base::Time end,
     const base::Callback<bool(const GURL& url)>& filter) {
   for (const auto& provider : providers_) {
     provider->ClearHistory(begin, end, filter);
   }
   category_ranker_->ClearHistory(begin, end);
@@ skipping 397 matching lines @@
 void ContentSuggestionsService::StoreDismissedCategoriesToPrefs() {
   base::ListValue list;
   for (const auto& category_provider_pair : dismissed_providers_by_category_) {
     list.AppendInteger(category_provider_pair.first.id());
   }
 
   pref_service_->Set(prefs::kDismissedCategories, list);
 }
 
 }  // namespace ntp_snippets