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

components/ntp_snippets/remote/scheduling_remote_suggestions_provider.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 <random>
 #include <string>
 #include <utility>
 
@@ skipping 77 matching lines @@
             arraysize(kFetchingIntervalParamNameActiveSuggestionsConsumer),
     "Fill in all the info for fetching intervals.");
 
 const char* kTriggerTypeNames[] = {"persistent_scheduler_wake_up", "ntp_opened",
                                    "browser_foregrounded",
                                    "browser_cold_start"};
 
 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(
     FetchingInterval interval,
     UserClassifier::UserClass user_class) {
   DCHECK(interval != FetchingInterval::COUNT);
   const unsigned int index = static_cast<unsigned int>(interval);
   DCHECK(index < arraysize(kDefaultFetchingIntervalHoursRareNtpUser));
 
   double default_value_hours = 0.0;
@@ skipping 77 matching lines @@
       background_fetch_in_progress_(false),
       user_classifier_(user_classifier),
       pref_service_(pref_service),
       clock_(std::move(clock)),
       enabled_triggers_(GetEnabledTriggerTypes()) {
   DCHECK(user_classifier);
   DCHECK(pref_service);
 
   LoadLastFetchingSchedule();
 
-  provider_->SetProviderStatusCallback(
-      base::MakeUnique<RemoteSuggestionsProvider::ProviderStatusCallback>(
-          base::BindRepeating(
-              &SchedulingRemoteSuggestionsProvider::OnProviderStatusChanged,
-              base::Unretained(this))));
+  provider_->SetRemoteSuggestionsScheduler(this);
 }
 
 SchedulingRemoteSuggestionsProvider::~SchedulingRemoteSuggestionsProvider() =
     default;
 
 // static
 void SchedulingRemoteSuggestionsProvider::RegisterProfilePrefs(
     PrefRegistrySimple* registry) {
   registry->RegisterInt64Pref(prefs::kSnippetPersistentFetchingIntervalWifi, 0);
   registry->RegisterInt64Pref(prefs::kSnippetPersistentFetchingIntervalFallback,
                               0);
   registry->RegisterInt64Pref(prefs::kSnippetSoftFetchingIntervalOnUsageEvent,
                               0);
   registry->RegisterInt64Pref(prefs::kSnippetLastFetchAttempt, 0);
   registry->RegisterInt64Pref(prefs::kSnippetSoftFetchingIntervalOnNtpOpened,
                               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();
   StartScheduling();
 }
 
 void SchedulingRemoteSuggestionsProvider::OnPersistentSchedulerWakeUp() {
   RefetchInTheBackgroundIfEnabled(TriggerType::PERSISTENT_SCHEDULER_WAKE_UP);
 }
 
@@ skipping 18 matching lines @@
 }
 
 void SchedulingRemoteSuggestionsProvider::OnNTPOpened() {
   if (!ShouldRefetchInTheBackgroundNow(TriggerType::NTP_OPENED)) {
     return;
   }
 
   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(
     std::unique_ptr<FetchStatusCallback> callback) {
   if (background_fetch_in_progress_) {
     if (callback) {
       callback->Run(
           Status(StatusCode::TEMPORARY_ERROR, "Background fetch in progress"));
     }
     return;
@@ skipping 68 matching lines @@
     Category category,
     const DismissedSuggestionsCallback& callback) {
   provider_->GetDismissedSuggestionsForDebugging(category, callback);
 }
 
 void SchedulingRemoteSuggestionsProvider::ClearDismissedSuggestionsForDebugging(
     Category category) {
   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();
 
   if (schedule_ == new_schedule) {
     // Do not schedule if nothing has changed;
     return;
   }
 
   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() {
   if (schedule_.is_empty()) {
     // Do not unschedule if already switched off.
     return;
   }
 
   schedule_ = FetchingSchedule::Empty();
   StoreFetchingSchedule();
@@ skipping 81 matching lines @@
     case TriggerType::BROWSER_FOREGROUNDED:
     case TriggerType::BROWSER_COLD_START:
       first_allowed_fetch_time =
           last_fetch_attempt_time + schedule_.interval_soft_on_usage_event;
       break;
     case TriggerType::PERSISTENT_SCHEDULER_WAKE_UP:
     case TriggerType::COUNT:
       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(
     SchedulingRemoteSuggestionsProvider::TriggerType trigger) const {
   if (schedule_.is_empty()) {
     return true;  // Background fetches are disabled in general.
   }
 
   if (enabled_triggers_.count(trigger) == 0) {
     return true;  // Background fetches for |trigger| are not enabled.
@@ skipping 29 matching lines @@
   // Reschedule after a fetch. The persistent schedule is applied only after a
   // successful fetch. After a failed fetch, we want to keep the previous
   // persistent schedule intact so that we eventually get a persistent
   // fallback fetch (if the wifi persistent fetches keep failing).
   if (fetch_status.code != StatusCode::SUCCESS) {
     return;
   }
   ApplyPersistentFetchingSchedule();
 }
 
+void SchedulingRemoteSuggestionsProvider::ClearLastFetchAttemptTime() {
+  pref_service_->ClearPref(prefs::kSnippetLastFetchAttempt);
+}
+
 std::set<SchedulingRemoteSuggestionsProvider::TriggerType>
 SchedulingRemoteSuggestionsProvider::GetEnabledTriggerTypes() {
   static_assert(static_cast<unsigned int>(TriggerType::COUNT) ==
                     arraysize(kTriggerTypeNames),
                 "Fill in names for trigger types.");
 
   std::string param_value = base::GetFieldTrialParamValueByFeature(
       ntp_snippets::kArticleSuggestionsFeature, kTriggerTypesParamName);
   if (param_value == kTriggerTypesParamValueForEmptyList) {
     return std::set<TriggerType>();
@@ skipping 26 matching lines @@
   return enabled_types;
 }
 
 std::set<SchedulingRemoteSuggestionsProvider::TriggerType>
 SchedulingRemoteSuggestionsProvider::GetDefaultEnabledTriggerTypes() {
   return {TriggerType::PERSISTENT_SCHEDULER_WAKE_UP, TriggerType::NTP_OPENED,
           TriggerType::BROWSER_FOREGROUNDED};
 }
 
 }  // namespace ntp_snippets