[NTP Snippets] Persist remote categories in prefs

components/ntp_snippets/remote/ntp_snippets_service.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/ntp_snippets/remote/ntp_snippets_service.h"
 
 #include <algorithm>
 #include <iterator>
 #include <utility>
 
@@ skipping 175 matching lines @@
       snippets_fetcher_(std::move(snippets_fetcher)),
       image_fetcher_(std::move(image_fetcher)),
       image_decoder_(std::move(image_decoder)),
       database_(std::move(database)),
       snippets_status_service_(std::move(status_service)),
       fetch_when_ready_(false),
       nuke_when_initialized_(false),
       thumbnail_requests_throttler_(
           pref_service,
           RequestThrottler::RequestType::CONTENT_SUGGESTION_THUMBNAIL) {
-  // Articles category always exists; others will be added as needed.
-  categories_[articles_category_] = CategoryContent();
-  categories_[articles_category_].localized_title =
-      l10n_util::GetStringUTF16(IDS_NTP_ARTICLE_SUGGESTIONS_SECTION_HEADER);
-  observer->OnCategoryStatusChanged(this, articles_category_,
-                                    categories_[articles_category_].status);
+  RestoreCategoriesFromPrefs();
+  // The articles category always exists. Add it if we didn't get it from prefs.
+  // TODO(treib): Rethink this.
+  if (!base::ContainsKey(categories_, articles_category_)) {
+    categories_[articles_category_] = CategoryContent();
+    categories_[articles_category_].localized_title =
+        l10n_util::GetStringUTF16(IDS_NTP_ARTICLE_SUGGESTIONS_SECTION_HEADER);
+  }
+  // Tell the observer about all the categories.
+  for (const auto& entry : categories_) {
+    observer->OnCategoryStatusChanged(this, entry.first, entry.second.status);
+  }
+
   if (database_->IsErrorState()) {
     EnterState(State::ERROR_OCCURRED);
     UpdateAllCategoryStatus(CategoryStatus::LOADING_ERROR);
     return;
   }
 
   database_->SetErrorCallback(base::Bind(&NTPSnippetsService::OnDatabaseError,
                                          base::Unretained(this)));
 
   // We transition to other states while finalizing the initialization, when the
   // database is done loading.
   database_->LoadSnippets(base::Bind(&NTPSnippetsService::OnDatabaseLoaded,
                                      base::Unretained(this)));
 }
 
 NTPSnippetsService::~NTPSnippetsService() = default;
 
 // static
 void NTPSnippetsService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
-  registry->RegisterListPref(prefs::kSnippetHosts); //TODO remove
+  // TODO(treib): Add cleanup logic for prefs::kSnippetHosts, then remove it
+  // completely after M56.
+  registry->RegisterListPref(prefs::kSnippetHosts);
+  registry->RegisterListPref(prefs::kRemoteSuggestionCategories);
   registry->RegisterInt64Pref(prefs::kSnippetBackgroundFetchingIntervalWifi, 0);
   registry->RegisterInt64Pref(prefs::kSnippetBackgroundFetchingIntervalFallback,
                               0);
 
   NTPSnippetsStatusService::RegisterProfilePrefs(registry);
 }
 
 void NTPSnippetsService::FetchSnippets(bool interactive_request) {
   if (ready())
     FetchSnippetsFromHosts(std::set<std::string>(), interactive_request);
@@ skipping 60 matching lines @@
     if (state_ != State::NOT_INITED || force) {
       scheduler_->Unschedule();
       pref_service_->ClearPref(prefs::kSnippetBackgroundFetchingIntervalWifi);
       pref_service_->ClearPref(
           prefs::kSnippetBackgroundFetchingIntervalFallback);
     }
   }
 }
 
 CategoryStatus NTPSnippetsService::GetCategoryStatus(Category category) {
-  DCHECK(categories_.find(category) != categories_.end());
+  DCHECK(base::ContainsKey(categories_, category));
   return categories_[category].status;
 }
 
 CategoryInfo NTPSnippetsService::GetCategoryInfo(Category category) {
-  DCHECK(categories_.find(category) != categories_.end());
+  DCHECK(base::ContainsKey(categories_, category));
   const CategoryContent& content = categories_[category];
   return CategoryInfo(content.localized_title,
                       ContentSuggestionsCardLayout::FULL_CARD,
                       /* has_more_button */ false,
                       /* show_if_empty */ true);
 }
 
 void NTPSnippetsService::DismissSuggestion(
     const ContentSuggestion::ID& suggestion_id) {
   if (!ready())
@@ skipping 38 matching lines @@
   if (!ready())
     nuke_when_initialized_ = true;
   else
     NukeAllSnippets();
 }
 
 void NTPSnippetsService::ClearCachedSuggestions(Category category) {
   if (!initialized())
     return;
 
-  if (categories_.find(category) == categories_.end())
+  if (!base::ContainsKey(categories_, category))
     return;
   CategoryContent* content = &categories_[category];
   if (content->snippets.empty())
     return;
 
   if (category == articles_category_) {
     database_->DeleteSnippets(GetSnippetIDVector(content->snippets));
     database_->DeleteImages(GetSnippetIDVector(content->snippets));
   }
   content->snippets.clear();
 
   NotifyNewSuggestions();
 }
 
 void NTPSnippetsService::GetDismissedSuggestionsForDebugging(
     Category category,
     const DismissedSuggestionsCallback& callback) {
-  DCHECK(categories_.find(category) != categories_.end());
+  DCHECK(base::ContainsKey(categories_, category));
 
   std::vector<ContentSuggestion> result;
   const CategoryContent& content = categories_[category];
   for (const std::unique_ptr<NTPSnippet>& snippet : content.dismissed) {
     if (!snippet->is_complete())
       continue;
     ContentSuggestion suggestion(category, snippet->id(),
                                  snippet->best_source().url);
     suggestion.set_amp_url(snippet->best_source().amp_url);
     suggestion.set_title(base::UTF8ToUTF16(snippet->title()));
     suggestion.set_snippet_text(base::UTF8ToUTF16(snippet->snippet()));
     suggestion.set_publish_date(snippet->publish_date());
     suggestion.set_publisher_name(
         base::UTF8ToUTF16(snippet->best_source().publisher_name));
     suggestion.set_score(snippet->score());
     result.emplace_back(std::move(suggestion));
   }
   callback.Run(std::move(result));
 }
 
 void NTPSnippetsService::ClearDismissedSuggestionsForDebugging(
     Category category) {
-  DCHECK(categories_.find(category) != categories_.end());
+  DCHECK(base::ContainsKey(categories_, category));
 
   if (!initialized())
     return;
 
   CategoryContent* content = &categories_[category];
   if (content->dismissed.empty())
     return;
 
   if (category == articles_category_) {
     // The image got already deleted when the suggestion was dismissed.
     database_->DeleteSnippets(GetSnippetIDVector(content->dismissed));
   }
   content->dismissed.clear();
 }
 
 // static
 int NTPSnippetsService::GetMaxSnippetCountForTesting() {
   return kMaxSnippetCount;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Private methods
 
 GURL NTPSnippetsService::FindSnippetImageUrl(
     const ContentSuggestion::ID& suggestion_id) const {
-  DCHECK(categories_.find(suggestion_id.category()) != categories_.end());
+  DCHECK(base::ContainsKey(categories_, suggestion_id.category()));
 
   const CategoryContent& content = categories_.at(suggestion_id.category());
   const NTPSnippet* snippet =
       content.FindSnippet(suggestion_id.id_within_category());
   if (!snippet)
     return GURL();
   return snippet->salient_image_url();
 }
 
 // image_fetcher::ImageFetcherDelegate implementation.
@@ skipping 16 matching lines @@
 
   // Only cache the data in the DB, the actual serving is done in the callback
   // provided to |image_fetcher_| (OnSnippetImageDecodedFromNetwork()).
   database_->SaveImage(id_within_category, image_data);
 }
 
 void NTPSnippetsService::OnDatabaseLoaded(NTPSnippet::PtrVector snippets) {
   if (state_ == State::ERROR_OCCURRED)
     return;
   DCHECK(state_ == State::NOT_INITED);
-  DCHECK_EQ(1u, categories_.size());  // Only articles category, so far.
-  DCHECK(categories_.find(articles_category_) != categories_.end());
+  DCHECK(base::ContainsKey(categories_, articles_category_));
 
-  // TODO(sfiera): support non-article categories in database.
+  // TODO(treib): Support non-article categories in database. crbug.com/653476
   CategoryContent* content = &categories_[articles_category_];
   for (std::unique_ptr<NTPSnippet>& snippet : snippets) {
     if (snippet->is_dismissed())
       content->dismissed.emplace_back(std::move(snippet));
     else
       content->snippets.emplace_back(std::move(snippet));
   }
 
   std::sort(content->snippets.begin(), content->snippets.end(),
             [](const std::unique_ptr<NTPSnippet>& lhs,
@@ skipping 12 matching lines @@
 void NTPSnippetsService::OnDatabaseError() {
   EnterState(State::ERROR_OCCURRED);
   UpdateAllCategoryStatus(CategoryStatus::LOADING_ERROR);
 }
 
 void NTPSnippetsService::OnFetchFinished(
     NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories) {
   if (!ready())
     return;
 
+  // Mark all categories as not provided by the server in the latest fetch. The
+  // ones we got will be marked again below.
   for (auto& item : categories_) {
     CategoryContent* content = &item.second;
     content->provided_by_server = false;
   }
 
   // Clear up expired dismissed snippets before we use them to filter new ones.
   ClearExpiredDismissedSnippets();
 
   // If snippets were fetched successfully, update our |categories_| from each
   // category provided by the server.
   if (fetched_categories) {
+    // TODO(treib): Reorder |categories_| to match the order we received from
+    // the server. crbug.com/653816
     // TODO(jkrcal): A bit hard to understand with so many variables called
     // "*categor*". Isn't here some room for simplification?
     for (NTPSnippetsFetcher::FetchedCategory& fetched_category :
          *fetched_categories) {
       Category category = fetched_category.category;
 
       // The ChromeReader backend doesn't provide category titles, so don't
       // overwrite the existing title for ARTICLES if the new one is empty.
       // TODO(treib): Remove this check after we fully switch to the content
       // suggestions backend.
@@ skipping 11 matching lines @@
       if (category == articles_category_) {
         UMA_HISTOGRAM_SPARSE_SLOWLY(
             "NewTabPage.Snippets.NumArticlesFetched",
             std::min(fetched_category.snippets.size(),
                      static_cast<size_t>(kMaxSnippetCount + 1)));
       }
       ReplaceSnippets(category, std::move(fetched_category.snippets));
     }
   }
 
+  // We might have gotten new categories (or updated the titles of existing
+  // ones), so update the pref.
+  StoreCategoriesToPrefs();
+
   for (const auto& item : categories_) {
     Category category = item.first;
     UpdateCategoryStatus(category, CategoryStatus::AVAILABLE);
   }
 
   // TODO(sfiera): equivalent metrics for non-articles.
   const CategoryContent& content = categories_[articles_category_];
   UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.NumArticles",
                               content.snippets.size());
   if (content.snippets.empty() && !content.dismissed.empty()) {
     UMA_HISTOGRAM_COUNTS("NewTabPage.Snippets.NumArticlesZeroDueToDiscarded",
                          content.dismissed.size());
   }
 
   // TODO(sfiera): notify only when a category changed above.
   NotifyNewSuggestions();
 
   // Reschedule after a successful fetch. This resets all currently scheduled
   // fetches, to make sure the fallback interval triggers only if no wifi fetch
   // succeeded, and also that we don't do a background fetch immediately after
   // a user-initiated one.
   if (fetched_categories)
     RescheduleFetching(true);
 }
 
 void NTPSnippetsService::ArchiveSnippets(Category category,
                                          NTPSnippet::PtrVector* to_archive) {
   CategoryContent* content = &categories_[category];
 
-  // TODO(sfiera): handle DB for non-articles too.
+  // TODO(treib): Handle DB for non-articles too. crbug.com/653476
   if (category == articles_category_) {
     database_->DeleteSnippets(GetSnippetIDVector(*to_archive));
     // Do not delete the thumbnail images as they are still handy on open NTPs.
   }
 
   // Archive previous snippets - move them at the beginning of the list.
   content->archived.insert(content->archived.begin(),
                            std::make_move_iterator(to_archive->begin()),
                            std::make_move_iterator(to_archive->end()));
   Compact(to_archive);
@@ skipping 97 matching lines @@
     if (content->snippets.empty() && content->dismissed.empty() &&
         category != articles_category_ && !content->provided_by_server) {
       categories_to_erase.push_back(category);
     }
   }
 
   for (Category category : categories_to_erase) {
     UpdateCategoryStatus(category, CategoryStatus::NOT_PROVIDED);
     categories_.erase(category);
   }
+
+  StoreCategoriesToPrefs();
 }
 
 void NTPSnippetsService::ClearOrphanedImages() {
   auto alive_snippets = base::MakeUnique<std::set<std::string>>();
   for (const auto& snippet_ptr : categories_[articles_category_].snippets) {
     alive_snippets->insert(snippet_ptr->id());
   }
   for (const auto& snippet_ptr : categories_[articles_category_].dismissed) {
     alive_snippets->insert(snippet_ptr->id());
   }
@@ skipping 14 matching lines @@
     UpdateCategoryStatus(category, CategoryStatus::NOT_PROVIDED);
 
     // Remove the category entirely; it may or may not reappear.
     if (category != articles_category_)
       categories_to_erase.push_back(category);
   }
 
   for (Category category : categories_to_erase) {
     categories_.erase(category);
   }
+
+  StoreCategoriesToPrefs();
 }
 
 void NTPSnippetsService::OnSnippetImageFetchedFromDatabase(
     const ImageFetchedCallback& callback,
     const ContentSuggestion::ID& suggestion_id,
     std::string data) {
   // |image_decoder_| is null in tests.
   if (image_decoder_ && !data.empty()) {
     image_decoder_->DecodeImage(
         data, base::Bind(&NTPSnippetsService::OnSnippetImageDecodedFromDatabase,
@@ skipping 16 matching lines @@
 
   // If decoding the image failed, delete the DB entry.
   database_->DeleteImage(suggestion_id.id_within_category());
 
   FetchSnippetImageFromNetwork(suggestion_id, callback);
 }
 
 void NTPSnippetsService::FetchSnippetImageFromNetwork(
     const ContentSuggestion::ID& suggestion_id,
     const ImageFetchedCallback& callback) {
-  if (categories_.find(suggestion_id.category()) == categories_.end()) {
+  if (!base::ContainsKey(categories_, suggestion_id.category())) {
     OnSnippetImageDecodedFromNetwork(
         callback, suggestion_id.id_within_category(), gfx::Image());
     return;
   }
 
   GURL image_url = FindSnippetImageUrl(suggestion_id);
 
   if (image_url.is_empty() ||
       !thumbnail_requests_throttler_.DemandQuotaForRequest(
           /*interactive_request=*/true)) {
@@ skipping 184 matching lines @@
     }
 
     DVLOG(1) << "NotifyNewSuggestions(): " << result.size()
              << " items in category " << category;
     observer()->OnNewSuggestions(this, category, std::move(result));
   }
 }
 
 void NTPSnippetsService::UpdateCategoryStatus(Category category,
                                               CategoryStatus status) {
-  DCHECK(categories_.find(category) != categories_.end());
+  DCHECK(base::ContainsKey(categories_, category));
   CategoryContent& content = categories_[category];
   if (status == content.status)
     return;
 
   DVLOG(1) << "UpdateCategoryStatus(): " << category.id() << ": "
            << static_cast<int>(content.status) << " -> "
            << static_cast<int>(status);
   content.status = status;
   observer()->OnCategoryStatusChanged(this, category, content.status);
 }
@@ skipping 19 matching lines @@
       archived.begin(), archived.end(),
       [&id_within_category](const std::unique_ptr<NTPSnippet>& snippet) {
         return snippet->id() == id_within_category;
       });
   if (it != archived.end())
     return it->get();
 
   return nullptr;
 }
 
+namespace {
+
+const char kCategoryContentId[] = "id";

[Bernhard Bauer, 2016/10/07 16:21:27]

Hm... Shouldn't the anonymous namespace go to the top of the file?

[Marc Treib, 2016/10/10 08:33:43]

That's certainly the common thing, though the style guide doesn't actually
enforce it as far as I can tell. In fact, from
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts:
"Note that if there is more than one class in the .cc file, it can aid clarity
to define your file-scope helpers in an anonymous namespace just above the class
that uses them, instead of at the top of the file."
(Which of course doesn't actually apply here, since it's not a separate
class...)
Oh well, moved it to the top. Consistency!

+const char kCategoryContentTitle[] = "title";
+const char kCategoryContentProvidedByServer[] = "provided_by_server";
+
+}  // namespace
+
+void NTPSnippetsService::RestoreCategoriesFromPrefs() {
+  // This must only be called at startup, before there are any categories.
+  DCHECK(categories_.empty());
+
+  const base::ListValue* list =
+      pref_service_->GetList(prefs::kRemoteSuggestionCategories);
+  for (const std::unique_ptr<base::Value>& entry : *list) {
+    const base::DictionaryValue* dict = nullptr;
+    if (!entry->GetAsDictionary(&dict)) {
+      DLOG(WARNING) << "Invalid category pref value: " << *entry;
+      continue;
+    }
+    int id = 0;
+    if (!dict->GetInteger(kCategoryContentId, &id)) {
+      DLOG(WARNING) << "Invalid category pref value, missing '"
+                    << kCategoryContentId << "': " << *entry;
+      continue;
+    }
+    base::string16 title;
+    if (!dict->GetString(kCategoryContentTitle, &title)) {
+      DLOG(WARNING) << "Invalid category pref value, missing '"
+                    << kCategoryContentTitle << "': " << *entry;
+      continue;
+    }
+    bool provided_by_server = false;
+    if (!dict->GetBoolean(kCategoryContentProvidedByServer,
+                          &provided_by_server)) {
+      DLOG(WARNING) << "Invalid category pref value, missing '"
+                    << kCategoryContentProvidedByServer << "': " << *entry;
+      continue;
+    }
+
+    Category category = category_factory()->FromIDValue(id);
+    categories_[category] = CategoryContent();
+    categories_[category].localized_title = title;
+    categories_[category].provided_by_server = provided_by_server;
+  }
+}
+
+void NTPSnippetsService::StoreCategoriesToPrefs() {
+  // Collect all the CategoryContents.
+  std::vector<std::pair<Category, const CategoryContent*>> to_store;
+  for (const auto& entry : categories_)
+    to_store.emplace_back(entry.first, &entry.second);
+  // Sort them into the proper category order.
+  std::sort(to_store.begin(), to_store.end(),
+            [this](const std::pair<Category, const CategoryContent*>& left,
+                   const std::pair<Category, const CategoryContent*>& right) {
+              return category_factory()->CompareCategories(left.first,
+                                                           right.first);
+            });
+  // Convert the relevant info into a base::ListValue for storage.
+  base::ListValue list;
+  for (const auto& entry : to_store) {
+    Category category = entry.first;
+    const base::string16& title = entry.second->localized_title;
+    bool provided_by_server = entry.second->provided_by_server;
+    auto dict = base::MakeUnique<base::DictionaryValue>();
+    dict->SetInteger(kCategoryContentId, category.id());
+    dict->SetString(kCategoryContentTitle, title);
+    dict->SetBoolean(kCategoryContentProvidedByServer, provided_by_server);
+    list.Append(std::move(dict));
+  }
+  // Finally, store the result in the pref service.
+  pref_service_->Set(prefs::kRemoteSuggestionCategories, list);
+}
+
 NTPSnippetsService::CategoryContent::CategoryContent() = default;
 NTPSnippetsService::CategoryContent::CategoryContent(CategoryContent&&) =
     default;
 NTPSnippetsService::CategoryContent::~CategoryContent() = default;
 NTPSnippetsService::CategoryContent& NTPSnippetsService::CategoryContent::
 operator=(CategoryContent&&) = default;
 
 }  // namespace ntp_snippets