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

components/ntp_snippets/remote/remote_suggestions_provider_impl.h

Index: components/ntp_snippets/remote/remote_suggestions_provider_impl.h
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_impl.h b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
index 5fc9f77a9ebd8ccfe401e24fdeb321d0a2522c32..df9f67122c9bdba0c01fab3b4170d551cb6832c5 100644
--- a/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
+++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
@@ -47,6 +47,7 @@ namespace ntp_snippets {
 
 class CategoryRanker;
 class RemoteSuggestionsDatabase;
+class RemoteSuggestionsScheduler;
 
 // CachedImageFetcher takes care of fetching images from the network and caching
 // them in the database.
@@ -137,8 +138,8 @@ class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
   bool initialized() const { return ready() || state_ == State::DISABLED; }
 
   // RemoteSuggestionsProvider implementation.
-  void SetProviderStatusCallback(
-      std::unique_ptr<ProviderStatusCallback> callback) override;
+  void SetRemoteSuggestionsScheduler(
+      RemoteSuggestionsScheduler* scheduler) override;
   void RefetchInTheBackground(
       std::unique_ptr<FetchStatusCallback> callback) override;
 
@@ -196,21 +197,19 @@ class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
  private:
   friend class RemoteSuggestionsProviderImplTest;
 
+  // TODO(jkrcal): Mock the database to trigger the error naturally (or remote

[Marc Treib, 2017/02/23 13:52:24]

s/remote/remove/ ?

[jkrcal, 2017/02/24 09:21:43]

Done.

+  // the error state and get rid of the test).
   FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
-                           CallsProviderStatusCallbackWhenReady);
+                           CallsSchedulerOnError);
+  // TODO(jkrcal): Mock the status service and remove these friend declarations.
   FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
-                           CallsProviderStatusCallbackOnError);
-  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
-                           CallsProviderStatusCallbackWhenDisabled);
-  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
-                           ShouldNotCrashWhenCallingEmptyCallback);
+                           CallsSchedulerWhenDisabled);
   FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
                            DontNotifyIfNotAvailable);
   FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
-                           RemoveExpiredDismissedContent);
-  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest, StatusChanges);
+                           CallsSchedulerWhenSignedIn);
   FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
-                           SuggestionsFetchedOnSignInAndSignOut);
+                           CallsSchedulerWhenSignedOut);
 
   // Possible state transitions:
   //       NOT_INITED --------+
@@ -220,6 +219,8 @@ class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
   //       \       /          |
   //        v     v           |
   //     ERROR_OCCURRED <-----+
+  // TODO(jkrcal): Do we need to keep the distinction between states DISABLED
+  // and ERROR_OCCURED?
   enum class State {
     // The service has just been created. Can change to states:
     // - DISABLED: After the database is done loading,
@@ -352,6 +353,13 @@ class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
   // the file system.
   void ClearOrphanedImages();
 
+  // Clears suggestions because any history item has been removed.
+  void ClearAnyHistory();

[Marc Treib, 2017/02/23 13:52:24]

Should this be "AnyHistoryCleared"? It doesn't clear history itself; it's called
when a history clear event occurs (right?)
Or alternatively something like ClearHistoryDependentState? (with a less-awkward
name, if possible :D)

[jkrcal, 2017/02/24 09:21:43]

I prefer the active form, stick to ClearHistoryDependentState().

+
+  // Clears suggestions for any non-history related reason (e.g., sign-in status
+  // change, etc.).
+  void ClearSuggestions();
+
   // Clears all stored suggestions and updates the observer.
   void NukeAllSuggestions();
 
@@ -442,12 +450,12 @@ class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
   bool fetch_when_ready_interactive_;
   std::unique_ptr<FetchStatusCallback> fetch_when_ready_callback_;
 
-  std::unique_ptr<ProviderStatusCallback> provider_status_callback_;
+  RemoteSuggestionsScheduler* remote_suggestions_scheduler_;
 
-  // Set to true if NukeAllSuggestions is called while the service isn't ready.
+  // Set to true if ClearAnyHistory is called while the service isn't ready.
   // The nuke will be executed once the service finishes initialization or
   // enters the READY state.
-  bool nuke_when_initialized_;
+  bool clear_any_history_when_initialized_;
 
   // A clock for getting the time. This allows to inject a clock in tests.
   std::unique_ptr<base::Clock> clock_;