MostVisitedSites: simplify prefs

BUG=none

Review-Url: https://codereview.chromium.org/2131863002
Cr-Commit-Position: refs/heads/master@{#405193}

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);
   // 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;
@@ -268,13 +272,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() {
@@ -468,8 +469,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
@@ -484,17 +492,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_) {

components/ntp_tiles/most_visited_sites.h

@@ -221,8 +221,6 @@ class MostVisitedSites : public history::TopSitesObserver,
       SuggestionsVector whitelist_suggestions,
       SuggestionsVector popular_suggestions);
 
-  void SaveCurrentSuggestionsToPrefs();
-
   // Notifies the observer about the availability of suggestions.
   // Also records impressions UMA if not done already.
   void NotifyMostVisitedURLsObserver();

components/ntp_tiles/pref_names.cc

@@ -7,12 +7,13 @@
 namespace ntp_tiles {
 namespace prefs {
 
-// Ordered list of website suggestions shown on the new tab page that will allow
-// retaining the order even if the suggestions change over time.
-const char kNTPSuggestionsURL[] = "ntp.suggestions_url";
+const char kDeprecatedNTPSuggestionsURL[] = "ntp.suggestions_url";
+const char kDeprecatedNTPSuggestionsIsPersonal[] =
+    "ntp.suggestions_is_personal";
 
-// Whether the suggestion was derived from personal data.
-const char kNTPSuggestionsIsPersonal[] = "ntp.suggestions_is_personal";
+// The number of personal suggestions we had previously. Used to figure out
+// whether we need popular sites.
+const char kNumPersonalSuggestions[] = "ntp.num_personal_suggestions";
 
 // If set, overrides the URL for popular sites, including the individual
 // overrides for country and version below.

components/ntp_tiles/pref_names.h

@@ -8,8 +8,11 @@
 namespace ntp_tiles {
 namespace prefs {
 
-extern const char kNTPSuggestionsURL[];
-extern const char kNTPSuggestionsIsPersonal[];
+// TODO(treib): Remove after M55.
+extern const char kDeprecatedNTPSuggestionsURL[];
+extern const char kDeprecatedNTPSuggestionsIsPersonal[];
+
+extern const char kNumPersonalSuggestions[];
 
 extern const char kPopularSitesOverrideURL[];
 extern const char kPopularSitesOverrideCountry[];