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);