MostVisitedSites: simplify prefs

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 b03fdaa5b315baa8a10b1be485370d95b90952d4..453097a61489509e988b6c4fcdc03bafe7849ed5 100644
--- a/components/ntp_tiles/most_visited_sites.cc
+++ b/components/ntp_tiles/most_visited_sites.cc
@@ -77,16 +77,20 @@ bool ShouldShowPopularSites() {
 
 // Determine whether we need any popular suggestions to fill up a grid of
 // |num_tiles| tiles.
-bool NeedPopularSites(const PrefService* prefs, size_t num_tiles) {
+bool NeedPopularSites(const PrefService* prefs, int num_tiles) {
+  if (num_tiles <= prefs->GetInteger(prefs::kNumPersonalSuggestions))
+    return false;
+
+  // TODO(treib): Remove after M55.
   const base::ListValue* source_list =
-      prefs->GetList(ntp_tiles::prefs::kNTPSuggestionsIsPersonal);
+      prefs->GetList(ntp_tiles::prefs::kDeprecatedNTPSuggestionsIsPersonal);

[sfiera, 2016/07/08 09:59:15]

How important is this to preserve? Seems like a user would hit popular sites
once after upgrading, then never again, using maybe 1kB of unnecessary data and
delaying startup after first launch by a few ms.

There's nothing wrong with keeping it, though.

[Marc Treib, 2016/07/08 10:02:17]

Well, accumulated, it'd still be a few hundred million unnecessary requests.
Since we have some other "remove after M55" code around, I see no reason not to
keep this as well.

   // If there aren't enough previous suggestions to fill the grid, we need
   // popular suggestions.
-  if (source_list->GetSize() < num_tiles)
+  if (static_cast<int>(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) {
+  for (int i = 0; i < num_tiles; ++i) {
     bool is_personal = false;
     if (source_list->GetBoolean(i, &is_personal) && !is_personal)
       return true;
@@ -271,13 +275,10 @@ void MostVisitedSites::OnBlockedSitesChanged() {
 // static
 void MostVisitedSites::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
-  // TODO(treib): Remove this, it's unused. Do we need migration code to clean
-  // up existing entries?
-  registry->RegisterListPref(ntp_tiles::prefs::kNTPSuggestionsURL);
-  // TODO(treib): Remove this. It's only used to determine if we need
-  // PopularSites at all. Find a way to do that without prefs, or failing that,
-  // replace this list pref by a simple bool.
-  registry->RegisterListPref(ntp_tiles::prefs::kNTPSuggestionsIsPersonal);
+  registry->RegisterIntegerPref(prefs::kNumPersonalSuggestions, 0);
+  // TODO(treib): Remove after M55.
+  registry->RegisterListPref(prefs::kDeprecatedNTPSuggestionsURL);
+  registry->RegisterListPref(prefs::kDeprecatedNTPSuggestionsIsPersonal);
 }
 
 void MostVisitedSites::BuildCurrentSuggestions() {
@@ -471,8 +472,15 @@ void MostVisitedSites::SaveNewSuggestions(
                                           std::move(popular_sites_suggestions));
   DCHECK_EQ(num_actual_tiles, current_suggestions_.size());
 
-  if (received_popular_sites_)
-    SaveCurrentSuggestionsToPrefs();
+  int num_personal_suggestions = 0;
+  for (const auto& suggestion : current_suggestions_) {
+    if (suggestion.source != POPULAR)
+      num_personal_suggestions++;
+  }
+  prefs_->SetInteger(prefs::kNumPersonalSuggestions, num_personal_suggestions);
+  // TODO(treib): Remove after M55.
+  prefs_->ClearPref(prefs::kDeprecatedNTPSuggestionsIsPersonal);
+  prefs_->ClearPref(prefs::kDeprecatedNTPSuggestionsURL);
 }
 
 // static
@@ -487,17 +495,6 @@ MostVisitedSites::SuggestionsVector MostVisitedSites::MergeSuggestions(
   return merged_suggestions;
 }
 
-void MostVisitedSites::SaveCurrentSuggestionsToPrefs() {
-  base::ListValue url_list;
-  base::ListValue source_list;
-  for (const auto& suggestion : current_suggestions_) {
-    url_list.AppendString(suggestion.url.spec());
-    source_list.AppendBoolean(suggestion.source != POPULAR);
-  }
-  prefs_->Set(ntp_tiles::prefs::kNTPSuggestionsIsPersonal, source_list);
-  prefs_->Set(ntp_tiles::prefs::kNTPSuggestionsURL, url_list);
-}
-
 void MostVisitedSites::NotifyMostVisitedURLsObserver() {
   if (received_most_visited_sites_ && received_popular_sites_ &&
       !recorded_uma_) {