ntp_tiles: fetch popular sites icons for 2nd NTP

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 f3695c9946ec90bfe44a7e89456e6e539e820fd9..3b65bf1882aeb7b851225912afd3ebae2337d647 100644
--- a/components/ntp_tiles/most_visited_sites.cc
+++ b/components/ntp_tiles/most_visited_sites.cc
@@ -418,6 +418,12 @@ void MostVisitedSites::OnPopularSitesDownloaded(bool success) {
     LOG(WARNING) << "Download of popular sites failed";
     return;
   }
+
+  for (const PopularSites::Site& popular_site : popular_sites_->sites()) {
+    // Ignore callback; these icons will be seen on the *next* NTP.
+    icon_cacher_->StartFetch(popular_site,

[Marc Treib, 2017/02/27 12:33:35]

Hrm. So if the NTP gets closed in the meantime, then all this will get aborted,
right?

That indeed sounds like we should make MostVisitedSites long-lived. On desktop,
it already is, through being owned by InstantService.

[sfiera, 2017/02/27 12:51:33]

Yes. But that's already true of PopularSites as a whole.

[fhorschig, 2017/02/27 15:40:28]

+1 to making MVS/PS long-lived.
(Maybe not in this CL, though)

+                             base::Bind([](bool newly_available) {}));

[Marc Treib, 2017/02/27 12:33:35]

Can we just pass a null Callback here?

[sfiera, 2017/02/27 12:51:33]

Do you mean to change the signature of StartFetch(), or some other way to make
this callback?

[Marc Treib, 2017/02/27 13:09:35]

No, I mean just passing "PopularSites::FinishedCallback()" which IMO would be a
bit easier to read than the "empty" Bind() call.
(But then PopularSites would have to check the callback for validity before
calling it, so maybe that's more churn than it's worth.)

[fhorschig, 2017/02/27 15:40:28]

Agree to the readability but I think it's more likely that 
we drop the callback in the future than checking it for validity.
We currently use it only for warning, testing and debugging.
(And all of those uses are debatable...)

+  }
 }
 
 void MostVisitedSites::OnIconMadeAvailable(const GURL& site_url,