Use whitelist large icon for corresponding most visited suggestions.

chrome/browser/android/most_visited_sites.cc

Index: chrome/browser/android/most_visited_sites.cc
diff --git a/chrome/browser/android/most_visited_sites.cc b/chrome/browser/android/most_visited_sites.cc
index 1e138f191eca86a2e035ca5a7c323b381fda238b..b2fdd1830fa513e3782a92787bbb8c16850009e7 100644
--- a/chrome/browser/android/most_visited_sites.cc
+++ b/chrome/browser/android/most_visited_sites.cc
@@ -166,8 +166,13 @@ bool NeedPopularSites(const PrefService* prefs, size_t num_tiles) {
 
 MostVisitedSites::Suggestion::Suggestion(const base::string16& title,
                                          const std::string& url,
-                                         MostVisitedSource source)
-    : title(title), url(url), source(source), provider_index(-1) {}
+                                         MostVisitedSource source,
+                                         const base::FilePath& large_icon_path)
+    : title(title),
+      url(url),
+      source(source),
+      large_icon_path(large_icon_path),
+      provider_index(-1) {}
 
 MostVisitedSites::Suggestion::Suggestion(const base::string16& title,
                                          const GURL& url,
@@ -177,9 +182,7 @@ MostVisitedSites::Suggestion::Suggestion(const base::string16& title,
       url(url),
       source(source),
       large_icon_path(large_icon_path),
-      provider_index(-1) {
-  DCHECK_EQ(MostVisitedSites::WHITELIST, source);
-}
+      provider_index(-1) {}
 
 MostVisitedSites::Suggestion::Suggestion(const base::string16& title,
                                          const GURL& url,
@@ -189,8 +192,13 @@ MostVisitedSites::Suggestion::Suggestion(const base::string16& title,
 MostVisitedSites::Suggestion::Suggestion(const base::string16& title,
                                          const std::string& url,
                                          MostVisitedSource source,
+                                         const base::FilePath& large_icon_path,
                                          int provider_index)
-    : title(title), url(url), source(source), provider_index(provider_index) {
+    : title(title),
+      url(url),
+      source(source),
+      large_icon_path(large_icon_path),
+      provider_index(provider_index) {
   DCHECK_EQ(MostVisitedSites::SUGGESTIONS_SERVICE, source);
 }
 
@@ -486,11 +494,23 @@ void MostVisitedSites::InitiateTopSitesQuery() {
       false);
 }
 
+base::FilePath MostVisitedSites::GetWhitelistLargeIconPath(const GURL& url) {
+  SupervisedUserService* supervised_user_service =
+      SupervisedUserServiceFactory::GetForProfile(profile_);
+
+  for (const auto& whitelist : supervised_user_service->whitelists()) {
+    if (whitelist->entry_point().host() == url.host())

[Marc Treib, 2016/03/11 11:42:02]

Hmmm, I'm not sure about this. Different URLs on the same host can have
different icons (example: www.heise.de vs www.heise.de/ct).

[atanasova, 2016/03/11 13:58:18]

We currently do not display more than 1 website from the same host. That is why
I though that following the logic here would be good, but I see your point. I
have changed it to "GetContent" instead of "host()". I was trying to think of
edge cases where this would be a problem, but I think that if a user accesses a
different URL, they either have to enter it manually (so NTP has to effect) or
visiting it often enough for it to replace the entry point. So I think using the
GetContent should get us the results we want.

[Marc Treib, 2016/03/11 14:21:01]

Are you sure about the "1 website per host"? I'm fairly sure both TopSites and
MostLikely can give more than 1 result per host.

Any particular reason for not just comparing the URLs? (Not that it will make
much of a difference, just wondering)

[atanasova, 2016/03/11 15:59:25]

So we are doing this "host" check when adding whitelist or popular sites
suggestions. If we already had a host suggested from MostVisited, we are not
adding it. The check is not performed inside the suggestions that come from
MostVisited or PopularSites. So you do indeed can end up with 2 websites with
the same host name coming from MostVisites or PopularSites. I think that the
idea of comparing hosts when it comes to PopularSites and whitelists comes from
not wanting to spam the user with suggestions that are not extactly what they
are looking for. If they already have a website with that host coming from
MostVisited, there is likely no need to add it again in a different form from
PopularSites.

[Marc Treib, 2016/03/11 16:11:00]

Yup exactly, for PopSites, we don't want to spam the user with almost-the-same
suggestions. (And PopSites will always be only a host anyway.) For whitelists, I
guess the same applies, though they can have paths after the host.
For replacing the icon, we should be more careful IMO. Better to occasionally
end up without an icon, than with a wrong icon.

[atanasova, 2016/03/11 16:16:53]

Agreed. The next patch has the change where "GetContents" is used to acknowledge
this.

+      return whitelist->large_icon_path();
+  }
+  return base::FilePath();
+}
+
 void MostVisitedSites::OnMostVisitedURLsAvailable(
     const history::MostVisitedURLList& visited_list) {
   SupervisedUserURLFilter* url_filter =
       SupervisedUserServiceFactory::GetForProfile(profile_)
           ->GetURLFilterForUIThread();
+
   MostVisitedSites::SuggestionsVector suggestions;
   size_t num_tiles =
       std::min(visited_list.size(), static_cast<size_t>(num_sites_));
@@ -506,7 +526,8 @@ void MostVisitedSites::OnMostVisitedURLsAvailable(
     }
 
     suggestions.push_back(make_scoped_ptr(
-        new Suggestion(visited.title, visited.url.spec(), TOP_SITES)));
+        new Suggestion(visited.title, visited.url.spec(), TOP_SITES,
+                       GetWhitelistLargeIconPath(visited.url))));
   }
 
   received_most_visited_sites_ = true;
@@ -539,7 +560,7 @@ void MostVisitedSites::OnSuggestionsProfileAvailable(
 
     suggestions.push_back(make_scoped_ptr(new Suggestion(
         base::UTF8ToUTF16(suggestion.title()), suggestion.url(),
-        SUGGESTIONS_SERVICE,
+        SUGGESTIONS_SERVICE, GetWhitelistLargeIconPath(GURL(suggestion.url())),
         suggestion.providers_size() > 0 ? suggestion.providers(0) : -1)));
   }