UpdateManager: Do not schedule update checks if OOBE did not complete.
This abides by the current logic, as found in
UpdateCheckScheduler::StaticCheck(). New unit test added to verify this
behavior.
BUG=chromium:358269
TEST=Unit tests.
Change-Id: I747e8a59408d6b93ceea62ef741b36e09f937a08
Reviewed-on: https://chromium-review.googlesource.com/207241
Reviewed-by: Alex Deymo <deymo@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/update_manager/chromeos_policy.cc b/update_manager/chromeos_policy.cc
index adbef4f..ba3241e 100644
--- a/update_manager/chromeos_policy.cc
+++ b/update_manager/chromeos_policy.cc
@@ -156,9 +156,12 @@
result->updates_enabled = true;
result->target_channel.clear();
+ DevicePolicyProvider* const dp_provider = state->device_policy_provider();
+ SystemProvider* const system_provider = state->system_provider();
+
// Unofficial builds should not perform any automatic update checks.
const bool* is_official_build_p = ec->GetValue(
- state->system_provider()->var_is_official_build());
+ system_provider->var_is_official_build());
if (is_official_build_p && !(*is_official_build_p)) {
result->updates_enabled = false;
return EvalStatus::kSucceeded;
@@ -166,13 +169,21 @@
// Do not perform any automatic update checks if booted from removable device.
const bool* is_boot_device_removable_p = ec->GetValue(
- state->system_provider()->var_is_boot_device_removable());
+ system_provider->var_is_boot_device_removable());
if (is_boot_device_removable_p && *is_boot_device_removable_p) {
result->updates_enabled = false;
return EvalStatus::kSucceeded;
}
- DevicePolicyProvider* const dp_provider = state->device_policy_provider();
+ // If OOBE is enabled, wait until it is completed.
+ const bool* is_oobe_enabled_p = ec->GetValue(
+ state->config_provider()->var_is_oobe_enabled());
+ if (is_oobe_enabled_p && *is_oobe_enabled_p) {
+ const bool* is_oobe_complete_p = ec->GetValue(
+ system_provider->var_is_oobe_complete());
+ if (is_oobe_complete_p && !(*is_oobe_complete_p))
+ return EvalStatus::kAskMeAgainLater;
+ }
const bool* device_policy_is_loaded_p = ec->GetValue(
dp_provider->var_device_policy_is_loaded());
@@ -180,10 +191,8 @@
// Check whether updates are disabled by policy.
const bool* update_disabled_p = ec->GetValue(
dp_provider->var_update_disabled());
- if (update_disabled_p && *update_disabled_p) {
- result->updates_enabled = false;
+ if (update_disabled_p && *update_disabled_p)
return EvalStatus::kAskMeAgainLater;
- }
// Determine whether a target channel is dictated by policy.
const bool* release_channel_delegated_p = ec->GetValue(
diff --git a/update_manager/chromeos_policy.h b/update_manager/chromeos_policy.h
index b7bbdfc..75e31cc 100644
--- a/update_manager/chromeos_policy.h
+++ b/update_manager/chromeos_policy.h
@@ -67,6 +67,7 @@
FRIEND_TEST(UmChromeOSPolicyTest, RecurringCheckBackoffIntervalAndFuzz);
FRIEND_TEST(UmChromeOSPolicyTest, ExponentialBackoffIsCapped);
FRIEND_TEST(UmChromeOSPolicyTest, UpdateCheckAllowedWaitsForTheTimeout);
+ FRIEND_TEST(UmChromeOSPolicyTest, UpdateCheckAllowedWaitsForOOBE);
FRIEND_TEST(UmChromeOSPolicyTest,
UpdateCanStartNotAllowedScatteringNewWaitPeriodApplies);
FRIEND_TEST(UmChromeOSPolicyTest,
diff --git a/update_manager/chromeos_policy_unittest.cc b/update_manager/chromeos_policy_unittest.cc
index 17eda83..d0a2114 100644
--- a/update_manager/chromeos_policy_unittest.cc
+++ b/update_manager/chromeos_policy_unittest.cc
@@ -57,6 +57,10 @@
fake_state_.device_policy_provider()->var_device_policy_is_loaded()->reset(
new bool(false));
+ // OOBE is enabled by default.
+ fake_state_.config_provider()->var_is_oobe_enabled()->reset(
+ new bool(true));
+
// For the purpose of the tests, this is an official build and OOBE was
// completed.
fake_state_.system_provider()->var_is_official_build()->reset(
@@ -259,6 +263,43 @@
fake_clock_.SetWallclockTime(next_update_check + TimeDelta::FromSeconds(1));
ExpectPolicyStatus(EvalStatus::kSucceeded,
&Policy::UpdateCheckAllowed, &result);
+ EXPECT_TRUE(result.updates_enabled);
+}
+
+TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedWaitsForOOBE) {
+ // Update checks are defferred until OOBE is completed.
+
+ // Ensure that update is not allowed even if wait period is satisfied.
+ Time next_update_check;
+ Time last_checked_time =
+ fake_clock_.GetWallclockTime() + TimeDelta::FromMinutes(1234);
+
+ fake_state_.updater_provider()->var_last_checked_time()->reset(
+ new Time(last_checked_time));
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &ChromeOSPolicy::NextUpdateCheckTime, &next_update_check);
+
+ SetUpDefaultClock();
+ SetUpDefaultState();
+ fake_state_.updater_provider()->var_last_checked_time()->reset(
+ new Time(last_checked_time));
+ fake_clock_.SetWallclockTime(next_update_check + TimeDelta::FromSeconds(1));
+ fake_state_.system_provider()->var_is_oobe_complete()->reset(
+ new bool(false));
+
+ UpdateCheckParams result;
+ ExpectPolicyStatus(EvalStatus::kAskMeAgainLater,
+ &Policy::UpdateCheckAllowed, &result);
+
+ // Now check that it is allowed if OOBE is completed.
+ SetUpDefaultClock();
+ SetUpDefaultState();
+ fake_state_.updater_provider()->var_last_checked_time()->reset(
+ new Time(last_checked_time));
+ fake_clock_.SetWallclockTime(next_update_check + TimeDelta::FromSeconds(1));
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &Policy::UpdateCheckAllowed, &result);
+ EXPECT_TRUE(result.updates_enabled);
}
TEST_F(UmChromeOSPolicyTest, UpdateCheckAllowedWithAttributes) {
diff --git a/update_manager/fake_config_provider.h b/update_manager/fake_config_provider.h
index 373ff0a..fab01d1 100644
--- a/update_manager/fake_config_provider.h
+++ b/update_manager/fake_config_provider.h
@@ -15,13 +15,12 @@
public:
FakeConfigProvider() {}
- protected:
virtual FakeVariable<bool>* var_is_oobe_enabled() override {
return &var_is_oobe_enabled_;
}
private:
- FakeVariable<bool> var_is_oobe_enabled_{
+ FakeVariable<bool> var_is_oobe_enabled_{ // NOLINT(whitespace/braces)
"is_oobe_enabled", kVariableModeConst};
DISALLOW_COPY_AND_ASSIGN(FakeConfigProvider);