commit 38b1402462bd15025a53bea993ca3d93cdf0d8de
author Gilad Arnold <garnold@chromium.org> Wed Jul 09 12:45:56 2014 -0700
committer chrome-internal-fetch <chrome-internal-fetch@google.com> Thu Jul 10 21:16:33 2014 +0000
tree 52904c202d8b7e20abb2ebbadcbc0066bcfead68
parent 28d6be6ba06170dc99b2b9fd1685a7a6405abc1a

UpdateManager: Fix update scheduling interval/fuzz inference.

This addresses two differences wrt current logic:

* In the case of recurring periodic check, we should use
  kTimeoutPeriodicInterval as the base check interval (and not
  kTimeoutInitialInterval).

* When doing exponential backoff, we should be using the interval / 2 as
  the fuzz factor (and not the fixed kTimeoutRegularFuzz).

Added two new tests (RecurringCheckBaseIntervalAndFuzz and
RecurringCheckBackoffIntervalAndFuzz) to ensure the correct values are
used.

Also fixed existing unit tests to properly distinguish a first update
check from recurring ones (FirstCheckIsAtMostInitialIntervalAfterStart)
and to properly verify that deferred update check times are within the
expected buzz boundaries (FirstCheckIsAtMostInitialIntervalAfterStart
and ExponentialBackoffIsCapped).

BUG=chromium:392582
TEST=Unit tests.

Change-Id: I3085502c57616cba2eff4cb0372ca5699427ae20
Reviewed-on: https://chromium-review.googlesource.com/207192
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>

update_manager/chromeos_policy_unittest.cc

diff --git a/update_manager/chromeos_policy_unittest.cc b/update_manager/chromeos_policy_unittest.cc
index 2eba5d2..23c0c2b 100644
--- a/update_manager/chromeos_policy_unittest.cc
+++ b/update_manager/chromeos_policy_unittest.cc
@@ -147,13 +147,62 @@
 TEST_F(UmChromeOSPolicyTest, FirstCheckIsAtMostInitialIntervalAfterStart) {
   Time next_update_check;
 
+  // Set the last update time so it'll appear as if this is a first update check
+  // in the lifetime of the current updater.
+  fake_state_.updater_provider()->var_last_checked_time()->reset(
+      new Time(fake_clock_.GetWallclockTime() - TimeDelta::FromMinutes(10)));
+
   ExpectPolicyStatus(EvalStatus::kSucceeded,
                      &ChromeOSPolicy::NextUpdateCheckTime, &next_update_check);
 
   EXPECT_LE(fake_clock_.GetWallclockTime(), next_update_check);
-  EXPECT_GE(fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
-      ChromeOSPolicy::kTimeoutInitialInterval +
-      ChromeOSPolicy::kTimeoutRegularFuzz), next_update_check);
+  EXPECT_GE(
+      fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
+          ChromeOSPolicy::kTimeoutInitialInterval +
+          ChromeOSPolicy::kTimeoutRegularFuzz / 2),
+      next_update_check);
+}
+
+TEST_F(UmChromeOSPolicyTest, RecurringCheckBaseIntervalAndFuzz) {
+  // Ensure that we're using the correct interval (kPeriodicInterval) and fuzz
+  // (kTimeoutRegularFuzz) as base values for period updates.
+  Time next_update_check;
+
+  ExpectPolicyStatus(EvalStatus::kSucceeded,
+                     &ChromeOSPolicy::NextUpdateCheckTime, &next_update_check);
+
+  EXPECT_LE(
+      fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
+          ChromeOSPolicy::kTimeoutPeriodicInterval -
+          ChromeOSPolicy::kTimeoutRegularFuzz / 2),
+      next_update_check);
+  EXPECT_GE(
+      fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
+          ChromeOSPolicy::kTimeoutPeriodicInterval +
+          ChromeOSPolicy::kTimeoutRegularFuzz / 2),
+      next_update_check);
+}
+
+TEST_F(UmChromeOSPolicyTest, RecurringCheckBackoffIntervalAndFuzz) {
+  // Ensure that we're properly backing off and fuzzing in the presence of
+  // failed updates attempts.
+  Time next_update_check;
+
+  fake_state_.updater_provider()->var_consecutive_failed_update_checks()->
+      reset(new unsigned int(2));  // NOLINT(readability/casting)
+
+  ExpectPolicyStatus(EvalStatus::kSucceeded,
+                     &ChromeOSPolicy::NextUpdateCheckTime, &next_update_check);
+
+  int expected_interval = ChromeOSPolicy::kTimeoutPeriodicInterval * 4;
+  EXPECT_LE(
+      fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
+          expected_interval - expected_interval / 2),
+      next_update_check);
+  EXPECT_GE(
+      fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
+          expected_interval + expected_interval / 2),
+      next_update_check);
 }
 
 TEST_F(UmChromeOSPolicyTest, ExponentialBackoffIsCapped) {
@@ -161,15 +210,20 @@
 
   fake_state_.updater_provider()->var_consecutive_failed_update_checks()->
       reset(new unsigned int(100));  // NOLINT(readability/casting)
+
   ExpectPolicyStatus(EvalStatus::kSucceeded,
                      &ChromeOSPolicy::NextUpdateCheckTime, &next_update_check);
 
-  EXPECT_LE(fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
-      ChromeOSPolicy::kTimeoutMaxBackoffInterval -
-      ChromeOSPolicy::kTimeoutRegularFuzz - 1), next_update_check);
-  EXPECT_GE(fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
-      ChromeOSPolicy::kTimeoutMaxBackoffInterval +
-      ChromeOSPolicy::kTimeoutRegularFuzz), next_update_check);
+  EXPECT_LE(
+      fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
+          ChromeOSPolicy::kTimeoutMaxBackoffInterval -
+          ChromeOSPolicy::kTimeoutMaxBackoffInterval / 2),
+      next_update_check);
+  EXPECT_GE(
+      fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
+          ChromeOSPolicy::kTimeoutMaxBackoffInterval +
+          ChromeOSPolicy::kTimeoutMaxBackoffInterval /2),
+      next_update_check);
 }
 
 TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedWaitsForTheTimeout) {