[NTP Snippets] Scheduler: Fetch on Wifi (without charging) only 6AM-10PM

components/ntp_snippets/ntp_snippets_service.cc

Index: components/ntp_snippets/ntp_snippets_service.cc
diff --git a/components/ntp_snippets/ntp_snippets_service.cc b/components/ntp_snippets/ntp_snippets_service.cc
index 38c98f199319b6398624718deb225e539524eac1..bc2ba79ec9e118c72ccfc5f40616c05fe1640957 100644
--- a/components/ntp_snippets/ntp_snippets_service.cc
+++ b/components/ntp_snippets/ntp_snippets_service.cc
@@ -35,6 +35,11 @@ const int kFetchingIntervalWifiChargingSeconds = 30 * 60;
 const int kFetchingIntervalWifiSeconds = 2 * 60 * 60;
 const int kFetchingIntervalFallbackSeconds = 24 * 60 * 60;
 
+// These define the times of day during which we will fetch via Wifi (without
+// charging) - 6 AM to 10 PM.
+const int kWifiFetchingHourMin = 6;
+const int kWifiFetchingHourMax = 22;
+
 const int kDefaultExpiryTimeMins = 24 * 60;
 
 base::TimeDelta GetFetchingInterval(const char* switch_name,
@@ -57,9 +62,16 @@ base::TimeDelta GetFetchingIntervalWifiCharging() {
                              kFetchingIntervalWifiChargingSeconds);
 }
 
-base::TimeDelta GetFetchingIntervalWifi() {
-  return GetFetchingInterval(switches::kFetchingIntervalWifiSeconds,
-                             kFetchingIntervalWifiSeconds);
+base::TimeDelta GetFetchingIntervalWifi(const base::Time& now) {
+  // Only fetch via Wifi (without charging) during the proper times of day.
+  base::Time::Exploded exploded;
+  now.LocalExplode(&exploded);
+  if (kWifiFetchingHourMin <= exploded.hour &&
+      exploded.hour < kWifiFetchingHourMax) {
+    return GetFetchingInterval(switches::kFetchingIntervalWifiSeconds,
+                               kFetchingIntervalWifiSeconds);
+  }
+  return base::TimeDelta();

[Bernhard Bauer, 2016/04/11 13:00:58]

Would it make sense to use the fallback value here? Otherwise between 10PM and
6AM we would not fetch all if we're on Wifi, but we might fetch if we're on
cellular o_O

[Marc Treib, 2016/04/11 13:25:26]

No, the "Fallback" wakeup would still trigger even if we're on Wifi.

Semi-related: It can happen that multiple wakeups trigger at the same time, e.g.
"Wifi" and "WifiCharging". Right now we'd fetch snippets twice in a row in that
case. I guess a minimum interval between automatic fetches would handle that.

 }
 
 base::TimeDelta GetFetchingIntervalFallback() {
@@ -67,6 +79,33 @@ base::TimeDelta GetFetchingIntervalFallback() {
                              kFetchingIntervalFallbackSeconds);
 }
 
+base::TimeDelta GetRescheduleDelay(const base::Time& now) {
+  base::Time::Exploded exploded;
+  now.LocalExplode(&exploded);
+  // The scheduling changes at both |kWifiFetchingHourMin| and
+  // |kWifiFetchingHourMax|. Find the time of the next one that we'll hit.
+  bool next_day = false;
+  if (exploded.hour < kWifiFetchingHourMin) {
+    exploded.hour = kWifiFetchingHourMin;
+  } else if (exploded.hour < kWifiFetchingHourMax) {
+    exploded.hour = kWifiFetchingHourMax;
+  } else {
+    next_day = true;
+    exploded.hour = kWifiFetchingHourMin;
+  }
+  // In any case, reschedule at the full hour. Add some fudge to avoid being
+  // called back before it's actually time to reschedule, due to clock skew
+  // or other shenanigans.

[Bernhard Bauer, 2016/04/18 08:03:00]

Wouldn't this just mean we'd reschedule for a couple of seconds later?

[Marc Treib, 2016/04/19 11:32:13]

Yup, which would mean one extra pointless wakeup that I'd like to avoid, and IMO
there's no harm in the 1 min delay. But if you feel strongly, I can of course
remove it..?

[Bernhard Bauer, 2016/04/19 15:45:35]

Yeah, because in that minute (or actually less than that, because it's just
going to be the value of the clock skew), the browser isn't that likely to get
killed again.

[Marc Treib, 2016/04/19 16:18:44]

Alright. Done.

+  exploded.minute = 1;
+  exploded.second = 0;
+  exploded.millisecond = 0;
+  base::Time reschedule = base::Time::FromLocalExploded(exploded);
+  if (next_day)
+    reschedule += base::TimeDelta::FromDays(1);
+
+  return reschedule - now;
+}
+
 // Extracts the hosts from |suggestions| and returns them in a set.
 std::set<std::string> GetSuggestionsHosts(
     const SuggestionsProfile& suggestions) {
@@ -124,7 +163,8 @@ NTPSnippetsService::NTPSnippetsService(
     NTPSnippetsScheduler* scheduler,
     scoped_ptr<NTPSnippetsFetcher> snippets_fetcher,
     const ParseJSONCallback& parse_json_callback)
-    : pref_service_(pref_service),
+    : enabled_(false),
+      pref_service_(pref_service),
       suggestions_service_(suggestions_service),
       file_task_runner_(file_task_runner),
       application_language_code_(application_language_code),
@@ -146,7 +186,8 @@ void NTPSnippetsService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
 }
 
 void NTPSnippetsService::Init(bool enabled) {
-  if (enabled) {
+  enabled_ = enabled;
+  if (enabled_) {
     // |suggestions_service_| can be null in tests.
     if (suggestions_service_) {
       suggestions_service_subscription_ = suggestions_service_->AddCallback(
@@ -163,22 +204,13 @@ void NTPSnippetsService::Init(bool enabled) {
       FetchSnippets();
   }
 
-  // The scheduler only exists on Android so far, it's null on other platforms.
-  if (!scheduler_)
-    return;
-
-  if (enabled) {
-    scheduler_->Schedule(GetFetchingIntervalWifiCharging(),
-                         GetFetchingIntervalWifi(),
-                         GetFetchingIntervalFallback());
-  } else {
-    scheduler_->Unschedule();
-  }
+  RescheduleFetching();
 }
 
 void NTPSnippetsService::Shutdown() {
   FOR_EACH_OBSERVER(NTPSnippetsServiceObserver, observers_,
                     NTPSnippetsServiceShutdown());
+  enabled_ = false;
 }
 
 void NTPSnippetsService::FetchSnippets() {
@@ -204,6 +236,21 @@ bool NTPSnippetsService::DiscardSnippet(const GURL& url) {
   return true;
 }
 
+void NTPSnippetsService::RescheduleFetching() {
+  // The scheduler only exists on Android so far, it's null on other platforms.
+  if (!scheduler_)
+    return;
+
+  if (enabled_) {
+    base::Time now = base::Time::Now();
+    scheduler_->Schedule(
+        GetFetchingIntervalWifiCharging(), GetFetchingIntervalWifi(now),
+        GetFetchingIntervalFallback(), GetRescheduleDelay(now));
+  } else {
+    scheduler_->Unschedule();
+  }
+}
+
 void NTPSnippetsService::AddObserver(NTPSnippetsServiceObserver* observer) {
   observers_.AddObserver(observer);
   observer->NTPSnippetsServiceLoaded();