Change NTPSnippetsService to implement ContentSuggestionsProvider

components/ntp_snippets/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/ntp_snippets_service.h"
 
 #include <algorithm>
 #include <iterator>
 #include <utility>
 
@@ skipping 164 matching lines @@
 void Compact(NTPSnippet::PtrVector* snippets) {
   snippets->erase(
       std::remove_if(
           snippets->begin(), snippets->end(),
           [](const std::unique_ptr<NTPSnippet>& snippet) { return !snippet; }),
       snippets->end());
 }
 
 }  // namespace
 
+// TODO(pke): Rename this service to ArticleSuggestionsService and move to
+// a subdirectory.
 NTPSnippetsService::NTPSnippetsService(
     bool enabled,
     PrefService* pref_service,
     SuggestionsService* suggestions_service,
     const std::string& application_language_code,
     NTPSnippetsScheduler* scheduler,
     std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher,
     std::unique_ptr<ImageFetcher> image_fetcher,
     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),
+      category_status_(ContentSuggestionsCategoryStatus::LOADING),
       pref_service_(pref_service),
       suggestions_service_(suggestions_service),
       application_language_code_(application_language_code),
+      observer_(nullptr),
       scheduler_(scheduler),
       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_after_load_(false) {
   // TODO(dgn) should be removed after branch point (https://crbug.com/617585).
   ClearDeprecatedPrefs();
 
-  if (!enabled || database_->IsErrorState()) {
-    // Don't even bother loading the database.
-    EnterState(State::SHUT_DOWN);
+  // In some cases, don't even bother loading the database.
+  if (!enabled) {
+    EnterState(State::SHUT_DOWN,
+               ContentSuggestionsCategoryStatus::CATEGORY_EXPLICITLY_DISABLED);
+    return;
+  }
+  if (database_->IsErrorState()) {
+    EnterState(State::SHUT_DOWN, ContentSuggestionsCategoryStatus::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() {
   DCHECK(state_ == State::SHUT_DOWN);
 }
 
 // static
 void NTPSnippetsService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterListPref(prefs::kDeprecatedSnippets);
   registry->RegisterListPref(prefs::kDeprecatedDiscardedSnippets);
   registry->RegisterListPref(prefs::kSnippetHosts);
 }
 
 // Inherited from KeyedService.
 void NTPSnippetsService::Shutdown() {
-  EnterState(State::SHUT_DOWN);
+  EnterState(State::SHUT_DOWN, ContentSuggestionsCategoryStatus::NOT_PROVIDED);
 }
 
 void NTPSnippetsService::FetchSnippets() {
   if (ready())
     FetchSnippetsFromHosts(GetSuggestionsHosts());
   else
     fetch_after_load_ = true;
 }
 
 void NTPSnippetsService::FetchSnippetsFromHosts(
@@ skipping 12 matching lines @@
   if (ready()) {
     base::Time now = base::Time::Now();
     scheduler_->Schedule(
         GetFetchingIntervalWifiCharging(), GetFetchingIntervalWifi(now),
         GetFetchingIntervalFallback(), GetRescheduleTime(now));
   } else {
     scheduler_->Unschedule();
   }
 }
 
-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;
 
   if (snippets_.empty())
     return;
 
   database_->DeleteSnippets(snippets_);
   snippets_.clear();
 
-  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
-                    NTPSnippetsServiceLoaded());
+  NotifyNewSuggestions();
 }
 
 std::set<std::string> NTPSnippetsService::GetSuggestionsHosts() const {
   // |suggestions_service_| can be null in tests.
   if (!suggestions_service_)
     return std::set<std::string>();
 
   // TODO(treib): This should just call GetSnippetHostsFromPrefs.
   return GetSuggestionsHostsImpl(
       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);
 
   database_->SaveSnippet(**it);
   database_->DeleteImage((*it)->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;
 
   if (discarded_snippets_.empty())
     return;
 
   database_->DeleteSnippets(discarded_snippets_);
   discarded_snippets_.clear();
 }
 
+void NTPSnippetsService::SetObserver(Observer* observer) {
+  observer_ = observer;
+}
+
+ContentSuggestionsCategoryStatus NTPSnippetsService::GetCategoryStatus(
+    ContentSuggestionsCategory category) {
+  DCHECK_EQ(ContentSuggestionsCategory::ARTICLES, category);
+  return category_status_;
+}
+
 void NTPSnippetsService::AddObserver(NTPSnippetsServiceObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void NTPSnippetsService::RemoveObserver(NTPSnippetsServiceObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
 // static
 int NTPSnippetsService::GetMaxSnippetCountForTesting() {
@@ skipping 38 matching lines @@
             [](const std::unique_ptr<NTPSnippet>& lhs,
                const std::unique_ptr<NTPSnippet>& rhs) {
               return lhs->score() > rhs->score();
             });
 
   ClearExpiredSnippets();
   FinishInitialization();
 }
 
 void NTPSnippetsService::OnDatabaseError() {
-  EnterState(State::SHUT_DOWN);
+  EnterState(State::SHUT_DOWN, ContentSuggestionsCategoryStatus::ERROR);
 }
 
 void NTPSnippetsService::OnSuggestionsChanged(
     const SuggestionsProfile& suggestions) {
   DCHECK(initialized());
 
   std::set<std::string> hosts = GetSuggestionsHostsImpl(suggestions);
   if (hosts == GetSnippetHostsFromPrefs())
     return;
 
   // Remove existing snippets that aren't in the suggestions anymore.
   // TODO(treib,maybelle): If there is another source with an allowed host,
   // then we should fall back to that.
   // First, move them over into |to_delete|.
   NTPSnippet::PtrVector to_delete;
   for (std::unique_ptr<NTPSnippet>& snippet : snippets_) {
     if (!hosts.count(snippet->best_source().url.host()))
       to_delete.emplace_back(std::move(snippet));
   }
   Compact(&snippets_);
   // Then delete the removed snippets from the database.
   database_->DeleteSnippets(to_delete);
 
   StoreSnippetHostsToPrefs(hosts);
 
-  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
-                    NTPSnippetsServiceLoaded());
+  NotifyNewSuggestions();
 
   FetchSnippetsFromHosts(hosts);
 }
 
 void NTPSnippetsService::OnFetchFinished(
     NTPSnippetsFetcher::OptionalSnippets snippets) {
   if (!ready())
     return;
 
   if (snippets) {
@@ skipping 16 matching lines @@
     database_->DeleteSnippets(to_delete);
   }
 
   UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.NumArticles",
                               snippets_.size());
   if (snippets_.empty() && !discarded_snippets_.empty()) {
     UMA_HISTOGRAM_COUNTS("NewTabPage.Snippets.NumArticlesZeroDueToDiscarded",
                          discarded_snippets_.size());
   }
 
-  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
-                    NTPSnippetsServiceLoaded());
+  NotifyNewSuggestions();
 }
 
 void NTPSnippetsService::MergeSnippets(NTPSnippet::PtrVector new_snippets) {
   DCHECK(ready());
 
   // Remove new snippets that we already have, or that have been discarded.
   std::set<std::string> old_snippet_ids;
   InsertAllIDs(discarded_snippets_, &old_snippet_ids);
   InsertAllIDs(snippets_, &old_snippet_ids);
   new_snippets.erase(
@@ skipping 171 matching lines @@
   if (snippets_fetcher_->UsesHostRestrictions() && suggestions_service_) {
     suggestions_service_subscription_ =
         suggestions_service_->AddCallback(base::Bind(
             &NTPSnippetsService::OnSuggestionsChanged, base::Unretained(this)));
   }
 
   RescheduleFetching();
 }
 
 void NTPSnippetsService::EnterStateDisabled() {
-  ClearSnippets();
-  ClearDiscardedSnippets();
+  ClearCachedSuggestionsForDebugging();
+  ClearDiscardedSuggestionsForDebugging();
 
   expiry_timer_.Stop();
   suggestions_service_subscription_.reset();
   RescheduleFetching();
 }
 
 void NTPSnippetsService::EnterStateShutdown() {
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceShutdown());
 
@@ skipping 10 matching lines @@
 
   // |image_fetcher_| can be null in tests.
   if (image_fetcher_)
     image_fetcher_->SetImageFetcherDelegate(this);
 
   // Note: Initializing the status service will run the callback right away with
   // the current state.
   snippets_status_service_->Init(base::Bind(
       &NTPSnippetsService::UpdateStateForStatus, base::Unretained(this)));
 
-  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
-                    NTPSnippetsServiceLoaded());
+  NotifyNewSuggestions();
 }
 
 void NTPSnippetsService::UpdateStateForStatus(DisabledReason disabled_reason) {
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceDisabledReasonChanged(disabled_reason));
 
   State new_state;
+  ContentSuggestionsCategoryStatus new_status;
   switch (disabled_reason) {
     case DisabledReason::NONE:
       new_state = State::READY;
+      new_status = ContentSuggestionsCategoryStatus::AVAILABLE;
       break;
 
     case DisabledReason::HISTORY_SYNC_STATE_UNKNOWN:
       // HistorySync is not initialized yet, so we don't know what the actual
       // state is and we just return the current one. If things change,
       // |OnStateChanged| will call this function again to update the state.
       DVLOG(1) << "Sync configuration incomplete, continuing based on the "
                   "current state.";
       new_state = state_;
+      new_status = ContentSuggestionsCategoryStatus::LOADING;
       break;
 
     case DisabledReason::EXPLICITLY_DISABLED:
-    case DisabledReason::SIGNED_OUT:
-    case DisabledReason::SYNC_DISABLED:
-    case DisabledReason::PASSPHRASE_ENCRYPTION_ENABLED:
-    case DisabledReason::HISTORY_SYNC_DISABLED:
+      new_status =
+          ContentSuggestionsCategoryStatus::CATEGORY_EXPLICITLY_DISABLED;
       new_state = State::DISABLED;
       break;
 
+    case DisabledReason::SIGNED_OUT:
+      new_status = ContentSuggestionsCategoryStatus::SIGNED_OUT;
+      new_state = State::DISABLED;
+      break;
+
+    case DisabledReason::SYNC_DISABLED:
+      new_status = ContentSuggestionsCategoryStatus::SYNC_DISABLED;
+      new_state = State::DISABLED;
+      break;
+
+    case DisabledReason::PASSPHRASE_ENCRYPTION_ENABLED:
+      new_status =
+          ContentSuggestionsCategoryStatus::PASSPHRASE_ENCRYPTION_ENABLED;
+      new_state = State::DISABLED;
+      break;
+
+    case DisabledReason::HISTORY_SYNC_DISABLED:
+      new_status = ContentSuggestionsCategoryStatus::HISTORY_SYNC_DISABLED;
+      new_state = State::DISABLED;
+      break;
+
     default:
       // All cases should be handled by the above switch
       NOTREACHED();
       new_state = State::DISABLED;
+      new_status = ContentSuggestionsCategoryStatus::ERROR;

[Marc Treib, 2016/07/11 10:12:04]

nit: Can we get a consistent order of assigning new_state and new_status please?

[Philipp Keck, 2016/07/11 10:30:56]

Done.

       break;
   }
 
-  EnterState(new_state);
+  EnterState(new_state, new_status);
 }
 
-void NTPSnippetsService::EnterState(State state) {
-  if (state == state_)
+void NTPSnippetsService::EnterState(
+    State state,
+    ContentSuggestionsCategoryStatus new_status) {
+  if (state == state_) {
+    category_status_ = new_status;

[Philipp Keck, 2016/07/11 09:41:49]

This code duplicates line 806. Alternative control structure with one more level
of indentation and no early-returns:

if (state != state_) {
   switch ...
}

if (new_status != category_status_) {
   update
   notify
}

[Marc Treib, 2016/07/11 10:12:04]

Hm. I guess we don't want to notify about the changed status before we've
updated the state too? If that were okay, you could just put the status update
before the if.
Otherwise, the version without early return is probably better.

nit: Please either rename state to new_state, or remove the new_ from
new_status.

[Philipp Keck, 2016/07/11 10:30:56]

That's what I thought initially, but actually it shouldn't matter too much if
the status is updated beforehand because it is pretty independent anyway --
either someone accesses state+disabledReason or the status.
Using your first suggestion, therefore.

+    NotifyCategoryStatusChanged();
     return;
+  }
 
   switch (state) {
     case State::NOT_INITED:
       // Initial state, it should not be possible to get back there.
       NOTREACHED();
       return;
 
     case State::READY: {
       DCHECK(state_ == State::NOT_INITED || state_ == State::DISABLED);
 
@@ skipping 12 matching lines @@
       state_ = State::DISABLED;
       EnterStateDisabled();
       return;
 
     case State::SHUT_DOWN:
       DVLOG(1) << "Entering state: SHUT_DOWN";
       state_ = State::SHUT_DOWN;
       EnterStateShutdown();
       return;
   }
+
+  category_status_ = new_status;
+  NotifyCategoryStatusChanged();
 }
 
 void NTPSnippetsService::ClearDeprecatedPrefs() {
   pref_service_->ClearPref(prefs::kDeprecatedSnippets);
   pref_service_->ClearPref(prefs::kDeprecatedDiscardedSnippets);
 }
 
+void NTPSnippetsService::NotifyNewSuggestions() {
+  // TODO(pke): Remove this as soon as this becomes a pure provider.
+  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
+                    NTPSnippetsServiceLoaded());
+
+  if (!observer_)
+    return;
+
+  std::vector<ContentSuggestion> result;
+  for (const std::unique_ptr<NTPSnippet>& snippet : snippets_) {
+    if (!snippet->is_complete())
+      continue;
+    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));
+}
+
+void NTPSnippetsService::NotifyCategoryStatusChanged() {
+  if (observer_) {
+    observer_->OnCategoryStatusChanged(ContentSuggestionsCategory::ARTICLES,
+                                       category_status_);
+  }
+}
+
 }  // namespace ntp_snippets