Fixed the timeout when state keys generation is late

The state keys are not generated as long as the time hasn't been
synchronized. These changes fix the case when the state keys generation
is late, possibly for the network missing reason, and the controller
hits the timeout.

The timeout can happen at establishing the ownership, or later at
state keys check. Without state keys check we cannot assume we can
skip FRE. If the ownership is established and FRE can be skipped
(OWNERSHIP_TAKEN or OWNERSHIP_UNKNOWN), the new state is set
immediately, so the timeout cannot trigger.

TEST=Manually tested on a falco chromebook.

BUG=673270, 645464

Review-Url: https://codereview.chromium.org/2587033002
Cr-Commit-Position: refs/heads/master@{#440748}

chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.cc

@@ -23,7 +23,7 @@ namespace {
 
 // Maximum time to wait before forcing a decision.  Note that download time for
 // state key buckets can be non-negligible, especially on 2G connections.
-const int kSafeguardTimeoutSeconds = 60;
+const int kSafeguardTimeoutSeconds = 90;
 
 // Returns the int value of the |switch_name| argument, clamped to the [0, 62]
 // interval. Returns 0 if the argument doesn't exist or isn't an int value.
@@ -52,23 +52,50 @@ int GetSanitizedArg(const std::string& switch_name) {
   return int_value;
 }
 
-// Determine whether the auto-enrollment check can be skipped. This is true when
-// kCheckEnrollmentKey VPD entry doesn't indicate the device as being enrolled
-// in the past. For backward compatibility with devices upgrading from an older
-// version of Chrome OS, the kActivateDateKey VPD entry should be missing too.
-// The requirement for the machine serial number to be present as well is a
-// sanity-check to ensure that the VPD has actually been read successfully.
-bool CanSkipFRE() {
+// Returns whether the auto-enrollment check is required. When
+// kCheckEnrollmentKey VPD entry is present, it is explicitly stating whether
+// the forced re-enrollment is required or not. Otherwise, for backward
+// compatibility with devices upgrading from an older version of Chrome OS, the
+// kActivateDateKey VPD entry is queried. If it's missing, FRE is not required.
+// This enables factories to start full guest sessions for testing, see
+// http://crbug.com/397354 for more context. The requirement for the machine
+// serial number to be present is a sanity-check to ensure that the VPD has
+// actually been read successfully. If VPD read failed, the FRE check is
+// required.
+AutoEnrollmentController::FRERequirement GetFRERequirement() {
+  std::string check_enrollment_value;
   system::StatisticsProvider* provider =
       system::StatisticsProvider::GetInstance();
-  std::string check_enrollment_value;
-  bool is_enrolled =
-      provider->GetMachineStatistic(
-          system::kCheckEnrollmentKey, &check_enrollment_value) &&
-      check_enrollment_value == "1";
-  return !provider->GetMachineStatistic(system::kActivateDateKey, nullptr) &&
-         !is_enrolled &&
-         !provider->GetEnterpriseMachineID().empty();
+  bool fre_flag_found = provider->GetMachineStatistic(
+      system::kCheckEnrollmentKey, &check_enrollment_value);
+
+  if (fre_flag_found) {
+    if (check_enrollment_value == "0")
+      return AutoEnrollmentController::EXPLICITLY_NOT_REQUIRED;
+    if (check_enrollment_value == "1")
+      return AutoEnrollmentController::EXPLICITLY_REQUIRED;
+  }
+  if (!provider->GetMachineStatistic(system::kActivateDateKey, nullptr) &&
+      !provider->GetEnterpriseMachineID().empty())
+    return AutoEnrollmentController::NOT_REQUIRED;
+  return AutoEnrollmentController::REQUIRED;
+}
+
+std::string FRERequirementToString(
+    AutoEnrollmentController::FRERequirement requirement) {
+  switch (requirement) {
+    case AutoEnrollmentController::REQUIRED:
+      return "Auto-enrollment required.";
+    case AutoEnrollmentController::NOT_REQUIRED:
+      return "Auto-enrollment disabled: first setup.";
+    case AutoEnrollmentController::EXPLICITLY_REQUIRED:
+      return "Auto-enrollment required: flag in VPD.";
+    case AutoEnrollmentController::EXPLICITLY_NOT_REQUIRED:
+      return "Auto-enrollment disabled: flag in VPD.";
+  }
+
+  NOTREACHED();
+  return std::string();
 }
 
 }  // namespace
@@ -136,10 +163,10 @@ void AutoEnrollmentController::Start() {
     return;
   }
 
-  // This enables factories to start full guest sessions for testing, see
-  // http://crbug.com/397354 for more context.
-  if (CanSkipFRE()) {
-    VLOG(1) << "Auto-enrollment disabled: first setup.";
+  fre_requirement_ = GetFRERequirement();
+  VLOG(1) << FRERequirementToString(fre_requirement_);
+  if (fre_requirement_ == EXPLICITLY_NOT_REQUIRED ||
+      fre_requirement_ == NOT_REQUIRED) {
     UpdateState(policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT);
     return;
   }
@@ -274,17 +301,17 @@ void AutoEnrollmentController::UpdateState(
 
 void AutoEnrollmentController::Timeout() {
   // TODO(mnissler): Add UMA to track results of auto-enrollment checks.
-  if (client_start_weak_factory_.HasWeakPtrs()) {
+  if (client_start_weak_factory_.HasWeakPtrs() &&
+      fre_requirement_ != EXPLICITLY_REQUIRED) {
     // If the callbacks to check ownership status or state keys are still
     // pending, there's a bug in the code running on the device. No use in
     // retrying anything, need to fix that bug.
     LOG(ERROR) << "Failed to start auto-enrollment check, fix the code!";
     UpdateState(policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT);
   } else {
-    // If the AutoEnrollmentClient didn't manage to come back with a result in
-    // time, blame it on the network. This can actually happen in some cases,
-    // for example when the server just doesn't reply and keeps the connection
-    // open.
+    // This can actually happen in some cases, for example when state key
+    // generation is waiting for time sync or the server just doesn't reply and
+    // keeps the connection open.
     LOG(ERROR) << "AutoEnrollmentClient didn't complete within time limit.";
     UpdateState(policy::AUTO_ENROLLMENT_STATE_CONNECTION_ERROR);
   }

chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.h

@@ -39,6 +39,20 @@ class AutoEnrollmentController {
     MODE_FORCED_RE_ENROLLMENT,
   };
 
+  // Requirement for forced re-enrollment check.
+  enum FRERequirement {
+    // The device was setup (has kActivateDateKey) but doesn't have the
+    // kCheckEnrollmentKey entry in VPD, or the VPD is corrupted.
+    REQUIRED,
+    // The device doesn't have kActivateDateKey, nor kCheckEnrollmentKey entry
+    // while the serial number has been successfully read from VPD.
+    NOT_REQUIRED,
+    // FRE check explicitly required by the flag in VPD.
+    EXPLICITLY_REQUIRED,
+    // FRE check to be skipped, explicitly stated by the flag in VPD.
+    EXPLICITLY_NOT_REQUIRED,
+  };
+
   // Gets the auto-enrollment mode based on command-line flags and official
   // build status.
   static Mode GetMode();
@@ -90,6 +104,9 @@ class AutoEnrollmentController {
   // http://crbug.com/433634 for background.
   base::Timer safeguard_timer_;
 
+  // Whether the forced re-enrollment check has to be applied.
+  FRERequirement fre_requirement_ = REQUIRED;
+
   base::WeakPtrFactory<AutoEnrollmentController> client_start_weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(AutoEnrollmentController);