Update enrollment OOBE screens result reporting

Stops using ScreenExitCode enums reported by enrollment screens (which
is part of a larger effort to deprecate ScreenExitCode enum all together).
Instead, defines result enums per screen when needed (i.e. if the screen
can report multiple values).

Updates how screens relay the exit code to WizardController - instead of
using BaseScreenDelegate::OnExit (indirectly though
BaseScreen::Finish), the screens now take a callback that is run when
the screen exits.

Affected screens:
  * EnrollmentScreen
  * AutoEnrollmentCheckScreen

Deprecated exit codes:
  * ENTERPRISE_ENROLLMENT_COMPLETED
  * ENTERPRISE_ENROLLMENT_BACK
  * ENTERPRISE_AUTO_ENROLLMENT_CHECK_COMPLETED

  BUG=930267

Change-Id: I2ade27b1a8d8a3b1e2a0993d76d3056c63feb61a
Reviewed-on: https://chromium-review.googlesource.com/c/1484079Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636198}

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

@@ -45,10 +45,12 @@ AutoEnrollmentCheckScreen* AutoEnrollmentCheckScreen::Get(
 
 AutoEnrollmentCheckScreen::AutoEnrollmentCheckScreen(
     BaseScreenDelegate* base_screen_delegate,
-    AutoEnrollmentCheckScreenView* view)
+    AutoEnrollmentCheckScreenView* view,
+    const base::RepeatingClosure& exit_callback)
     : BaseScreen(base_screen_delegate,
                  OobeScreen::SCREEN_AUTO_ENROLLMENT_CHECK),
       view_(view),
+      exit_callback_(exit_callback),
       auto_enrollment_controller_(nullptr),
       captive_portal_status_(
           NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_UNKNOWN),
@@ -246,13 +248,11 @@ void AutoEnrollmentCheckScreen::SignalCompletion() {
   auto_enrollment_progress_subscription_.reset();
   connect_request_subscription_.reset();
 
-  // Calling Finish() can cause |this| destruction, so let other methods finish
-  // their work before.
+  // Running exit callback can cause |this| destruction, so let other methods
+  // finish their work before.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          &AutoEnrollmentCheckScreen::Finish, weak_ptr_factory_.GetWeakPtr(),
-          ScreenExitCode::ENTERPRISE_AUTO_ENROLLMENT_CHECK_COMPLETED));
+      FROM_HERE, base::BindOnce(&AutoEnrollmentCheckScreen::RunExitCallback,
+                                weak_ptr_factory_.GetWeakPtr()));
 }
 
 bool AutoEnrollmentCheckScreen::IsCompleted() const {

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

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
@@ -34,7 +35,8 @@ class AutoEnrollmentCheckScreen
       public NetworkPortalDetector::Observer {
  public:
   AutoEnrollmentCheckScreen(BaseScreenDelegate* base_screen_delegate,
-                            AutoEnrollmentCheckScreenView* view);
+                            AutoEnrollmentCheckScreenView* view,
+                            const base::RepeatingClosure& exit_callback);
   ~AutoEnrollmentCheckScreen() override;
 
   static AutoEnrollmentCheckScreen* Get(ScreenManager* manager);
@@ -47,6 +49,10 @@ class AutoEnrollmentCheckScreen
     auto_enrollment_controller_ = auto_enrollment_controller;
   }
 
+  void set_exit_callback_for_testing(const base::RepeatingClosure& callback) {
+    exit_callback_ = callback;
+  }
+
   // BaseScreen implementation:
   void Show() override;
   void Hide() override;
@@ -59,6 +65,12 @@ class AutoEnrollmentCheckScreen
       const NetworkState* network,
       const NetworkPortalDetector::CaptivePortalState& state) override;
 
+ protected:
+  // Runs |exit_callback_| - used to prevent |exit_callback_| from running after
+  // |this| has been destroyed (by wrapping it with a callback bound to a weak
+  // ptr).
+  void RunExitCallback() { exit_callback_.Run(); }
+
  private:
   // Handles update notifications regarding the auto-enrollment check.
   void OnAutoEnrollmentCheckProgressed(policy::AutoEnrollmentState state);
@@ -97,6 +109,7 @@ class AutoEnrollmentCheckScreen
   bool ShouldBlockOnServerError() const;
 
   AutoEnrollmentCheckScreenView* view_;
+  base::RepeatingClosure exit_callback_;
   AutoEnrollmentController* auto_enrollment_controller_;
 
   std::unique_ptr<AutoEnrollmentController::ProgressCallbackList::Subscription>

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

@@ -94,9 +94,11 @@ EnrollmentScreen* EnrollmentScreen::Get(ScreenManager* manager) {
 }
 
 EnrollmentScreen::EnrollmentScreen(BaseScreenDelegate* base_screen_delegate,
-                                   EnrollmentScreenView* view)
+                                   EnrollmentScreenView* view,
+                                   const ScreenExitCallback& exit_callback)
     : BaseScreen(base_screen_delegate, OobeScreen::SCREEN_OOBE_ENROLLMENT),
       view_(view),
+      exit_callback_(exit_callback),
       weak_ptr_factory_(this) {
   retry_policy_.num_errors_to_ignore = 0;
   retry_policy_.initial_delay_ms = kInitialDelayMS;
@@ -308,16 +310,19 @@ void EnrollmentScreen::OnCancel() {
   if (authpolicy_login_helper_)
     authpolicy_login_helper_->CancelRequestsAndRestart();
 
-  const ScreenExitCode exit_code =
-      config_.is_forced() ? ScreenExitCode::ENTERPRISE_ENROLLMENT_BACK
-                          : ScreenExitCode::ENTERPRISE_ENROLLMENT_COMPLETED;
-  ClearAuth(
-      base::Bind(&EnrollmentScreen::Finish, base::Unretained(this), exit_code));
+  // The callback passed to ClearAuth is called either immediately or gets
+  // wrapped in a callback bound to a weak pointer from |weak_factory_| - in
+  // either case, passing exit_callback_ directly should be safe.
+  ClearAuth(base::BindRepeating(
+      exit_callback_, config_.is_forced() ? Result::BACK : Result::COMPLETED));
 }
 
 void EnrollmentScreen::OnConfirmationClosed() {
-  ClearAuth(base::Bind(&EnrollmentScreen::Finish, base::Unretained(this),
-                       ScreenExitCode::ENTERPRISE_ENROLLMENT_COMPLETED));
+  // The callback passed to ClearAuth is called either immediately or gets
+  // wrapped in a callback bound to a weak pointer from |weak_factory_| - in
+  // either case, passing exit_callback_ directly should be safe.
+  ClearAuth(base::BindRepeating(exit_callback_, Result::COMPLETED));
+
   // Restart browser to switch from DeviceCloudPolicyManagerChromeOS to
   // DeviceActiveDirectoryPolicyManager.
   if (g_browser_process->platform_part()

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

@@ -8,7 +8,7 @@
 #include <memory>
 #include <string>
 
-#include "base/callback_forward.h"
+#include "base/callback.h"
 #include "base/cancelable_callback.h"
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
@@ -45,8 +45,12 @@ class EnrollmentScreen
       public EnrollmentScreenView::Controller,
       public ActiveDirectoryJoinDelegate {
  public:
+  enum class Result { COMPLETED, BACK };
+
+  using ScreenExitCallback = base::RepeatingCallback<void(Result result)>;
   EnrollmentScreen(BaseScreenDelegate* base_screen_delegate,
-                   EnrollmentScreenView* view);
+                   EnrollmentScreenView* view,
+                   const ScreenExitCallback& exit_callback);
   ~EnrollmentScreen() override;
 
   static EnrollmentScreen* Get(ScreenManager* manager);
@@ -92,6 +96,14 @@ class EnrollmentScreen
   // Used for testing.
   EnrollmentScreenView* GetView() { return view_; }
 
+  void set_exit_callback_for_testing(const ScreenExitCallback& callback) {
+    exit_callback_ = callback;
+  }
+
+ protected:
+  // Expose the exit_callback to test screen overrides.
+  ScreenExitCallback* exit_callback() { return &exit_callback_; }
+
  private:
   friend class MultiLicenseEnrollmentScreenUnitTest;
   friend class ZeroTouchEnrollmentScreenUnitTest;
@@ -175,6 +187,7 @@ class EnrollmentScreen
                                const std::string& machine_domain);
 
   EnrollmentScreenView* view_;
+  ScreenExitCallback exit_callback_;
   policy::EnrollmentConfig config_;
   policy::EnrollmentConfig enrollment_config_;
   Auth current_auth_ = AUTH_OAUTH;

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

@@ -5,6 +5,7 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/run_loop.h"
 #include "chrome/browser/chromeos/login/enrollment/enrollment_screen.h"
 #include "chrome/browser/chromeos/login/enrollment/mock_enrollment_screen.h"
@@ -47,6 +48,8 @@ class EnrollmentScreenTest : public InProcessBrowserTest {
               enrollment_screen());
     static_cast<BaseScreen*>(enrollment_screen())->base_screen_delegate_ =
         &mock_base_screen_delegate_;
+    enrollment_screen()->set_exit_callback_for_testing(base::BindRepeating(
+        &EnrollmentScreenTest::HandleScreenExit, base::Unretained(this)));
   }
   void TearDownOnMainThread() override {
     static_cast<BaseScreen*>(enrollment_screen())->base_screen_delegate_ =
@@ -66,30 +69,51 @@ class EnrollmentScreenTest : public InProcessBrowserTest {
     return &mock_base_screen_delegate_;
   }
 
+  // Runs loop until the enrollment screen reports exit. It will return the
+  // last result returned by the enrollment screen.
+  // NOTE: This will not work if the screen is expected to be shown more than
+  // once - to support that use case, screen_result_ would have to be reset
+  // before showing another EnrollmentScreen.
+  EnrollmentScreen::Result WaitForScreenExit() {
+    if (screen_result_.has_value())
+      return screen_result_.value();
+    if (!screen_exit_waiter_)
+      screen_exit_waiter_ = std::make_unique<base::RunLoop>();
+    screen_exit_waiter_->Run();
+    return screen_result_.value();
+  }
+
  private:
+  void HandleScreenExit(EnrollmentScreen::Result result) {
+    EXPECT_FALSE(screen_result_.has_value());
+    screen_result_ = result;
+    if (screen_exit_waiter_)
+      screen_exit_waiter_->Quit();
+  }
+
   MockBaseScreenDelegate mock_base_screen_delegate_;
+  base::Optional<EnrollmentScreen::Result> screen_result_;
+
+  // Created lazily in WaitForScreenExit().
+  // Note: unique_ptr because RunLoops cannot be created without a thread
+  // context, which does not exist in the test ctor.
+  std::unique_ptr<base::RunLoop> screen_exit_waiter_;
 
   DISALLOW_COPY_AND_ASSIGN(EnrollmentScreenTest);
 };
 
 IN_PROC_BROWSER_TEST_F(EnrollmentScreenTest, TestCancel) {
-  base::RunLoop run_loop;
-  EXPECT_CALL(*mock_base_screen_delegate(),
-              OnExit(ScreenExitCode::ENTERPRISE_ENROLLMENT_COMPLETED))
-      .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   enrollment_screen()->OnCancel();
-  run_loop.Run();
+  EnrollmentScreen::Result screen_result = WaitForScreenExit();
+  EXPECT_EQ(EnrollmentScreen::Result::COMPLETED, screen_result);
 }
 
 IN_PROC_BROWSER_TEST_F(EnrollmentScreenTest, TestSuccess) {
   WizardController::SkipEnrollmentPromptsForTesting();
-  base::RunLoop run_loop;
-  EXPECT_FALSE(StartupUtils::IsDeviceRegistered());
-  EXPECT_CALL(*mock_base_screen_delegate(),
-              OnExit(ScreenExitCode::ENTERPRISE_ENROLLMENT_COMPLETED))
-      .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   enrollment_screen()->OnDeviceAttributeUpdatePermission(false /* granted */);
-  run_loop.Run();
+  EnrollmentScreen::Result screen_result = WaitForScreenExit();
+  EXPECT_EQ(EnrollmentScreen::Result::COMPLETED, screen_result);
+
   EXPECT_TRUE(StartupUtils::IsDeviceRegistered());
 }
 
@@ -109,14 +133,10 @@ class AttestationAuthEnrollmentScreenTest : public EnrollmentScreenTest {
 };
 
 IN_PROC_BROWSER_TEST_F(AttestationAuthEnrollmentScreenTest, TestCancel) {
-  base::RunLoop run_loop;
-
-  EXPECT_CALL(*mock_base_screen_delegate(),
-              OnExit(ScreenExitCode::ENTERPRISE_ENROLLMENT_COMPLETED))
-      .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   ASSERT_FALSE(enrollment_screen()->AdvanceToNextAuth());
   enrollment_screen()->OnCancel();
-  run_loop.Run();
+  EnrollmentScreen::Result screen_result = WaitForScreenExit();
+  EXPECT_EQ(EnrollmentScreen::Result::COMPLETED, screen_result);
 }
 
 IN_PROC_BROWSER_TEST_F(EnrollmentScreenTest, EnrollmentSpinner) {
@@ -161,13 +181,10 @@ class ForcedAttestationAuthEnrollmentScreenTest : public EnrollmentScreenTest {
 };
 
 IN_PROC_BROWSER_TEST_F(ForcedAttestationAuthEnrollmentScreenTest, TestCancel) {
-  base::RunLoop run_loop;
-  EXPECT_CALL(*mock_base_screen_delegate(),
-              OnExit(ScreenExitCode::ENTERPRISE_ENROLLMENT_BACK))
-      .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   ASSERT_FALSE(enrollment_screen()->AdvanceToNextAuth());
   enrollment_screen()->OnCancel();
-  run_loop.Run();
+  EnrollmentScreen::Result screen_result = WaitForScreenExit();
+  EXPECT_EQ(EnrollmentScreen::Result::BACK, screen_result);
 }
 
 class MultiAuthEnrollmentScreenTest : public EnrollmentScreenTest {
@@ -193,13 +210,10 @@ class MultiAuthEnrollmentScreenTest : public EnrollmentScreenTest {
 };
 
 IN_PROC_BROWSER_TEST_F(MultiAuthEnrollmentScreenTest, TestCancel) {
-  base::RunLoop run_loop;
-  EXPECT_CALL(*mock_base_screen_delegate(),
-              OnExit(ScreenExitCode::ENTERPRISE_ENROLLMENT_BACK))
-      .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   ASSERT_TRUE(enrollment_screen()->AdvanceToNextAuth());
   enrollment_screen()->OnCancel();
-  run_loop.Run();
+  EnrollmentScreen::Result screen_result = WaitForScreenExit();
+  EXPECT_EQ(EnrollmentScreen::Result::BACK, screen_result);
 }
 
 class ProvisionedEnrollmentScreenTest : public EnrollmentScreenTest {
@@ -223,12 +237,9 @@ class ProvisionedEnrollmentScreenTest : public EnrollmentScreenTest {
 };
 
 IN_PROC_BROWSER_TEST_F(ProvisionedEnrollmentScreenTest, TestBackButton) {
-  base::RunLoop run_loop;
-  EXPECT_CALL(*mock_base_screen_delegate(),
-              OnExit(ScreenExitCode::ENTERPRISE_ENROLLMENT_BACK))
-      .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   enrollment_screen()->OnCancel();
-  run_loop.Run();
+  EnrollmentScreen::Result screen_result = WaitForScreenExit();
+  EXPECT_EQ(EnrollmentScreen::Result::BACK, screen_result);
 }
 
 }  // namespace chromeos

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

@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
+#include "base/optional.h"
 #include "base/test/scoped_mock_time_message_loop_task_runner.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -35,8 +36,11 @@ class EnrollmentScreenUnitTest : public testing::Test {
 
   // Creates the EnrollmentScreen and sets required parameters.
   virtual void SetUpEnrollmentScreen() {
-    enrollment_screen_.reset(
-        new EnrollmentScreen(&mock_delegate_, &mock_view_));
+    enrollment_screen_ = std::make_unique<EnrollmentScreen>(
+        &mock_delegate_, &mock_view_,
+        base::BindRepeating(&EnrollmentScreenUnitTest::HandleScreenExit,
+                            base::Unretained(this)));
+
     enrollment_screen_->SetEnrollmentConfig(enrollment_config_);
   }
 
@@ -62,9 +66,17 @@ class EnrollmentScreenUnitTest : public testing::Test {
  protected:
   std::unique_ptr<EnrollmentScreen> enrollment_screen_;
 
+  // The last result reported by |enrollment_screen_|.
+  base::Optional<EnrollmentScreen::Result> last_screen_result_;
+
   policy::EnrollmentConfig enrollment_config_;
 
  private:
+  void HandleScreenExit(EnrollmentScreen::Result screen_result) {
+    EXPECT_FALSE(last_screen_result_.has_value());
+    last_screen_result_ = screen_result;
+  }
+
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   // Replace main thread's task runner with a mock for duration of test.
   base::ScopedMockTimeMessageLoopTaskRunner runner_;
@@ -180,16 +192,12 @@ class ZeroTouchEnrollmentScreenUnitTest : public EnrollmentScreenUnitTest {
 
     SetUpEnrollmentScreen();
 
-    // Set up expectation for BaseScreenDelegate::OnExit to be called
-    // with BaseScreenDelegate::ENTERPRISE_ENROLLMENT_COMPLETED
-    // This is how we check that the code finishes and cleanly exits
-    // the enterprise enrollment flow.
-    EXPECT_CALL(*GetBaseScreenDelegate(),
-                OnExit(ScreenExitCode::ENTERPRISE_ENROLLMENT_COMPLETED))
-        .Times(1);
-
     // Start zero-touch enrollment.
     enrollment_screen_->Show();
+
+    // Verify that enrollment flow finished and exited cleanly.
+    ASSERT_TRUE(last_screen_result_.has_value());
+    EXPECT_EQ(EnrollmentScreen::Result::COMPLETED, last_screen_result_.value());
   }
 
   void TestFallback() {

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

@@ -8,8 +8,9 @@ namespace chromeos {
 
 MockAutoEnrollmentCheckScreen::MockAutoEnrollmentCheckScreen(
     BaseScreenDelegate* base_screen_delegate,
-    AutoEnrollmentCheckScreenView* view)
-    : AutoEnrollmentCheckScreen(base_screen_delegate, view) {}
+    AutoEnrollmentCheckScreenView* view,
+    const base::RepeatingClosure& exit_callback)
+    : AutoEnrollmentCheckScreen(base_screen_delegate, view, exit_callback) {}
 
 MockAutoEnrollmentCheckScreen::~MockAutoEnrollmentCheckScreen() {}
 
@@ -17,6 +18,10 @@ void MockAutoEnrollmentCheckScreen::RealShow() {
   AutoEnrollmentCheckScreen::Show();
 }
 
+void MockAutoEnrollmentCheckScreen::ExitScreen() {
+  RunExitCallback();
+}
+
 MockAutoEnrollmentCheckScreenView::MockAutoEnrollmentCheckScreenView() =
     default;
 

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

@@ -14,13 +14,15 @@ namespace chromeos {
 class MockAutoEnrollmentCheckScreen : public AutoEnrollmentCheckScreen {
  public:
   MockAutoEnrollmentCheckScreen(BaseScreenDelegate* base_screen_delegate,
-                                AutoEnrollmentCheckScreenView* view);
+                                AutoEnrollmentCheckScreenView* view,
+                                const base::RepeatingClosure& exit_callback);
   ~MockAutoEnrollmentCheckScreen() override;
 
   MOCK_METHOD0(Show, void());
   MOCK_METHOD0(Hide, void());
 
   void RealShow();
+  void ExitScreen();
 };
 
 class MockAutoEnrollmentCheckScreenView : public AutoEnrollmentCheckScreenView {

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

@@ -8,8 +8,13 @@ namespace chromeos {
 
 MockEnrollmentScreen::MockEnrollmentScreen(
     BaseScreenDelegate* base_screen_delegate,
-    EnrollmentScreenView* view)
-    : EnrollmentScreen(base_screen_delegate, view) {}
+    EnrollmentScreenView* view,
+    const ScreenExitCallback& exit_callback)
+    : EnrollmentScreen(base_screen_delegate, view, exit_callback) {}
+
+void MockEnrollmentScreen::ExitScreen(Result screen_result) {
+  exit_callback()->Run(screen_result);
+}
 
 MockEnrollmentScreen::~MockEnrollmentScreen() {}
 

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

@@ -17,11 +17,14 @@ namespace chromeos {
 class MockEnrollmentScreen : public EnrollmentScreen {
  public:
   MockEnrollmentScreen(BaseScreenDelegate* base_screen_delegate,
-                       EnrollmentScreenView* view);
+                       EnrollmentScreenView* view,
+                       const ScreenExitCallback& exit_callback);
   ~MockEnrollmentScreen() override;
 
   MOCK_METHOD0(Show, void());
   MOCK_METHOD0(Hide, void());
+
+  void ExitScreen(Result result);
 };
 
 class MockEnrollmentScreenView : public EnrollmentScreenView {

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

@@ -30,11 +30,11 @@ std::string ExitCodeToString(ScreenExitCode code) {
       return "EULA_ACCEPTED";
     case ScreenExitCode::EULA_BACK:
       return "EULA_BACK";
-    case ScreenExitCode::ENTERPRISE_AUTO_ENROLLMENT_CHECK_COMPLETED:
+    case ScreenExitCode::DEPRECATED_ENTERPRISE_AUTO_ENROLLMENT_CHECK_COMPLETED:
       return "ENTERPRISE_AUTO_ENROLLMENT_CHECK_COMPLETED";
-    case ScreenExitCode::ENTERPRISE_ENROLLMENT_COMPLETED:
+    case ScreenExitCode::DEPRECATED_ENTERPRISE_ENROLLMENT_COMPLETED:
       return "ENTERPRISE_ENROLLMENT_COMPLETED";
-    case ScreenExitCode::ENTERPRISE_ENROLLMENT_BACK:
+    case ScreenExitCode::DEPRECATED_ENTERPRISE_ENROLLMENT_BACK:
       return "ENTERPRISE_ENROLLMENT_BACK";
     case ScreenExitCode::RESET_CANCELED:
       return "RESET_CANCELED";

chrome/browser/chromeos/login/screens/screen_exit_code.h

@@ -34,9 +34,9 @@ enum class ScreenExitCode {
   USER_IMAGE_SELECTED = 7,
   EULA_ACCEPTED = 8,
   EULA_BACK = 9,
-  ENTERPRISE_AUTO_ENROLLMENT_CHECK_COMPLETED = 10,
-  ENTERPRISE_ENROLLMENT_COMPLETED = 11,
-  ENTERPRISE_ENROLLMENT_BACK = 12,
+  DEPRECATED_ENTERPRISE_AUTO_ENROLLMENT_CHECK_COMPLETED = 10,
+  DEPRECATED_ENTERPRISE_ENROLLMENT_COMPLETED = 11,
+  DEPRECATED_ENTERPRISE_ENROLLMENT_BACK = 12,
   RESET_CANCELED = 13,
   KIOSK_AUTOLAUNCH_CANCELED = 14,
   KIOSK_AUTOLAUNCH_CONFIRMED = 15,

chrome/browser/chromeos/login/wizard_controller.cc

@@ -383,7 +383,9 @@ std::unique_ptr<BaseScreen> WizardController::CreateScreen(OobeScreen screen) {
     return std::make_unique<EulaScreen>(this, this, oobe_ui->GetEulaView());
   } else if (screen == OobeScreen::SCREEN_OOBE_ENROLLMENT) {
     return std::make_unique<EnrollmentScreen>(
-        this, oobe_ui->GetEnrollmentScreenView());
+        this, oobe_ui->GetEnrollmentScreenView(),
+        base::BindRepeating(&WizardController::OnEnrollmentScreenExit,
+                            weak_factory_.GetWeakPtr()));
   } else if (screen == OobeScreen::SCREEN_OOBE_RESET) {
     return std::make_unique<chromeos::ResetScreen>(this,
                                                    oobe_ui->GetResetView());
@@ -425,7 +427,9 @@ std::unique_ptr<BaseScreen> WizardController::CreateScreen(OobeScreen screen) {
         this, oobe_ui->GetHIDDetectionView());
   } else if (screen == OobeScreen::SCREEN_AUTO_ENROLLMENT_CHECK) {
     return std::make_unique<AutoEnrollmentCheckScreen>(
-        this, oobe_ui->GetAutoEnrollmentCheckScreenView());
+        this, oobe_ui->GetAutoEnrollmentCheckScreenView(),
+        base::BindRepeating(&WizardController::OnAutoEnrollmentCheckScreenExit,
+                            weak_factory_.GetWeakPtr()));
   } else if (screen == OobeScreen::SCREEN_DEVICE_DISABLED) {
     return std::make_unique<DeviceDisabledScreen>(
         this, oobe_ui->GetDeviceDisabledScreenView());
@@ -711,7 +715,9 @@ void WizardController::SkipUpdateEnrollAfterEula() {
 }
 
 void WizardController::OnScreenExit(OobeScreen screen) {
-  DCHECK_EQ(screen, current_screen_->screen_id());
+  DCHECK(current_screen_->screen_id() == screen ||
+         (current_screen_->screen_id() == OobeScreen::SCREEN_ERROR_MESSAGE &&
+          previous_screen_->screen_id() == screen));
 
   if (IsOOBEStepToTrack(screen)) {
     RecordUMAHistogramForOOBEStepCompletionTime(
@@ -745,6 +751,54 @@ void WizardController::OnUpdateCompleted() {
   ShowAutoEnrollmentCheckScreen();
 }
 
+void WizardController::OnAutoEnrollmentCheckScreenExit() {
+  VLOG(1) << "Auto enrollment check screen exit.";
+  OnScreenExit(OobeScreen::SCREEN_AUTO_ENROLLMENT_CHECK);
+
+  // Check whether the device is disabled. OnDeviceDisabledChecked() will be
+  // invoked when the result of this check is known. Until then, the current
+  // screen will remain visible and will continue showing a spinner.
+  g_browser_process->platform_part()
+      ->device_disabling_manager()
+      ->CheckWhetherDeviceDisabledDuringOOBE(
+          base::BindRepeating(&WizardController::OnDeviceDisabledChecked,
+                              weak_factory_.GetWeakPtr()));
+}
+
+void WizardController::OnEnrollmentScreenExit(EnrollmentScreen::Result result) {
+  VLOG(1) << "Enrollment screen exit: " << static_cast<int>(result);
+  OnScreenExit(OobeScreen::SCREEN_OOBE_ENROLLMENT);
+
+  switch (result) {
+    case EnrollmentScreen::Result::COMPLETED:
+      OnEnrollmentDone();
+      break;
+    case EnrollmentScreen::Result::BACK:
+      retry_auto_enrollment_check_ = true;
+      ShowAutoEnrollmentCheckScreen();
+      break;
+  }
+}
+
+void WizardController::OnEnrollmentDone() {
+  PerformOOBECompletedActions();
+
+  // Restart to make the login page pick up the policy changes resulting from
+  // enrollment recovery.  (Not pretty, but this codepath is rarely exercised.)
+  if (prescribed_enrollment_config_.mode ==
+          policy::EnrollmentConfig::MODE_RECOVERY ||
+      prescribed_enrollment_config_.mode ==
+          policy::EnrollmentConfig::MODE_ENROLLED_ROLLBACK) {
+    chrome::AttemptRestart();
+    return;
+  }
+
+  if (KioskAppManager::Get()->IsAutoLaunchEnabled())
+    AutoLaunchKioskApp();
+  else
+    ShowLoginScreen(LoginScreenContext());
+}
+
 void WizardController::OnHIDDetectionCompleted() {
   // Check for tests configuration.
   if (!StartupUtils::IsOobeCompleted())
@@ -855,26 +909,6 @@ void WizardController::OnUserImageSelected() {
   OnOobeFlowFinished();
 }
 
-void WizardController::OnEnrollmentDone() {
-  PerformOOBECompletedActions();
-
-  // Restart to make the login page pick up the policy changes resulting from
-  // enrollment recovery.  (Not pretty, but this codepath is rarely exercised.)
-  if (prescribed_enrollment_config_.mode ==
-          policy::EnrollmentConfig::MODE_RECOVERY ||
-      prescribed_enrollment_config_.mode ==
-          policy::EnrollmentConfig::MODE_ENROLLED_ROLLBACK) {
-    chrome::AttemptRestart();
-  }
-
-  // TODO(mnissler): Unify the logic for auto-login for Public Sessions and
-  // Kiosk Apps and make this code cover both cases: http://crbug.com/234694.
-  if (KioskAppManager::Get()->IsAutoLaunchEnabled())
-    AutoLaunchKioskApp();
-  else
-    ShowLoginScreen(LoginScreenContext());
-}
-
 void WizardController::OnDeviceModificationCanceled() {
   if (previous_screen_) {
     SetCurrentScreen(previous_screen_);
@@ -996,17 +1030,6 @@ void WizardController::OnMultiDeviceSetupFinished() {
   OnOobeFlowFinished();
 }
 
-void WizardController::OnAutoEnrollmentCheckCompleted() {
-  // Check whether the device is disabled. OnDeviceDisabledChecked() will be
-  // invoked when the result of this check is known. Until then, the current
-  // screen will remain visible and will continue showing a spinner.
-  g_browser_process->platform_part()
-      ->device_disabling_manager()
-      ->CheckWhetherDeviceDisabledDuringOOBE(
-          base::Bind(&WizardController::OnDeviceDisabledChecked,
-                     weak_factory_.GetWeakPtr()));
-}
-
 void WizardController::OnDemoSetupFinished() {
   DCHECK(demo_setup_controller_);
   demo_setup_controller_.reset();
@@ -1415,16 +1438,6 @@ void WizardController::OnExit(ScreenExitCode exit_code) {
     case ScreenExitCode::ENABLE_DEBUGGING_FINISHED:
       OnDeviceModificationCanceled();
       break;
-    case ScreenExitCode::ENTERPRISE_AUTO_ENROLLMENT_CHECK_COMPLETED:
-      OnAutoEnrollmentCheckCompleted();
-      break;
-    case ScreenExitCode::ENTERPRISE_ENROLLMENT_COMPLETED:
-      OnEnrollmentDone();
-      break;
-    case ScreenExitCode::ENTERPRISE_ENROLLMENT_BACK:
-      retry_auto_enrollment_check_ = true;
-      ShowAutoEnrollmentCheckScreen();
-      break;
     case ScreenExitCode::RESET_CANCELED:
       OnDeviceModificationCanceled();
       break;

chrome/browser/chromeos/login/wizard_controller.h

@@ -20,6 +20,7 @@
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
 #include "chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.h"
+#include "chrome/browser/chromeos/login/enrollment/enrollment_screen.h"
 #include "chrome/browser/chromeos/login/screen_manager.h"
 #include "chrome/browser/chromeos/login/screens/base_screen_delegate.h"
 #include "chrome/browser/chromeos/login/screens/eula_screen.h"
@@ -197,6 +198,9 @@ class WizardController : public BaseScreenDelegate,
   // Exit handlers:
   void OnUpdateScreenExit(UpdateScreen::Result result);
   void OnUpdateCompleted();
+  void OnAutoEnrollmentCheckScreenExit();
+  void OnEnrollmentScreenExit(EnrollmentScreen::Result result);
+  void OnEnrollmentDone();
   void OnHIDDetectionCompleted();
   void OnWelcomeContinued();
   void OnNetworkBack();
@@ -206,7 +210,6 @@ class WizardController : public BaseScreenDelegate,
   void OnEulaAccepted();
   void OnEulaBack();
   void OnUserImageSelected();
-  void OnEnrollmentDone();
   void OnDeviceModificationCanceled();
   void OnKioskAutolaunchCanceled();
   void OnKioskAutolaunchConfirmed();
@@ -223,7 +226,6 @@ class WizardController : public BaseScreenDelegate,
   void OnRecommendAppsSkipped();
   void OnRecommendAppsSelected();
   void OnAppDownloadingFinished();
-  void OnAutoEnrollmentCheckCompleted();
   void OnDemoSetupFinished();
   void OnDemoSetupCanceled();
   void OnDemoPreferencesContinued();