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..610b2ecc68d521740e175b6f1851ede2301f9ac8 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; |
@@ -514,6 +525,8 @@ void NTPSnippetsService::OnFetchFinished( |
// 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 : |
@@ -545,6 +558,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 +591,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 +709,8 @@ void NTPSnippetsService::ClearExpiredDismissedSnippets() { |
UpdateCategoryStatus(category, CategoryStatus::NOT_PROVIDED); |
categories_.erase(category); |
} |
+ |
+ StoreCategoriesToPrefs(); |
} |
void NTPSnippetsService::ClearOrphanedImages() { |
@@ -726,6 +745,8 @@ void NTPSnippetsService::NukeAllSnippets() { |
for (Category category : categories_to_erase) { |
categories_.erase(category); |
} |
+ |
+ StoreCategoriesToPrefs(); |
} |
void NTPSnippetsService::OnSnippetImageFetchedFromDatabase( |
@@ -762,7 +783,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 +988,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 +1028,81 @@ const NTPSnippet* NTPSnippetsService::CategoryContent::FindSnippet( |
return nullptr; |
} |
+namespace { |
+ |
+const char kCategoryContentId[] = "id"; |
Bernhard Bauer
2016/10/07 16:21:27
Hm... Shouldn't the anonymous namespace go to the
Marc Treib
2016/10/10 08:33:43
That's certainly the common thing, though the styl
|
+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; |