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

components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc

Index: components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc
diff --git a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc
index 0acb45cd640d47245145ae045a1a632d4fd392ad..8485c672027e71c1a61080c4146e2a8eaf311c39 100644
--- a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc
+++ b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc
@@ -69,20 +69,14 @@ class MockRemoteSuggestionsProvider : public RemoteSuggestionsProvider {
   MockRemoteSuggestionsProvider(Observer* observer)
       : RemoteSuggestionsProvider(observer) {}
 
+  MOCK_METHOD1(SetRemoteSuggestionsScheduler,
+               void(RemoteSuggestionsScheduler*));
+
   // Move-only params are not supported by GMock. We want to mock out
   // RefetchInTheBackground() which takes a unique_ptr<>. Instead, we add a new
   // mock function which takes a copy of the callback and override the
   // RemoteSuggestionsProvider's method to forward the call into the new mock
   // function.
-  void SetProviderStatusCallback(
-      std::unique_ptr<RemoteSuggestionsProvider::ProviderStatusCallback>
-          callback) override {
-    SetProviderStatusCallback(*callback);
-  }
-  MOCK_METHOD1(SetProviderStatusCallback,
-               void(RemoteSuggestionsProvider::ProviderStatusCallback));
-
-  // Move-only params are not supported by GMock (same work-around as above).
   void RefetchInTheBackground(
       std::unique_ptr<RemoteSuggestionsProvider::FetchStatusCallback> callback)
       override {
@@ -141,10 +135,10 @@ class SchedulingRemoteSuggestionsProviderTest
             /*observer=*/nullptr);
     underlying_provider_ = underlying_provider.get();
 
-    // SchedulingRemoteSuggestionsProvider calls SetProviderStatusCallback(_) to
-    // stay in the loop of status changes.
-    EXPECT_CALL(*underlying_provider_, SetProviderStatusCallback(_))
-        .WillOnce(SaveArg<0>(&provider_status_callback_));
+    // SchedulingRemoteSuggestionsProvider calls
+    // SetRemoteSuggestionsScheduler(_) to stay in the loop of status changes.
+    EXPECT_CALL(*underlying_provider_, SetRemoteSuggestionsScheduler(_))
+        .WillOnce(SaveArg<0>(&remote_suggestions_scheduler_));
 
     auto test_clock = base::MakeUnique<base::SimpleTestClock>();
     test_clock_ = test_clock.get();
@@ -174,12 +168,15 @@ class SchedulingRemoteSuggestionsProviderTest
   StrictMock<MockPersistentScheduler> persistent_scheduler_;
   StrictMock<MockRemoteSuggestionsProvider>* underlying_provider_;
   std::unique_ptr<SchedulingRemoteSuggestionsProvider> scheduling_provider_;
-  RemoteSuggestionsProvider::ProviderStatusCallback provider_status_callback_;
+  RemoteSuggestionsScheduler* remote_suggestions_scheduler_;
   base::SimpleTestClock* test_clock_;
 
-  void ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus new_status) {
-    provider_status_callback_.Run(new_status);
+  void ActivateUnderlyingProvider() {
+    remote_suggestions_scheduler_->OnProviderActivated();
+  }
+
+  void InactivateUnderlyingProvider() {
+    remote_suggestions_scheduler_->OnProviderInactivated();
   }
 
  private:
@@ -205,8 +202,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   scheduling_provider_->OnPersistentSchedulerWakeUp();
   scheduling_provider_->OnNTPOpened();
@@ -223,8 +219,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   // For instance, persistent scheduler wake up should be enabled by default.
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
@@ -239,8 +234,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   // For instance, persistent scheduler wake up should be enabled by default.
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
@@ -256,8 +250,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   scheduling_provider_->OnPersistentSchedulerWakeUp();
@@ -275,8 +268,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   }
   // First enable the scheduler -- calling Schedule() for the first time.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
   // Make the first persistent fetch successful -- calling Schedule() again.
   scheduling_provider_->OnPersistentSchedulerWakeUp();
   signal_fetch_done.Run(Status::Success());
@@ -293,8 +285,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
     // RefetchInTheBackground is not called after the second trigger.
   }
   // First enable the scheduler -- calling Schedule() for the first time.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
   // Make the first persistent fetch never finish.
   scheduling_provider_->OnPersistentSchedulerWakeUp();
   // Make the second fetch.
@@ -309,8 +300,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   scheduling_provider_->OnNTPOpened();
@@ -324,8 +314,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   scheduling_provider_->OnBrowserForegrounded();
@@ -339,8 +328,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   scheduling_provider_->OnBrowserColdStart();
@@ -351,8 +339,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
   // First enable the scheduler; the second Schedule is called after the
   // successful fetch.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _)).Times(2);
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   // Make the first soft fetch successful.
   RemoteSuggestionsProvider::FetchStatusCallback signal_fetch_done;
@@ -369,8 +356,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
   // First enable the scheduler; the second Schedule is called after the
   // successful fetch.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _)).Times(2);
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   // Make the first persistent fetch successful.
   RemoteSuggestionsProvider::FetchStatusCallback signal_fetch_done;
@@ -386,8 +372,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldNotFetchOnNTPOpenedAfterFailedSoftFetch) {
   // First enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   // Make the first soft fetch failed.
   RemoteSuggestionsProvider::FetchStatusCallback signal_fetch_done;
@@ -404,8 +389,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldNotFetchOnNTPOpenedAfterFailedPersistentFetch) {
   // First enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   // Make the first persistent fetch failed.
   RemoteSuggestionsProvider::FetchStatusCallback signal_fetch_done;
@@ -435,8 +419,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
   }
 
   // First enable the scheduler.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
   // Make the first soft fetch successful.
   scheduling_provider_->OnBrowserForegrounded();
   signal_fetch_done.Run(Status::Success());
@@ -453,8 +436,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest, ShouldScheduleOnActivation) {
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
@@ -464,28 +446,23 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     EXPECT_CALL(persistent_scheduler_, Unschedule());
   }
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::INACTIVE);
+  ActivateUnderlyingProvider();
+  InactivateUnderlyingProvider();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldScheduleOnLaterActivation) {
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
   // There is no schedule yet, so inactivation does not trigger unschedule.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::INACTIVE);
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  InactivateUnderlyingProvider();
+  ActivateUnderlyingProvider();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldRescheduleAfterSuccessfulFetch) {
   // First reschedule on becoming active.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _)).Times(2);
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   RemoteSuggestionsProvider::FetchStatusCallback signal_fetch_done;
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
@@ -501,8 +478,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldNotRescheduleAfterFailedFetch) {
   // Only reschedule on becoming active.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   RemoteSuggestionsProvider::FetchStatusCallback signal_fetch_done;
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
@@ -516,11 +492,9 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest, ShouldScheduleOnlyOnce) {
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
   // No further call to Schedule on a second status callback.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest, ShouldUnscheduleOnlyOnce) {
@@ -530,35 +504,29 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest, ShouldUnscheduleOnlyOnce) {
     EXPECT_CALL(persistent_scheduler_, Unschedule());
   }
   // First schedule so that later we really unschedule.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::INACTIVE);
+  ActivateUnderlyingProvider();
+  InactivateUnderlyingProvider();
   // No further call to Unschedule on second status callback.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::INACTIVE);
+  InactivateUnderlyingProvider();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ReschedulesWhenWifiParamChanges) {
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _)).Times(2);
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   // UserClassifier defaults to UserClass::ACTIVE_NTP_USER if PrefService is
   // null. Change the wifi interval for this class.
   SetVariationParameter("fetching_interval_hours-wifi-active_ntp_user", "1.5");
 
   // Schedule() should get called for the second time after params have changed.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ReschedulesWhenFallbackParamChanges) {
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _)).Times(2);
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   // UserClassifier defaults to UserClass::ACTIVE_NTP_USER if PrefService is
   // null. Change the fallback interval for this class.
@@ -566,15 +534,13 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
                         "1.5");
 
   // Schedule() should get called for the second time after params have changed.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ReschedulesWhenOnUsageEventParamChanges) {
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _)).Times(2);
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   // UserClassifier defaults to UserClass::ACTIVE_NTP_USER if PrefService is
   // null. Change the on usage interval for this class.
@@ -582,15 +548,13 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
                         "1.5");
 
   // Schedule() should get called for the second time after params have changed.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ReschedulesWhenOnNtpOpenedParamChanges) {
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _)).Times(2);
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   // UserClassifier defaults to UserClass::ACTIVE_NTP_USER if PrefService is
   // null. Change the fallback interval for this class.
@@ -598,8 +562,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
                         "1.5");
 
   // Schedule() should get called for the second time after params have changed.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
@@ -618,8 +581,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   }
 
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   scheduling_provider_->OnNTPOpened();
   signal_fetch_done.Run(Status::Success());
@@ -658,8 +620,7 @@ TEST_F(SchedulingRemoteSuggestionsProviderTest,
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   }
 
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   scheduling_provider_->OnNTPOpened();
   signal_fetch_done.Run(Status::Success());