Skip to content

Commit

Permalink
Demo Mode: Display descriptive progress during Demo setup
Browse files Browse the repository at this point in the history
This work builds upon 2086358: Demo Mode: Display progress bar rather
than spinner during Demo setup
https://chromium-review.googlesource.com/c/chromium/src/+/2086358

Now Demo Mode setup progress is displayed to the user as a list of
steps with individual progress indicators (not begun, in progress,
complete). This is hidden behind the `ShowStepsInDemoModeSetup`
flag, disabled by default.

Bug: b:148753805
Change-Id: I5bdfe643e80f291755df64e28d671f5fc7069e64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2114014
Commit-Queue: Joseph Kim <josephkimsh@google.com>
Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756229}
  • Loading branch information
Joseph Kim authored and Commit Bot committed Apr 3, 2020
1 parent 6bcada4 commit 074aa44
Show file tree
Hide file tree
Showing 25 changed files with 385 additions and 146 deletions.
9 changes: 8 additions & 1 deletion chrome/app/chromeos_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -670,14 +670,21 @@
OK
</message>
<message name="IDS_OOBE_DEMO_SETUP_PROGRESS_SCREEN_TITLE" desc="The title of the dialog that is shown when demo mode setup is in progress.">
Starting demo mode
Starting Demo Mode
</message>
<message name="IDS_OOBE_DEMO_SETUP_ERROR_SCREEN_TITLE" desc="The title of the dialog that is shown when demo mode setup failed.">
Couldn't start demo mode
</message>
<message name="IDS_OOBE_DEMO_SETUP_ERROR_SCREEN_RETRY_BUTTON_LABEL" desc="The label of the button that is shown on error screen and that retries demo mode setup with the previously chosen configuration.">
OK
</message>
<!-- Demo setup progress steps -->
<message name="IDS_OOBE_DEMO_SETUP_PROGRESS_STEP_DOWNLOAD" desc="List item shown when demo resources are being downloaded">
Downloading Resources
</message>
<message name="IDS_OOBE_DEMO_SETUP_PROGRESS_STEP_ENROLL" desc="List item shown when enterprise enrolling for demo mode">
Enrolling in Demo Mode
</message>
<!-- Demo setup flow errors -->
<message name="IDS_DEMO_SETUP_OFFLINE_POLICY_ERROR" desc="Error message shown on demo setup screen when loading or parsing offline policy failed.">
Could not read offline demo mode policy.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4c1599982a05093bbcbeec264265b8d32f870e53
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4c1599982a05093bbcbeec264265b8d32f870e53
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4c1599982a05093bbcbeec264265b8d32f870e53
83 changes: 83 additions & 0 deletions chrome/browser/chromeos/login/demo_mode/demo_setup_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "base/run_loop.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "base/time/time_to_iso8601.h"
#include "base/timer/timer.h"
#include "base/values.h"
Expand All @@ -39,6 +40,7 @@
#include "chrome/browser/ui/webui/chromeos/login/network_screen_handler.h"
#include "chrome/browser/ui/webui/chromeos/login/welcome_screen_handler.h"
#include "chrome/grit/generated_resources.h"
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/constants/chromeos_switches.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/fake_update_engine_client.h"
Expand Down Expand Up @@ -1169,6 +1171,87 @@ IN_PROC_BROWSER_TEST_F(DemoSetupArcSupportedTest,
EXPECT_FALSE(IsCustomNetworkListElementShown("offlineDemoSetupListItemName"));
}

class DemoSetupProgressStepsTest : public DemoSetupTestBase {
public:
DemoSetupProgressStepsTest() {
scoped_feature_list_.InitAndEnableFeature(
features::kShowStepsInDemoModeSetup);
}
~DemoSetupProgressStepsTest() override = default;

// Checks how many steps have been rendered in the demo setup screen.
int CountNumberOfStepsInUi() {
const std::string query =
"$('demo-setup-content').$$('oobe-dialog').querySelectorAll('progress-"
"list-item').length";

return test::OobeJS().GetInt(query);
}

// Checks how many steps are marked as pending in the demo setup screen.
int CountPendingStepsInUi() {
const std::string query =
"Object.values($('demo-setup-content').$$('oobe-dialog')."
"querySelectorAll('progress-list-item')).filter(node => "
"node.shadowRoot.querySelector('#icon-pending:not([hidden])')).length";

return test::OobeJS().GetInt(query);
}

// Checks how many steps are marked as active in the demo setup screen.
int CountActiveStepsInUi() {
const std::string query =
"Object.values($('demo-setup-content').$$('oobe-dialog')."
"querySelectorAll('progress-list-item')).filter(node => "
"node.shadowRoot.querySelector('#icon-active:not([hidden])')).length";

return test::OobeJS().GetInt(query);
}

// Checks how many steps are marked as complete in the demo setup screen.
int CountCompletedStepsInUi() {
const std::string query =
"Object.values($('demo-setup-content').$$('oobe-dialog')."
"querySelectorAll('progress-list-item')).filter(node => "
"node.shadowRoot.querySelector('#icon-completed:not([hidden])'))."
"length";

return test::OobeJS().GetInt(query);
}

private:
base::test::ScopedFeatureList scoped_feature_list_;
DISALLOW_COPY_AND_ASSIGN(DemoSetupProgressStepsTest);
};

IN_PROC_BROWSER_TEST_F(DemoSetupProgressStepsTest,
SetupProgessStepsDisplayCorrectly) {
auto* const wizard_controller = WizardController::default_controller();
wizard_controller->SimulateDemoModeSetupForTesting(
DemoSession::DemoModeConfig::kOnline);
SimulateNetworkConnected();
SkipToScreen(DemoSetupScreenView::kScreenId);

DemoSetupScreen* demoSetupScreen = GetDemoSetupScreen();

DemoSetupController::DemoSetupStep orderedSteps[] = {
DemoSetupController::DemoSetupStep::kDownloadResources,
DemoSetupController::DemoSetupStep::kEnrollment,
DemoSetupController::DemoSetupStep::kComplete};

// Subtract 1 to account for kComplete step
int numSteps =
static_cast<int>(sizeof(orderedSteps) / sizeof(*orderedSteps)) - 1;
ASSERT_EQ(CountNumberOfStepsInUi(), numSteps);

for (int i = 0; i < numSteps; i++) {
demoSetupScreen->SetCurrentSetupStepForTest(orderedSteps[i]);
ASSERT_EQ(CountPendingStepsInUi(), numSteps - i - 1);
ASSERT_EQ(CountActiveStepsInUi(), 1);
ASSERT_EQ(CountCompletedStepsInUi(), i);
}
}

class DemoSetupArcUnsupportedTest : public DemoSetupTestBase {
public:
DemoSetupArcUnsupportedTest() = default;
Expand Down
58 changes: 49 additions & 9 deletions chrome/browser/chromeos/login/demo_mode/demo_setup_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,19 @@ constexpr char kDemoSetupLoadingDurationHistogram[] =
"DemoMode.Setup.LoadingDuration";
constexpr char kDemoSetupNumRetriesHistogram[] = "DemoMode.Setup.NumRetries";

struct DemoSetupStepInfo {
DemoSetupController::DemoSetupStep step;
const int step_index;
};

base::span<const DemoSetupStepInfo> GetDemoSetupStepsInfo() {
static const DemoSetupStepInfo kDemoModeSetupStepsInfo[] = {
{DemoSetupController::DemoSetupStep::kDownloadResources, 0},
{DemoSetupController::DemoSetupStep::kEnrollment, 1},
{DemoSetupController::DemoSetupStep::kComplete, 2}};
return kDemoModeSetupStepsInfo;
}

// Get the DeviceLocalAccountPolicyStore for the account_id.
policy::CloudPolicyStore* GetDeviceLocalAccountPolicyStore(
const std::string& account_id) {
Expand Down Expand Up @@ -483,6 +496,32 @@ std::string DemoSetupController::GetSubOrganizationEmail() {
return std::string();
}

// static
base::Value DemoSetupController::GetDemoSetupSteps() {
base::Value setup_steps_dict(base::Value::Type::DICTIONARY);
for (auto entry : GetDemoSetupStepsInfo()) {
setup_steps_dict.SetIntPath(GetDemoSetupStepString(entry.step),
entry.step_index);
}

return setup_steps_dict;
}

// static
std::string DemoSetupController::GetDemoSetupStepString(
const DemoSetupStep step_enum) {
switch (step_enum) {
case DemoSetupStep::kDownloadResources:
return "downloadResources";
case DemoSetupStep::kEnrollment:
return "enrollment";
case DemoSetupStep::kComplete:
return "complete";
}

NOTREACHED();
}

DemoSetupController::DemoSetupController() {}

DemoSetupController::~DemoSetupController() {
Expand All @@ -497,18 +536,20 @@ bool DemoSetupController::IsOfflineEnrollment() const {
void DemoSetupController::Enroll(
OnSetupSuccess on_setup_success,
OnSetupError on_setup_error,
const OnIncrementSetupProgress& increment_setup_progress) {
const OnSetCurrentSetupStep& set_current_setup_step) {
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;
set_current_setup_step_ = set_current_setup_step;
on_setup_success_ = std::move(on_setup_success);
on_setup_error_ = std::move(on_setup_error);

VLOG(1) << "Starting demo setup "
<< DemoSession::DemoConfigToString(demo_config_);

SetCurrentSetupStep(DemoSetupStep::kDownloadResources);

switch (demo_config_) {
case DemoSession::DemoModeConfig::kOnline:
LoadDemoResourcesCrOSComponent();
Expand Down Expand Up @@ -577,7 +618,7 @@ void DemoSetupController::OnDemoResourcesCrOSComponentLoaded() {
base::TimeTicks::Now() - download_start_time_;
base::UmaHistogramLongTimes100(kDemoSetupDownloadDurationHistogram,
download_duration);
IncrementSetupProgress(/*complete=*/false);
SetCurrentSetupStep(DemoSetupStep::kEnrollment);

if (demo_resources_->component_error().value() !=
component_updater::CrOSComponentManager::Error::NONE) {
Expand Down Expand Up @@ -662,7 +703,6 @@ 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) {
Expand Down Expand Up @@ -764,8 +804,6 @@ void DemoSetupController::OnDeviceLocalAccountPolicyLoaded(
}

void DemoSetupController::OnDeviceRegistered() {
IncrementSetupProgress(/*complete=*/true);

VLOG(1) << "Demo mode setup finished successfully.";

if (demo_config_ == DemoSession::DemoModeConfig::kOnline) {
Expand All @@ -782,6 +820,8 @@ void DemoSetupController::OnDeviceRegistered() {
base::UmaHistogramCounts100(kDemoSetupNumRetriesHistogram,
num_setup_retries_);

SetCurrentSetupStep(DemoSetupStep::kComplete);

PrefService* prefs = g_browser_process->local_state();
prefs->SetInteger(prefs::kDemoModeConfig, static_cast<int>(demo_config_));
prefs->CommitPendingWrite();
Expand All @@ -790,9 +830,9 @@ 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::SetCurrentSetupStep(DemoSetupStep current_step) {
if (!set_current_setup_step_.is_null())
set_current_setup_step_.Run(current_step);
}

void DemoSetupController::SetupFailed(const DemoSetupError& error) {
Expand Down
35 changes: 26 additions & 9 deletions chrome/browser/chromeos/login/demo_mode/demo_setup_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,21 @@ namespace chromeos {

class DemoResources;

// Controlls enrollment flow for setting up Demo Mode.
// Controls enrollment flow for setting up Demo Mode.
class DemoSetupController
: public EnterpriseEnrollmentHelper::EnrollmentStatusConsumer,
public policy::CloudPolicyStore::Observer {
public:
// All steps required for setup.
enum class DemoSetupStep {
// Downloading Demo Mode resources.
kDownloadResources,
// Enrolling in Demo Mode.
kEnrollment,
// Setup is complete.
kComplete
};

// Contains information related to setup error.
class DemoSetupError {
public:
Expand Down Expand Up @@ -155,7 +165,8 @@ class DemoSetupController
// Demo mode setup callbacks.
using OnSetupSuccess = base::OnceClosure;
using OnSetupError = base::OnceCallback<void(const DemoSetupError&)>;
using OnIncrementSetupProgress = base::RepeatingCallback<void(bool)>;
using OnSetCurrentSetupStep =
base::RepeatingCallback<void(const DemoSetupStep)>;
using HasPreinstalledDemoResourcesCallback = base::OnceCallback<void(bool)>;

static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
Expand All @@ -177,6 +188,12 @@ class DemoSetupController
// Otherwise, returns an empty string.
static std::string GetSubOrganizationEmail();

// Returns a dictionary mapping setup steps to step indices.
static base::Value GetDemoSetupSteps();

// Converts a step enum to a string e.g. to sent to JavaScript.
static std::string GetDemoSetupStepString(const DemoSetupStep step_enum);

DemoSetupController();
~DemoSetupController() override;

Expand All @@ -194,11 +211,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. |update_setup_progress| will be called when enrollment progress
// is updated.
// an error. |set_current_setup_step| will be called when an enrollment step
// completes.
void Enroll(OnSetupSuccess on_setup_success,
OnSetupError on_setup_error,
const OnIncrementSetupProgress& increment_setup_progress);
const OnSetCurrentSetupStep& set_current_setup_step);

// Tries to mount the preinstalled offline resources necessary for offline
// Demo Mode.
Expand Down Expand Up @@ -257,8 +274,8 @@ 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);
// Sets current setup step.
void SetCurrentSetupStep(DemoSetupStep current_step);

// Finish the flow with an error.
void SetupFailed(const DemoSetupError& error);
Expand Down Expand Up @@ -292,8 +309,8 @@ 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 setup step is updated.
OnSetCurrentSetupStep set_current_setup_step_;

// Callback to call when enrollment finishes with an error.
OnSetupError on_setup_error_;
Expand Down
Loading

0 comments on commit 074aa44

Please sign in to comment.