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

components/ntp_snippets/ntp_snippets_service.h

 // 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.
 
 #ifndef COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_SERVICE_H_
 #define COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_SERVICE_H_
 
 #include <stddef.h>
 
 #include <memory>
@@ skipping 31 matching lines @@
 namespace suggestions {
 class SuggestionsProfile;
 }
 
 namespace sync_driver {
 class SyncService;
 }
 
 namespace ntp_snippets {
 
+enum class DisabledReason {
+  // Snippets are enabled
+  NONE,
+  // Snippets have been disabled as part of the service configuration.
+  EXPLICITLY_DISABLED,
+  // History sync is not enabled, and the service requires it to be enabled.
+  HISTORY_SYNC_DISABLED,
+  // The sync service is not completely initialized, and the status is unknown.
+  HISTORY_SYNC_STATE_UNKNOWN
+};
+
 class NTPSnippetsDatabase;
 class NTPSnippetsServiceObserver;
 
 // Stores and vends fresh content data for the NTP.
 class NTPSnippetsService : public KeyedService,
                            public sync_driver::SyncServiceObserver {
  public:
   using ImageFetchedCallback =
       base::Callback<void(const std::string& snippet_id, const gfx::Image&)>;
 
@@ skipping 10 matching lines @@
                      std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher,
                      std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher,
                      std::unique_ptr<NTPSnippetsDatabase> database);
   ~NTPSnippetsService() override;
 
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
   // Inherited from KeyedService.
   void Shutdown() override;
 
-  // Returns whether the initial set of snippets has been loaded from the
-  // database. While this is false, the list of snippets will be empty.
-  bool loaded() const { return state_ == State::LOADED; }
+  // Returns whether the service is ready. While this is false, the list of
+  // snippets will be empty, and all modifications to it (fetch, discard, etc)
+  // will be ignored.
+  bool ready() const { return state_ == State::READY; }
+
+  // Returns whether the service is initialized. While this is false, some
+  // calls may trigger DCHECKs.
+  bool initialized() const { return ready() || state_ == State::DISABLED; }
 
   // Fetches snippets from the server and adds them to the current ones.
   void FetchSnippets();
   // Fetches snippets from the server for specified hosts (overriding
   // suggestions from the suggestion service) and adds them to the current ones.
   // Only called from chrome://snippets-internals, DO NOT USE otherwise!
   // Ignored while |loaded()| is false.
   void FetchSnippetsFromHosts(const std::set<std::string>& hosts);
 
   // Available snippets.
@@ skipping 29 matching lines @@
   // Clears the lists of snippets previously discarded by the user.
   void ClearDiscardedSnippets();
 
   // Returns the lists of suggestion hosts the snippets are restricted to.
   std::set<std::string> GetSuggestionsHosts() const;
 
   // Observer accessors.
   void AddObserver(NTPSnippetsServiceObserver* observer);
   void RemoveObserver(NTPSnippetsServiceObserver* observer);
 
+  // Returns a reason why the service could be disabled, or DisabledReason::NONE
+  // if it's not.
+  DisabledReason GetDisabledReason() const;
+
   // Returns the maximum number of snippets that will be shown at once.
   static int GetMaxSnippetCountForTesting();
 
  private:
   FRIEND_TEST_ALL_PREFIXES(NTPSnippetsServiceWithSyncTest,
                            SyncStateCompatibility);
   FRIEND_TEST_ALL_PREFIXES(NTPSnippetsServiceWithSyncTest,
                            HistorySyncStateChanges);
 
+  // Possible state transitions:
+  //  +------- NOT_INITED ------+
+  //  |        /       \        |
+  //  |   READY <--> DISABLED <-+
+  //  |       \        /
+  //  +-----> SHUT_DOWN
+  enum class State {
+    // The service has just been created. Can change to states:
+    // - DISABLED: if the constructor was called with |enabled == false| . In
+    //             that case the service will stay disabled until it is shut
+    //             down. It can also enter this state after the database is
+    //             done loading and GetStateForDependenciesStatus identifies
+    //             the next state to be DISABLED.
+    // - READY: if GetStateForDependenciesStatus returns it, after the database
+    //          is done loading.
+    NOT_INITED,
+
+    // The service registered observers, timers, etc. and is ready to answer to
+    // queries, fetch snippets... Can change to states:
+    // - READY: noop

[Marc Treib, 2016/06/07 08:55:39]

nit: I'd remove the "noop" comments, IMO they don't add anything, and they're
not actually a *change*.

[dgn, 2016/06/07 11:22:47]

I added them since transitions from states not written down here hit DCHECKs and
crash, and in a case where it's supported, I suppose it's useful to know that we
don't go through the observer setup, fetch, etc. again.

[Marc Treib, 2016/06/07 11:50:37]

Acknowledged.

+    // - DISABLED: when the global Chrome state changes, for example after
+    //             |OnStateChanged| is called and sync is disabled.
+    // - SHUT_DOWN: when |Shutdown| is called, during the browser shutdown.
+    READY,
+
+    // The service is disabled and unregistered the related resources.
+    // Can change to states:
+    // - DISABLED: noop
+    // - READY: when the global Chrome state changes, for example after
+    //          |OnStateChanged| is called and sync is enabled.
+    // - SHUT_DOWN: when |Shutdown| is called, during the browser shutdown.
+    DISABLED,
+
+    // The service shutdown and can't be used anymore. This state is checked
+    // for early exit in callbacks from observers.
+    SHUT_DOWN
+  };
+
   // sync_driver::SyncServiceObserver implementation.
   void OnStateChanged() override;
 
   // Callback for the NTPSnippetsDatabase.
   void OnDatabaseLoaded(NTPSnippet::PtrVector snippets);
 
   // Callback for the SuggestionsService.
   void OnSuggestionsChanged(const suggestions::SuggestionsProfile& suggestions);
 
   // Callback for the NTPSnippetsFetcher.
   void OnFetchFinished(NTPSnippetsFetcher::OptionalSnippets snippets);
 
   // Merges newly available snippets with the previously available list.
   void MergeSnippets(NTPSnippet::PtrVector new_snippets);
 
   std::set<std::string> GetSnippetHostsFromPrefs() const;
   void StoreSnippetHostsToPrefs(const std::set<std::string>& hosts);
 
   void LoadingSnippetsFinished();
 
-  // Checks whether the state of the sync service is incompatible with showing
-  // snippets. We need history sync to be active.
-  // Note: The state is considered compatible if the service is still
-  // initializing and the sync state is not known.
-  bool IsSyncStateIncompatible();
+  // Returns whether the service should be enabled or disable depending on its
+  // internal state and the state of its dependencies.
+  State GetStateForDependenciesStatus();
+
+  // Verifies state transitions (see |State|'s documentation) and applies them.
+  void EnterState(State state);
+
+  // Enables the service and triggers a fetch if required. Do not call directly,
+  // use |EnterState| instead.
+  void EnterStateEnabled(bool fetch_snippets);
+
+  // Disables the service. Do not call directly, use |EnterState| instead.
+  void EnterStateDisabled();
+
+  // Applies the effects of the transition to the SHUT_DOWN state. Do not call
+  // directly, use |EnterState| instead.
+  void EnterStateShutdown();
 
   void ClearDeprecatedPrefs();
 
-  enum class State {
-    INITED,    // Initial state before the DB has been loaded; no snippets yet.
-    LOADED,    // DB has been loaded and the service is ready for action.
-    SHUT_DOWN  // Shutdown has been called, service is inactive.
-  } state_;
+  State state_;
 
-  // When |enabled_| is true the service will fetch snippets from the server
-  // using |snippets_fetcher_|, load snippets from device storage, and schedule
-  // the |scheduler_|.
-  bool enabled_;
+  // The service was set up to be disabled and should not be enabled by any
+  // state change. We track this to make sure we clear scheduled tasks, which
+  // persist even when Chrome is stopped.
+  bool explicitly_disabled_;
 
   PrefService* pref_service_;
 
   sync_driver::SyncService* sync_service_;
 
   // The observer for the SyncService. When the sync state changes,
   // SyncService will call |OnStateChanged|, which is propagated to the
   // snippet observers.
   ScopedObserver<sync_driver::SyncService, sync_driver::SyncServiceObserver>
       sync_service_observer_;
@@ skipping 51 matching lines @@
   // or because a requirement (e.g. History Sync) is not fulfilled anymore.
   virtual void NTPSnippetsServiceDisabled() = 0;
 
  protected:
   virtual ~NTPSnippetsServiceObserver() {}
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_SERVICE_H_