[Remote suggestions] Move all decisions to fetch to the scheduler

components/ntp_snippets/remote/remote_suggestions_provider.h

Index: components/ntp_snippets/remote/remote_suggestions_provider.h
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider.h b/components/ntp_snippets/remote/remote_suggestions_provider.h
index c9f58c45fb10639a62bdddc4a7131906f75552a7..e94d5e6fc26abfb4990bc7c17767e1e35d9130cb 100644
--- a/components/ntp_snippets/remote/remote_suggestions_provider.h
+++ b/components/ntp_snippets/remote/remote_suggestions_provider.h
@@ -14,19 +14,12 @@
 namespace ntp_snippets {
 
 class RemoteSuggestionsFetcher;
+class RemoteSuggestionsScheduler;
 
 // Retrieves fresh content data (articles) from the server, stores them and
 // provides them as content suggestions.
 class RemoteSuggestionsProvider : public ContentSuggestionsProvider {
  public:
-  // TODO(jkrcal): Would be nice to get rid of this another level of statuses.
-  // Maybe possible while refactoring the RemoteSuggestionsStatusService? (and
-  // letting it notify both the SchedulingRemoteSuggestionsProvider and
-  // RemoteSuggestionsProviderImpl or just the scheduling one).
-  enum class ProviderStatus { ACTIVE, INACTIVE };
-  using ProviderStatusCallback =
-      base::RepeatingCallback<void(ProviderStatus status)>;
-
   // Callback to notify with the result of a fetch.
   // TODO(jkrcal): Change to OnceCallback? A OnceCallback does only have a
   // move-constructor which seems problematic for google mock.
@@ -34,13 +27,12 @@ class RemoteSuggestionsProvider : public ContentSuggestionsProvider {
 
   ~RemoteSuggestionsProvider() override;
 
-  // Set a callback to be notified whenever the status of the provider changes.
-  // The initial change is also notified (switching from an initial undecided
-  // status). If the callback is set after the first change, it is called back
-  // immediately.
-  // TODO(treib): Get rid of unique_ptrs to callbacks.
-  virtual void SetProviderStatusCallback(
-      std::unique_ptr<ProviderStatusCallback> callback) = 0;
+  // Set the scheduler to be notified whenever the provider becomes active /
+  // in-active and whenever history is deleted. The initial change is also
+  // notified (switching from an initial undecided status). If the scheduler is
+  // set after the first change, it is called back immediately.
+  virtual void SetRemoteSuggestionsScheduler(
+      RemoteSuggestionsScheduler* scheduler) = 0;

[tschumann, 2017/02/21 11:57:32]

I'm not a fan of this cyclic dependency -- it feels weird that the provider
could schedule calls for itself.

Also, the new methods added to the RemoteSuggestionsScheduler could make a nice
separate interface.

Adding another Observer interface to the RemoteSuggestionsProvider however seems
also odd because we already have an observer class on it's base class...

Maybe it's best for now to go with your plan :-)

[jkrcal, 2017/02/21 15:19:00]

I am also not super happy about that. However, some generic Observer interface
would obfuscate the meaning of these functions, IMO, as they are really targeted
at someone who cares about when to fetch and when not to fetch - the scheduler.

Thus (and for the reason you mention), I prefer to stick with the current
design.

 
   // Fetches suggestions from the server for all remote categories and replaces
   // old suggestions by the new ones. The request to the server is performed as