Change NTPSnippetsService to implement ContentSuggestionsProvider

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 5c0ed8c0a6a88c89120da5c94f692c040c1fea0d..96f74f95eb9544b6ad324fd1a00218ee43123d99 100644
--- a/components/ntp_snippets/ntp_snippets_service.h
+++ b/components/ntp_snippets/ntp_snippets_service.h
@@ -19,6 +19,8 @@
 #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/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"
@@ -59,10 +61,11 @@ class NTPSnippetsServiceObserver;
 
 // Stores and vends fresh content data for the NTP.
 class NTPSnippetsService : public KeyedService,
-                           public image_fetcher::ImageFetcherDelegate {
+                           public image_fetcher::ImageFetcherDelegate,
+                           public ContentSuggestionsProvider {
  public:
   using ImageFetchedCallback =
-      base::Callback<void(const std::string& snippet_id, const gfx::Image&)>;
+      base::Callback<void(const std::string& suggestion_id, const gfx::Image&)>;
 
   // |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
@@ -126,21 +129,17 @@ class NTPSnippetsService : public KeyedService,
   // the schedule depends on the time of day.
   void RescheduleFetching();
 
-  // Fetches the image for the snippet with the given |snippet_id| and runs the
-  // |callback|. If that snippet doesn't exist or the fetch fails, the callback
-  // gets a null image.
-  void FetchSnippetImage(const std::string& snippet_id,
-                         const ImageFetchedCallback& callback);
-
-  // Deletes all currently stored snippets.
-  void ClearSnippets();
-
-  // Discards the snippet with the given |snippet_id|, if it exists. Returns
-  // true iff a snippet was discarded.
-  bool DiscardSnippet(const std::string& snippet_id);
-
-  // Clears the lists of snippets previously discarded by the user.
-  void ClearDiscardedSnippets();
+  // ContentSuggestionsProvider implementation
+  // TODO(pke): At some point reorder the implementations in the .cc file
+  // accordingly.
+  void SetObserver(Observer* observer) override;
+  ContentSuggestionsCategoryStatus GetCategoryStatus(
+      ContentSuggestionsCategory category) override;
+  void DiscardSuggestion(const std::string& suggestion_id) override;
+  void FetchSuggestionImage(const std::string& suggestion_id,
+                            const ImageFetchedCallback& callback) override;
+  void ClearCachedSuggestionsForDebugging() override;
+  void ClearDiscardedSuggestionsForDebugging() override;
 
   // Returns the lists of suggestion hosts the snippets are restricted to.
   std::set<std::string> GetSuggestionsHosts() const;
@@ -254,8 +253,19 @@ class NTPSnippetsService : public KeyedService,
 
   void ClearDeprecatedPrefs();
 
+  // Converts the cached snippets to article content suggestions and notifies
+  // the observers.
+  void NotifyNewSuggestions();
+
+  // Notifies the content suggestions observer about a change in the
+  // |category_status_|.
+  void NotifyCategoryStatusChanged();
+
   State state_;
 
+  ContentSuggestionsCategoryStatus category_status_ =
+      ContentSuggestionsCategoryStatus::LOADING;

[Marc Treib, 2016/07/08 15:17:23]

Ah, I didn't know this style of initialization was allowed yet.
I'd prefer to keep all initialization in one place though, i.e. the constructor.
Maybe we can switch to this style at some point, but then we should switch all
other members (for which it makes sense) too.

[Philipp Keck, 2016/07/08 16:17:01]

Done.

+
   PrefService* pref_service_;
 
   suggestions::SuggestionsService* suggestions_service_;
@@ -271,8 +281,12 @@ class NTPSnippetsService : public KeyedService,
   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_ = nullptr;

[Marc Treib, 2016/07/08 15:17:23]

Same here.

[Philipp Keck, 2016/07/08 16:17:01]

Done.

+
   // Scheduler for fetching snippets. Not owned.
   NTPSnippetsScheduler* scheduler_;
 
@@ -305,9 +319,13 @@ 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.
+  // This is called in addition to the ContentSuggestionsProvider::Observer and
+  // should only be used by snippets-internals.
   virtual void NTPSnippetsServiceLoaded() = 0;
 
   // Sent when the service is shutting down.