Popular sites on the NTP: re-download popular suggestions once per Chrome run

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 03fb06c4781e12b829d7b0520c01d73148923153..f3b75e63f8097b85da56a594247447d71de4327d 100644
--- a/chrome/browser/android/most_visited_sites.cc
+++ b/chrome/browser/android/most_visited_sites.cc
@@ -134,6 +134,25 @@ std::string GetPopularSitesFilename() {
                                             "filename");
 }
 
+bool NeedPopularSites(const PrefService* prefs, size_t num_tiles) {

[battre, 2015/09/14 14:31:25]

Please document the meaning of num_tiles.

[Marc Treib, 2015/09/18 11:44:24]

Done.

+  const base::ListValue* source_list =
+      prefs->GetList(prefs::kNTPSuggestionsIsPersonal);
+  // If there aren't enough previous suggestions to fill the grid, we need
+  // popular suggestions.
+  if (source_list->GetSize() < num_tiles)
+    return true;
+  // Otherwise, if any of the previous suggestions is not personal, then also
+  // get popular suggestions.
+  for (size_t i = 0; i < num_tiles; ++i) {
+    bool is_personal;

[battre, 2015/09/14 14:31:25]

please initialize this variable.

[Marc Treib, 2015/09/18 11:44:24]

Done.

+    if (source_list->GetBoolean(i, &is_personal) && !is_personal)
+      return true;
+  }
+  // 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,
@@ -189,17 +208,6 @@ MostVisitedSites::MostVisitedSites(Profile* profile)
       ProfileSyncServiceFactory::GetForProfile(profile_);
   if (profile_sync_service)
     profile_sync_service->AddObserver(this);
-
-  if (ShouldShowPopularSites()) {
-    popular_sites_.reset(new PopularSites(
-        profile,
-        GetPopularSitesFilename(),
-        profile_->GetRequestContext(),
-        base::Bind(&MostVisitedSites::OnPopularSitesAvailable,
-                   base::Unretained(this))));
-  } else {
-    received_popular_sites_ = true;
-  }
 }
 
 MostVisitedSites::~MostVisitedSites() {
@@ -224,6 +232,18 @@ void MostVisitedSites::SetMostVisitedURLsObserver(JNIEnv* env,
   observer_.Reset(env, j_observer);
   num_sites_ = num_sites;
 
+  if (ShouldShowPopularSites() &&
+      NeedPopularSites(profile_->GetPrefs(), num_sites_)) {
+    popular_sites_.reset(new PopularSites(
+        profile_,
+        GetPopularSitesFilename(),
+        profile_->GetRequestContext(),
+        base::Bind(&MostVisitedSites::OnPopularSitesAvailable,
+                   base::Unretained(this))));
+  } else {
+    received_popular_sites_ = true;
+  }
+
   QueryMostVisitedURLs();
 
   scoped_refptr<history::TopSites> top_sites =