Remove dev screen for choosing online/offline setup and run final

demo mode setup flow.

* Change DemoSetupController delegate into callback passed to Enroll()
* Pass info about enrollment type separately from Enroll() request
* Store DemoSetupController instance in WizardController instead of
  DemoSetupScreen for easier storage and access to demo setup flow status

Change-Id: I43f2f28dfc2daea519849d222ea5a73561522516
Bug: 862445
Test: Run DemoSetupTest, DemoSetupControllerTest and wizard_controller_browsertest.
Reviewed-on: https://chromium-review.googlesource.com/1155152
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580937}

chrome/browser/chromeos/arc/arc_util.cc

@@ -21,6 +21,7 @@
 #include "base/threading/thread_restrictions.h"
 #include "chrome/browser/chromeos/arc/arc_session_manager.h"
 #include "chrome/browser/chromeos/arc/policy/arc_policy_util.h"
+#include "chrome/browser/chromeos/login/demo_mode/demo_setup_controller.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/login/user_flow.h"
 #include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
@@ -553,14 +554,7 @@ bool IsArcStatsReportingEnabled() {
 }
 
 bool IsArcDemoModeSetupFlow() {
-  chromeos::LoginDisplayHost* const host =
-      chromeos::LoginDisplayHost::default_host();
-  if (!host)
-    return false;
-
-  const chromeos::WizardController* const wizard_controller =
-      host->GetWizardController();
-  return wizard_controller && wizard_controller->is_in_demo_mode_setup_flow();
+  return chromeos::DemoSetupController::IsOobeDemoSetupFlowInProgress();
 }
 
 void UpdateArcFileSystemCompatibilityPrefIfNeeded(

chrome/browser/chromeos/login/demo_mode/demo_setup_controller.cc

-// Copyright (c) 2018 The Chromium Authors. All rights reserved.
+// Copyright 2018 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/login/demo_mode/demo_setup_controller.h"
 
-#include <string>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
@@ -26,6 +26,11 @@ constexpr char kOfflineDevicePolicyFileName[] = "device_policy";
 constexpr char kOfflineDeviceLocalAccountPolicyFileName[] =
     "local_account_policy";
 
+// The policy blob data for offline demo-mode is embedded into the filesystem.
+// TODO(mukai, agawronska): fix this when switching to dm-verity image.
+constexpr const base::FilePath::CharType kOfflineDemoModeDir[] =
+    FILE_PATH_LITERAL("/usr/share/chromeos-assets/demo_mode_resources/policy");
+
 bool CheckOfflinePolicyFilesExist(const base::FilePath& policy_dir,
                                   std::string* message) {
   base::FilePath device_policy_path =
@@ -93,22 +98,48 @@ constexpr char DemoSetupController::kDemoModeDomain[];
 bool DemoSetupController::IsOobeDemoSetupFlowInProgress() {
   const WizardController* const wizard_controller =
       WizardController::default_controller();
-  return wizard_controller && wizard_controller->is_in_demo_mode_setup_flow();
+  return wizard_controller &&
+         wizard_controller->demo_setup_controller() != nullptr;
 }
 
-DemoSetupController::DemoSetupController(Delegate* delegate)
-    : delegate_(delegate), weak_ptr_factory_(this) {
-  DCHECK(delegate_);
-}
+DemoSetupController::DemoSetupController() : weak_ptr_factory_(this) {}
 
 DemoSetupController::~DemoSetupController() {
   if (device_local_account_policy_store_)
     device_local_account_policy_store_->RemoveObserver(this);
 }
 
-void DemoSetupController::EnrollOnline() {
+bool DemoSetupController::IsOfflineEnrollment() const {
+  return enrollment_type_ && *enrollment_type_ == EnrollmentType::kOffline;
+}
+
+void DemoSetupController::Enroll(OnSetupSuccess on_setup_success,
+                                 OnSetupError on_setup_error) {
+  DCHECK(enrollment_type_)
+      << "Enrollment type needs to be explicitly set before calling Enroll()";
   DCHECK_EQ(mode_, policy::EnrollmentConfig::MODE_NONE);
   DCHECK(!enrollment_helper_);
+
+  on_setup_success_ = std::move(on_setup_success);
+  on_setup_error_ = std::move(on_setup_error);
+
+  switch (*enrollment_type_) {
+    case EnrollmentType::kOnline:
+      EnrollOnline();
+      return;
+    case EnrollmentType::kOffline: {
+      const base::FilePath offline_data_dir =
+          policy_dir_for_tests_.empty() ? base::FilePath(kOfflineDemoModeDir)
+                                        : policy_dir_for_tests_;
+      EnrollOffline(offline_data_dir);
+      return;
+    }
+    default:
+      NOTREACHED() << "Unknown demo mode enrollment type";
+  }
+}
+
+void DemoSetupController::EnrollOnline() {
   policy::BrowserPolicyConnectorChromeOS* connector =
       g_browser_process->platform_part()->browser_policy_connector_chromeos();
   connector->GetDeviceCloudPolicyManager()->SetDeviceRequisition(
@@ -124,8 +155,6 @@ void DemoSetupController::EnrollOnline() {
 }
 
 void DemoSetupController::EnrollOffline(const base::FilePath& policy_dir) {
-  DCHECK_EQ(mode_, policy::EnrollmentConfig::MODE_NONE);
-  DCHECK(!enrollment_helper_);
   DCHECK(policy_dir_.empty());
   policy_dir_ = policy_dir;
   mode_ = policy::EnrollmentConfig::MODE_OFFLINE_DEMO;
@@ -145,7 +174,7 @@ void DemoSetupController::OnOfflinePolicyFilesExisted(std::string* message,
   DCHECK(!policy_dir_.empty());
 
   if (!ok) {
-    SetupFailed(*message, false);
+    SetupFailed(*message, DemoSetupError::kRecoverable);
     return;
   }
 
@@ -172,12 +201,13 @@ void DemoSetupController::OnEnrollmentError(policy::EnrollmentStatus status) {
           "validation_status: %d lock_status: %d",
           status.status(), status.client_status(), status.store_status(),
           status.validation_status(), status.lock_status()),
-      false);
+      DemoSetupError::kRecoverable);
 }
 
 void DemoSetupController::OnOtherError(
     EnterpriseEnrollmentHelper::OtherError error) {
-  SetupFailed(base::StringPrintf("Other error: %d", error), false);
+  SetupFailed(base::StringPrintf("Other error: %d", error),
+              DemoSetupError::kRecoverable);
 }
 
 void DemoSetupController::OnDeviceEnrolled(
@@ -200,7 +230,8 @@ void DemoSetupController::OnDeviceEnrolled(
     return;
   }
   Reset();
-  delegate_->OnSetupSuccess();
+  if (!on_setup_success_.is_null())
+    std::move(on_setup_success_).Run();
 }
 
 void DemoSetupController::OnMultipleLicensesAvailable(
@@ -221,18 +252,25 @@ void DemoSetupController::SetDeviceLocalAccountPolicyStoreForTest(
   device_local_account_policy_store_ = store;
 }
 
+void DemoSetupController::SetOfflineDataDirForTest(
+    const base::FilePath& offline_dir) {
+  policy_dir_for_tests_ = offline_dir;
+}
+
 void DemoSetupController::OnDeviceLocalAccountPolicyLoaded(
     base::Optional<std::string> blob) {
   if (!blob.has_value()) {
     // This is very unlikely to happen since the file existence is already
     // checked as CheckOfflinePolicyFilesExist.
-    SetupFailed("Policy file for the device local account not found", true);
+    SetupFailed("Policy file for the device local account not found",
+                DemoSetupError::kFatal);
     return;
   }
 
   enterprise_management::PolicyFetchResponse policy;
   if (!policy.ParseFromString(blob.value())) {
-    SetupFailed("Error parsing local account policy blob.", true);
+    SetupFailed("Error parsing local account policy blob.",
+                DemoSetupError::kFatal);
     return;
   }
 
@@ -240,7 +278,8 @@ void DemoSetupController::OnDeviceLocalAccountPolicyLoaded(
   enterprise_management::PolicyData policy_data;
   if (policy.policy_data().empty() ||
       !policy_data.ParseFromString(policy.policy_data())) {
-    SetupFailed("Error parsing local account policy data.", true);
+    SetupFailed("Error parsing local account policy data.",
+                DemoSetupError::kFatal);
     return;
   }
 
@@ -252,17 +291,20 @@ void DemoSetupController::OnDeviceLocalAccountPolicyLoaded(
   }
 
   if (!device_local_account_policy_store_) {
-    SetupFailed("Can't find the store for the local account policy.", true);
+    SetupFailed("Can't find the store for the local account policy.",
+                DemoSetupError::kFatal);
     return;
   }
   device_local_account_policy_store_->AddObserver(this);
   device_local_account_policy_store_->Store(policy);
 }
 
-void DemoSetupController::SetupFailed(const std::string& message, bool fatal) {
+void DemoSetupController::SetupFailed(const std::string& message,
+                                      DemoSetupError error) {
   Reset();
-  LOG(ERROR) << message << " fatal=" << fatal;
-  delegate_->OnSetupError(fatal);
+  LOG(ERROR) << message << " fatal=" << (error == DemoSetupError::kFatal);
+  if (!on_setup_error_.is_null())
+    std::move(on_setup_error_).Run(error);
 }
 
 void DemoSetupController::Reset() {
@@ -281,12 +323,14 @@ void DemoSetupController::Reset() {
 void DemoSetupController::OnStoreLoaded(policy::CloudPolicyStore* store) {
   DCHECK_EQ(store, device_local_account_policy_store_);
   Reset();
-  delegate_->OnSetupSuccess();
+  if (!on_setup_success_.is_null())
+    std::move(on_setup_success_).Run();
 }
 
 void DemoSetupController::OnStoreError(policy::CloudPolicyStore* store) {
   DCHECK_EQ(store, device_local_account_policy_store_);
-  SetupFailed("Failed to store the local account policy", true);
+  SetupFailed("Failed to store the local account policy",
+              DemoSetupError::kFatal);
 }
 
 }  //  namespace chromeos

chrome/browser/chromeos/login/demo_mode/demo_setup_controller.h

@@ -5,6 +5,9 @@
 #ifndef CHROME_BROWSER_CHROMEOS_LOGIN_DEMO_MODE_DEMO_SETUP_CONTROLLER_H_
 #define CHROME_BROWSER_CHROMEOS_LOGIN_DEMO_MODE_DEMO_SETUP_CONTROLLER_H_
 
+#include <string>
+
+#include "base/callback_forward.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
@@ -20,38 +23,54 @@ class DemoSetupController
     : public EnterpriseEnrollmentHelper::EnrollmentStatusConsumer,
       public policy::CloudPolicyStore::Observer {
  public:
-  // Domain that demo mode devices are enrolled into.
-  static constexpr char kDemoModeDomain[] = "cros-demo-mode.com";
+  // Type of demo mode enrollment.
+  enum class EnrollmentType {
+    // Online enrollment into demo mode that is established with DMServer.
+    kOnline,
+    // Offline enrollment into demo mode that is established locally.
+    kOffline,
+  };
 
-  // Delegate that will be notified about result of setup flow when it is
-  // finished.
-  class Delegate {
-   public:
-    virtual ~Delegate() = default;
+  // Type of demo mode setup error.
+  enum class DemoSetupError {
+    // Recoverable or temporary demo mode setup error. Another attempt to setup
+    // demo mode may succeed.
+    kRecoverable,
+    // Fatal demo mode setup error. Device requires powerwash to recover from
+    // the resulting state.
+    kFatal,
+  };
 
-    // Called when the setup flow finished with error. |fatal| is true if the
-    // error isn't recoverable and needs powerwash.
-    virtual void OnSetupError(bool fatal) = 0;
+  // Demo mode setup callbacks.
+  using OnSetupSuccess = base::OnceClosure;
+  using OnSetupError = base::OnceCallback<void(DemoSetupError)>;
 
-    // Called when the setup flow finished successfully.
-    virtual void OnSetupSuccess() = 0;
-  };
+  // Domain that demo mode devices are enrolled into.
+  static constexpr char kDemoModeDomain[] = "cros-demo-mode.com";
 
-  // Returns whether demo mode setup flow is in progress in OOBE.
+  // Utility method that returns whether demo mode setup flow is in progress in
+  // OOBE.
   static bool IsOobeDemoSetupFlowInProgress();
 
-  explicit DemoSetupController(Delegate* delegate);
+  DemoSetupController();
   ~DemoSetupController() override;
 
-  // Initiates online enrollment that registers and sets up the device in the
-  // Demo Mode domain.
-  void EnrollOnline();
+  // Sets enrollment type that will be used to setup the device. It has to be
+  // set before calling Enroll().
+  void set_enrollment_type(EnrollmentType enrollment_type) {
+    enrollment_type_ = enrollment_type;
+  }
 
-  // Initiates offline enrollment that locks the device and sets up offline
-  // policies required by Demo Mode. It requires no network connectivity since
-  // and all setup will be done locally. The policy files will be loaded
-  // from the |base_path|.
-  void EnrollOffline(const base::FilePath& base_path);
+  // Whether offline enrollment is used for setup.
+  bool IsOfflineEnrollment() const;
+
+  // Initiates enrollment that sets up the device in the demo mode domain. The
+  // |enrollment_type_| determines whether online or offline setup will be
+  // performed and it should be set with set_enrollment_type() before calling
+  // Enroll(). |on_setup_success| will be called when enrollment finishes
+  // successfully. |on_setup_error| will be called when enrollment finishes with
+  // an error.
+  void Enroll(OnSetupSuccess on_setup_success, OnSetupError on_setup_error);
 
   // EnterpriseEnrollmentHelper::EnrollmentStatusConsumer:
   void OnDeviceEnrolled(const std::string& additional_token) override;
@@ -64,8 +83,19 @@ class DemoSetupController
       const EnrollmentLicenseMap& licenses) override;
 
   void SetDeviceLocalAccountPolicyStoreForTest(policy::CloudPolicyStore* store);
+  void SetOfflineDataDirForTest(const base::FilePath& offline_dir);
 
  private:
+  // Initiates online enrollment that registers and sets up the device in the
+  // demo mode domain.
+  void EnrollOnline();
+
+  // Initiates offline enrollment that locks the device and sets up offline
+  // policies required by demo mode. It requires no network connectivity since
+  // all setup will be done locally. The policy files will be loaded from the
+  // |policy_dir|.
+  void EnrollOffline(const base::FilePath& policy_dir);
+
   // Called when the checks of policy files for the offline demo mode is done.
   void OnOfflinePolicyFilesExisted(std::string* message, bool ok);
 
@@ -74,7 +104,7 @@ class DemoSetupController
   void OnDeviceLocalAccountPolicyLoaded(base::Optional<std::string> blob);
 
   // Finish the flow with an error message.
-  void SetupFailed(const std::string& message, bool fatal);
+  void SetupFailed(const std::string& message, DemoSetupError error);
 
   // Clears the internal state.
   void Reset();
@@ -83,7 +113,15 @@ class DemoSetupController
   void OnStoreLoaded(policy::CloudPolicyStore* store) override;
   void OnStoreError(policy::CloudPolicyStore* store) override;
 
-  Delegate* delegate_ = nullptr;
+  // Enrollment type that will be performed when Enroll() is called. Should be
+  // set explicitly.
+  base::Optional<EnrollmentType> enrollment_type_;
+
+  // Callback to call when enrollment finishes with an error.
+  OnSetupError on_setup_error_;
+
+  // Callback to call when enrollment finishes successfully.
+  OnSetupSuccess on_setup_success_;
 
   // The mode of the current enrollment flow.
   policy::EnrollmentConfig::Mode mode_ = policy::EnrollmentConfig::MODE_NONE;
@@ -93,6 +131,9 @@ class DemoSetupController
   // on the online enrollment.
   base::FilePath policy_dir_;
 
+  // The directory containing policy blob files used for testing.
+  base::FilePath policy_dir_for_tests_;
+
   // The CloudPolicyStore for the device local account for the offline policy.
   policy::CloudPolicyStore* device_local_account_policy_store_ = nullptr;
 

chrome/browser/chromeos/login/demo_mode/demo_setup_test_utils.h

@@ -7,10 +7,14 @@
 
 #include <string>
 
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
 #include "chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper.h"
 #include "chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_mock.h"
 #include "chrome/browser/chromeos/policy/enrollment_config.h"
 #include "chrome/browser/chromeos/policy/enrollment_status_chromeos.h"
+#include "components/policy/proto/device_management_backend.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -72,6 +76,40 @@ EnterpriseEnrollmentHelper* MockDemoModeOfflineEnrollmentHelperCreator(
   return mock;
 }
 
+// Creates fake offline policy directory to be used in tests.
+bool SetupDummyOfflinePolicyDir(const std::string& account_id,
+                                base::ScopedTempDir* temp_dir) {
+  if (!temp_dir->CreateUniqueTempDir()) {
+    LOG(ERROR) << "Failed to create unique tempdir";
+    return false;
+  }
+
+  if (base::WriteFile(temp_dir->GetPath().AppendASCII("device_policy"), "",
+                      0) != 0) {
+    LOG(ERROR) << "Failed to create device_policy file";
+    return false;
+  }
+
+  // We use MockCloudPolicyStore for the device local account policy in the
+  // tests, thus actual policy content can be empty. account_id is specified
+  // since it is used by DemoSetupController to look up the store.
+  std::string policy_blob;
+  if (!account_id.empty()) {
+    enterprise_management::PolicyData policy_data;
+    policy_data.set_username(account_id);
+    enterprise_management::PolicyFetchResponse policy;
+    policy.set_policy_data(policy_data.SerializeAsString());
+    policy_blob = policy.SerializeAsString();
+  }
+  if (base::WriteFile(temp_dir->GetPath().AppendASCII("local_account_policy"),
+                      policy_blob.data(), policy_blob.size()) !=
+      static_cast<int>(policy_blob.size())) {
+    LOG(ERROR) << "Failed to create local_account_policy file";
+    return false;
+  }
+  return true;
+}
+
 }  // namespace test
 
 }  // namespace chromeos

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

@@ -4,24 +4,16 @@
 
 #include "chrome/browser/chromeos/login/screens/demo_setup_screen.h"
 
-#include "base/files/file_path.h"
-#include "base/logging.h"
-#include "chrome/browser/chromeos/login/screen_manager.h"
+#include "base/bind.h"
 #include "chrome/browser/chromeos/login/screens/base_screen_delegate.h"
 #include "chrome/browser/chromeos/login/screens/demo_setup_screen_view.h"
-#include "chrome/browser/chromeos/policy/enrollment_config.h"
+#include "chrome/browser/chromeos/login/wizard_controller.h"
 
 namespace {
 
-constexpr char kUserActionOnlineSetup[] = "online-setup";
-constexpr char kUserActionOfflineSetup[] = "offline-setup";
+constexpr char kUserActionStartSetup[] = "start-setup";
 constexpr char kUserActionClose[] = "close-setup";
 
-// The policy blob data for offline demo-mode is embedded into the filesystem.
-// TODO(mukai, agawronska): fix this when switching to dm-verity image.
-constexpr const base::FilePath::CharType kOfflineDemoModeDir[] =
-    FILE_PATH_LITERAL("/usr/share/chromeos-assets/demo_mode_resources/policy");
-
 }  // namespace
 
 namespace chromeos {
@@ -29,10 +21,10 @@ namespace chromeos {
 DemoSetupScreen::DemoSetupScreen(BaseScreenDelegate* base_screen_delegate,
                                  DemoSetupScreenView* view)
     : BaseScreen(base_screen_delegate, OobeScreen::SCREEN_OOBE_DEMO_SETUP),
-      view_(view) {
+      view_(view),
+      weak_ptr_factory_(this) {
   DCHECK(view_);
   view_->Bind(this);
-  demo_controller_.reset(new DemoSetupController(this));
 }
 
 DemoSetupScreen::~DemoSetupScreen() {
@@ -51,10 +43,8 @@ void DemoSetupScreen::Hide() {
 }
 
 void DemoSetupScreen::OnUserAction(const std::string& action_id) {
-  if (action_id == kUserActionOnlineSetup) {
-    demo_controller_->EnrollOnline();
-  } else if (action_id == kUserActionOfflineSetup) {
-    demo_controller_->EnrollOffline(base::FilePath(kOfflineDemoModeDir));
+  if (action_id == kUserActionStartSetup) {
+    StartEnrollment();
   } else if (action_id == kUserActionClose) {
     Finish(ScreenExitCode::DEMO_MODE_SETUP_CANCELED);
   } else {
@@ -62,11 +52,22 @@ void DemoSetupScreen::OnUserAction(const std::string& action_id) {
   }
 }
 
-void DemoSetupScreen::OnSetupError(bool fatal) {
-  // TODO(mukai): propagate |fatal| information and change the error message.
+void DemoSetupScreen::OnSetupError(DemoSetupController::DemoSetupError error) {
+  // TODO(mukai): propagate |error| information and change the error message.
   view_->OnSetupFinished(false, std::string());
 }
 
+void DemoSetupScreen::StartEnrollment() {
+  // Demo setup screen is only shown in OOBE.
+  DCHECK(DemoSetupController::IsOobeDemoSetupFlowInProgress());
+  DemoSetupController* demo_controller =
+      WizardController::default_controller()->demo_setup_controller();
+  demo_controller->Enroll(base::BindOnce(&DemoSetupScreen::OnSetupSuccess,
+                                         weak_ptr_factory_.GetWeakPtr()),
+                          base::BindOnce(&DemoSetupScreen::OnSetupError,
+                                         weak_ptr_factory_.GetWeakPtr()));
+}
+
 void DemoSetupScreen::OnSetupSuccess() {
   Finish(ScreenExitCode::DEMO_MODE_SETUP_FINISHED);
 }
@@ -74,7 +75,6 @@ void DemoSetupScreen::OnSetupSuccess() {
 void DemoSetupScreen::OnViewDestroyed(DemoSetupScreenView* view) {
   if (view_ == view)
     view_ = nullptr;
-  demo_controller_.reset();
 }
 
 }  // namespace chromeos

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

@@ -5,8 +5,10 @@
 #ifndef CHROME_BROWSER_CHROMEOS_LOGIN_SCREENS_DEMO_SETUP_SCREEN_H_
 #define CHROME_BROWSER_CHROMEOS_LOGIN_SCREENS_DEMO_SETUP_SCREEN_H_
 
-#include <memory>
+#include <string>
 
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_setup_controller.h"
 #include "chrome/browser/chromeos/login/screens/base_screen.h"
 
@@ -17,8 +19,7 @@ class DemoSetupScreenView;
 
 // Controlls demo mode setup. The screen can be shown during OOBE. It allows
 // user to setup retail demo mode on the device.
-class DemoSetupScreen : public BaseScreen,
-                        public DemoSetupController::Delegate {
+class DemoSetupScreen : public BaseScreen {
  public:
   DemoSetupScreen(BaseScreenDelegate* base_screen_delegate,
                   DemoSetupScreenView* view);
@@ -29,17 +30,22 @@ class DemoSetupScreen : public BaseScreen,
   void Hide() override;
   void OnUserAction(const std::string& action_id) override;
 
-  // DemoSetupManager::Delegate:
-  void OnSetupError(bool fatal) override;
-  void OnSetupSuccess() override;
-
   // Called when view is being destroyed. If Screen is destroyed earlier
   // then it has to call Bind(nullptr).
   void OnViewDestroyed(DemoSetupScreenView* view);
 
  private:
+  void StartEnrollment();
+
+  // Called when the setup flow finished with error.
+  void OnSetupError(DemoSetupController::DemoSetupError error);
+
+  // Called when the setup flow finished successfully.
+  void OnSetupSuccess();
+
   DemoSetupScreenView* view_;
-  std::unique_ptr<DemoSetupController> demo_controller_;
+
+  base::WeakPtrFactory<DemoSetupScreen> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DemoSetupScreen);
 };

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

-// Copyright (c) 2018 The Chromium Authors. All rights reserved.
+// Copyright 2018 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 

chrome/browser/chromeos/login/wizard_controller.cc

@@ -32,6 +32,7 @@
 #include "chrome/browser/chromeos/arc/arc_util.h"
 #include "chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.h"
 #include "chrome/browser/chromeos/customization/customization_document.h"
+#include "chrome/browser/chromeos/login/demo_mode/demo_setup_controller.h"
 #include "chrome/browser/chromeos/login/enrollment/auto_enrollment_check_screen.h"
 #include "chrome/browser/chromeos/login/enrollment/enrollment_screen.h"
 #include "chrome/browser/chromeos/login/existing_user_controller.h"
@@ -816,7 +817,7 @@ void WizardController::OnWelcomeContinued() {
 }
 
 void WizardController::OnNetworkBack() {
-  if (is_in_demo_setup_flow_) {
+  if (demo_setup_controller_) {
     ShowDemoModePreferencesScreen();
   } else {
     ShowWelcomeScreen();
@@ -824,6 +825,11 @@ void WizardController::OnNetworkBack() {
 }
 
 void WizardController::OnNetworkConnected() {
+  if (demo_setup_controller_) {
+    demo_setup_controller_->set_enrollment_type(
+        DemoSetupController::EnrollmentType::kOnline);
+  }
+
   if (is_official_build_) {
     if (!StartupUtils::IsEulaAccepted()) {
       ShowEulaScreen();
@@ -842,8 +848,10 @@ void WizardController::OnNetworkConnected() {
 }
 
 void WizardController::OnOfflineDemoModeSetup() {
-  DCHECK(is_in_demo_setup_flow_);
-  is_offline_demo_setup_ = true;
+  DCHECK(demo_setup_controller_);
+  demo_setup_controller_->set_enrollment_type(
+      DemoSetupController::EnrollmentType::kOffline);
+
   if (is_official_build_) {
     if (!StartupUtils::IsEulaAccepted()) {
       ShowEulaScreen();
@@ -865,7 +873,7 @@ void WizardController::OnConnectionFailed() {
 }
 
 void WizardController::OnUpdateCompleted() {
-  if (is_in_demo_setup_flow_) {
+  if (demo_setup_controller_) {
     ShowDemoModeSetupScreen();
     return;
   }
@@ -889,8 +897,7 @@ void WizardController::OnEulaAccepted() {
   PerformPostEulaActions();
 
   // TODO(crbug.com/857275): Show Play Store ToS when available offline.
-  if (is_offline_demo_setup_) {
-    DCHECK(is_in_demo_setup_flow_);
+  if (demo_setup_controller_ && demo_setup_controller_->IsOfflineEnrollment()) {
     ShowDemoModeSetupScreen();
     return;
   }
@@ -1031,7 +1038,7 @@ void WizardController::OnArcTermsOfServiceSkipped() {
 }
 
 void WizardController::OnArcTermsOfServiceAccepted() {
-  if (is_in_demo_setup_flow_) {
+  if (demo_setup_controller_) {
     InitiateOOBEUpdate();
     return;
   }
@@ -1052,7 +1059,7 @@ void WizardController::OnArcTermsOfServiceAccepted() {
 }
 
 void WizardController::OnArcTermsOfServiceBack() {
-  DCHECK(is_in_demo_setup_flow_);
+  DCHECK(demo_setup_controller_);
   DCHECK(StartupUtils::IsEulaAccepted());
   ShowNetworkScreen();
 }
@@ -1102,29 +1109,26 @@ void WizardController::OnAutoEnrollmentCheckCompleted() {
 }
 
 void WizardController::OnDemoSetupFinished() {
-  DCHECK(is_in_demo_setup_flow_);
-  is_in_demo_setup_flow_ = false;
-  is_offline_demo_setup_ = false;
+  DCHECK(demo_setup_controller_);
+  demo_setup_controller_.reset();
   PerformOOBECompletedActions();
   ShowLoginScreen(LoginScreenContext());
 }
 
 void WizardController::OnDemoSetupCanceled() {
-  DCHECK(is_in_demo_setup_flow_);
-  is_in_demo_setup_flow_ = false;
-  is_offline_demo_setup_ = false;
+  DCHECK(demo_setup_controller_);
+  demo_setup_controller_.reset();
   ShowWelcomeScreen();
 }
 
 void WizardController::OnDemoPreferencesContinued() {
-  DCHECK(is_in_demo_setup_flow_);
+  DCHECK(demo_setup_controller_);
   ShowNetworkScreen();
 }
 
 void WizardController::OnDemoPreferencesCanceled() {
-  DCHECK(is_in_demo_setup_flow_);
-  is_in_demo_setup_flow_ = false;
-  is_offline_demo_setup_ = false;
+  DCHECK(demo_setup_controller_);
+  demo_setup_controller_.reset();
   ShowWelcomeScreen();
 }
 
@@ -1427,12 +1431,12 @@ void WizardController::AdvanceToScreen(OobeScreen screen) {
 }
 
 void WizardController::StartDemoModeSetup() {
-  is_in_demo_setup_flow_ = true;
+  demo_setup_controller_ = std::make_unique<DemoSetupController>();
   ShowDemoModePreferencesScreen();
 }
 
 void WizardController::SimulateDemoModeSetupForTesting() {
-  is_in_demo_setup_flow_ = true;
+  demo_setup_controller_ = std::make_unique<DemoSetupController>();
 }
 
 ///////////////////////////////////////////////////////////////////////////////

chrome/browser/chromeos/login/wizard_controller.h

@@ -44,6 +44,7 @@ namespace login {
 class NetworkStateHelper;
 }  // namespace login
 
+class DemoSetupController;
 class ErrorScreen;
 struct Geoposition;
 class LoginDisplayHost;
@@ -106,7 +107,10 @@ class WizardController : public BaseScreenDelegate,
   // Starts Demo Mode setup flow. The flow starts from network screen and reuses
   // some of regular OOBE screens. It consists of the following screens:
   //    chromeos::OobeScreen::SCREEN_OOBE_DEMO_PREFERENCES
+  //    chromeos::OobeScreen::SCREEN_OOBE_NETWORK
   //    chromeos::OobeScreen::SCREEN_OOBE_EULA
+  //    chromeos::OobeScreen::SCREEN_ARC_TERMS_OF_SERVICE
+  //    chromeos::OobeScreen::SCREEN_OOBE_UPDATE
   //    chromeos::OobeScreen::SCREEN_OOBE_DEMO_SETUP
   void StartDemoModeSetup();
 
@@ -127,6 +131,12 @@ class WizardController : public BaseScreenDelegate,
   // reworked at hackaton.
   void EnableUserImageScreenReturnToPreviousHack();
 
+  // Returns current DemoSetupController if demo setup flow is in progress or
+  // nullptr otherwise.
+  DemoSetupController* demo_setup_controller() const {
+    return demo_setup_controller_.get();
+  }
+
   // Returns a pointer to the current screen or nullptr if there's no such
   // screen.
   BaseScreen* current_screen() const { return current_screen_; }
@@ -134,9 +144,6 @@ class WizardController : public BaseScreenDelegate,
   // Returns true if the current wizard instance has reached the login screen.
   bool login_screen_started() const { return login_screen_started_; }
 
-  // Whether demo mode setup OOBE flow is currently in progress.
-  bool is_in_demo_mode_setup_flow() const { return is_in_demo_setup_flow_; }
-
   // Returns a given screen. Creates it lazily.
   BaseScreen* GetScreen(OobeScreen screen);
 
@@ -434,16 +441,6 @@ class WizardController : public BaseScreenDelegate,
 
   bool is_in_session_oobe_ = false;
 
-  // Whether the currently presented flow is Demo Mode setup.
-  bool is_in_demo_setup_flow_ = false;
-
-  // TODO(agawronska): Refactor |is_in_demo_setup_flow_| and
-  // |is_offline_demo_setup_| to DemoSetupController and use it here to
-  // determine demo setup configuration.
-  // Whether offline demo setup was chosen as a part of
-  // demo mode setup. Should be check together with |is_in_demo_setup_flow_|.
-  bool is_offline_demo_setup_ = false;
-
   // Indicates that once image selection screen finishes we should return to
   // a previous screen instead of proceeding with usual flow.
   bool user_image_screen_return_to_previous_hack_ = false;
@@ -480,6 +477,10 @@ class WizardController : public BaseScreenDelegate,
   // Helper for network realted operations.
   std::unique_ptr<login::NetworkStateHelper> network_state_helper_;
 
+  // Controller of the demo mode setup. It has the lifetime of the single demo
+  // mode setup flow.
+  std::unique_ptr<DemoSetupController> demo_setup_controller_;
+
   // Maps screen names to last time of their shows.
   std::map<std::string, base::Time> screen_show_times_;
 

chrome/browser/resources/chromeos/login/demo_setup.html

@@ -27,29 +27,6 @@
     <link rel="stylesheet" href="demo_setup.css">
     <link rel="stylesheet" href="oobe_flex_layout.css">
 
-    <oobe-dialog id="demoSetupSettingsDialog" role="dialog" has-buttons>
-      <h1 slot="title">Choose demo mode settings</h1>
-      <div slot="footer" class="flex layout vertical">
-        <paper-radio-group id="setupGroup"
-            selectable="cr-radio-button" selected="onlineSetup">
-          <cr-radio-button name="onlineSetup"
-              class="options-list-item flex layout horizontal center">
-            <div>Online setup</div>
-          </cr-radio-button>
-          <template is="dom-if" if="[[offlineDemoModeEnabled_]]">
-            <cr-radio-button name="offlineSetup"
-                class="options-list-item flex layout horizontal center">
-              <div>Offline setup</div>
-            </cr-radio-button>
-          </template>
-        </paper-radio-group>
-      </div>
-      <div slot="bottom-buttons" class="layout horizontal justified">
-        <oobe-back-button inverse on-tap="onCloseClicked_"></oobe-back-button>
-        <oobe-next-button inverse on-tap="onNextClicked_"></oobe-next-button>
-      </div>
-    </oobe-dialog>
-
     <oobe-dialog id="demoSetupProgressDialog" role="dialog" has-buttons hidden>
       <hd-iron-icon slot="oobe-icon"
           icon1x="demo-setup-32:computer" icon2x="demo-setup-64:computer">
@@ -77,7 +54,7 @@
       </div>
       <div slot="bottom-buttons" class="layout horizontal justified">
         <oobe-back-button inverse on-tap="onCloseClicked_"></oobe-back-button>
-        <oobe-text-button inverse on-tap="startSetup_">
+        <oobe-text-button inverse on-tap="onRetryClicked_">
           <div>
             [[i18nDynamic(locale, 'demoSetupErrorScreenRetryButtonLabel')]]
           </div>

chrome/browser/resources/chromeos/login/demo_setup.js

@@ -13,40 +13,20 @@ Polymer({
   behaviors: [I18nBehavior, OobeDialogHostBehavior],
 
   properties: {
-    /**
-     * Whether offline demo mode is enabled. If it is disabled offline setup
-     * option will not be shown in UI.
-     */
-    offlineDemoModeEnabled_: {
-      type: Boolean,
-      value: false,
-    },
-
-    /**
-     * Whether offline demo setup was selected. Available setup types: online
-     * and offline.
-     */
-    isOfflineSetup_: {
-      type: Boolean,
-      value: false,
-    },
-
     /** Ordered array of screen ids that are a part of demo setup flow. */
     screens_: {
       type: Array,
       readonly: true,
       value: function() {
-        return [
-          'demoSetupSettingsDialog', 'demoSetupProgressDialog',
-          'demoSetupErrorDialog'
-        ];
+        return ['demoSetupProgressDialog', 'demoSetupErrorDialog'];
       },
     },
   },
 
-  /** Resets demo setup flow to the initial screen. */
+  /** Resets demo setup flow to the initial screen and starts setup. */
   reset: function() {
-    this.showScreen_(this.screens_[0]);
+    this.showScreen_('demoSetupProgressDialog');
+    chrome.send('login.DemoSetupScreen.userActed', ['start-setup']);
   },
 
   /** Called after resources are updated. */
@@ -74,19 +54,6 @@ Polymer({
     this.showScreen_(id);
   },
 
-  /**
-   * Shows progress dialog and starts demo setup.
-   * @private
-   */
-  startSetup_: function() {
-    this.showScreen_('demoSetupProgressDialog');
-    if (this.isOfflineSetup_) {
-      chrome.send('login.DemoSetupScreen.userActed', ['offline-setup']);
-    } else {
-      chrome.send('login.DemoSetupScreen.userActed', ['online-setup']);
-    }
-  },
-
   /**
    * Shows screen with the given id.
    * @param {string} id Screen id.
@@ -113,22 +80,12 @@ Polymer({
     }
   },
 
-  /**
-   * Next button click handler.
-   * @private
-   */
-  onNextClicked_: function() {
-    const selected = this.$.setupGroup.selected;
-    this.isOfflineSetup_ = (selected == 'offlineSetup');
-    this.startSetup_();
-  },
-
   /**
    * Retry button click handler.
    * @private
    */
   onRetryClicked_: function() {
-    this.startSetup_();
+    this.reset();
   },
 
   /**

chrome/browser/resources/chromeos/login/oobe_screen_demo_setup.js

@@ -10,26 +10,31 @@ login.createScreen('DemoSetupScreen', 'demo-setup', function() {
   return {
     EXTERNAL_API: ['onSetupFinished'],
 
+    /**
+     * Demo setup module.
+     * @private
+     */
+    demoSetupModule_: null,
+
+
     /** @override */
     decorate: function() {
-      var demoSetupScreen = $('demo-setup-content');
-      demoSetupScreen.offlineDemoModeEnabled_ =
-          loadTimeData.getValue('offlineDemoModeEnabled');
+      this.demoSetupModule_ = $('demo-setup-content');
     },
 
     /** Returns a control which should receive an initial focus. */
     get defaultControl() {
-      return $('demo-setup-content');
+      return this.demoSetupModule_;
     },
 
     /** Called after resources are updated. */
     updateLocalizedContent: function() {
-      $('demo-setup-content').updateLocalizedContent();
+      this.demoSetupModule_.updateLocalizedContent();
     },
 
     /** @override */
-    onBeforeShow: function(data) {
-      $('demo-setup-content').reset();
+    onBeforeShow: function() {
+      this.demoSetupModule_.reset();
     },
 
     /**
@@ -39,7 +44,7 @@ login.createScreen('DemoSetupScreen', 'demo-setup', function() {
      *  populated if setup finished with an error.
      */
     onSetupFinished: function(isSuccess, message) {
-      $('demo-setup-content').onSetupFinished(isSuccess, message);
+      this.demoSetupModule_.onSetupFinished(isSuccess, message);
     },
   };
 });

chrome/browser/ui/webui/chromeos/login/demo_setup_screen_handler.cc

@@ -4,13 +4,9 @@
 
 #include "chrome/browser/ui/webui/chromeos/login/demo_setup_screen_handler.h"
 
-#include "base/command_line.h"
-#include "base/values.h"
 #include "chrome/browser/chromeos/login/oobe_screen.h"
 #include "chrome/browser/chromeos/login/screens/demo_setup_screen.h"
-#include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chrome/grit/generated_resources.h"
-#include "chromeos/chromeos_switches.h"
 #include "components/login/localized_values_builder.h"
 
 namespace {
@@ -61,12 +57,4 @@ void DemoSetupScreenHandler::DeclareLocalizedValues(
                IDS_OOBE_DEMO_SETUP_ERROR_SCREEN_RETRY_BUTTON_LABEL);
 }
 
-void DemoSetupScreenHandler::GetAdditionalParameters(
-    base::DictionaryValue* dict) {
-  const bool is_offline_demo_mode_enabled =
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableOfflineDemoMode);
-  dict->SetBoolean("offlineDemoModeEnabled", is_offline_demo_mode_enabled);
-}
-
 }  // namespace chromeos

chrome/browser/ui/webui/chromeos/login/demo_setup_screen_handler.h

@@ -30,7 +30,6 @@ class DemoSetupScreenHandler : public BaseScreenHandler,
   void Initialize() override;
   void DeclareLocalizedValues(
       ::login::LocalizedValuesBuilder* builder) override;
-  void GetAdditionalParameters(base::DictionaryValue* dict) override;
 
  private:
   DemoSetupScreen* screen_ = nullptr;