PM: OOBE prevents scattering.
This implements a TODO item whereas we can only scatter if OOBE was
completed.
BUG=chromium:358323
TEST=Unit tests.
Change-Id: I55d95020d738cbba08b77bc580353e69256d1a7e
Reviewed-on: https://chromium-review.googlesource.com/200590
Reviewed-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/policy_manager/chromeos_policy.cc b/policy_manager/chromeos_policy.cc
index 99e13e9..f0d6e3d 100644
--- a/policy_manager/chromeos_policy.cc
+++ b/policy_manager/chromeos_policy.cc
@@ -69,6 +69,7 @@
}
DevicePolicyProvider* const dp_provider = state->device_policy_provider();
+ SystemProvider* const system_provider = state->system_provider();
const bool* device_policy_is_loaded_p = ec->GetValue(
dp_provider->var_device_policy_is_loaded());
@@ -82,14 +83,29 @@
return EvalStatus::kAskMeAgainLater;
}
- // Check whether scattering applies to this update attempt.
- // TODO(garnold) We should not be scattering during OOBE. We'll need to read
- // the OOBE status (via SystemProvider) and only scatter if not enacted.
- // TODO(garnold) Current code further suppresses scattering if a "deadline"
+ // Check whether scattering applies to this update attempt. We should not be
+ // scattering if this is an interactive update check, or if OOBE is enabled
+ // but not completed.
+ //
+ // Note: current code further suppresses scattering if a "deadline"
// attribute is found in the Omaha response. However, it appears that the
- // presence of this attribute is merely indicative of an OOBE update, which
- // we should support anyway (see above).
+ // presence of this attribute is merely indicative of an OOBE update, during
+ // which we suppress scattering anyway.
+ bool scattering_applies = false;
if (!interactive) {
+ const bool* is_oobe_enabled_p = ec->GetValue(
+ state->config_provider()->var_is_oobe_enabled());
+ if (is_oobe_enabled_p && !(*is_oobe_enabled_p)) {
+ scattering_applies = true;
+ } else {
+ const bool* is_oobe_complete_p = ec->GetValue(
+ system_provider->var_is_oobe_complete());
+ scattering_applies = (is_oobe_complete_p && *is_oobe_complete_p);
+ }
+ }
+
+ // Compute scattering values.
+ if (scattering_applies) {
UpdateScatteringResult scatter_result;
EvalStatus scattering_status = UpdateScattering(
ec, state, error, &scatter_result, update_state);
@@ -108,7 +124,7 @@
// Determine whether HTTP downloads are forbidden by policy. This only
// applies to official system builds; otherwise, HTTP is always enabled.
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) {
const bool* policy_http_downloads_enabled_p = ec->GetValue(
dp_provider->var_http_downloads_enabled());
diff --git a/policy_manager/chromeos_policy_unittest.cc b/policy_manager/chromeos_policy_unittest.cc
index 69f092f..88a6d28 100644
--- a/policy_manager/chromeos_policy_unittest.cc
+++ b/policy_manager/chromeos_policy_unittest.cc
@@ -54,9 +54,12 @@
fake_state_.device_policy_provider()->var_device_policy_is_loaded()->reset(
new bool(false));
- // For the purpose of the tests, this is an official build.
+ // For the purpose of the tests, this is an official build and OOBE was
+ // completed.
fake_state_.system_provider()->var_is_official_build()->reset(
new bool(true));
+ fake_state_.system_provider()->var_is_oobe_complete()->reset(
+ new bool(true));
// Connection is wifi, untethered.
fake_state_.shill_provider()->var_conn_type()->
@@ -442,6 +445,31 @@
EXPECT_EQ(0, result.scatter_check_threshold);
}
+TEST_F(PmChromeOSPolicyTest,
+ UpdateCanStartAllowedOobePreventsScattering) {
+ // The UpdateCanStart policy returns true; device policy is loaded and
+ // scattering would have applied, except that OOBE was not completed and so it
+ // is suppressed.
+
+ SetUpdateCheckAllowed(false);
+ fake_state_.device_policy_provider()->var_scatter_factor()->reset(
+ new TimeDelta(TimeDelta::FromSeconds(1)));
+ fake_state_.system_provider()->var_is_oobe_complete()->reset(new bool(false));
+
+ UpdateState update_state = GetDefaultUpdateState(TimeDelta::FromSeconds(1));
+ update_state.scatter_check_threshold = 0;
+ update_state.scatter_check_threshold_min = 2;
+ update_state.scatter_check_threshold_max = 5;
+
+ // Check that the UpdateCanStart returns true.
+ UpdateCanStartResult result;
+ ExpectPolicyStatus(EvalStatus::kSucceeded, &Policy::UpdateCanStart, &result,
+ true, update_state);
+ EXPECT_TRUE(result.update_can_start);
+ EXPECT_EQ(TimeDelta(), result.scatter_wait_period);
+ EXPECT_EQ(0, result.scatter_check_threshold);
+}
+
TEST_F(PmChromeOSPolicyTest, UpdateCanStartAllowedWithAttributes) {
// The UpdateCanStart policy returns true; device policy permits both HTTP and
// P2P updates, as well as a non-empty target channel string.