Demo Mode: Display progress bar rather than spinner during Demo setup

Currently when setting up demo mode, users are presented with a
non-descriptive spinner loader while components are being installed
and loaded, and the device is enterprise enrolled. Now the progress is
displayed to the user as a progress bar (hidden behind the
`ShowProgressBarInDemoModeSetup` flag, disabled by default). Future
work should consider the possibility of decreasing granularity of
progress to help users notice if download speeds are low.


Bug: b:148753805
Change-Id: I9ccb4bca664b6fbc6ab51289df9900fda62e3f33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2086358
Commit-Queue: Joseph Kim <josephkimsh@google.com>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Patti <patricialor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750816}

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

@@ -494,12 +494,15 @@ bool DemoSetupController::IsOfflineEnrollment() const {
   return demo_config_ == DemoSession::DemoModeConfig::kOffline;
 }
 
-void DemoSetupController::Enroll(OnSetupSuccess on_setup_success,
-                                 OnSetupError on_setup_error) {
+void DemoSetupController::Enroll(
+    OnSetupSuccess on_setup_success,
+    OnSetupError on_setup_error,
+    const OnIncrementSetupProgress& increment_setup_progress) {
   DCHECK_NE(demo_config_, DemoSession::DemoModeConfig::kNone)
       << "Demo config needs to be explicitly set before calling Enroll()";
   DCHECK(!enrollment_helper_);
 
+  increment_setup_progress_ = increment_setup_progress;
   on_setup_success_ = std::move(on_setup_success);
   on_setup_error_ = std::move(on_setup_error);
 
@@ -574,6 +577,7 @@ void DemoSetupController::OnDemoResourcesCrOSComponentLoaded() {
       base::TimeTicks::Now() - download_start_time_;
   base::UmaHistogramLongTimes100(kDemoSetupDownloadDurationHistogram,
                                  download_duration);
+  IncrementSetupProgress(/*complete=*/false);
 
   if (demo_resources_->component_error().value() !=
       component_updater::CrOSComponentManager::Error::NONE) {
@@ -658,6 +662,7 @@ void DemoSetupController::OnDeviceEnrolled() {
     base::UmaHistogramLongTimes100(kDemoSetupEnrollDurationHistogram,
                                    enroll_duration);
   }
+  IncrementSetupProgress(/*complete=*/false);
 
   // Try to load the policy for the device local account.
   if (demo_config_ == DemoSession::DemoModeConfig::kOffline) {
@@ -759,6 +764,8 @@ void DemoSetupController::OnDeviceLocalAccountPolicyLoaded(
 }
 
 void DemoSetupController::OnDeviceRegistered() {
+  IncrementSetupProgress(/*complete=*/true);
+
   VLOG(1) << "Demo mode setup finished successfully.";
 
   if (demo_config_ == DemoSession::DemoModeConfig::kOnline) {
@@ -783,6 +790,11 @@ void DemoSetupController::OnDeviceRegistered() {
     std::move(on_setup_success_).Run();
 }
 
+void DemoSetupController::IncrementSetupProgress(bool complete) {
+  if (!increment_setup_progress_.is_null())
+    increment_setup_progress_.Run(complete);
+}
+
 void DemoSetupController::SetupFailed(const DemoSetupError& error) {
   num_setup_retries_++;
   Reset();

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

@@ -155,6 +155,7 @@ class DemoSetupController
   // Demo mode setup callbacks.
   using OnSetupSuccess = base::OnceClosure;
   using OnSetupError = base::OnceCallback<void(const DemoSetupError&)>;
+  using OnIncrementSetupProgress = base::RepeatingCallback<void(bool)>;
   using HasPreinstalledDemoResourcesCallback = base::OnceCallback<void(bool)>;
 
   static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
@@ -193,8 +194,11 @@ class DemoSetupController
   // 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);
+  // an error. |update_setup_progress| will be called when enrollment progress
+  // is updated.
+  void Enroll(OnSetupSuccess on_setup_success,
+              OnSetupError on_setup_error,
+              const OnIncrementSetupProgress& increment_setup_progress);
 
   // Tries to mount the preinstalled offline resources necessary for offline
   // Demo Mode.
@@ -253,6 +257,9 @@ class DemoSetupController
   // is completed. This is the last step of demo mode setup flow.
   void OnDeviceRegistered();
 
+  // Increments setup progress percentage for UI.
+  void IncrementSetupProgress(bool complete);
+
   // Finish the flow with an error.
   void SetupFailed(const DemoSetupError& error);
 
@@ -285,6 +292,9 @@ class DemoSetupController
   // Path at which to mount preinstalled offline demo resources for tests.
   base::FilePath preinstalled_offline_resources_path_for_tests_;
 
+  // Callback to call when setup progress is updated.
+  OnIncrementSetupProgress increment_setup_progress_;
+
   // Callback to call when enrollment finishes with an error.
   OnSetupError on_setup_error_;
 

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

@@ -63,6 +63,8 @@ class DemoSetupControllerTestHelper {
     run_loop_->Quit();
   }
 
+  void IncrementSetupProgress(const bool complete) {}
+
   // Wait until the setup result arrives (either OnSetupError or OnSetupSuccess
   // is called), returns true when the result matches with |expected|.
   bool WaitResult(bool expected) {
@@ -159,7 +161,10 @@ TEST_F(DemoSetupControllerTest, OfflineSuccess) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(true));
   EXPECT_EQ("", GetDeviceRequisition());
@@ -184,7 +189,10 @@ TEST_F(DemoSetupControllerTest, OfflineDeviceLocalAccountPolicyStoreFailed) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(false));
   EXPECT_TRUE(helper_->RequiresPowerwash());
@@ -204,7 +212,10 @@ TEST_F(DemoSetupControllerTest, OfflineInvalidDeviceLocalAccountPolicyBlob) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(false));
   EXPECT_TRUE(helper_->RequiresPowerwash());
@@ -229,7 +240,10 @@ TEST_F(DemoSetupControllerTest, OfflineErrorDefault) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(false));
   EXPECT_FALSE(helper_->RequiresPowerwash());
@@ -255,7 +269,10 @@ TEST_F(DemoSetupControllerTest, OfflineErrorPowerwashRequired) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(false));
   EXPECT_TRUE(helper_->RequiresPowerwash());
@@ -270,7 +287,10 @@ TEST_F(DemoSetupControllerTest, OnlineSuccess) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(true));
   EXPECT_EQ("", GetDeviceRequisition());
@@ -284,7 +304,10 @@ TEST_F(DemoSetupControllerTest, OnlineErrorDefault) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(false));
   EXPECT_FALSE(helper_->RequiresPowerwash());
@@ -300,7 +323,10 @@ TEST_F(DemoSetupControllerTest, OnlineErrorPowerwashRequired) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(false));
   EXPECT_TRUE(helper_->RequiresPowerwash());
@@ -319,7 +345,10 @@ TEST_F(DemoSetupControllerTest, OnlineComponentError) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(false));
   EXPECT_FALSE(helper_->RequiresPowerwash());
@@ -334,7 +363,10 @@ TEST_F(DemoSetupControllerTest, EnrollTwice) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(false));
   EXPECT_FALSE(helper_->RequiresPowerwash());
@@ -349,7 +381,10 @@ TEST_F(DemoSetupControllerTest, EnrollTwice) {
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupSuccess,
                      base::Unretained(helper_.get())),
       base::BindOnce(&DemoSetupControllerTestHelper::OnSetupError,
-                     base::Unretained(helper_.get())));
+                     base::Unretained(helper_.get())),
+      base::BindRepeating(
+          &DemoSetupControllerTestHelper::IncrementSetupProgress,
+          base::Unretained(helper_.get())));
 
   EXPECT_TRUE(helper_->WaitResult(true));
   EXPECT_EQ("", GetDeviceRequisition());

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

@@ -61,10 +61,17 @@ void DemoSetupScreen::StartEnrollment() {
   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()));
+  demo_controller->Enroll(
+      base::BindOnce(&DemoSetupScreen::OnSetupSuccess,
+                     weak_ptr_factory_.GetWeakPtr()),
+      base::BindOnce(&DemoSetupScreen::OnSetupError,
+                     weak_ptr_factory_.GetWeakPtr()),
+      base::BindRepeating(&DemoSetupScreen::IncrementSetupProgress,
+                          weak_ptr_factory_.GetWeakPtr()));
+}
+
+void DemoSetupScreen::IncrementSetupProgress(bool complete) {
+  view_->IncrementSetupProgress(complete);
 }
 
 void DemoSetupScreen::OnSetupError(

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

@@ -43,6 +43,9 @@ class DemoSetupScreen : public BaseScreen {
  private:
   void StartEnrollment();
 
+  // Increments setup progress percentage for UI.
+  void IncrementSetupProgress(bool complete);
+
   // Called when the setup flow finished with error.
   void OnSetupError(const DemoSetupController::DemoSetupError& error);
 

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

@@ -35,6 +35,7 @@ class MockDemoSetupScreenView : public DemoSetupScreenView {
   MOCK_METHOD0(OnSetupSucceeded, void());
   MOCK_METHOD1(OnSetupFailed,
                void(const DemoSetupController::DemoSetupError& error));
+  MOCK_METHOD1(IncrementSetupProgress, void(bool complete));
 
   void Bind(DemoSetupScreen* screen) override;
 

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

@@ -10,7 +10,18 @@
           icon1x="oobe-32:computer" icon2x="oobe-64:computer">
       </hd-iron-icon>
       <div slot="footer" class="flex layout vertical center center-justified">
-        <paper-spinner-lite id="spinner" dir="ltr" active></paper-spinner-lite>
+        <div hidden$="[[showProgressBarInDemoModeSetup_]]">
+          <paper-spinner-lite id="spinner" dir="ltr"
+              active></paper-spinner-lite>
+        </div>
+        <div hidden$="[[!showProgressBarInDemoModeSetup_]]">
+          <!-- TODO(josephkimsh): Confirm UI input for progress bar and
+            accompanying text. -->
+          <!-- Paper progress is indeterminate when percentage is 0. -->
+          <paper-progress indeterminate="[[!progressPercentage_]]"
+              value="[[progressPercentage_]]"></paper-progress>
+          <span>Starting up Demo Mode...</span>
+        </div>
       </div>
     </oobe-dialog>
 

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

@@ -13,6 +13,12 @@ Polymer({
   behaviors: [OobeI18nBehavior, OobeDialogHostBehavior],
 
   properties: {
+    /** Percentage of progress completed in Demo Mode setup. */
+    progressPercentage_: {
+      type: Number,
+      value: 0,
+    },
+
     /** Error message displayed on demoSetupErrorDialog screen. */
     errorMessage_: {
       type: String,
@@ -33,6 +39,15 @@ Polymer({
         return ['demoSetupProgressDialog', 'demoSetupErrorDialog'];
       },
     },
+
+    /** Feature flag to display progress bar instead of spinner during setup. */
+    showProgressBarInDemoModeSetup_: {
+      type: Boolean,
+      readonly: true,
+      value() {
+        return loadTimeData.getBoolean('showProgressBarInDemoModeSetup');
+      }
+    }
   },
 
   /** Resets demo setup flow to the initial screen and starts setup. */
@@ -46,6 +61,21 @@ Polymer({
     this.i18nUpdateLocale();
   },
 
+  /**
+   * Called when the progress bar needs to be incremented. Every time progress
+   * is incremented, remaining progress is halved.
+   * @param {boolean} complete Set to true if progress is complete
+   */
+  incrementSetupProgress(complete) {
+    const maxPercentage = 100;
+    if (complete) {
+      this.progressPercentage_ = maxPercentage;
+    } else {
+      const remaining = maxPercentage - this.progressPercentage_;
+      this.progressPercentage_ += remaining / 2;
+    }
+  },
+
   /** Called when demo mode setup succeeded. */
   onSetupSucceeded() {
     this.errorMessage_ = '';

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

@@ -8,7 +8,8 @@
 
 login.createScreen('DemoSetupScreen', 'demo-setup', function() {
   return {
-    EXTERNAL_API: ['onSetupSucceeded', 'onSetupFailed'],
+    EXTERNAL_API:
+        ['incrementSetupProgress', 'onSetupSucceeded', 'onSetupFailed'],
 
     /**
      * Demo setup module.
@@ -37,6 +38,15 @@ login.createScreen('DemoSetupScreen', 'demo-setup', function() {
       this.demoSetupModule_.reset();
     },
 
+    /**
+     * Called when the progress bar needs updating with a new percentage value.
+     * @param {number} percentage Number in range 0-100 denoting progress
+     * percentage.
+     */
+    incrementSetupProgress(complete) {
+      this.demoSetupModule_.incrementSetupProgress(complete);
+    },
+
     /** Called when demo mode setup succeeded. */
     onSetupSucceeded() {
       this.demoSetupModule_.onSetupSucceeded();

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

@@ -10,6 +10,7 @@
 #include "chrome/browser/chromeos/login/oobe_screen.h"
 #include "chrome/browser/chromeos/login/screens/demo_setup_screen.h"
 #include "chrome/grit/generated_resources.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "components/login/localized_values_builder.h"
 
 namespace chromeos {
@@ -51,6 +52,10 @@ void DemoSetupScreenHandler::OnSetupFailed(
              DemoSetupController::DemoSetupError::RecoveryMethod::kPowerwash);
 }
 
+void DemoSetupScreenHandler::IncrementSetupProgress(bool complete) {
+  CallJS("login.DemoSetupScreen.incrementSetupProgress", complete);
+}
+
 void DemoSetupScreenHandler::OnSetupSucceeded() {
   CallJS("login.DemoSetupScreen.onSetupSucceeded");
 }
@@ -69,4 +74,11 @@ void DemoSetupScreenHandler::DeclareLocalizedValues(
                IDS_LOCAL_STATE_ERROR_POWERWASH_BUTTON);
 }
 
+void DemoSetupScreenHandler::GetAdditionalParameters(
+    base::DictionaryValue* parameters) {
+  parameters->SetBoolKey(
+      "showProgressBarInDemoModeSetup",
+      base::FeatureList::IsEnabled(features::kShowProgressBarInDemoModeSetup));
+}
+
 }  // namespace chromeos

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

@@ -28,6 +28,9 @@ class DemoSetupScreenView {
   // Sets view and screen.
   virtual void Bind(DemoSetupScreen* screen) = 0;
 
+  // Increments setup progress percentage for UI.
+  virtual void IncrementSetupProgress(bool complete) = 0;
+
   // Handles successful setup.
   virtual void OnSetupSucceeded() = 0;
 
@@ -50,6 +53,7 @@ class DemoSetupScreenHandler : public BaseScreenHandler,
   void Show() override;
   void Hide() override;
   void Bind(DemoSetupScreen* screen) override;
+  void IncrementSetupProgress(bool complete) override;
   void OnSetupFailed(const DemoSetupController::DemoSetupError& error) override;
   void OnSetupSucceeded() override;
 
@@ -58,6 +62,9 @@ class DemoSetupScreenHandler : public BaseScreenHandler,
   void DeclareLocalizedValues(
       ::login::LocalizedValuesBuilder* builder) override;
 
+  // BaseWebUIHandler:
+  void GetAdditionalParameters(base::DictionaryValue* parameters) override;
+
  private:
   DemoSetupScreen* screen_ = nullptr;
 

chromeos/constants/chromeos_features.cc

@@ -276,6 +276,10 @@ const base::Feature kShowBluetoothDeviceBattery{
 const base::Feature kShowPlayInDemoMode{"ShowPlayInDemoMode",
                                         base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Shows the progress bar during Demo Mode setup.
+const base::Feature kShowProgressBarInDemoModeSetup{
+    "ShowProgressBarInDemoModeSetup", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Uses the smart dim component updater to provide smart dim model and
 // preprocessor configuration.
 const base::Feature kSmartDimNewMlAgent{"SmartDimNewMlAgent",

chromeos/constants/chromeos_features.h

@@ -126,6 +126,8 @@ extern const base::Feature kShowBluetoothDeviceBattery;
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
 extern const base::Feature kShowPlayInDemoMode;
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
+extern const base::Feature kShowProgressBarInDemoModeSetup;
+COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
 extern const base::Feature kSmartDimNewMlAgent;
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
 extern const base::Feature kSmartDimModelV3;