Combine all suggestions factories into ContentSuggestionsServiceFactory

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 a2245e5c6446f6584ea562bfd0e6849db4a197eb..72d470b48fcf543433ac652235c0d7de81a8ff0a 100644
--- a/components/ntp_snippets/ntp_snippets_service.h
+++ b/components/ntp_snippets/ntp_snippets_service.h
@@ -15,10 +15,8 @@
 #include "base/callback_forward.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
-#include "base/observer_list.h"
 #include "base/timer/timer.h"
 #include "components/image_fetcher/image_fetcher_delegate.h"
-#include "components/keyed_service/core/keyed_service.h"
 #include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/category_factory.h"
 #include "components/ntp_snippets/category_status.h"
@@ -66,18 +64,17 @@ class NTPSnippetsServiceObserver;
 // provides them as content suggestions.
 // TODO(pke): Rename this service to ArticleSuggestionsProvider and move to
 // a subdirectory.
-class NTPSnippetsService : public KeyedService,
-                           public image_fetcher::ImageFetcherDelegate,
+class NTPSnippetsService : public image_fetcher::ImageFetcherDelegate,
                            public ContentSuggestionsProvider {
  public:
   // |application_language_code| should be a ISO 639-1 compliant string, e.g.
   // 'en' or 'en-US'. Note that this code should only specify the language, not
   // the locale, so 'en_US' (English language with US locale) and 'en-GB_US'
   // (British English person in the US) are not language codes.
-  NTPSnippetsService(bool enabled,
+  NTPSnippetsService(Observer* observer,
+                     CategoryFactory* category_factory,
                      PrefService* pref_service,
                      suggestions::SuggestionsService* suggestions_service,
-                     CategoryFactory* category_factory,
                      const std::string& application_language_code,
                      NTPSnippetsScheduler* scheduler,
                      std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher,
@@ -90,9 +87,6 @@ class NTPSnippetsService : public KeyedService,
 
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
-  // Inherited from KeyedService.
-  void Shutdown() override;
-
   // Returns whether the service is ready. While this is false, the list of
   // snippets will be empty, and all modifications to it (fetch, dismiss, etc)
   // will be ignored.
@@ -142,7 +136,6 @@ class NTPSnippetsService : public KeyedService,
   // TODO(pke): At some point reorder the implementations in the .cc file
   // accordingly.
   std::vector<Category> GetProvidedCategories() override;
-  void SetObserver(Observer* observer) override;
   CategoryStatus GetCategoryStatus(Category category) override;
   void DismissSuggestion(const std::string& suggestion_id) override;
   void FetchSuggestionImage(const std::string& suggestion_id,
@@ -153,10 +146,6 @@ class NTPSnippetsService : public KeyedService,
   // 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 the maximum number of snippets that will be shown at once.
   static int GetMaxSnippetCountForTesting();
 
@@ -164,15 +153,12 @@ class NTPSnippetsService : public KeyedService,
   friend class NTPSnippetsServiceTest;
   FRIEND_TEST_ALL_PREFIXES(NTPSnippetsServiceTest, StatusChanges);
 
-  // TODO(pke): As soon as the DisabledReason is replaced with the new status,
-  // also remove the old State enum and replace it with CategoryStatus and a
-  // similar status diagram.
   // Possible state transitions:
   //  +------- NOT_INITED ------+
-  //  |        /       \        |
-  //  |   READY <--> DISABLED <-+
-  //  |       \        /
-  //  +-----> SHUT_DOWN
+  //  |                         |
+  //  +-> READY <--> DISABLED <-+
+  //  |
+  //  +------->  ERROR

[Marc Treib, 2016/08/03 11:53:23]

This doesn't really express that any state can go to ERROR. How about:

//  +------- NOT_INITED
//  |        /       \
//  |   READY <--> DISABLED
//  |       \        /
//  +----->   ERROR

or maybe, if you want to express the direction of the downward arrows,

//  +------- NOT_INITED
//  |        /       \
//  |       v         v
//  |   READY <--> DISABLED
//  |       \        /
//  |        v      v
//  +----->   ERROR

WDYT?

[Philipp Keck, 2016/08/03 12:28:25]

Right. I missed that OnDatabaseError can occur at any time.
+1 for advanced ASCII art

   enum class State {
     // The service has just been created. Can change to states:
     // - DISABLED: if the constructor was called with |enabled == false| . In

[Marc Treib, 2016/08/03 11:53:23]

The enabled param doesn't exist anymore

[Philipp Keck, 2016/08/03 12:28:25]

Done.

@@ -182,25 +168,26 @@ class NTPSnippetsService : public KeyedService,
     //             the next state to be DISABLED.
     // - READY: if GetStateForDependenciesStatus returns it, after the database
     //          is done loading.
+    // - ERROR: when an unrecoverable error occurred.
     NOT_INITED,
 
     // The service registered observers, timers, etc. and is ready to answer to
     // queries, fetch snippets... Can change to states:
     // - 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.
+    // - ERROR: when an unrecoverable error occurred.
     READY,
 
     // The service is disabled and unregistered the related resources.
     // Can change to states:
     // - 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.
+    // - ERROR: when an unrecoverable error occurred.
     DISABLED,
 
     // The service shutdown and can't be used anymore. This state is checked
     // for early exit in callbacks from observers.
-    SHUT_DOWN
+    ERROR
   };
 
   // image_fetcher::ImageFetcherDelegate implementation.
@@ -259,9 +246,9 @@ class NTPSnippetsService : public KeyedService,
   // 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();
+  // Disables the service permanently because an unrecoverable error occurred.
+  // Do not call directly, use |EnterState| instead.
+  void EnterStateError();
 
   // Converts the cached snippets to article content suggestions and notifies
   // the observers.
@@ -289,13 +276,6 @@ class NTPSnippetsService : public KeyedService,
   // The ISO 639-1 code of the language used by the application.
   const std::string application_language_code_;
 
-  // The observers.
-  // TODO(pke): This is an old kind of observers that will be removed.
-  base::ObserverList<NTPSnippetsServiceObserver> observers_;
-
-  // The observer to deliver data to.
-  Observer* observer_;
-
   // Scheduler for fetching snippets. Not owned.
   NTPSnippetsScheduler* scheduler_;
 
@@ -330,25 +310,6 @@ class NTPSnippetsService : public KeyedService,
   DISALLOW_COPY_AND_ASSIGN(NTPSnippetsService);
 };
 
-// TODO(pke): Remove this when snippets internals don't access this service
-// directly anymore.
-class NTPSnippetsServiceObserver {
- public:
-  // Sent every time the service loads a new set of data.
-  virtual void NTPSnippetsServiceLoaded() = 0;
-
-  // Sent when the service is shutting down.
-  virtual void NTPSnippetsServiceShutdown() = 0;
-
-  // Sent when the state of the service is changing. Something changed in its
-  // dependencies so it's notifying observers about incoming data changes.
-  // If the service might be enabled, DisabledReason::NONE will be provided.
-  virtual void NTPSnippetsServiceDisabledReasonChanged(DisabledReason) = 0;
-
- protected:
-  virtual ~NTPSnippetsServiceObserver() {}
-};
-
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_SERVICE_H_