Skip to content

Commit

Permalink
UpdateScreen: Move to MultiStepBehavior
Browse files Browse the repository at this point in the history 
Bug: 1156673
Change-Id: Id2bb1c935a13efaf58642be2eab025b69c9d82d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2718247
Commit-Queue: Roman Aleksandrov <raleksandrov@google.com>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#859009}
  • Loading branch information
Roman Aleksandrov authored and Chromium LUCI CQ committed Mar 2, 2021
1 parent 29975ae commit b8bfdbf
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 127 deletions.
3 changes: 1 addition & 2 deletions chrome/browser/ash/login/screens/mock_update_screen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class MockUpdateView : public UpdateView {
MOCK_METHOD(void, MockBind, (UpdateScreen * screen));
MOCK_METHOD(void, MockUnbind, ());

MOCK_METHOD(void, SetUIState, (UpdateView::UIState value));
MOCK_METHOD(void, SetUpdateState, (UpdateView::UIState value));
MOCK_METHOD(void,
SetUpdateStatus,
(int percent,
Expand All @@ -49,7 +49,6 @@ class MockUpdateView : public UpdateView {
MOCK_METHOD(void, SetShowCurtain, (bool value));
MOCK_METHOD(void, SetProgressMessage, (const base::string16& value));
MOCK_METHOD(void, SetProgress, (int value));
MOCK_METHOD(void, SetRequiresPermissionForCellular, (bool value));
MOCK_METHOD(void, SetCancelUpdateShortcutEnabled, (bool value));
MOCK_METHOD(void, ShowLowBatteryWarningMessage, (bool value));
MOCK_METHOD(void, SetAutoTransition, (bool value));
Expand Down
37 changes: 16 additions & 21 deletions chrome/browser/ash/login/screens/update_screen.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,10 @@ void UpdateScreen::ShowImpl() {
view_->SetCancelUpdateShortcutEnabled(true);
}
#endif
RefreshView(version_updater_->update_info());
if (version_updater_->update_info().requires_permission_for_cellular &&
view_) {
view_->SetUpdateState(UpdateView::UIState::kCellularPermission);
}
show_timer_.Start(FROM_HERE, kShowDelay,
base::BindOnce(&UpdateScreen::MakeSureScreenIsShown,
weak_factory_.GetWeakPtr()));
Expand Down Expand Up @@ -231,7 +234,7 @@ void UpdateScreen::OnWaitForRebootTimeElapsed() {
MakeSureScreenIsShown();
if (!view_)
return;
view_->SetUIState(UpdateView::UIState::kManualReboot);
view_->SetUpdateState(UpdateView::UIState::kManualReboot);
}

void UpdateScreen::PrepareForUpdateCheck() {
Expand Down Expand Up @@ -288,25 +291,27 @@ void UpdateScreen::UpdateInfoChanged(
const VersionUpdater::UpdateInfo& update_info) {
const update_engine::StatusResult& status = update_info.status;
hide_progress_on_exit_ = false;
bool need_refresh_view = true;
if (update_info.requires_permission_for_cellular && view_) {
view_->SetUpdateState(UpdateView::UIState::kCellularPermission);
MakeSureScreenIsShown();
return;
}
switch (status.current_operation()) {
case update_engine::Operation::CHECKING_FOR_UPDATE:
if (view_)
view_->SetUIState(UpdateView::UIState::kCheckingForUpdate);
view_->SetUpdateState(UpdateView::UIState::kCheckingForUpdate);
if (start_update_stage_.is_null())
start_update_stage_ = tick_clock_->NowTicks();
need_refresh_view = false;
break;
// Do nothing in these cases, we don't want to notify the user of the
// check unless there is an update.
case update_engine::Operation::ATTEMPTING_ROLLBACK:
case update_engine::Operation::DISABLED:
case update_engine::Operation::IDLE:
need_refresh_view = false;
break;
case update_engine::Operation::UPDATE_AVAILABLE:
if (view_)
view_->SetUIState(UpdateView::UIState::kCheckingForUpdate);
view_->SetUpdateState(UpdateView::UIState::kCheckingForUpdate);
if (start_update_stage_.is_null())
start_update_stage_ = tick_clock_->NowTicks();
MakeSureScreenIsShown();
Expand All @@ -318,7 +323,7 @@ void UpdateScreen::UpdateInfoChanged(
break;
case update_engine::Operation::DOWNLOADING:
if (view_)
view_->SetUIState(UpdateView::UIState::KUpdateInProgress);
view_->SetUpdateState(UpdateView::UIState::kUpdateInProgress);
SetUpdateStatusMessage(update_info.better_update_progress,
update_info.total_time_left);
MakeSureScreenIsShown();
Expand All @@ -341,7 +346,7 @@ void UpdateScreen::UpdateInfoChanged(
break;
case update_engine::Operation::VERIFYING:
if (view_)
view_->SetUIState(UpdateView::UIState::KUpdateInProgress);
view_->SetUpdateState(UpdateView::UIState::kUpdateInProgress);
SetUpdateStatusMessage(update_info.better_update_progress,
update_info.total_time_left);
// Make sure that VERIFYING and DOWNLOADING stages are recorded correctly.
Expand All @@ -353,7 +358,7 @@ void UpdateScreen::UpdateInfoChanged(
break;
case update_engine::Operation::FINALIZING:
if (view_)
view_->SetUIState(UpdateView::UIState::KUpdateInProgress);
view_->SetUpdateState(UpdateView::UIState::kUpdateInProgress);
SetUpdateStatusMessage(update_info.better_update_progress,
update_info.total_time_left);
// Make sure that VERIFYING and FINALIZING stages are recorded correctly.
Expand Down Expand Up @@ -393,14 +398,11 @@ void UpdateScreen::UpdateInfoChanged(
} else {
ExitUpdate(Result::UPDATE_ERROR);
}
need_refresh_view = false;
break;
default:
NOTREACHED();
}
UpdateBatteryWarningVisibility();
if (need_refresh_view)
RefreshView(update_info);
}

void UpdateScreen::FinishExitUpdate(Result result) {
Expand All @@ -422,7 +424,7 @@ void UpdateScreen::PowerChanged(
void UpdateScreen::ShowRebootInProgress() {
MakeSureScreenIsShown();
if (view_)
view_->SetUIState(UpdateView::UIState::kRestartInProgress);
view_->SetUpdateState(UpdateView::UIState::kRestartInProgress);
}

void UpdateScreen::SetUpdateStatusMessage(int percent,
Expand Down Expand Up @@ -462,13 +464,6 @@ void UpdateScreen::UpdateBatteryWarningVisibility() {
proto->battery_percent() < kInsufficientBatteryPercent);
}

void UpdateScreen::RefreshView(const VersionUpdater::UpdateInfo& update_info) {
if (view_) {
view_->SetRequiresPermissionForCellular(
update_info.requires_permission_for_cellular);
}
}

bool UpdateScreen::HasCriticalUpdate() {
if (ignore_update_deadlines_)
return true;
Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/ash/login/screens/update_screen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,6 @@ class UpdateScreen : public BaseScreen,
FRIEND_TEST_ALL_PREFIXES(UpdateScreenTest, TestAPReselection);
friend class UpdateScreenUnitTest;

void RefreshView(const VersionUpdater::UpdateInfo& update_info);

// Returns true if there is critical system update that requires installation
// and immediate reboot.
bool HasCriticalUpdate();
Expand Down
22 changes: 14 additions & 8 deletions chrome/browser/ash/login/screens/update_screen_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,6 @@ IN_PROC_BROWSER_TEST_F(BetterUpdateScreenTest, TestUpdateCheckDoneBeforeShow) {

ASSERT_NE(GetOobeUI()->current_screen(), UpdateView::kScreenId);

// Show another screen, and verify the Update screen in not shown before it.
GetOobeUI()->GetView<NetworkScreenHandler>()->Show();
OobeScreenWaiter network_screen_waiter(NetworkScreenView::kScreenId);
network_screen_waiter.set_assert_next_screen();
network_screen_waiter.Wait();
histogram_tester_.ExpectTotalCount("OOBE.UpdateScreen.UpdateDownloadingTime",
0);
histogram_tester_.ExpectTotalCount(kTimeCheck, 1);
Expand Down Expand Up @@ -676,6 +671,9 @@ IN_PROC_BROWSER_TEST_F(BetterUpdateScreenTest, UpdateOverCellularAccepted) {
test::OobeJS().ExpectHiddenPath(kBetterUpdateCheckingForUpdatesDialog);

test::OobeJS().TapOnPath(kCellularPermissionNext);
status.set_current_operation(update_engine::Operation::CHECKING_FOR_UPDATE);
update_engine_client()->set_default_status(status);
update_engine_client()->NotifyObserversThatStatusChanged(status);

test::OobeJS()
.CreateVisibilityWaiter(true, kBetterUpdateCheckingForUpdatesDialog)
Expand Down Expand Up @@ -920,9 +918,14 @@ IN_PROC_BROWSER_TEST_F(BetterUpdateScreenTest, UpdateScreenSteps) {
update_engine_client()->set_default_status(status);
update_engine_client()->NotifyObserversThatStatusChanged(status);

test::OobeJS()
.CreateVisibilityWaiter(true, kBetterUpdateCheckingForUpdatesDialog)
->Wait();
test::OobeJS().ExpectHiddenPath(kBetterUpdateCheckingForUpdatesDialog);
test::OobeJS().ExpectHiddenPath(kUpdateInProgressDialog);
test::OobeJS().ExpectHiddenPath(kRestartingDialog);
test::OobeJS().ExpectHiddenPath(kBetterUpdateCompletedDialog);

update_screen_->GetShowTimerForTesting()->FireNow();

test::OobeJS().ExpectVisiblePath(kBetterUpdateCheckingForUpdatesDialog);
test::OobeJS().ExpectHiddenPath(kUpdateInProgressDialog);
test::OobeJS().ExpectHiddenPath(kRestartingDialog);
test::OobeJS().ExpectHiddenPath(kBetterUpdateCompletedDialog);
Expand Down Expand Up @@ -1021,6 +1024,9 @@ IN_PROC_BROWSER_TEST_F(BetterUpdateScreenTest, UpdateOverCellularShown) {
test::OobeJS().ExpectHiddenPath(kBetterUpdateCheckingForUpdatesDialog);

test::OobeJS().TapOnPath(kCellularPermissionNext);
status.set_current_operation(update_engine::Operation::UPDATE_AVAILABLE);
update_engine_client()->set_default_status(status);
update_engine_client()->NotifyObserversThatStatusChanged(status);

test::OobeJS()
.CreateVisibilityWaiter(true, kBetterUpdateCheckingForUpdatesDialog)
Expand Down
18 changes: 7 additions & 11 deletions chrome/browser/resources/chromeos/login/oobe_update.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<template>
<style include="oobe-dialog-host"></style>
<link rel="stylesheet" href="oobe_update.css">
<oobe-dialog hidden="[[!requiresPermissionForCellular]]" tabindex="0"
<oobe-dialog for-step="cellular" tabindex="0"
subtitle-key="updateOverCellularPromptMessage"
aria-live="polite" has-buttons footer-shrinkable
id="cellular-permission-dialog">
Expand All @@ -44,8 +44,7 @@ <h1 slot="title">
</div>
</oobe-dialog>
<oobe-dialog footer-shrinkable id="checking-for-updates-dialog"
hidden="[[!isCheckingForUpdate_(uiState,
requiresPermissionForCellular)]]"
for-step="checking"
title-key="checkingForUpdates" aria-live="polite">
<hd-iron-icon slot="oobe-icon"
icon1x="oobe-32:googleg" icon2x="oobe-64:googleg">
Expand All @@ -59,8 +58,7 @@ <h1 slot="title">
</div>
</oobe-dialog>
<oobe-dialog footer-shrinkable id="update-in-progress-dialog"
hidden="[[!isUpdateInProgress_(uiState,
requiresPermissionForCellular)]]"
for-step="update"
title-key="updateStatusTitle" aria-live="polite">
<hd-iron-icon slot="oobe-icon"
icon1x="oobe-32:googleg" icon2x="oobe-64:googleg">
Expand Down Expand Up @@ -91,8 +89,8 @@ <h1 slot="title">
</div>
<div hidden="[[showLowBatteryWarning]]" id="carousel" class="slide-view"
slot="footer">
<oobe-carousel slide-duration-in-seconds=3
auto-transition="[[getAutoTransition_(uiState, autoTransition)]]"
<oobe-carousel slide-duration-in-seconds=5
auto-transition="[[getAutoTransition_(uiStep, autoTransition)]]"
slide-label="slideLabel"
selected-button-label="slideSelectedButtonLabel"
unselected-button-label="slideUnselectedButtonLabel">
Expand Down Expand Up @@ -141,9 +139,7 @@ <h1 slot="title">
</oobe-carousel>
</div>
</oobe-dialog>
<oobe-dialog footer-shrinkable id="restarting-dialog"
hidden="[[!isRestartInProgress_(uiState,
requiresPermissionForCellular)]]"
<oobe-dialog footer-shrinkable id="restarting-dialog" for-step="restart"
title-key="updateCompeletedRebootingMsg" aria-live="polite">
<hd-iron-icon slot="oobe-icon"
icon1x="oobe-32:googleg" icon2x="oobe-64:googleg">
Expand All @@ -152,7 +148,7 @@ <h1 slot="title">
</paper-progress>
</oobe-dialog>
<oobe-dialog footer-shrinkable id="better-update-complete-dialog"
hidden="[[!isManualReboot_(uiState, requiresPermissionForCellular)]]"
for-step="reboot"
title-key="updateCompeletedMsg" aria-live="polite">
<hd-iron-icon slot="oobe-icon"
icon1x="oobe-32:googleg" icon2x="oobe-64:googleg">
Expand Down
Loading

0 comments on commit b8bfdbf

Please sign in to comment.