Skip to content

Commit

Permalink
arc: Check battery state before showing ARCVM /data migration screen
Browse files Browse the repository at this point in the history 
When the device does not have enough battery, show a warning and disable
the update button (and also ask the user to connect their device to a
charger if not).

Screenshots:
https://screenshot.googleplex.com/87VrAjHYFDyKAs3 (low storage)
https://screenshot.googleplex.com/9ccCQc44Qn9MWMZ (low battery,charged)
https://screenshot.googleplex.com/B7dReN9bWpiboDJ (low battery,!charged)

Bug: b:258278176
Test: --enable-features=ArcVmDataMigration
Test: Connect/Disconnect the device to/from a charger. Confirm that a
warning to ask charging disappears/appears if it has enough free disk
space but does not have enough battery.
Test: Keep connecting the device to a charger when it has enough free
disk space but does not have enough battery. Confirm that a warning
about the required battery % disappears and the update button is enabled
when the battery % reaches the threshold.

Change-Id: I2209d950cf863103dfb70dba38ed45518255b22f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4049541
Reviewed-by: Satoshi Niwa <niwa@chromium.org>
Commit-Queue: Youkichi Hosoi <youkichihosoi@chromium.org>
Reviewed-by: Danila Kuzmin <dkuzmin@google.com>
Cr-Commit-Position: refs/heads/main@{#1100894}
  • Loading branch information
Youkichi Hosoi authored and Chromium LUCI CQ committed Feb 3, 2023
1 parent 9fefa9a commit 758e476
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 7 deletions.
46 changes: 44 additions & 2 deletions chrome/browser/ash/login/screens/arc_vm_data_migration_screen.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ namespace ash {
namespace {

constexpr char kPathToCheckFreeDiskSpace[] = "/home/chronos/user";
// TODO(b/258278176): Set appropriate thresholds based on experiments.
constexpr int64_t kMinimumFreeDiskSpaceForMigration = 1LL << 30; // 1 GB.
constexpr double kMinimumBatteryPercent = 30.0;

constexpr char kUserActionSkip[] = "skip";
constexpr char kUserActionUpdate[] = "update";
Expand All @@ -42,6 +44,32 @@ ArcVmDataMigrationScreen::ArcVmDataMigrationScreen(

ArcVmDataMigrationScreen::~ArcVmDataMigrationScreen() = default;

void ArcVmDataMigrationScreen::PowerChanged(
const power_manager::PowerSupplyProperties& proto) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (proto.has_battery_percent()) {
battery_percent_ = proto.battery_percent();
}

if (proto.has_external_power()) {
is_connected_to_charger_ =
proto.external_power() !=
power_manager::PowerSupplyProperties_ExternalPower_DISCONNECTED;
}

if (!view_) {
return;
}
view_->SetBatteryState(battery_percent_ >= kMinimumBatteryPercent,
is_connected_to_charger_);

// TODO(b/258278176): Properly handle cases like the resume screen and the
// progress screen.
if (current_ui_state_ == ArcVmDataMigrationScreenView::UIState::kLoading) {
UpdateUIState(ArcVmDataMigrationScreenView::UIState::kWelcome);
}
}

void ArcVmDataMigrationScreen::ShowImpl() {
if (!view_) {
return;
Expand Down Expand Up @@ -84,7 +112,6 @@ void ArcVmDataMigrationScreen::OnUserAction(const base::Value::List& args) {
}

void ArcVmDataMigrationScreen::SetUpInitialView() {
// TODO(b/258278176): Check battery state.
arc::ArcVmDataMigrationStatus data_migration_status =
arc::GetArcVmDataMigrationStatus(profile_->GetPrefs());
switch (data_migration_status) {
Expand Down Expand Up @@ -126,13 +153,28 @@ void ArcVmDataMigrationScreen::OnGetFreeDiskSpace(
VLOG(1) << "Free disk space is " << free_disk_space;
if (free_disk_space < kMinimumFreeDiskSpaceForMigration) {
view_->SetRequiredFreeDiskSpace(kMinimumFreeDiskSpaceForMigration);
// Update the UI to show the low disk space warning and return, because the
// user cannot free up the disk space while in the screen, and thus there is
// no point in reporting the battery state in this case.
DCHECK_EQ(current_ui_state_,
ArcVmDataMigrationScreenView::UIState::kLoading);
UpdateUIState(ArcVmDataMigrationScreenView::UIState::kWelcome);
return;
}

UpdateUIState(ArcVmDataMigrationScreenView::UIState::kWelcome);
view_->SetMinimumBatteryPercent(kMinimumBatteryPercent);

// Request PowerManager to report the battery status updates. The UI will be
// updated on PowerChanged().
DCHECK(chromeos::PowerManagerClient::Get());
power_manager_observation_.Observe(chromeos::PowerManagerClient::Get());
chromeos::PowerManagerClient::Get()->RequestStatusUpdate();
}

void ArcVmDataMigrationScreen::UpdateUIState(
ArcVmDataMigrationScreenView::UIState state) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
current_ui_state_ = state;
if (view_) {
view_->SetUIState(state);
}
Expand Down
21 changes: 20 additions & 1 deletion chrome/browser/ash/login/screens/arc_vm_data_migration_screen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
#define CHROME_BROWSER_ASH_LOGIN_SCREENS_ARC_VM_DATA_MIGRATION_SCREEN_H_

#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
#include "base/sequence_checker.h"
#include "chrome/browser/ash/login/screens/base_screen.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/webui/ash/login/arc_vm_data_migration_screen_handler.h"
#include "chromeos/dbus/power/power_manager_client.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/device/public/mojom/wake_lock.mojom.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
Expand All @@ -17,14 +20,18 @@ namespace ash {

class ScopedScreenLockBlocker;

class ArcVmDataMigrationScreen : public BaseScreen {
class ArcVmDataMigrationScreen : public BaseScreen,
public chromeos::PowerManagerClient::Observer {
public:
explicit ArcVmDataMigrationScreen(
base::WeakPtr<ArcVmDataMigrationScreenView> view);
~ArcVmDataMigrationScreen() override;
ArcVmDataMigrationScreen(const ArcVmDataMigrationScreen&) = delete;
ArcVmDataMigrationScreen& operator=(const ArcVmDataMigrationScreen&) = delete;

// chromeos::PowerManagerClient::Observer override:
void PowerChanged(const power_manager::PowerSupplyProperties& proto) override;

private:
// BaseScreen overrides:
void ShowImpl() override;
Expand All @@ -46,11 +53,23 @@ class ArcVmDataMigrationScreen : public BaseScreen {

Profile* profile_;

ArcVmDataMigrationScreenView::UIState current_ui_state_ =
ArcVmDataMigrationScreenView::UIState::kLoading;

double battery_percent_ = 100.0;
bool is_connected_to_charger_ = true;

base::ScopedObservation<chromeos::PowerManagerClient,
chromeos::PowerManagerClient::Observer>
power_manager_observation_{this};

mojo::Remote<device::mojom::WakeLock> wake_lock_;
std::unique_ptr<ScopedScreenLockBlocker> scoped_screen_lock_blocker_;

base::WeakPtr<ArcVmDataMigrationScreenView> view_;

SEQUENCE_CHECKER(sequence_checker_);

base::WeakPtrFactory<ArcVmDataMigrationScreen> weak_ptr_factory_{this};
};

Expand Down
28 changes: 24 additions & 4 deletions chrome/browser/resources/chromeos/login/screens/login/arc_vm_data_migration.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@
}
div.warning {
border-bottom: var(--cr-separator-line);
padding-bottom: 11px;
padding-top: 22px;
padding-bottom: 19px;
padding-top: 21px;
}
div.warning-last-child {
padding-bottom: 19px;
padding-top: 21px;
}
.warning-message {
margin-inline-start: 20px;
Expand All @@ -34,13 +38,28 @@ <h1 slot="title">[[i18nDynamic(locale, 'welcomeScreenTitle')]]</h1>
[[i18nDynamic(locale, 'welcomeScreenDescriptionBody')]]
</div>
<div class="message-container">
<div class="warning" hidden="[[hasEnoughFreeDiskSpace]]">
<div class="warning-last-child" hidden="[[hasEnoughFreeDiskSpace]]">
<iron-icon slot="icon" icon="oobe-32:warning"></iron-icon>
<span class="warning-message">
[[i18nDynamic(locale, 'notEnoughFreeDiskSpaceMessage',
requiredFreeDiskSpaceInString)]]
</span>
</div>
<template is="dom-if" if="[[!hasEnoughBattery]]">
<div class="warning" hidden="[[isConnectedToCharger]]">
<iron-icon icon="oobe-32:warning"></iron-icon>
<span class="warning-message">
[[i18nDynamic(locale, 'connectToChargerMessage')]]
</span>
</div>
<div class="warning-last-child">
<iron-icon icon="oobe-32:warning"></iron-icon>
<span class="warning-message">
[[i18nDynamic(locale, 'notEnoughBatteryMessage',
minimumBatteryPercent)]]
</span>
</div>
</template>
</div>
</div>
<div slot="bottom-buttons">
Expand All @@ -49,7 +68,8 @@ <h1 slot="title">[[i18nDynamic(locale, 'welcomeScreenTitle')]]</h1>
text-key="skipButtonLabel">
</oobe-text-button>
<oobe-text-button inverse id="update-button"
disabled="[[!hasEnoughFreeDiskSpace]]"
disabled="[[shouldDisableUpdateButton_(hasEnoughFreeDiskSpace,
hasEnoughBattery)]]"
on-click="onUpdateButtonClicked_"
text-key="updateButtonLabel">
</oobe-text-button>
Expand Down
21 changes: 21 additions & 0 deletions chrome/browser/resources/chromeos/login/screens/login/arc_vm_data_migration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,19 @@ class ArcVmDataMigrationScreen extends ArcVmDataMigrationScreenElementBase {
return {
hasEnoughFreeDiskSpace: Boolean,
requiredFreeDiskSpaceInString: String,
minimumBatteryPercent: Number,
hasEnoughBattery: Boolean,
isConnectedToCharger: Boolean,
};
}

constructor() {
super();
this.hasEnoughFreeDiskSpace = true;
this.requiredFreeDiskSpaceInString = '';
this.minimumBatteryPercent = 0;
this.hasEnoughBattery = true;
this.isConnectedToCharger = true;
}

defaultUIStep() {
Expand All @@ -83,6 +89,8 @@ class ArcVmDataMigrationScreen extends ArcVmDataMigrationScreenElementBase {
return [
'setUIState',
'setRequiredFreeDiskSpace',
'setMinimumBatteryPercent',
'setBatteryState',
];
}

Expand All @@ -104,6 +112,19 @@ class ArcVmDataMigrationScreen extends ArcVmDataMigrationScreenElementBase {
this.requiredFreeDiskSpaceInString = requiredFreeDiskSpaceInString;
}

setMinimumBatteryPercent(minimumBatteryPercent) {
this.minimumBatteryPercent = Math.floor(minimumBatteryPercent);
}

setBatteryState(hasEnoughBattery, isConnectedToCharger) {
this.hasEnoughBattery = hasEnoughBattery;
this.isConnectedToCharger = isConnectedToCharger;
}

shouldDisableUpdateButton_(hasEnoughFreeDiskSpace, hasEnoughBattery) {
return !hasEnoughFreeDiskSpace || !hasEnoughBattery;
}

onSkipButtonClicked_() {
this.userActed(ArcVmDataMigrationUserAction.SKIP);
}
Expand Down
12 changes: 12 additions & 0 deletions chrome/browser/ui/webui/ash/login/arc_vm_data_migration_screen_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ void ArcVmDataMigrationScreenHandler::DeclareLocalizedValues(
u"your device connected to a charger during the update.");
builder->Add("notEnoughFreeDiskSpaceMessage",
u"Free up more than $1 of space");
builder->Add("notEnoughBatteryMessage",
u"Your battery must be charged above $1%");
builder->Add("connectToChargerMessage", u"Connect your device to a charger");
builder->Add("skipButtonLabel", u"Remind me later");
builder->Add("updateButtonLabel", u"Next");
}
Expand All @@ -45,4 +48,13 @@ void ArcVmDataMigrationScreenHandler::SetRequiredFreeDiskSpace(
ui::FormatBytes(required_free_disk_space));
}

void ArcVmDataMigrationScreenHandler::SetMinimumBatteryPercent(double percent) {
CallExternalAPI("setMinimumBatteryPercent", percent);
}

void ArcVmDataMigrationScreenHandler::SetBatteryState(bool enough,
bool connected) {
CallExternalAPI("setBatteryState", enough, connected);
}

} // namespace ash
4 changes: 4 additions & 0 deletions chrome/browser/ui/webui/ash/login/arc_vm_data_migration_screen_handler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ class ArcVmDataMigrationScreenView
virtual void Show() = 0;
virtual void SetUIState(UIState state) = 0;
virtual void SetRequiredFreeDiskSpace(int64_t required_free_disk_space) = 0;
virtual void SetMinimumBatteryPercent(double percent) = 0;
virtual void SetBatteryState(bool enough, bool connected) = 0;
};

class ArcVmDataMigrationScreenHandler : public BaseScreenHandler,
Expand All @@ -53,6 +55,8 @@ class ArcVmDataMigrationScreenHandler : public BaseScreenHandler,
void Show() override;
void SetUIState(UIState state) override;
void SetRequiredFreeDiskSpace(int64_t required_free_disk_space) override;
void SetMinimumBatteryPercent(double percent) override;
void SetBatteryState(bool enough, bool connected) override;
};

} // namespace ash
Expand Down

0 comments on commit 758e476

Please sign in to comment.