Change NTPSnippetsService to implement ContentSuggestionsProvider

components/ntp_snippets/ntp_snippets_service.cc

Index: components/ntp_snippets/ntp_snippets_service.cc
diff --git a/components/ntp_snippets/ntp_snippets_service.cc b/components/ntp_snippets/ntp_snippets_service.cc
index 4b18a58f6469486d06d8de08277f7ab909edd59d..f97d6d5f0c02d7fcc2cb215c09fa4432d1edfa27 100644
--- a/components/ntp_snippets/ntp_snippets_service.cc
+++ b/components/ntp_snippets/ntp_snippets_service.cc
@@ -182,6 +182,8 @@ void Compact(NTPSnippet::PtrVector* snippets) {
 
 }  // namespace
 
+// TODO(pke): Rename this service to ArticleSuggestionsService and move to
+// nested namespace.

[Marc Treib, 2016/07/08 13:26:02]

Eh, not sure if a nested namespace is necessary.

[Philipp Keck, 2016/07/08 14:02:54]

Done. Changed to "subdirectory" to be consistent with the "offline_pages"
subdirectory that I have for the other provider.

 NTPSnippetsService::NTPSnippetsService(
     bool enabled,
     PrefService* pref_service,
@@ -193,7 +195,8 @@ NTPSnippetsService::NTPSnippetsService(
     std::unique_ptr<ImageDecoder> image_decoder,
     std::unique_ptr<NTPSnippetsDatabase> database,
     std::unique_ptr<NTPSnippetsStatusService> status_service)
-    : state_(State::NOT_INITED),
+    : ContentSuggestionsProvider({ContentSuggestionsCategory::ARTICLES}),
+      state_(State::NOT_INITED),
       pref_service_(pref_service),
       suggestions_service_(suggestions_service),
       application_language_code_(application_language_code),
@@ -207,9 +210,18 @@ NTPSnippetsService::NTPSnippetsService(
   // TODO(dgn) should be removed after branch point (https://crbug.com/617585).
   ClearDeprecatedPrefs();
 
-  if (!enabled || database_->IsErrorState()) {
-    // Don't even bother loading the database.
+  // In some cases, don't even bother loading the database.
+  if (!enabled) {
+    category_status_ =
+        ContentSuggestionsCategoryStatus::CATEGORY_EXPLICITLY_DISABLED;
     EnterState(State::SHUT_DOWN);
+    NotifyCategoryStatusChanged();
+    return;
+  }
+  if (database_->IsErrorState()) {
+    category_status_ = ContentSuggestionsCategoryStatus::ERROR;
+    EnterState(State::SHUT_DOWN);
+    NotifyCategoryStatusChanged();
     return;
   }
 
@@ -235,7 +247,9 @@ void NTPSnippetsService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
 
 // Inherited from KeyedService.
 void NTPSnippetsService::Shutdown() {
+  category_status_ = ContentSuggestionsCategoryStatus::NOT_PROVIDED;
   EnterState(State::SHUT_DOWN);
+  NotifyCategoryStatusChanged();
 }
 
 void NTPSnippetsService::FetchSnippets() {
@@ -268,16 +282,16 @@ void NTPSnippetsService::RescheduleFetching() {
   }
 }
 
-void NTPSnippetsService::FetchSnippetImage(
-    const std::string& snippet_id,
+void NTPSnippetsService::FetchSuggestionImage(
+    const std::string& suggestion_id,
     const ImageFetchedCallback& callback) {
   database_->LoadImage(
-      snippet_id,
+      suggestion_id,
       base::Bind(&NTPSnippetsService::OnSnippetImageFetchedFromDatabase,
-                 base::Unretained(this), snippet_id, callback));
+                 base::Unretained(this), suggestion_id, callback));
 }
 
-void NTPSnippetsService::ClearSnippets() {
+void NTPSnippetsService::ClearCachedSuggestionsForDebugging() {
   if (!initialized())
     return;
 
@@ -287,8 +301,7 @@ void NTPSnippetsService::ClearSnippets() {
   database_->DeleteSnippets(snippets_);
   snippets_.clear();
 
-  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
-                    NTPSnippetsServiceLoaded());
+  NotifyNewSuggestions();
 }
 
 std::set<std::string> NTPSnippetsService::GetSuggestionsHosts() const {
@@ -301,17 +314,17 @@ std::set<std::string> NTPSnippetsService::GetSuggestionsHosts() const {
       suggestions_service_->GetSuggestionsDataFromCache());
 }
 
-bool NTPSnippetsService::DiscardSnippet(const std::string& snippet_id) {
+void NTPSnippetsService::DiscardSuggestion(const std::string& suggestion_id) {
   if (!ready())
-    return false;
+    return;
 
-  auto it =
-      std::find_if(snippets_.begin(), snippets_.end(),
-                   [&snippet_id](const std::unique_ptr<NTPSnippet>& snippet) {
-                     return snippet->id() == snippet_id;
-                   });
+  auto it = std::find_if(
+      snippets_.begin(), snippets_.end(),
+      [&suggestion_id](const std::unique_ptr<NTPSnippet>& snippet) {
+        return snippet->id() == suggestion_id;
+      });
   if (it == snippets_.end())
-    return false;
+    return;
 
   (*it)->set_discarded(true);
 
@@ -321,12 +334,10 @@ bool NTPSnippetsService::DiscardSnippet(const std::string& snippet_id) {
   discarded_snippets_.push_back(std::move(*it));
   snippets_.erase(it);
 
-  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
-                    NTPSnippetsServiceLoaded());
-  return true;
+  NotifyNewSuggestions();
 }
 
-void NTPSnippetsService::ClearDiscardedSnippets() {
+void NTPSnippetsService::ClearDiscardedSuggestionsForDebugging() {
   if (!initialized())
     return;
 
@@ -337,6 +348,15 @@ void NTPSnippetsService::ClearDiscardedSnippets() {
   discarded_snippets_.clear();
 }
 
+void NTPSnippetsService::SetObserver(Observer* observer) {
+  observer_ = observer;
+}
+
+ContentSuggestionsCategoryStatus NTPSnippetsService::GetCategoryStatus(
+    ContentSuggestionsCategory category) {
+  return category_status_;
+}
+
 void NTPSnippetsService::AddObserver(NTPSnippetsServiceObserver* observer) {
   observers_.AddObserver(observer);
 }
@@ -395,7 +415,9 @@ void NTPSnippetsService::OnDatabaseLoaded(NTPSnippet::PtrVector snippets) {
 }
 
 void NTPSnippetsService::OnDatabaseError() {
+  category_status_ = ContentSuggestionsCategoryStatus::ERROR;
   EnterState(State::SHUT_DOWN);
+  NotifyCategoryStatusChanged();
 }
 
 void NTPSnippetsService::OnSuggestionsChanged(
@@ -421,8 +443,7 @@ void NTPSnippetsService::OnSuggestionsChanged(
 
   StoreSnippetHostsToPrefs(hosts);
 
-  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
-                    NTPSnippetsServiceLoaded());
+  NotifyNewSuggestions();
 
   FetchSnippetsFromHosts(hosts);
 }
@@ -459,8 +480,7 @@ void NTPSnippetsService::OnFetchFinished(
                          discarded_snippets_.size());
   }
 
-  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
-                    NTPSnippetsServiceLoaded());
+  NotifyNewSuggestions();
 }
 
 void NTPSnippetsService::MergeSnippets(NTPSnippet::PtrVector new_snippets) {
@@ -652,8 +672,8 @@ void NTPSnippetsService::EnterStateEnabled(bool fetch_snippets) {
 }
 
 void NTPSnippetsService::EnterStateDisabled() {
-  ClearSnippets();
-  ClearDiscardedSnippets();
+  ClearCachedSuggestionsForDebugging();
+  ClearDiscardedSuggestionsForDebugging();
 
   expiry_timer_.Stop();
   suggestions_service_subscription_.reset();
@@ -684,8 +704,7 @@ void NTPSnippetsService::FinishInitialization() {
   snippets_status_service_->Init(base::Bind(
       &NTPSnippetsService::UpdateStateForStatus, base::Unretained(this)));
 
-  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
-                    NTPSnippetsServiceLoaded());
+  NotifyNewSuggestions();
 }
 
 void NTPSnippetsService::UpdateStateForStatus(DisabledReason disabled_reason) {
@@ -696,6 +715,7 @@ void NTPSnippetsService::UpdateStateForStatus(DisabledReason disabled_reason) {
   switch (disabled_reason) {
     case DisabledReason::NONE:
       new_state = State::READY;
+      category_status_ = ContentSuggestionsCategoryStatus::AVAILABLE;
       break;
 
     case DisabledReason::HISTORY_SYNC_STATE_UNKNOWN:
@@ -705,13 +725,34 @@ void NTPSnippetsService::UpdateStateForStatus(DisabledReason disabled_reason) {
       DVLOG(1) << "Sync configuration incomplete, continuing based on the "
                   "current state.";
       new_state = state_;
+      category_status_ = ContentSuggestionsCategoryStatus::LOADING;
       break;
 
     case DisabledReason::EXPLICITLY_DISABLED:
+      category_status_ =
+          ContentSuggestionsCategoryStatus::CATEGORY_EXPLICITLY_DISABLED;
+      new_state = State::DISABLED;
+      break;
+
     case DisabledReason::SIGNED_OUT:
+      category_status_ = ContentSuggestionsCategoryStatus::SIGNED_OUT;
+      new_state = State::DISABLED;
+      break;
+
     case DisabledReason::SYNC_DISABLED:
+      category_status_ = ContentSuggestionsCategoryStatus::SYNC_DISABLED;
+      new_state = State::DISABLED;
+      break;
+
     case DisabledReason::PASSPHRASE_ENCRYPTION_ENABLED:
+      category_status_ =
+          ContentSuggestionsCategoryStatus::PASSPHRASE_ENCRYPTION_ENABLED;
+      new_state = State::DISABLED;
+      break;
+
     case DisabledReason::HISTORY_SYNC_DISABLED:
+      category_status_ =
+          ContentSuggestionsCategoryStatus::HISTORY_SYNC_DISABLED;
       new_state = State::DISABLED;
       break;
 
@@ -723,6 +764,7 @@ void NTPSnippetsService::UpdateStateForStatus(DisabledReason disabled_reason) {
   }
 
   EnterState(new_state);
+  NotifyCategoryStatusChanged();
 }
 
 void NTPSnippetsService::EnterState(State state) {
@@ -767,4 +809,35 @@ void NTPSnippetsService::ClearDeprecatedPrefs() {
   pref_service_->ClearPref(prefs::kDeprecatedDiscardedSnippets);
 }
 
+void NTPSnippetsService::NotifyNewSuggestions() {
+  if (observer_) {
+    std::vector<ContentSuggestion> result;
+    for (const std::unique_ptr<NTPSnippet>& snippet : snippets_) {
+      if (!snippet->is_discarded() && snippet->is_complete()) {

[Marc Treib, 2016/07/08 13:26:02]

snippets_ contains only non-discarded snippets, no need to check here.
Also maybe "if (!complete) continue" to save a level of indentation?

[Philipp Keck, 2016/07/08 14:02:54]

Done.

+        ContentSuggestion suggestion(
+            MakeUniqueID(ContentSuggestionsCategory::ARTICLES, snippet->id()),
+            snippet->best_source().url);
+        suggestion.set_amp_url(snippet->best_source().amp_url);
+        suggestion.set_title(snippet->title());
+        suggestion.set_snippet_text(snippet->snippet());
+        suggestion.set_publish_date(snippet->publish_date());
+        suggestion.set_publisher_name(snippet->best_source().publisher_name);
+        suggestion.set_score(snippet->score());
+        result.emplace_back(std::move(suggestion));
+      }
+    }
+    observer_->OnNewSuggestions(ContentSuggestionsCategory::ARTICLES,
+                                std::move(result));
+  }
+  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,

[Marc Treib, 2016/07/08 13:26:02]

nit: move this up, so you can do "if (!observer_) return" and save one level of
indentation? Also maybe add a(nother) TODO to remove this?

[Philipp Keck, 2016/07/08 14:02:55]

Done.

+                    NTPSnippetsServiceLoaded());
+}
+
+void NTPSnippetsService::NotifyCategoryStatusChanged() {
+  if (observer_) {
+    observer_->OnCategoryStatusChanged(ContentSuggestionsCategory::ARTICLES,
+                                       category_status_);
+  }
+}
+
 }  // namespace ntp_snippets