Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(320)

Unified Diff: components/ntp_snippets/remote/ntp_snippets_service.cc

Issue 2395273003: [NTP Snippets] Persist remote categories in prefs (Closed)
Patch Set: Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/ntp_snippets/remote/ntp_snippets_service.cc
diff --git a/components/ntp_snippets/remote/ntp_snippets_service.cc b/components/ntp_snippets/remote/ntp_snippets_service.cc
index 78aea3a04b60edba453d0f2562906971ae05a88e..7c7cc1fbaf237941592e2134048e510b29c85c18 100644
--- a/components/ntp_snippets/remote/ntp_snippets_service.cc
+++ b/components/ntp_snippets/remote/ntp_snippets_service.cc
@@ -193,12 +193,19 @@ NTPSnippetsService::NTPSnippetsService(
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);
@@ -218,7 +225,10 @@ 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);
@@ -299,12 +309,12 @@ void NTPSnippetsService::RescheduleFetching(bool force) {
}
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,
@@ -363,7 +373,7 @@ 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())
@@ -381,7 +391,7 @@ void NTPSnippetsService::ClearCachedSuggestions(Category category) {
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];
@@ -404,7 +414,7 @@ void NTPSnippetsService::GetDismissedSuggestionsForDebugging(
void NTPSnippetsService::ClearDismissedSuggestionsForDebugging(
Category category) {
- DCHECK(categories_.find(category) != categories_.end());
+ DCHECK(base::ContainsKey(categories_, category));
if (!initialized())
return;
@@ -430,7 +440,7 @@ int NTPSnippetsService::GetMaxSnippetCountForTesting() {
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 =
@@ -467,10 +477,9 @@ 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())
@@ -503,6 +512,8 @@ void NTPSnippetsService::OnFetchFinished(
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;
@@ -545,6 +556,10 @@ void NTPSnippetsService::OnFetchFinished(
}
}
+ // 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);
@@ -574,7 +589,7 @@ 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.
@@ -692,6 +707,8 @@ void NTPSnippetsService::ClearExpiredDismissedSnippets() {
UpdateCategoryStatus(category, CategoryStatus::NOT_PROVIDED);
categories_.erase(category);
}
+
+ StoreCategoriesToPrefs();
}
void NTPSnippetsService::ClearOrphanedImages() {
@@ -726,6 +743,8 @@ void NTPSnippetsService::NukeAllSnippets() {
for (Category category : categories_to_erase) {
categories_.erase(category);
}
+
+ StoreCategoriesToPrefs();
}
void NTPSnippetsService::OnSnippetImageFetchedFromDatabase(
@@ -762,7 +781,7 @@ void NTPSnippetsService::OnSnippetImageDecodedFromDatabase(
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;
@@ -967,7 +986,7 @@ void NTPSnippetsService::NotifyNewSuggestions() {
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;
@@ -1007,6 +1026,81 @@ const NTPSnippet* NTPSnippetsService::CategoryContent::FindSnippet(
return nullptr;
}
+namespace {
+
+const char kCategoryContentId[] = "id";
+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;

Powered by Google App Engine
This is Rietveld 408576698