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

components/ntp_snippets/remote/scheduling_remote_suggestions_provider_unittest.cc

 // Copyright 2016 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.
 
 #include "components/ntp_snippets/remote/scheduling_remote_suggestions_provider.h"
 
 #include <memory>
 #include <set>
 #include <string>
 #include <utility>
@@ skipping 51 matching lines @@
                     base::TimeDelta period_fallback));
   MOCK_METHOD0(Unschedule, bool());
 };
 
 // TODO(jkrcal): Move into its own library to reuse in other unit-tests?
 class MockRemoteSuggestionsProvider : public RemoteSuggestionsProvider {
  public:
   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 {
     RefetchInTheBackground(*callback);
   }
   MOCK_METHOD1(RefetchInTheBackground,
                void(RemoteSuggestionsProvider::FetchStatusCallback));
 
   MOCK_CONST_METHOD0(suggestions_fetcher_for_debugging,
                      const RemoteSuggestionsFetcher*());
@@ skipping 38 matching lines @@
         utils_.pref_service()->registry());
     ResetProvider();
   }
 
   void ResetProvider() {
     auto underlying_provider =
         base::MakeUnique<StrictMock<MockRemoteSuggestionsProvider>>(
             /*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();
     test_clock_->SetNow(base::Time::Now());
 
     scheduling_provider_ =
         base::MakeUnique<SchedulingRemoteSuggestionsProvider>(
             /*observer=*/nullptr, std::move(underlying_provider),
             &persistent_scheduler_, &user_classifier_, utils_.pref_service(),
             std::move(test_clock));
   }
 
   void SetVariationParameter(const std::string& param_name,
                              const std::string& param_value) {
     std::map<std::string, std::string> params = default_variation_params_;
     params[param_name] = param_value;
 
     params_manager_.ClearAllVariationParams();
     params_manager_.SetVariationParamsWithFeatureAssociations(
         ntp_snippets::kStudyName, params,
         {ntp_snippets::kArticleSuggestionsFeature.name});
   }
 
  protected:
   std::map<std::string, std::string> default_variation_params_;
   variations::testing::VariationParamsManager params_manager_;
   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:
   test::RemoteSuggestionsTestUtils utils_;
   UserClassifier user_classifier_;
 
   DISALLOW_COPY_AND_ASSIGN(SchedulingRemoteSuggestionsProviderTest);
 };
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldIgnoreSignalsWhenNotEnabled) {
   scheduling_provider_->OnPersistentSchedulerWakeUp();
   scheduling_provider_->OnNTPOpened();
   scheduling_provider_->OnBrowserForegrounded();
   scheduling_provider_->OnBrowserColdStart();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldIgnoreSignalsWhenDisabledByParam) {
   // First set an empty list of allowed trigger types.
   SetVariationParameter("scheduler_trigger_types", "-");
   ResetProvider();
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   scheduling_provider_->OnPersistentSchedulerWakeUp();
   scheduling_provider_->OnNTPOpened();
   scheduling_provider_->OnBrowserForegrounded();
   scheduling_provider_->OnBrowserColdStart();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldHandleEmptyParamForTriggerTypes) {
   // First set an empty param for allowed trigger types -> should result in the
   // default list.
   SetVariationParameter("scheduler_trigger_types", "");
   ResetProvider();
 
   // 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(_));
   scheduling_provider_->OnPersistentSchedulerWakeUp();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldHandleIncorrentParamForTriggerTypes) {
   // First set an invalid list of allowed trigger types.
   SetVariationParameter("scheduler_trigger_types", "ntp_opened,foo;");
   ResetProvider();
 
   // 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(_));
   scheduling_provider_->OnPersistentSchedulerWakeUp();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldFetchOnPersistentSchedulerWakeUp) {
   // First set only this type to be allowed.
   SetVariationParameter("scheduler_trigger_types",
                         "persistent_scheduler_wake_up");
   ResetProvider();
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   scheduling_provider_->OnPersistentSchedulerWakeUp();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldFetchOnPersistentSchedulerWakeUpRepeated) {
   RemoteSuggestionsProvider::FetchStatusCallback signal_fetch_done;
   {
     InSequence s;
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
         .WillOnce(SaveArg<0>(&signal_fetch_done));
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     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());
   // Make the second fetch.
   scheduling_provider_->OnPersistentSchedulerWakeUp();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldNotTriggerBackgroundFetchIfAlreadyInProgess) {
   {
     InSequence s;
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
     // 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.
   scheduling_provider_->OnPersistentSchedulerWakeUp();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldFetchOnNTPOpenedForTheFirstTime) {
   // First set only this type to be allowed.
   SetVariationParameter("scheduler_trigger_types", "ntp_opened");
   ResetProvider();
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   scheduling_provider_->OnNTPOpened();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldFetchOnBrowserForegroundedForTheFirstTime) {
   // First set only this type to be allowed.
   SetVariationParameter("scheduler_trigger_types", "browser_foregrounded");
   ResetProvider();
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   scheduling_provider_->OnBrowserForegrounded();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldFetchOnBrowserColdStartForTheFirstTime) {
   // First set only this type to be allowed.
   SetVariationParameter("scheduler_trigger_types", "browser_cold_start");
   ResetProvider();
 
   // Then enable the scheduler.
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   scheduling_provider_->OnBrowserColdStart();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldNotFetchOnNTPOpenedAfterSuccessfulSoftFetch) {
   // 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;
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
       .WillOnce(SaveArg<0>(&signal_fetch_done));
   scheduling_provider_->OnNTPOpened();
   signal_fetch_done.Run(Status::Success());
   // The second call is ignored if it happens right after the first one.
   scheduling_provider_->OnNTPOpened();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldNotFetchOnNTPOpenedAfterSuccessfulPersistentFetch) {
   // 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;
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
       .WillOnce(SaveArg<0>(&signal_fetch_done));
   scheduling_provider_->OnPersistentSchedulerWakeUp();
   signal_fetch_done.Run(Status::Success());
   // The second call is ignored if it happens right after the first one.
   scheduling_provider_->OnNTPOpened();
 }
 
 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;
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
       .WillOnce(SaveArg<0>(&signal_fetch_done));
   scheduling_provider_->OnNTPOpened();
   signal_fetch_done.Run(Status(StatusCode::PERMANENT_ERROR, ""));
 
   // The second call is ignored if it happens right after the first one.
   scheduling_provider_->OnNTPOpened();
 }
 
 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;
   EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
       .WillOnce(SaveArg<0>(&signal_fetch_done));
   scheduling_provider_->OnPersistentSchedulerWakeUp();
   signal_fetch_done.Run(Status(StatusCode::PERMANENT_ERROR, ""));
 
   // The second call is ignored if it happens right after the first one.
   scheduling_provider_->OnNTPOpened();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldFetchAgainOnBrowserForgroundLaterAgain) {
   RemoteSuggestionsProvider::FetchStatusCallback signal_fetch_done;
   {
     InSequence s;
     // Initial scheduling after being enabled.
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     // The first call to NTPOpened results in a fetch.
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
         .WillOnce(SaveArg<0>(&signal_fetch_done));
     // Rescheduling after a succesful fetch.
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     // The second call to NTPOpened 2hrs later again results in a fetch.
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   }
 
   // First enable the scheduler.
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
   // Make the first soft fetch successful.
   scheduling_provider_->OnBrowserForegrounded();
   signal_fetch_done.Run(Status::Success());
   // Open NTP again after 2hrs.
   test_clock_->Advance(base::TimeDelta::FromHours(2));
   scheduling_provider_->OnBrowserForegrounded();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldRescheduleOnRescheduleFetching) {
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
   scheduling_provider_->RescheduleFetching();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest, ShouldScheduleOnActivation) {
   EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 }
 
 TEST_F(SchedulingRemoteSuggestionsProviderTest,
        ShouldUnscheduleOnLaterInactivation) {
   {
     InSequence s;
     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(_))
       .WillOnce(SaveArg<0>(&signal_fetch_done));
 
   // Trigger a fetch.
   scheduling_provider_->OnPersistentSchedulerWakeUp();
   // Second reschedule after a successful fetch.
   signal_fetch_done.Run(Status::Success());
 }
 
 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(_))
       .WillOnce(SaveArg<0>(&signal_fetch_done));
 
   // Trigger a fetch.
   scheduling_provider_->OnPersistentSchedulerWakeUp();
   // No furter reschedule after a failure.
   signal_fetch_done.Run(Status(StatusCode::PERMANENT_ERROR, ""));
 }
 
 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) {
   {
     InSequence s;
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     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.
   SetVariationParameter("fetching_interval_hours-fallback-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,
        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.
   SetVariationParameter("soft_fetching_interval_hours-active-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,
        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.
   SetVariationParameter("soft_on_ntp_opened_interval_hours-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,
        FetchIntervalForNtpOpenedTrigger) {
   RemoteSuggestionsProvider::FetchStatusCallback signal_fetch_done;
   {
     InSequence s;
     // Initial scheduling after being enabled.
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     // The first call to NTPOpened results in a fetch.
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
         .WillOnce(SaveArg<0>(&signal_fetch_done));
     // Rescheduling after a succesful fetch.
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     // The third call to NTPOpened 35min later again results in a fetch.
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   }
 
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   scheduling_provider_->OnNTPOpened();
   signal_fetch_done.Run(Status::Success());
 
   // UserClassifier defaults to UserClass::ACTIVE_NTP_USER which uses a 2h time
   // interval by default for soft backgroudn fetches on ntp open events.
 
   // Open NTP again after 20min. This time no fetch is executed.
   test_clock_->Advance(base::TimeDelta::FromMinutes(20));
   scheduling_provider_->OnNTPOpened();
@@ skipping 18 matching lines @@
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     // The first call to NTPOpened results in a fetch.
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
         .WillOnce(SaveArg<0>(&signal_fetch_done));
     // Rescheduling after a succesful fetch.
     EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
     // The third call to NTPOpened 35min later again results in a fetch.
     EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
   }
 
-  ChangeStatusOfUnderlyingProvider(
-      RemoteSuggestionsProvider::ProviderStatus::ACTIVE);
+  ActivateUnderlyingProvider();
 
   scheduling_provider_->OnNTPOpened();
   signal_fetch_done.Run(Status::Success());
 
   // Open NTP again after 20min. No fetch request is issues since the 30 min
   // time interval has not passed yet.
   test_clock_->Advance(base::TimeDelta::FromMinutes(20));
   scheduling_provider_->OnNTPOpened();
 
   // Open NTP again after 15min (35min since first opened)
   test_clock_->Advance(base::TimeDelta::FromMinutes(15));
   scheduling_provider_->OnNTPOpened();
 }
 
+TEST_F(SchedulingRemoteSuggestionsProviderTest,
+       ShouldBlockFetchingForSomeTimeAfterHistoryCleared) {
+  {
+    InSequence s;
+    // Initial scheduling after being enabled.
+    EXPECT_CALL(persistent_scheduler_, Schedule(_, _));

[tschumann, 2017/02/22 20:46:04]

i'd much rather setup the expectations where they should happen. This also
enforces an order but gives better context and makes reading the test easier
(it's usually also easier to maintain as expectations and trigger actions are
closer together).

// First enable the scheduler -- this will trigger a fetch.
EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
ActivateUnderlyingProvider();

// Clear the history.
scheduling_provider_->OnHistoryCleared();

// A trigger after 15 minutes is ignored.
test_clock_->Advance(base::TimeDelta::FromMinutes(15));
scheduling_provider_->OnBrowserForegrounded();

// A trigger after another 16 minutes is performed (more than 30m after
// clearing the history).
EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
test_clock_->Advance(base::TimeDelta::FromMinutes(16));
scheduling_provider_->OnBrowserForegrounded();

[jkrcal, 2017/02/23 12:20:13]

Done.

+    // The first call after 15m is ignored.
+    // The second call after 31m results in a fetch.
+    EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
+  }
+
+  // First enable the scheduler.
+  ActivateUnderlyingProvider();
+  // Clear the history.
+  scheduling_provider_->OnHistoryCleared();
+  // A trigger after 15 minutes is ignored.
+  test_clock_->Advance(base::TimeDelta::FromMinutes(15));
+  scheduling_provider_->OnBrowserForegrounded();
+  // A trigger after another 16 minutes is performed (more than 30m after
+  // clearing the history).
+  test_clock_->Advance(base::TimeDelta::FromMinutes(16));
+  scheduling_provider_->OnBrowserForegrounded();
+}
+
+TEST_F(SchedulingRemoteSuggestionsProviderTest,
+       ShouldAllowImmediateFetchingAfterSuggestionsObsoleted) {
+  RemoteSuggestionsProvider::FetchStatusCallback signal_fetch_done;
+  {
+    InSequence s;
+    // Initial scheduling after being enabled.
+    EXPECT_CALL(persistent_scheduler_, Schedule(_, _));

[tschumann, 2017/02/22 20:46:04]

same here. Consider moving the EXPECT_CALL statements where they get triggered.

[jkrcal, 2017/02/23 12:20:14]

Done.

+    // The first trigger results in a fetch.
+    EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_))
+        .WillOnce(SaveArg<0>(&signal_fetch_done));
+    // Rescheduling after a succesful fetch.
+    EXPECT_CALL(persistent_scheduler_, Schedule(_, _));
+    // The immediate trigger results in a fetch.
+    EXPECT_CALL(*underlying_provider_, RefetchInTheBackground(_));
+  }
+
+  // First enable the scheduler.
+  ActivateUnderlyingProvider();
+  // Make the first soft fetch successful.
+  scheduling_provider_->OnBrowserForegrounded();
+  signal_fetch_done.Run(Status::Success());
+
+  // Obsolete the suggestions.
+  scheduling_provider_->OnSuggestionsCleared();
+  // Another trigger right after results in a fetch again.
+  scheduling_provider_->OnBrowserForegrounded();
+}
+
 }  // namespace ntp_snippets