Skip non-critical updates during OOBE update check

If the system has not yet completed OOBE, and Omaha returns a response
with an empty deadline field, avoid initiating a background update
because doing so may violate an enterprise policy that gets pushed
down post-enrollment.

Bug: None
BUG=chromium:587101
TEST=`FEATURES=test emerge-link chromeos-base/update_engine`
TEST=powerwash, install lsb-release file from an old stable build,
     repeat OOBE, verify that download is skipped via logs, then verify
     that update proceeds after logging into a standard gmail account

Change-Id: I6e6fae02c609b2763abc6e9960bf6d87dd3534e2

8 files changed, 61 insertions, 1 deletions

diff --git a/common/error_code.h b/common/error_code.h
index 32155f27..e08ec460 100644
--- a/common/error_code.h
+++ b/common/error_code.h
@@ -72,6 +72,7 @@ enum class ErrorCode : int {
   kOmahaRequestXMLHasEntityDecl = 46,
   kFilesystemVerifierError = 47,
   kUserCanceled = 48,
+  kNonCriticalUpdateInOOBE = 49,
 
   // VERY IMPORTANT! When adding new error codes:
   //
diff --git a/common/error_code_utils.cc b/common/error_code_utils.cc
index dc9eaf4a..ad4aeeb0 100644
--- a/common/error_code_utils.cc
+++ b/common/error_code_utils.cc
@@ -142,6 +142,8 @@ string ErrorCodeToString(ErrorCode code) {
       return "ErrorCode::kFilesystemVerifierError";
     case ErrorCode::kUserCanceled:
       return "ErrorCode::kUserCanceled";
+    case ErrorCode::kNonCriticalUpdateInOOBE:
+      return "ErrorCode::kNonCriticalUpdateInOOBE";
     // Don't add a default case to let the compiler warn about newly added
     // error codes which should be added here.
   }
diff --git a/common/fake_hardware.h b/common/fake_hardware.h
index 23d64981..449787be 100644
--- a/common/fake_hardware.h
+++ b/common/fake_hardware.h
@@ -37,7 +37,8 @@ class FakeHardware : public HardwareInterface {
   FakeHardware()
       : is_official_build_(true),
         is_normal_boot_mode_(true),
-        is_oobe_complete_(false),
+        is_oobe_complete_(true),
+        oobe_timestamp_(base::Time::FromTimeT(1169280000)), // Jan 20, 2007
         hardware_class_("Fake HWID BLAH-1234"),
         firmware_version_("Fake Firmware v1.0.1"),
         ec_version_("Fake EC v1.0a"),
diff --git a/metrics_utils.cc b/metrics_utils.cc
index 11260fc6..e165e897 100644
--- a/metrics_utils.cc
+++ b/metrics_utils.cc
@@ -99,7 +99,10 @@ metrics::AttemptResult GetAttemptResult(ErrorCode code) {
     case ErrorCode::kDownloadInvalidMetadataSignature:
     case ErrorCode::kOmahaResponseInvalid:
     case ErrorCode::kOmahaUpdateIgnoredPerPolicy:
+    // TODO(deymo): The next two items belong in their own category; they
+    // should not be counted as internal errors. b/27112092
     case ErrorCode::kOmahaUpdateDeferredPerPolicy:
+    case ErrorCode::kNonCriticalUpdateInOOBE:
     case ErrorCode::kOmahaErrorInHTTPResponse:
     case ErrorCode::kDownloadMetadataSignatureMissingError:
     case ErrorCode::kOmahaUpdateDeferredForBackoff:
@@ -193,6 +196,7 @@ metrics::DownloadErrorCode GetDownloadErrorCode(ErrorCode code) {
     case ErrorCode::kOmahaResponseInvalid:
     case ErrorCode::kOmahaUpdateIgnoredPerPolicy:
     case ErrorCode::kOmahaUpdateDeferredPerPolicy:
+    case ErrorCode::kNonCriticalUpdateInOOBE:
     case ErrorCode::kOmahaErrorInHTTPResponse:
     case ErrorCode::kDownloadOperationHashMissingError:
     case ErrorCode::kDownloadMetadataSignatureMissingError:
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index e0176752..f785760e 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -1024,6 +1024,14 @@ void OmahaRequestAction::CompleteProcessing() {
   OmahaResponse& output_object = const_cast<OmahaResponse&>(GetOutputObject());
   PayloadStateInterface* payload_state = system_state_->payload_state();
 
+  if (!system_state_->hardware()->IsOOBEComplete(nullptr) &&
+      output_object.deadline.empty()) {
+    output_object.update_exists = false;
+    LOG(INFO) << "Ignoring non-critical Omaha updates until OOBE is done.";
+    completer.set_code(ErrorCode::kNonCriticalUpdateInOOBE);
+    return;
+  }
+
   if (ShouldDeferDownload(&output_object)) {
     output_object.update_exists = false;
     LOG(INFO) << "Ignoring Omaha updates as updates are deferred by policy.";
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 084d3aab..39bc5cd9 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -516,6 +516,40 @@ TEST_F(OmahaRequestActionTest, ValidUpdateBlockedByRollback) {
   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.
+TEST_F(OmahaRequestActionTest, SkipNonCriticalUpdatesBeforeOOBE) {
+  OmahaResponse response;
+
+  fake_system_state_.fake_hardware()->UnsetIsOOBEComplete();
+  ASSERT_FALSE(
+      TestUpdateCheck(nullptr,  // request_params
+                      fake_update_response_.GetUpdateResponse(),
+                      -1,
+                      false,  // ping_only
+                      ErrorCode::kNonCriticalUpdateInOOBE,
+                      metrics::CheckResult::kUnset,
+                      metrics::CheckReaction::kUnset,
+                      metrics::DownloadErrorCode::kUnset,
+                      &response,
+                      nullptr));
+  EXPECT_FALSE(response.update_exists);
+
+  fake_update_response_.deadline = "20101020";
+  ASSERT_TRUE(
+      TestUpdateCheck(nullptr,  // request_params
+                      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);
+}
+
 TEST_F(OmahaRequestActionTest, WallClockBasedWaitAloneCausesScattering) {
   OmahaResponse response;
   OmahaRequestParams params = request_params_;
@@ -2086,6 +2120,13 @@ bool OmahaRequestActionTest::InstallDateParseHelper(const string &elapsed_days,
 TEST_F(OmahaRequestActionTest, ParseInstallDateFromResponse) {
   OmahaResponse response;
 
+  // Simulate a successful update check that happens during OOBE.  The
+  // deadline in the response is needed to force the update attempt to
+  // occur; responses without a deadline seen during OOBE will normally
+  // return ErrorCode::kNonCriticalUpdateInOOBE.
+  fake_system_state_.fake_hardware()->UnsetIsOOBEComplete();
+  fake_update_response_.deadline = "20101020";
+
   // Check that we parse elapsed_days in the Omaha Response correctly.
   // and that the kPrefsInstallDateDays value is written to.
   EXPECT_FALSE(fake_prefs_.Exists(kPrefsInstallDateDays));
@@ -2123,6 +2164,7 @@ TEST_F(OmahaRequestActionTest, ParseInstallDateFromResponse) {
 // If there is no prefs and OOBE is not complete, we should not
 // report anything to Omaha.
 TEST_F(OmahaRequestActionTest, GetInstallDateWhenNoPrefsNorOOBE) {
+  fake_system_state_.fake_hardware()->UnsetIsOOBEComplete();
   EXPECT_EQ(OmahaRequestAction::GetInstallDate(&fake_system_state_), -1);
   EXPECT_FALSE(fake_prefs_.Exists(kPrefsInstallDateDays));
 }
diff --git a/payload_state.cc b/payload_state.cc
index d459d556..af1ad055 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -340,6 +340,7 @@ void PayloadState::UpdateFailed(ErrorCode error) {
     case ErrorCode::kOmahaResponseInvalid:
     case ErrorCode::kOmahaUpdateIgnoredPerPolicy:
     case ErrorCode::kOmahaUpdateDeferredPerPolicy:
+    case ErrorCode::kNonCriticalUpdateInOOBE:
     case ErrorCode::kOmahaUpdateDeferredForBackoff:
     case ErrorCode::kPostinstallPowerwashError:
     case ErrorCode::kUpdateCanceledByChannelChange:
diff --git a/update_manager/chromeos_policy.cc b/update_manager/chromeos_policy.cc
index fa321754..3fb680a8 100644
--- a/update_manager/chromeos_policy.cc
+++ b/update_manager/chromeos_policy.cc
@@ -125,6 +125,7 @@ bool HandleErrorCode(ErrorCode err_code, int* url_num_error_p) {
     case ErrorCode::kOmahaResponseInvalid:
     case ErrorCode::kOmahaUpdateIgnoredPerPolicy:
     case ErrorCode::kOmahaUpdateDeferredPerPolicy:
+    case ErrorCode::kNonCriticalUpdateInOOBE:
     case ErrorCode::kOmahaUpdateDeferredForBackoff:
     case ErrorCode::kPostinstallPowerwashError:
     case ErrorCode::kUpdateCanceledByChannelChange: