Ensure PopularSite icon availability in ntp_tiles.

components/ntp_tiles/most_visited_sites.cc

Index: components/ntp_tiles/most_visited_sites.cc
diff --git a/components/ntp_tiles/most_visited_sites.cc b/components/ntp_tiles/most_visited_sites.cc
index 309a0b48d1568e234d25a7e34c950e3634eaf245..89277c074deb139c51f243d47c740f8e61a0a58e 100644
--- a/components/ntp_tiles/most_visited_sites.cc
+++ b/components/ntp_tiles/most_visited_sites.cc
@@ -26,6 +26,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "components/history/core/browser/top_sites.h"
 #include "components/ntp_tiles/constants.h"
+#include "components/ntp_tiles/icon_cacher.h"
 #include "components/ntp_tiles/pref_names.h"
 #include "components/ntp_tiles/switches.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -144,11 +145,13 @@ MostVisitedSites::MostVisitedSites(PrefService* prefs,
                                    scoped_refptr<history::TopSites> top_sites,
                                    SuggestionsService* suggestions,
                                    std::unique_ptr<PopularSites> popular_sites,
+                                   std::unique_ptr<IconCacher> icon_cacher,
                                    MostVisitedSitesSupervisor* supervisor)
     : prefs_(prefs),
       top_sites_(top_sites),
       suggestions_service_(suggestions),
       popular_sites_(std::move(popular_sites)),
+      icon_cacher_(std::move(icon_cacher)),
       supervisor_(supervisor),
       observer_(nullptr),
       num_sites_(0),
@@ -281,6 +284,12 @@ void MostVisitedSites::RegisterProfilePrefs(
   registry->RegisterListPref(prefs::kDeprecatedNTPTilesIsPersonal);
 }
 
+void MostVisitedSites::OnIconMadeAvailable(const GURL& site_url,
+                                           bool newly_available) {
+  if (newly_available)
+    observer_->OnIconMadeAvailable(site_url);
+}
+
 void MostVisitedSites::BuildCurrentTiles() {
   // Get the current suggestions from cache. If the cache is empty, this will
   // fall back to TopSites.
@@ -518,9 +527,11 @@ void MostVisitedSites::OnPopularSitesAvailable(bool success) {
     return;
   }
 
-  // Pass the popular sites to the observer. This will cause it to fetch any
-  // missing icons, but will *not* cause it to display the popular sites.
-  observer_->OnPopularURLsAvailable(popular_sites_->sites());
+  for (const auto& site : popular_sites_->sites()) {
+    icon_cacher_->StartFetch(
+        site, base::Bind(&MostVisitedSites::OnIconMadeAvailable,
+                         weak_ptr_factory_.GetWeakPtr(), site.url));

[Marc Treib, 2016/10/12 09:11:28]

Is the WeakPtr necessary? Since we own the IconCacher, it'll get destroyed when
we get destroyed, so presumably it won't try to call us back anymore?

[sfiera, 2016/10/13 09:06:46]

No, I suppose it isn't. Switched for base::Unretained.

+  }
 
   // Re-build the tile list. Once done, this will notify the observer.
   BuildCurrentTiles();