Combine all suggestions factories into ContentSuggestionsServiceFactory

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>
 #include <set>
 #include <string>
 #include <vector>
 
 #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"
 #include "components/ntp_snippets/content_suggestion.h"
 #include "components/ntp_snippets/content_suggestions_provider.h"
 #include "components/ntp_snippets/ntp_snippet.h"
 #include "components/ntp_snippets/ntp_snippets_fetcher.h"
 #include "components/ntp_snippets/ntp_snippets_scheduler.h"
 #include "components/ntp_snippets/ntp_snippets_status_service.h"
 #include "components/suggestions/suggestions_service.h"
@@ skipping 27 matching lines @@
 
 namespace ntp_snippets {
 
 class NTPSnippetsDatabase;
 class NTPSnippetsServiceObserver;
 
 // Retrieves fresh content data (articles) from the server, stores them and
 // 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,
                      std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher,
                      std::unique_ptr<image_fetcher::ImageDecoder> image_decoder,
                      std::unique_ptr<NTPSnippetsDatabase> database,
                      std::unique_ptr<NTPSnippetsStatusService> status_service);
 
   ~NTPSnippetsService() override;
 
   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.
   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.
@@ skipping 29 matching lines @@
   }
 
   // (Re)schedules the periodic fetching of snippets. This is necessary because
   // the schedule depends on the time of day.
   void RescheduleFetching();
 
   // ContentSuggestionsProvider implementation
   // 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,
                             const ImageFetchedCallback& callback) override;
   void ClearCachedSuggestionsForDebugging() override;
   void ClearDismissedSuggestionsForDebugging() override;
 
   // 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();
 
  private:
   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.

     //             that case the service will stay disabled until it is shut

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

I believe this is also outdated - it can get enabled at runtime

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

Done.

     //             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.
+    // - 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.
   void OnImageDataFetched(const std::string& snippet_id,
                           const std::string& image_data) override;
 
   // Callbacks for the NTPSnippetsDatabase.
   void OnDatabaseLoaded(NTPSnippet::PtrVector snippets);
   void OnDatabaseError();
 
@@ skipping 38 matching lines @@
   // status if called with the current state.
   void EnterState(State state, CategoryStatus status);
 
   // 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();
+  // 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.
   void NotifyNewSuggestions();
 
   // Updates the internal status |category_status_| and notifies the content
   // suggestions observer if it changed.
   void UpdateCategoryStatus(CategoryStatus status);
 
   State state_;
 
   CategoryStatus category_status_;
 
   PrefService* pref_service_;
 
   suggestions::SuggestionsService* suggestions_service_;
 
   // All current suggestions (i.e. not dismissed ones).
   NTPSnippet::PtrVector snippets_;
 
   // Suggestions that the user dismissed. We keep these around until they expire
   // so we won't re-add them on the next fetch.
   NTPSnippet::PtrVector dismissed_snippets_;
 
   // 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_;
 
   // The subscription to the SuggestionsService. When the suggestions change,
   // SuggestionsService will call |OnSuggestionsChanged|, which triggers an
   // update to the set of snippets.
   using SuggestionsSubscription =
       suggestions::SuggestionsService::ResponseCallbackList::Subscription;
   std::unique_ptr<SuggestionsSubscription> suggestions_service_subscription_;
 
@@ skipping 14 matching lines @@
 
   // Set to true if FetchSnippets is called before the database has been loaded.
   // The fetch will be executed after the database load finishes.
   bool fetch_after_load_;
 
   const Category provided_category_;
 
   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_