📰Fix SnippetsService's status reporting

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 243 matching lines @@
     FetchSnippetsFromHosts(GetSuggestionsHosts(), force_request);
   else
     fetch_after_load_ = true;
 }
 
 void NTPSnippetsService::FetchSnippetsFromHosts(
     const std::set<std::string>& hosts,
     bool force_request) {
   if (!ready())
     return;
+
+  if (snippets_.empty())
+    UpdateCategoryStatus(CategoryStatus::AVAILABLE_LOADING);
+
   snippets_fetcher_->FetchSnippetsFromHosts(hosts, application_language_code_,
                                             kMaxSnippetCount, force_request);
 }
 
 void NTPSnippetsService::RescheduleFetching() {
   // The scheduler only exists on Android so far, it's null on other platforms.
   if (!scheduler_)
     return;
 
   if (ready()) {
@@ skipping 140 matching lines @@
             });
 
   ClearExpiredSnippets();
   FinishInitialization();
 }
 
 void NTPSnippetsService::OnDatabaseError() {
   EnterState(State::SHUT_DOWN, CategoryStatus::LOADING_ERROR);
 }
 
+// TODO(dgn): name clash between content suggestions and suggestions hosts.
+// method name should be changed.
 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);
 
+  // We removed some suggestions, so we want to let the client know about that.
+  // The fetch might take a long time or not complete so we don't want to wait
+  // for its callback.
   NotifyNewSuggestions();
 
   FetchSnippetsFromHosts(hosts, /*force_request=*/false);
 }
 
 void NTPSnippetsService::OnFetchFinished(
     NTPSnippetsFetcher::OptionalSnippets snippets) {
   if (!ready())
     return;
 
+  DCHECK(category_status_ == CategoryStatus::AVAILABLE ||
+         category_status_ == CategoryStatus::AVAILABLE_LOADING);
+
   if (snippets) {
     // Sparse histogram used because the number of snippets is small (bound by
     // kMaxSnippetCount).
     DCHECK_LE(snippets->size(), static_cast<size_t>(kMaxSnippetCount));
     UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.NumArticlesFetched",
                                 snippets->size());
     MergeSnippets(std::move(*snippets));
   }
 
   ClearExpiredSnippets();
 
   // If there are more snippets than we want to show, delete the extra ones.
   if (snippets_.size() > kMaxSnippetCount) {
     NTPSnippet::PtrVector to_delete(
         std::make_move_iterator(snippets_.begin() + kMaxSnippetCount),
         std::make_move_iterator(snippets_.end()));
     snippets_.resize(kMaxSnippetCount);
     database_->DeleteSnippets(to_delete);
   }
 
   UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.NumArticles",
                               snippets_.size());
   if (snippets_.empty() && !dismissed_snippets_.empty()) {
     UMA_HISTOGRAM_COUNTS("NewTabPage.Snippets.NumArticlesZeroDueToDiscarded",
                          dismissed_snippets_.size());
   }
 
+  UpdateCategoryStatus(CategoryStatus::AVAILABLE);
   NotifyNewSuggestions();
 }
 
 void NTPSnippetsService::MergeSnippets(NTPSnippet::PtrVector new_snippets) {
   DCHECK(ready());
 
   // Remove new snippets that we already have, or that have been dismissed.
   std::set<std::string> old_snippet_ids;
   InsertAllIDs(dismissed_snippets_, &old_snippet_ids);
   InsertAllIDs(snippets_, &old_snippet_ids);
@@ skipping 160 matching lines @@
 
   const NTPSnippet& snippet = *it->get();
   image_fetcher_->StartOrQueueNetworkRequest(
       snippet.id(), snippet.salient_image_url(), callback);
 }
 
 void NTPSnippetsService::EnterStateEnabled(bool fetch_snippets) {
   if (fetch_snippets)
     FetchSnippets(/*force_request=*/false);
 
+  // FetchSnippets should set the status to |AVAILABLE_LOADING| if relevant,
+  // otherwise we transition to |AVAILABLE| here.
+  if (category_status_ != CategoryStatus::AVAILABLE_LOADING)
+    UpdateCategoryStatus(CategoryStatus::AVAILABLE);
+
   // If host restrictions are enabled, register for host list updates.
   // |suggestions_service_| can be null in tests.
   if (snippets_fetcher_->UsesHostRestrictions() && suggestions_service_) {
     suggestions_service_subscription_ =
         suggestions_service_->AddCallback(base::Bind(
             &NTPSnippetsService::OnSuggestionsChanged, base::Unretained(this)));
   }
 
   RescheduleFetching();
 }
@@ skipping 27 matching lines @@
     image_fetcher_->SetImageFetcherDelegate(this);
     image_fetcher_->SetDataUseServiceName(
         data_use_measurement::DataUseUserData::NTP_SNIPPETS);
   }
 
   // Note: Initializing the status service will run the callback right away with
   // the current state.
   snippets_status_service_->Init(base::Bind(
       &NTPSnippetsService::OnDisabledReasonChanged, base::Unretained(this)));
 
+  // Always notify here even if we got nothing from the database, because we
+  // don't know how long the fetch will take or if it will even complete.
   NotifyNewSuggestions();
 }
 
 void NTPSnippetsService::OnDisabledReasonChanged(
     DisabledReason disabled_reason) {
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceDisabledReasonChanged(disabled_reason));
 
   switch (disabled_reason) {
     case DisabledReason::NONE:
-      EnterState(State::READY, CategoryStatus::AVAILABLE);
+      // Do not change the status. That will be done in EnterStateEnabled()
+      EnterState(State::READY, category_status_);
       break;
 
     case DisabledReason::EXPLICITLY_DISABLED:
       EnterState(State::DISABLED, CategoryStatus::CATEGORY_EXPLICITLY_DISABLED);
       break;
 
     case DisabledReason::SIGNED_OUT:
       EnterState(State::DISABLED, CategoryStatus::SIGNED_OUT);
       break;
   }
 }
 
 void NTPSnippetsService::EnterState(State state, CategoryStatus status) {
-  if (status != category_status_) {
-    category_status_ = status;
-    NotifyCategoryStatusChanged();
-  }
+  UpdateCategoryStatus(status);
 
   if (state == state_)
     return;
 
   switch (state) {
     case State::NOT_INITED:
       // Initial state, it should not be possible to get back there.
       NOTREACHED();
       return;
 
@@ skipping 44 matching lines @@
     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));
   }
   observer_->OnNewSuggestions(provided_category_, std::move(result));
 }
 
-void NTPSnippetsService::NotifyCategoryStatusChanged() {
+void NTPSnippetsService::UpdateCategoryStatus(CategoryStatus status) {
+  if (status == category_status_)
+    return;
+
+  category_status_ = status;
   if (observer_) {
     observer_->OnCategoryStatusChanged(provided_category_, category_status_);
   }
 }
 
 }  // namespace ntp_snippets