[Content suggestions] Allow to specify URLs for favicons

components/ntp_snippets/content_suggestions_service.cc

Index: components/ntp_snippets/content_suggestions_service.cc
diff --git a/components/ntp_snippets/content_suggestions_service.cc b/components/ntp_snippets/content_suggestions_service.cc
index 9d4955e1e2872eda23d76479904ebb3e65b90805..be7108eb969c4b0720a3aa916c84ec3fd4b4d16b 100644
--- a/components/ntp_snippets/content_suggestions_service.cc
+++ b/components/ntp_snippets/content_suggestions_service.cc
@@ -139,8 +139,6 @@ void ContentSuggestionsService::FetchSuggestionFavicon(
     int minimum_size_in_pixel,
     int desired_size_in_pixel,
     const ImageFetchedCallback& callback) {
-  // TODO(jkrcal): Allow the provider to provide (or possibly override) the URL
-  // for looking up the favicon.
   std::vector<ContentSuggestion>* suggestions =
       &suggestions_by_category_[suggestion_id.category()];
   auto position =
@@ -154,7 +152,10 @@ void ContentSuggestionsService::FetchSuggestionFavicon(
     return;
   }
 
-  const GURL& publisher_url = position->url().GetWithEmptyPath();
+  const GURL& publisher_url =

[Marc Treib, 2017/04/12 11:48:00]

s/publisher_url/url_with_favicon/ ?

[jkrcal, 2017/04/12 12:44:18]

Done. Renamed to domain_with_favicon to make the trimming of the URL even more
apparent.

+      position->url_with_favicon().is_valid()
+          ? position->url_with_favicon().GetWithEmptyPath()

[Marc Treib, 2017/04/12 11:48:00]

Hm, should the fallback logic live in ContentSuggestion directly?

[jkrcal, 2017/04/12 12:44:19]

I do not think so, because it is CSS who is using the fetching API of
LargeIconService so it is its responsibility to choose what URL trimming to use.

[Marc Treib, 2017/04/12 12:50:18]

I didn't mean the trimming, just the "url_with_favicon.is_valid() ?
url_with_favicon : url"

[jkrcal, 2017/04/12 12:58:17]

Ah, fair enough. Done.

+          : position->url().GetWithEmptyPath();
 
   // TODO(jkrcal): Create a general wrapper function in LargeIconService that
   // does handle the get-from-cache-and-fallback-to-google-server functionality