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

components/ntp_snippets/ntp_snippets_service.h

Index: components/ntp_snippets/ntp_snippets_service.h
diff --git a/components/ntp_snippets/ntp_snippets_service.h b/components/ntp_snippets/ntp_snippets_service.h
index e60bfa2b54936cce777eaadb2aea09fca2269a96..63ee6456b707eb2ff6ab3c72895110d18129db3a 100644
--- a/components/ntp_snippets/ntp_snippets_service.h
+++ b/components/ntp_snippets/ntp_snippets_service.h
@@ -49,6 +49,15 @@ 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
+};
+
 class NTPSnippetsDatabase;
 class NTPSnippetsServiceObserver;
 
@@ -131,6 +140,10 @@ class NTPSnippetsService : public KeyedService,
   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();
+
   // Returns the maximum number of snippets that will be shown at once.
   static int GetMaxSnippetCountForTesting();
 
@@ -140,6 +153,23 @@ class NTPSnippetsService : public KeyedService,
   FRIEND_TEST_ALL_PREFIXES(NTPSnippetsServiceWithSyncTest,
                            HistorySyncStateChanges);
 
+  // Possible state transitions:
+  //           NOT_INITED ---------------+
+  //               v                     |
+  //  +--------- INITED   <------+       |
+  //  |            v             |       |
+  //  |     LOADED = READY -> DISABLED <-+
+  //  |            v             |
+  //  +------> SHUT_DOWN  <------+
+  enum class State {
+    NOT_INITED,
+    INITED,
+    LOADED,
+    READY = LOADED,

[Marc Treib, 2016/05/24 09:04:46]

Why READY and LOADED? Can we stick with just one? (READY is probably a better
name than LOADED)

[dgn, 2016/05/24 18:13:30]

That was in case we have more intermediate states, to have the reference to the
READY state be able to easily move. Removed that.

+    DISABLED,
+    SHUT_DOWN
+  };
+
   // sync_driver::SyncServiceObserver implementation.
   void OnStateChanged() override;
 
@@ -160,20 +190,21 @@ class NTPSnippetsService : public KeyedService,
 
   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();
+  // Enable the service. Do not call directly, use |EnterState| instead.
+  void Enable();
 
-  enum class State {
-    NOT_INITED,
-    INITED,
-    LOADED,
-    SHUT_DOWN
-  } state_;
+  // Disable the service. Do not call directly, use |EnterState| instead.
+  void Disable();
+
+  // Applies state transitions (see |State|'s documentation )and verifies them.
+  void EnterState(State state);
+
+  State state_;
 
-  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_;