Adjust scheduling of extension autoupdate checks

chrome/browser/extensions/updater/extension_updater.cc

 // Copyright (c) 2012 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 "chrome/browser/extensions/updater/extension_updater.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <set>
@@ skipping 32 matching lines @@
 using base::RandInt;
 using base::Time;
 using base::TimeDelta;
 using content::BrowserThread;
 
 typedef extensions::ExtensionDownloaderDelegate::Error Error;
 typedef extensions::ExtensionDownloaderDelegate::PingResult PingResult;
 
 namespace {
 
-// Wait at least 5 minutes after browser startup before we do any checks. If you
-// change this value, make sure to update comments where it is used.
-const int kStartupWaitSeconds = 60 * 5;
+// Wait at least 30 seconds after browser startup before we do any checks. If

[lazyboy, 2016/08/05 00:04:19]

As discussed offline, I'd be comfortable if this value was at least 60 seconds,
30s feels a bit aggressive.
So I'd suggest something between 60s to 5mins.

[asargent_no_longer_on_chrome, 2016/08/05 06:23:31]

Done.

+// you change this value, make sure to update comments where it is used.
+const int kStartupWaitSeconds = 30;
 
 // For sanity checking on update frequency - enforced in release mode only.
 #if defined(NDEBUG)
 const int kMinUpdateFrequencySeconds = 30;
 #endif
 const int kMaxUpdateFrequencySeconds = 60 * 60 * 24 * 7;  // 7 days
 
 // When we've computed a days value, we want to make sure we don't send a
 // negative value (due to the system clock being set backwards, etc.), since -1
 // is a special sentinel value that means "never pinged", and other negative
@@ skipping 99 matching lines @@
 }
 
 // The overall goal here is to balance keeping clients up to date while
 // avoiding a thundering herd against update servers.
 TimeDelta ExtensionUpdater::DetermineFirstCheckDelay() {
   DCHECK(alive_);
   // If someone's testing with a quick frequency, just allow it.
   if (frequency_seconds_ < kStartupWaitSeconds)
     return TimeDelta::FromSeconds(frequency_seconds_);
 
-  // If we've never scheduled a check before, start at frequency_seconds_.
+  // If we've never scheduled a check before, start at a random time up to
+  // frequency_seconds_ away.
   if (!prefs_->HasPrefPath(pref_names::kNextUpdateCheck))
-    return TimeDelta::FromSeconds(frequency_seconds_);
+    return TimeDelta::FromSeconds(
+        RandInt(kStartupWaitSeconds, frequency_seconds_));
 
-  // If it's been a long time since our last actual check, we want to do one
-  // relatively soon.
-  Time now = Time::Now();
-  Time last = Time::FromInternalValue(prefs_->GetInt64(
-      pref_names::kLastUpdateCheck));
-  int days = (now - last).InDays();
-  if (days >= 30) {
-    // Wait 5-10 minutes.
-    return TimeDelta::FromSeconds(RandInt(kStartupWaitSeconds,
-                                          kStartupWaitSeconds * 2));
-  } else if (days >= 14) {
-    // Wait 10-20 minutes.
-    return TimeDelta::FromSeconds(RandInt(kStartupWaitSeconds * 2,
-                                          kStartupWaitSeconds * 4));
-  } else if (days >= 3) {
-    // Wait 20-40 minutes.
-    return TimeDelta::FromSeconds(RandInt(kStartupWaitSeconds * 4,
-                                          kStartupWaitSeconds * 8));
-  }
-
-  // Read the persisted next check time, and use that if it isn't too soon
-  // or too late. Otherwise pick something random.
   Time saved_next = Time::FromInternalValue(prefs_->GetInt64(
       pref_names::kNextUpdateCheck));
-  Time earliest = now + TimeDelta::FromSeconds(kStartupWaitSeconds);
-  Time latest = now + TimeDelta::FromSeconds(frequency_seconds_);
-  if (saved_next >= earliest && saved_next <= latest) {
+
+  Time now = Time::Now();
+
+  // Read the persisted next check time, and use that if it isn't in the past
+  // or too far in the future (this can happen with system clock changes).
+  if (saved_next > now &&
+      saved_next < now + TimeDelta::FromSeconds(frequency_seconds_))

[lazyboy, 2016/08/05 00:04:19]

nit: this requires {}

[asargent_no_longer_on_chrome, 2016/08/05 06:23:32]

Done.

     return saved_next - now;
-  } else {
-    return TimeDelta::FromSeconds(RandInt(kStartupWaitSeconds,
-                                          frequency_seconds_));
-  }
+
+  // In most cases we'll get here because the persisted next check time passed
+  // while we weren't running, so pick something soon.
+  return TimeDelta::FromSeconds(
+      RandInt(kStartupWaitSeconds, kStartupWaitSeconds * 2));
 }
 
 void ExtensionUpdater::Start() {
   DCHECK(!alive_);
   // If these are NULL, then that means we've been called after Stop()
   // has been called.
   DCHECK(service_);
   DCHECK(extension_prefs_);
   DCHECK(prefs_);
   DCHECK(profile_);
@@ skipping 388 matching lines @@
   const InProgressCheck& request = requests_in_progress_[request_id];
   if (request.in_progress_ids_.empty()) {
     VLOG(2) << "Finished update check " << request_id;
     if (!request.callback.is_null())
       request.callback.Run();
     requests_in_progress_.erase(request_id);
   }
 }
 
 }  // namespace extensions