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

components/ntp_snippets/remote/scheduling_remote_suggestions_provider.cc

Index: components/ntp_snippets/remote/scheduling_remote_suggestions_provider.cc
diff --git a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.cc b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.cc
index deb218a3197f5f1238b96408cbdfc9d0eb35c2ff..208fea63756d3821f2e006c7159f07b588e15234 100644
--- a/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.cc
+++ b/components/ntp_snippets/remote/scheduling_remote_suggestions_provider.cc
@@ -95,6 +95,8 @@ const char* kTriggerTypeNames[] = {"persistent_scheduler_wake_up", "ntp_opened",
 const char* kTriggerTypesParamName = "scheduler_trigger_types";
 const char* kTriggerTypesParamValueForEmptyList = "-";
 
+const int kBlockBackgroundFetchesMinutesAfterClearingHistory = 30;
+
 // Returns the time interval to use for scheduling remote suggestion fetches for
 // the given interval and user_class.
 base::TimeDelta GetDesiredFetchingInterval(
@@ -192,11 +194,7 @@ SchedulingRemoteSuggestionsProvider::SchedulingRemoteSuggestionsProvider(
 
   LoadLastFetchingSchedule();
 
-  provider_->SetProviderStatusCallback(
-      base::MakeUnique<RemoteSuggestionsProvider::ProviderStatusCallback>(
-          base::BindRepeating(
-              &SchedulingRemoteSuggestionsProvider::OnProviderStatusChanged,
-              base::Unretained(this))));
+  provider_->SetRemoteSuggestionsScheduler(this);
 }
 
 SchedulingRemoteSuggestionsProvider::~SchedulingRemoteSuggestionsProvider() =
@@ -215,6 +213,30 @@ void SchedulingRemoteSuggestionsProvider::RegisterProfilePrefs(
                               0);
 }
 
+void SchedulingRemoteSuggestionsProvider::OnProviderActivated() {
+  StartScheduling();
+}
+
+void SchedulingRemoteSuggestionsProvider::OnProviderInactivated() {
+  StopScheduling();
+}
+
+void SchedulingRemoteSuggestionsProvider::OnSuggestionsCleared() {
+  // There are no suggestions, be as eager to fetch as possible.
+  ClearLastFetchAttemptTime();
+}
+
+void SchedulingRemoteSuggestionsProvider::OnHistoryCleared() {
+  // Due to privacy, we should not fetch for a while (unless the user explicitly
+  // asks for new suggestions) do give sync the time to propagate the changes in
+  // history to the server.
+  background_fetches_allowed_after_ =
+      clock_->Now() + base::TimeDelta::FromMinutes(
+                          kBlockBackgroundFetchesMinutesAfterClearingHistory);
+  // After that time elapses, we should fetch as soon as possible.
+  ClearLastFetchAttemptTime();
+}
+
 void SchedulingRemoteSuggestionsProvider::RescheduleFetching() {
   // Force the reschedule by stopping and starting it again.
   StopScheduling();
@@ -253,9 +275,9 @@ void SchedulingRemoteSuggestionsProvider::OnNTPOpened() {
   RefetchInTheBackgroundIfEnabled(TriggerType::NTP_OPENED);
 }
 
-void SchedulingRemoteSuggestionsProvider::SetProviderStatusCallback(
-    std::unique_ptr<ProviderStatusCallback> callback) {
-  provider_->SetProviderStatusCallback(std::move(callback));
+void SchedulingRemoteSuggestionsProvider::SetRemoteSuggestionsScheduler(
+    RemoteSuggestionsScheduler* scheduler) {
+  provider_->SetRemoteSuggestionsScheduler(scheduler);
 }
 
 void SchedulingRemoteSuggestionsProvider::RefetchInTheBackground(
@@ -344,19 +366,6 @@ void SchedulingRemoteSuggestionsProvider::ClearDismissedSuggestionsForDebugging(
   provider_->ClearDismissedSuggestionsForDebugging(category);
 }
 
-void SchedulingRemoteSuggestionsProvider::OnProviderStatusChanged(
-    RemoteSuggestionsProvider::ProviderStatus status) {
-  switch (status) {
-    case RemoteSuggestionsProvider::ProviderStatus::ACTIVE:
-      StartScheduling();
-      return;
-    case RemoteSuggestionsProvider::ProviderStatus::INACTIVE:
-      StopScheduling();
-      return;
-  }
-  NOTREACHED();
-}
-
 void SchedulingRemoteSuggestionsProvider::StartScheduling() {
   FetchingSchedule new_schedule = GetDesiredFetchingSchedule();
 
@@ -368,6 +377,7 @@ void SchedulingRemoteSuggestionsProvider::StartScheduling() {
   schedule_ = new_schedule;
   StoreFetchingSchedule();
   ApplyPersistentFetchingSchedule();
+  ClearLastFetchAttemptTime();

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

This change is unexpected. Why is this needed now? Or was this a bug?

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

Not a bug, not strictly needed. Okay, I remove it as the improvement is arguable
;)

 }
 
 void SchedulingRemoteSuggestionsProvider::StopScheduling() {
@@ -469,7 +479,10 @@ bool SchedulingRemoteSuggestionsProvider::ShouldRefetchInTheBackgroundNow(
       NOTREACHED();
       break;
   }
-  return first_allowed_fetch_time <= clock_->Now();
+  base::Time now = clock_->Now();
+
+  return background_fetches_allowed_after_ <= now &&
+         first_allowed_fetch_time <= now;
 }
 
 bool SchedulingRemoteSuggestionsProvider::BackgroundFetchesDisabled(
@@ -519,6 +532,10 @@ void SchedulingRemoteSuggestionsProvider::OnFetchCompleted(
   ApplyPersistentFetchingSchedule();
 }
 
+void SchedulingRemoteSuggestionsProvider::ClearLastFetchAttemptTime() {
+  pref_service_->ClearPref(prefs::kSnippetLastFetchAttempt);
+}
+
 std::set<SchedulingRemoteSuggestionsProvider::TriggerType>
 SchedulingRemoteSuggestionsProvider::GetEnabledTriggerTypes() {
   static_assert(static_cast<unsigned int>(TriggerType::COUNT) ==