Make Hands-Off Zero-Touch Enrollment compatibile with tests

chrome/browser/chromeos/login/screens/update_screen.cc

Index: chrome/browser/chromeos/login/screens/update_screen.cc
diff --git a/chrome/browser/chromeos/login/screens/update_screen.cc b/chrome/browser/chromeos/login/screens/update_screen.cc
index 96a797ed24a134e20f9afe24d7822205b133bc7b..a5abc13d7a82e278733bba1ecfb059fb3a847b70 100644
--- a/chrome/browser/chromeos/login/screens/update_screen.cc
+++ b/chrome/browser/chromeos/login/screens/update_screen.cc
@@ -201,6 +201,9 @@ void UpdateScreen::ExitUpdate(UpdateScreen::ExitReason reason) {
         // Noncritical update, just exit screen as if there is no update.
         // no break
         case UpdateEngineClient::UPDATE_STATUS_IDLE:
+          // At this point, there is either no update available, or there is
+          // a noncritical update available. No need to update during OOBE.

[xiyuan, 2017/05/10 16:29:19]

nit: "No need to update during OOBE." -> "Record the time to block OOBE update
for sometime."

[kumarniranjan, 2017/05/13 04:42:36]

Done.

+          RecordUpdateCheckWithoutUpdate();
           Finish(ScreenExitCode::UPDATE_NOUPDATE);
           break;
         case UpdateEngineClient::UPDATE_STATUS_ERROR:
@@ -552,11 +555,18 @@ void UpdateScreen::StartUpdateCheck() {
   connect_request_subscription_.reset();
   if (state_ == State::STATE_ERROR)
     HideErrorMessage();
-  state_ = State::STATE_UPDATE;
-  DBusThreadManager::Get()->GetUpdateEngineClient()->AddObserver(this);
-  VLOG(1) << "Initiate update check";
-  DBusThreadManager::Get()->GetUpdateEngineClient()->RequestUpdateCheck(
-      base::Bind(StartUpdateCallback, this));
+
+  if (ShouldCheckForUpdate()) {
+    state_ = State::STATE_UPDATE;
+    DBusThreadManager::Get()->GetUpdateEngineClient()->AddObserver(this);
+    VLOG(1) << "Initiate update check";
+    DBusThreadManager::Get()->GetUpdateEngineClient()->RequestUpdateCheck(
+        base::Bind(StartUpdateCallback, this));
+  } else {
+    LOG(WARNING) << "Skipping update check since one was done recently "
+                    "which did not result in an update.";
+    CancelUpdate();
+  }
 }
 
 void UpdateScreen::ShowErrorMessage() {
@@ -627,4 +637,28 @@ void UpdateScreen::OnConnectRequested() {
   }
 }
 
+void UpdateScreen::RecordUpdateCheckWithoutUpdate() {
+  StartupUtils::SaveTimeOfLastUpdateCheckWithoutUpdate(base::Time::Now());
+}
+
+bool UpdateScreen::ShouldCheckForUpdate() {
+  // If we check for an update and there is no need to perform an update,
+  // this is the time in hours we should wait before checking again.
+  const int kUpdateCheckRecencyThresholdInHours = 1;

[xiyuan, 2017/05/10 16:29:19]

nit: since we are in a function scoped, we can use base::TimeDelta directly.

i.e.
  const base::TimeDelta kUpdateCheckRecencyThreshold
      = base::TimeDelta::FromHours(1);

[kumarniranjan, 2017/05/13 04:42:35]

Done.

+
+  base::Time now = base::Time::Now();
+  base::Time last = StartupUtils::GetTimeOfLastUpdateCheckWithoutUpdate();
+
+  if (WizardController::UsingHandsOffEnrollment()) {
+    if (now > last) {
+      return (now - last) >
+             base::TimeDelta::FromHours(kUpdateCheckRecencyThresholdInHours);
+    } else {
+      return false;

[xiyuan, 2017/05/10 16:29:19]

I think we should "return true" to allow update check if time goes backward. The
system might not be in a good shape when it happens. So we would want update
check to happen.

[kumarniranjan, 2017/05/13 04:42:35]

Done.

+    }
+  } else {
+    return true;

[xiyuan, 2017/05/10 16:29:20]

nit: move this to the beginning of the function and bail out earlier.

i.e.

bool UpdateScreen::ShouldCheckForUpdate() {
  // Always run update check for non hands-off enrollment.
  if (!WizardController::UsingHandsOffEnrollment())
    return true;
  
  ...
}

[kumarniranjan, 2017/05/13 04:42:35]

Done.

+  }
+}
+
 }  // namespace chromeos