[NTP Snippets] Unschedule fetches when the service should be disabled

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 169 matching lines @@
 NTPSnippetsService::NTPSnippetsService(
     bool enabled,
     PrefService* pref_service,
     sync_driver::SyncService* sync_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<NTPSnippetsDatabase> database)
-    : state_(State::INITED),
-      enabled_(enabled),
+    : state_(State::NOT_INITED),
+      explicitly_disabled_(!enabled),
       pref_service_(pref_service),
       sync_service_(sync_service),
       sync_service_observer_(this),
       suggestions_service_(suggestions_service),
       application_language_code_(application_language_code),
       scheduler_(scheduler),
       snippets_fetcher_(std::move(snippets_fetcher)),
       image_fetcher_(std::move(image_fetcher)),
       database_(std::move(database)),
       fetch_after_load_(false) {
+  // TODO(dgn) should that be removed after branch point?
+  ClearDeprecatedPrefs();
+
+  if (explicitly_disabled_) {
+    EnterState(State::DISABLED);
+    return;
+  }
+
   snippets_fetcher_->SetCallback(base::Bind(
       &NTPSnippetsService::OnFetchFinished, base::Unretained(this)));
 
   // |sync_service_| can be null in tests or if sync is disabled.
+  // This is a service we want to keep listening to all the time, independently
+  // from the state, since it will allow us to enable the snippets service.
   if (sync_service_)
     sync_service_observer_.Add(sync_service_);
 
-  if (enabled_) {
-    database_->Load(base::Bind(&NTPSnippetsService::OnDatabaseLoaded,
-                               base::Unretained(this)));
-  }
+  // Will trigger the transition to the READY state.
+  database_->Load(base::Bind(&NTPSnippetsService::OnDatabaseLoaded,
+                             base::Unretained(this)));
 
-  RescheduleFetching();

[dgn, 2016/06/03 19:42:22]

Significant change: we now call Reschedule fetching during the
transition to READY, rather than in the constructor. When the service
is disabled, it happens during the transition to disable, synchronously
from the constructor, so changes are minimal there. However here, it
would happen only once the DB is done loading. Worth modifying?

[Marc Treib, 2016/06/06 08:38:32]

I think this is fine.

-
-  ClearDeprecatedPrefs();
+  // Significant change: we now call Reschedule fetching during the

[dgn, 2016/06/03 19:42:22]

Was supposed to be a review comment. To be removed.

[Marc Treib, 2016/06/06 08:38:32]

Acknowledged.

+  // transition to READY, rather than in the constructor. When the service
+  // is disabled, it happens during the transition to disable, synchronously
+  // from the constructor, so changes are minimal there. However here, it
+  // would happen only once the DB is done loading. Worth modifying?
 }
 
 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() {
-  DCHECK(state_ == State::INITED || state_ == State::LOADED);
-  state_ = State::SHUT_DOWN;
+  EnterState(State::SHUT_DOWN);
 
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceShutdown());
   expiry_timer_.Stop();
   suggestions_service_subscription_.reset();
-  enabled_ = false;
 }
 
 void NTPSnippetsService::FetchSnippets() {
-  if (loaded())
+  if (ready())
     FetchSnippetsFromHosts(GetSuggestionsHosts());
   else
     fetch_after_load_ = true;
 }
 
 void NTPSnippetsService::FetchSnippetsFromHosts(
     const std::set<std::string>& hosts) {
-  if (!loaded())
+  if (!ready())
     return;
   snippets_fetcher_->FetchSnippetsFromHosts(hosts, application_language_code_,
                                             kMaxSnippetCount);
 }
 
 void NTPSnippetsService::RescheduleFetching() {
   // The scheduler only exists on Android so far, it's null on other platforms.
   if (!scheduler_)
     return;
 
-  if (enabled_) {
+  if (ready()) {
     base::Time now = base::Time::Now();
     scheduler_->Schedule(
         GetFetchingIntervalWifiCharging(), GetFetchingIntervalWifi(now),
         GetFetchingIntervalFallback(), GetRescheduleTime(now));
   } else {
     scheduler_->Unschedule();
   }
 }
 
 void NTPSnippetsService::FetchSnippetImage(
@@ skipping 10 matching lines @@
     return;
   }
 
   const NTPSnippet& snippet = *it->get();
   image_fetcher_->StartOrQueueNetworkRequest(
       snippet.id(), snippet.salient_image_url(), callback);
   // TODO(treib): Cache/persist the snippet image.
 }
 
 void NTPSnippetsService::ClearSnippets() {
-  if (!loaded())
+  if (!initialized())
+    return;
+
+  if (snippets_.empty())
     return;
 
   database_->Delete(snippets_);
   snippets_.clear();
 
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceLoaded());
 }
 
 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) {
-  if (!loaded())
+  if (!ready())
     return false;
 
   auto it =
       std::find_if(snippets_.begin(), snippets_.end(),
                    [&snippet_id](const std::unique_ptr<NTPSnippet>& snippet) {
                      return snippet->id() == snippet_id;
                    });
   if (it == snippets_.end())
     return false;
 
   (*it)->set_discarded(true);
 
   database_->Save(**it);
 
   discarded_snippets_.push_back(std::move(*it));
   snippets_.erase(it);
 
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceLoaded());
   return true;
 }
 
 void NTPSnippetsService::ClearDiscardedSnippets() {
-  if (!loaded())
+  if (!initialized())
     return;
 
   database_->Delete(discarded_snippets_);
   discarded_snippets_.clear();
 
   FetchSnippets();
 }
 
 void NTPSnippetsService::AddObserver(NTPSnippetsServiceObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void NTPSnippetsService::RemoveObserver(NTPSnippetsServiceObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+DisabledReason NTPSnippetsService::GetDisabledReason() {
+  if (explicitly_disabled_)
+    return DisabledReason::EXPLICITLY_DISABLED;
+
+  if (!sync_service_ || !sync_service_->CanSyncStart()) {
+    DVLOG(1) << "[GetDisabledReason] Sync disabled";
+    return DisabledReason::HISTORY_SYNC_DISABLED;
+  }
+
+  // !IsSyncActive in cases where CanSyncStart is true hints at the backend not
+  // being initialized.
+  // ConfigurationDone() verifies that the sync service has properly loaded its
+  // configuration and is aware of the different components to sync.
+  if (!sync_service_->IsSyncActive() || !sync_service_->ConfigurationDone()) {
+    DVLOG(1) << "[GetDisabledReason] Sync initialization is not complete.";
+    return DisabledReason::HISTORY_SYNC_STATE_UNKNOWN;
+  }
+
+  if (!sync_service_->GetActiveDataTypes().Has(
+          syncer::HISTORY_DELETE_DIRECTIVES)) {
+    DVLOG(1) << "[GetDisabledReason] History sync disabled";
+    return DisabledReason::HISTORY_SYNC_DISABLED;
+  }
+
+  DVLOG(1) << "[GetDisabledReason] Enabled";
+  return DisabledReason::NONE;
+}
+
 // static
 int NTPSnippetsService::GetMaxSnippetCountForTesting() {
   return kMaxSnippetCount;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Private methods
 
-// sync_driver::SyncServiceObserver implementation.
 void NTPSnippetsService::OnStateChanged() {
-  if (IsSyncStateIncompatible()) {
-    ClearSnippets();
-    FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
-                      NTPSnippetsServiceDisabled());
+  if (!initialized())
     return;
-  }
 
-  // TODO(dgn): When the data sources change, we may want to not fetch here,
-  // as we will get notified of changes from the snippet sources as well, and
-  // start multiple fetches.
-  FetchSnippets();
+  DVLOG(1) << "[OnStateChanged]";
+  EnterState(GetStateForDependenciesStatus());
 }
 
 void NTPSnippetsService::OnDatabaseLoaded(NTPSnippet::PtrVector snippets) {
-  DCHECK(state_ == State::INITED || state_ == State::SHUT_DOWN);
   if (state_ == State::SHUT_DOWN)
     return;
-  state_ = State::LOADED;
 
   DCHECK(snippets_.empty());
   DCHECK(discarded_snippets_.empty());
   for (std::unique_ptr<NTPSnippet>& snippet : snippets) {
     if (snippet->is_discarded())
       discarded_snippets_.emplace_back(std::move(snippet));
     else
       snippets_.emplace_back(std::move(snippet));
   }
   std::sort(snippets_.begin(), snippets_.end(),
             [](const std::unique_ptr<NTPSnippet>& lhs,
                const std::unique_ptr<NTPSnippet>& rhs) {
               return lhs->score() > rhs->score();
             });
-  LoadingSnippetsFinished();
 
-  // 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)));
-  }
-
-  // Start a fetch if we don't have any snippets yet, or a fetch was requested
-  // earlier.
-  if (snippets_.empty() || fetch_after_load_) {
-    fetch_after_load_ = false;
-    FetchSnippets();
-  }
+  EnterState(GetStateForDependenciesStatus());
 }
 
 void NTPSnippetsService::OnSuggestionsChanged(
     const SuggestionsProfile& suggestions) {
-  DCHECK(loaded());
+  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_->Delete(to_delete);
 
   StoreSnippetHostsToPrefs(hosts);
 
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceLoaded());
 
   FetchSnippetsFromHosts(hosts);
 }
 
 void NTPSnippetsService::OnFetchFinished(
     NTPSnippetsFetcher::OptionalSnippets snippets) {
-  if (!loaded())
+  if (!ready())
     return;
 
   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));
   }
   LoadingSnippetsFinished();
 }
 
 void NTPSnippetsService::MergeSnippets(NTPSnippet::PtrVector new_snippets) {
-  DCHECK(loaded());
+  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(
       std::remove_if(
           new_snippets.begin(), new_snippets.end(),
           [&old_snippet_ids](const std::unique_ptr<NTPSnippet>& snippet) {
             if (old_snippet_ids.count(snippet->id()))
@@ skipping 62 matching lines @@
 
 void NTPSnippetsService::StoreSnippetHostsToPrefs(
     const std::set<std::string>& hosts) {
   base::ListValue list;
   for (const std::string& host : hosts)
     list.AppendString(host);
   pref_service_->Set(prefs::kSnippetHosts, list);
 }
 
 void NTPSnippetsService::LoadingSnippetsFinished() {
-  DCHECK(loaded());
+  DCHECK(ready());
 
   // Remove expired snippets.
   base::Time expiry = base::Time::Now();
 
   // Move expired snippets over into |to_delete|.
   NTPSnippet::PtrVector to_delete;
   for (std::unique_ptr<NTPSnippet>& snippet : snippets_) {
     if (snippet->expiry_date() <= expiry)
       to_delete.emplace_back(std::move(snippet));
   }
@@ skipping 41 matching lines @@
   for (const auto& snippet : discarded_snippets_) {
     if (snippet->expiry_date() < next_expiry)
       next_expiry = snippet->expiry_date();
   }
   DCHECK_GT(next_expiry, expiry);
   expiry_timer_.Start(FROM_HERE, next_expiry - expiry,
                       base::Bind(&NTPSnippetsService::LoadingSnippetsFinished,
                                  base::Unretained(this)));
 }
 
-bool NTPSnippetsService::IsSyncStateIncompatible() {
-  if (!sync_service_ || !sync_service_->CanSyncStart())
-    return true;
-  if (!sync_service_->IsSyncActive() || !sync_service_->ConfigurationDone())
-    return false;  // Sync service is not initialized, yet not disabled.
-  return !sync_service_->GetActiveDataTypes().Has(
-      syncer::HISTORY_DELETE_DIRECTIVES);
+void NTPSnippetsService::Enable() {
+  // Start a fetch if we don't have any snippets yet, or a fetch was requested
+  // earlier.
+  if (snippets_.empty() || fetch_after_load_) {
+    fetch_after_load_ = false;
+    FetchSnippets();
+  }
+
+  // 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();
+}
+
+void NTPSnippetsService::Disable() {
+  // TODO(dgn) should we also clear the discarded snippets?
+  ClearSnippets();
+
+  suggestions_service_subscription_.reset();
+  expiry_timer_.Stop();
+
+  RescheduleFetching();
+  FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
+                    NTPSnippetsServiceDisabled());
+}
+
+NTPSnippetsService::State NTPSnippetsService::GetStateForDependenciesStatus() {
+  DisabledReason dr = GetDisabledReason();
+
+  if (dr == DisabledReason::NONE)
+    return State::READY;
+
+  // HistorySync is not initialized yet, so we don't know what the actual state
+  // is. However, because we retreived the previous snippets from the database,
+  // if we got something, we know that the service was previously enabled, so
+  // we just restore that state. If things changed, |OnStateChanged| will call
+  // this function again to fix the state.
+  if (dr == DisabledReason::HISTORY_SYNC_STATE_UNKNOWN) {
+    DVLOG(1) << "Sync configuration not done, continuing based on the current "
+                "state.";
+    return snippets_.empty() ? State::DISABLED : State::READY;
+  }
+
+  return State::DISABLED;
+}
+
+void NTPSnippetsService::EnterState(State state) {
+  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::READY ||
+             state_ == State::DISABLED);
+      if (state_ == State::READY)
+        return;
+
+      bool is_first_init = state_ == State::NOT_INITED;
+      DVLOG(1) << "Entering state: READY";
+      state_ = State::READY;
+
+      // At the first initialization, we loaded snippets from the database.
+      // We should notify listeners about them. We do so here because we have
+      // to be in the READY state.
+      if (is_first_init)
+        LoadingSnippetsFinished();
+
+      Enable();
+      return;
+    }
+
+    case State::DISABLED:
+      DCHECK(state_ == State::NOT_INITED || state_ == State::READY ||
+             state_ == State::DISABLED);
+      if (state_ == State::DISABLED)
+        return;
+
+      DVLOG(1) << "Entering state: DISABLED";
+      state_ = State::DISABLED;
+      Disable();
+      return;
+
+    case State::SHUT_DOWN:
+      DCHECK(state_ == State::NOT_INITED || state_ == State::DISABLED ||
+             state_ == State::READY);
+      DVLOG(1) << "Entering state: SHUT_DOWN";
+      state_ = State::SHUT_DOWN;
+      return;
+  }
 }
 
 void NTPSnippetsService::ClearDeprecatedPrefs() {
   pref_service_->ClearPref(prefs::kDeprecatedSnippets);
   pref_service_->ClearPref(prefs::kDeprecatedDiscardedSnippets);
 }
 
 }  // namespace ntp_snippets