update_engine: Ignore OOBE update check result after rollback
Ignore the result of the OOBE update check if a rollback has
happened, even if the deadline field is set in the response. This
is needed to skip forced updates after rollback, thus avoiding
infinite rollback-update loops happening (since policy containing
targetversionprefix is not yet available during OOBE).
BUG=chromium:831266
TEST='cros_run_unit_tests --board=caroline --packages update_engine'
Change-Id: I792f09e54a42c4b73116071f2d55e2a4ee196daf
Reviewed-on: https://chromium-review.googlesource.com/1057617
Commit-Ready: Marton Hunyady <hunyadym@chromium.org>
Tested-by: Marton Hunyady <hunyadym@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 9021e3f..13a58fb 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -1266,7 +1266,8 @@
if (system_state_->hardware()->IsOOBEEnabled() &&
!system_state_->hardware()->IsOOBEComplete(nullptr) &&
- output_object.deadline.empty() &&
+ (output_object.deadline.empty() ||
+ payload_state->GetRollbackHappened()) &&
params_->app_version() != "ForcedUpdate") {
output_object.update_exists = false;
LOG(INFO) << "Ignoring non-critical Omaha updates until OOBE is done.";
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index fa531d2..a70712a 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -1044,14 +1044,14 @@
EXPECT_FALSE(response.update_exists);
}
-// Verify that update checks called during OOBE will only try to download
-// an update if the response includes a non-empty deadline field.
+// Verify that update checks called during OOBE will not try to download an
+// update if the response doesn't include the deadline field.
TEST_F(OmahaRequestActionTest, SkipNonCriticalUpdatesBeforeOOBE) {
OmahaResponse response;
+ fake_system_state_.fake_hardware()->UnsetIsOOBEComplete();
// TODO(senj): set better default value for metrics::checkresult in
// OmahaRequestAction::ActionCompleted.
- fake_system_state_.fake_hardware()->UnsetIsOOBEComplete();
ASSERT_FALSE(TestUpdateCheck(fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
@@ -1062,23 +1062,15 @@
&response,
nullptr));
EXPECT_FALSE(response.update_exists);
+}
- // The IsOOBEComplete() value is ignored when the OOBE flow is not enabled.
+// Verify that the IsOOBEComplete() value is ignored when the OOBE flow is not
+// enabled.
+TEST_F(OmahaRequestActionTest, SkipNonCriticalUpdatesBeforeOOBEDisabled) {
+ OmahaResponse response;
+ fake_system_state_.fake_hardware()->UnsetIsOOBEComplete();
fake_system_state_.fake_hardware()->SetIsOOBEEnabled(false);
- ASSERT_TRUE(TestUpdateCheck(fake_update_response_.GetUpdateResponse(),
- -1,
- false, // ping_only
- ErrorCode::kSuccess,
- metrics::CheckResult::kUpdateAvailable,
- metrics::CheckReaction::kUpdating,
- metrics::DownloadErrorCode::kUnset,
- &response,
- nullptr));
- EXPECT_TRUE(response.update_exists);
- fake_system_state_.fake_hardware()->SetIsOOBEEnabled(true);
- // The payload is applied when a deadline was set in the response.
- fake_update_response_.deadline = "20101020";
ASSERT_TRUE(TestUpdateCheck(fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
@@ -1091,6 +1083,47 @@
EXPECT_TRUE(response.update_exists);
}
+// Verify that update checks called during OOBE will still try to download an
+// update if the response includes the deadline field.
+TEST_F(OmahaRequestActionTest, SkipNonCriticalUpdatesBeforeOOBEDeadlineSet) {
+ OmahaResponse response;
+ fake_system_state_.fake_hardware()->UnsetIsOOBEComplete();
+ fake_update_response_.deadline = "20101020";
+
+ ASSERT_TRUE(TestUpdateCheck(fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kUpdating,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+ EXPECT_TRUE(response.update_exists);
+}
+
+// Verify that update checks called during OOBE will not try to download an
+// update if a rollback happened, even when the response includes the deadline
+// field.
+TEST_F(OmahaRequestActionTest, SkipNonCriticalUpdatesBeforeOOBERollback) {
+ OmahaResponse response;
+ fake_system_state_.fake_hardware()->UnsetIsOOBEComplete();
+ fake_update_response_.deadline = "20101020";
+ EXPECT_CALL(*(fake_system_state_.mock_payload_state()), GetRollbackHappened())
+ .WillOnce(Return(true));
+
+ ASSERT_FALSE(TestUpdateCheck(fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kNonCriticalUpdateInOOBE,
+ metrics::CheckResult::kParsingError,
+ metrics::CheckReaction::kUnset,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+ EXPECT_FALSE(response.update_exists);
+}
+
TEST_F(OmahaRequestActionTest, WallClockBasedWaitAloneCausesScattering) {
OmahaResponse response;
request_params_.set_wall_clock_based_wait_enabled(true);