Use whitelist large icon for corresponding most visited suggestions.

chrome/browser/android/most_visited_sites.cc

 // Copyright 2013 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 "chrome/browser/android/most_visited_sites.h"
 
 #include <utility>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
@@ skipping 148 matching lines @@
   }
   // The whole grid is already filled with personal suggestions, no point in
   // bothering with popular ones.
   return false;
 }
 
 }  // namespace
 
 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,
                                          MostVisitedSource source,
                                          const base::FilePath& large_icon_path)
     : title(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,
                                          MostVisitedSource source)
     : title(title), url(url), source(source), provider_index(-1) {}
 
 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);
 }
 
 MostVisitedSites::Suggestion::~Suggestion() {}
 
 std::string MostVisitedSites::Suggestion::GetSourceHistogramName() const {
   switch (source) {
     case MostVisitedSites::TOP_SITES:
       return kHistogramClientName;
     case MostVisitedSites::POPULAR:
@@ skipping 275 matching lines @@
   scoped_refptr<TopSites> top_sites = TopSitesFactory::GetForProfile(profile_);
   if (!top_sites)
     return;
 
   top_sites->GetMostVisitedURLs(
       base::Bind(&MostVisitedSites::OnMostVisitedURLsAvailable,
                  weak_ptr_factory_.GetWeakPtr()),
       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_));
   for (size_t i = 0; i < num_tiles; ++i) {
     const history::MostVisitedURL& visited = visited_list[i];
     if (visited.url.is_empty()) {
       num_tiles = i;
       break;  // This is the signal that there are no more real visited sites.
     }
     if (url_filter->GetFilteringBehaviorForURL(visited.url) ==
         SupervisedUserURLFilter::FilteringBehavior::BLOCK) {
       continue;
     }
 
     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;
   mv_source_ = TOP_SITES;
   SaveNewNTPSuggestions(&suggestions);
   NotifyMostVisitedURLsObserver();
 }
 
 void MostVisitedSites::OnSuggestionsProfileAvailable(
     const SuggestionsProfile& suggestions_profile) {
@@ skipping 12 matching lines @@
   MostVisitedSites::SuggestionsVector suggestions;
   for (int i = 0; i < num_tiles; ++i) {
     const ChromeSuggestion& suggestion = suggestions_profile.suggestions(i);
     if (url_filter->GetFilteringBehaviorForURL(GURL(suggestion.url())) ==
         SupervisedUserURLFilter::FilteringBehavior::BLOCK) {
       continue;
     }
 
     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)));
   }
 
   received_most_visited_sites_ = true;
   mv_source_ = SUGGESTIONS_SERVICE;
   SaveNewNTPSuggestions(&suggestions);
   NotifyMostVisitedURLsObserver();
 }
 
 MostVisitedSites::SuggestionsVector
@@ skipping 325 matching lines @@
   }
 }
 
 static jlong Init(JNIEnv* env,
                   const JavaParamRef<jobject>& obj,
                   const JavaParamRef<jobject>& jprofile) {
   MostVisitedSites* most_visited_sites =
       new MostVisitedSites(ProfileAndroid::FromProfileAndroid(jprofile));
   return reinterpret_cast<intptr_t>(most_visited_sites);
 }