Skip to content

Commit

Permalink
Show rollback-specific strings for users during/after rollback happens
Browse files Browse the repository at this point in the history
Change text on the update screen:
- During rollback:
    "Your administrator is rolling back this device (25%)"
    https://screenshot.googleplex.com/WrMrGxdOU86.png
- After rollback:
    "Your administrator rolled back this device. Please save important
     files, then restart. All data on the device will be deleted."
- Button text:
    "Restart and reset"
    https://screenshot.googleplex.com/Zq2QsMTXUdt.png

Change text of the notification:
- https://screenshot.googleplex.com/vVdBEHcKzKR.png
- Title:
    "Device will be rolled back"
- Text:
    "Your administrator is rolling back your device. All data will be
     deleted when the device is restarted."
- Button:
    "Restart and reset"

Notification is also shown at warning level.

Bug: 863088
Change-Id: I99cd590fee90d7ac1ad5927b3459f9cb97a051a1
Reviewed-on: https://chromium-review.googlesource.com/1143481
Commit-Queue: Marton Hunyady <hunyadym@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576922}
  • Loading branch information
Marton Hunyady authored and Commit Bot committed Jul 20, 2018
1 parent 92409c5 commit 37ecdeb
Show file tree
Hide file tree
Showing 23 changed files with 162 additions and 54 deletions.
9 changes: 9 additions & 0 deletions ash/ash_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -366,15 +366,24 @@ This file contains the strings for ash.
Adobe Flash Player update available
</message>
</if>
<message name="IDS_ROLLBACK_NOTIFICATION_TITLE" desc="The title of the notification to notify that the user should restart to roll back the device.">
Device will be rolled back
</message>
<message name="IDS_UPDATE_NOTIFICATION_MESSAGE_LEARN_MORE" desc="The body of the notification to notify that the user should restart to get system updates. This notification body links to the info page on the update.">
Learn more about the latest <ph name="SYSTEM_APP_NAME">$1<ex>Chromium OS</ex></ph> update
</message>
<message name="IDS_UPDATE_NOTIFICATION_RESTART_BUTTON" meaning="button label" desc="The label used as the button to restart system and update. Displayed as the action button of the notification for system update.">
Restart to update
</message>
<message name="IDS_ROLLBACK_NOTIFICATION_RESTART_BUTTON" meaning="button label" desc="The label used as the button to restart system to rollback. Displayed as the action button of the notification for system rollback.">
Restart and reset
</message>
<message name="IDS_UPDATE_NOTIFICATION_MESSAGE_POWERWASH" desc="The notification message used in the system notification update when user need to powerwash the device in order to update the system.">
This update requires powerwashing your device. Learn more about the latest <ph name="SYSTEM_APP_NAME">$1<ex>Chromium OS</ex></ph> update.
</message>
<message name="IDS_UPDATE_NOTIFICATION_MESSAGE_ROLLBACK" desc="The notification message used in the system notification update when the device is rolled back and data will be deleted.">
Your administrator is rolling back your device. All data will be deleted when the device is restarted.
</message>

<message name="IDS_ASH_STATUS_TRAY_VOLUME" desc="The accessible text for the volume slider.">
Volume
Expand Down
16 changes: 13 additions & 3 deletions ash/public/interfaces/system_tray.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,21 @@ interface SystemTray {
// tracing is running.
SetPerformanceTracingIconVisible(bool visible);

// Shows an icon in the system tray indicating that a software update is
// available. Once shown the icon persists until reboot. |severity| and
// |factory_reset_required| are used to set the icon, color, and tooltip.
// Shows an icon in the system tray or a notification on the unified system
// menu indicating that a software update is available. Once shown, the icon
// or the notification persists until reboot.
// |severity| specifies how critical is the update.
// |factory_reset_required| is true if during the update the device will
// be wiped.
// |rollback| specifies whether the update is actually an admin-initiated
// rollback. This implies that a the device will be wiped.
// |update_type| specifies the component which has been updated.
//
// These values are used to control the icon, color and the text of the
// tooltip or the notification.
ShowUpdateIcon(UpdateSeverity severity,
bool factory_reset_required,
bool rollback,
UpdateType update_type);

// If |visible| is true, shows an icon in the system tray which indicates that
Expand Down
3 changes: 2 additions & 1 deletion ash/system/model/system_tray_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@ void SystemTrayModel::SetPerformanceTracingIconVisible(bool visible) {

void SystemTrayModel::ShowUpdateIcon(mojom::UpdateSeverity severity,
bool factory_reset_required,
bool rollback,
mojom::UpdateType update_type) {
update_model()->SetUpdateAvailable(severity, factory_reset_required,
update_model()->SetUpdateAvailable(severity, factory_reset_required, rollback,
update_type);
}

Expand Down
1 change: 1 addition & 0 deletions ash/system/model/system_tray_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class SystemTrayModel : public mojom::SystemTray {
void SetPerformanceTracingIconVisible(bool visible) override;
void ShowUpdateIcon(mojom::UpdateSeverity severity,
bool factory_reset_required,
bool rollback,
mojom::UpdateType update_type) override;
void SetUpdateOverCellularAvailableIconVisible(bool visible) override;
void ShowVolumeSliderBubble() override;
Expand Down
2 changes: 2 additions & 0 deletions ash/system/model/update_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ void UpdateModel::RemoveObserver(UpdateObserver* observer) {

void UpdateModel::SetUpdateAvailable(mojom::UpdateSeverity severity,
bool factory_reset_required,
bool rollback,
mojom::UpdateType update_type) {
update_required_ = true;
severity_ = severity;
factory_reset_required_ = factory_reset_required;
rollback_ = rollback;
update_type_ = update_type;
NotifyUpdateAvailable();
}
Expand Down
9 changes: 6 additions & 3 deletions ash/system/model/update_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@ class UpdateModel {
void RemoveObserver(UpdateObserver* observer);

// Store the state that a software update is available. The state persists
// until reboot. Based on |severity| and |factory_reset_required|, the
// observer views can indicate the severity of the update to users by changing
// the icon, color, and tooltip.
// until reboot. Based on |severity|, |factory_reset_required| and |rollback|,
// the observer views can indicate the severity of the update to users by
// changing the icon, color, and tooltip.
void SetUpdateAvailable(mojom::UpdateSeverity severity,
bool factory_reset_required,
bool rollback,
mojom::UpdateType update_type);

// If |available| is true, a software update is available but user's agreement
Expand All @@ -45,6 +46,7 @@ class UpdateModel {

bool update_required() const { return update_required_; }
bool factory_reset_required() const { return factory_reset_required_; }
bool rollback() const { return rollback_; }
mojom::UpdateType update_type() const { return update_type_; }
bool update_over_cellular_available() const {
return update_over_cellular_available_;
Expand All @@ -56,6 +58,7 @@ class UpdateModel {
bool update_required_ = false;
mojom::UpdateSeverity severity_ = mojom::UpdateSeverity::NONE;
bool factory_reset_required_ = false;
bool rollback_ = false;
mojom::UpdateType update_type_ = mojom::UpdateType::SYSTEM;
bool update_over_cellular_available_ = false;

Expand Down
4 changes: 2 additions & 2 deletions ash/system/update/tray_update_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ TEST_F(TrayUpdateTest, VisibilityAfterUpdate) {

// Simulate an update.
Shell::Get()->system_tray_model()->ShowUpdateIcon(
mojom::UpdateSeverity::LOW, false, mojom::UpdateType::SYSTEM);
mojom::UpdateSeverity::LOW, false, false, mojom::UpdateType::SYSTEM);

// Tray item is now visible.
EXPECT_TRUE(tray_update->tray_view()->visible());
Expand All @@ -58,7 +58,7 @@ TEST_F(TrayUpdateTest, VisibilityAfterFlashUpdate) {

// Simulate an update.
Shell::Get()->system_tray_model()->ShowUpdateIcon(
mojom::UpdateSeverity::LOW, false, mojom::UpdateType::FLASH);
mojom::UpdateSeverity::LOW, false, false, mojom::UpdateType::FLASH);

// Tray item is now visible.
EXPECT_TRUE(tray_update->tray_view()->visible());
Expand Down
25 changes: 18 additions & 7 deletions ash/system/update/update_notification_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ void UpdateNotificationController::OnUpdateAvailable() {
return;
}

message_center::SystemNotificationWarningLevel warning_level =
model_->rollback()
? message_center::SystemNotificationWarningLevel::WARNING
: message_center::SystemNotificationWarningLevel::NORMAL;
std::unique_ptr<Notification> notification =
Notification::CreateSystemNotification(
message_center::NOTIFICATION_TYPE_SIMPLE, kNotificationId,
Expand All @@ -57,15 +61,18 @@ void UpdateNotificationController::OnUpdateAvailable() {
base::BindRepeating(
&UpdateNotificationController::HandleNotificationClick,
weak_ptr_factory_.GetWeakPtr())),
kSystemMenuUpdateIcon,
message_center::SystemNotificationWarningLevel::NORMAL);
kSystemMenuUpdateIcon, warning_level);
notification->set_pinned(true);

if (model_->update_required()) {
std::vector<message_center::ButtonInfo> notification_actions;
message_center::ButtonInfo button_info = message_center::ButtonInfo(
l10n_util::GetStringUTF16(IDS_UPDATE_NOTIFICATION_RESTART_BUTTON));
notification_actions.push_back(button_info);
if (model_->rollback()) {
notification_actions.push_back(message_center::ButtonInfo(
l10n_util::GetStringUTF16(IDS_ROLLBACK_NOTIFICATION_RESTART_BUTTON)));
} else {
notification_actions.push_back(message_center::ButtonInfo(
l10n_util::GetStringUTF16(IDS_UPDATE_NOTIFICATION_RESTART_BUTTON)));
}
notification->set_buttons(notification_actions);
}

Expand All @@ -79,6 +86,9 @@ bool UpdateNotificationController::ShouldShowUpdate() const {
base::string16 UpdateNotificationController::GetNotificationMessage() const {
base::string16 system_app_name =
l10n_util::GetStringUTF16(IDS_ASH_MESSAGE_CENTER_SYSTEM_APP_NAME);
if (model_->rollback()) {
return l10n_util::GetStringUTF16(IDS_UPDATE_NOTIFICATION_MESSAGE_ROLLBACK);
}
if (model_->factory_reset_required()) {
return l10n_util::GetStringFUTF16(IDS_UPDATE_NOTIFICATION_MESSAGE_POWERWASH,
system_app_name);
Expand All @@ -94,8 +104,9 @@ base::string16 UpdateNotificationController::GetNotificationTitle() const {
IDS_UPDATE_NOTIFICATION_TITLE_FLASH_PLAYER);
}
#endif

return l10n_util::GetStringUTF16(IDS_UPDATE_NOTIFICATION_TITLE);
return model_->rollback()
? l10n_util::GetStringUTF16(IDS_ROLLBACK_NOTIFICATION_TITLE)
: l10n_util::GetStringUTF16(IDS_UPDATE_NOTIFICATION_TITLE);
}

void UpdateNotificationController::HandleNotificationClick(
Expand Down
25 changes: 22 additions & 3 deletions ash/system/update/update_notification_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ TEST_F(UpdateNotificationControllerTest, VisibilityAfterUpdate) {

// Simulate an update.
Shell::Get()->system_tray_model()->ShowUpdateIcon(
mojom::UpdateSeverity::LOW, false, mojom::UpdateType::SYSTEM);
mojom::UpdateSeverity::LOW, false, false, mojom::UpdateType::SYSTEM);

// The notification is now visible.
ASSERT_TRUE(HasNotification());
Expand All @@ -103,7 +103,7 @@ TEST_F(UpdateNotificationControllerTest, VisibilityAfterFlashUpdate) {

// Simulate an update.
Shell::Get()->system_tray_model()->ShowUpdateIcon(
mojom::UpdateSeverity::LOW, false, mojom::UpdateType::FLASH);
mojom::UpdateSeverity::LOW, false, false, mojom::UpdateType::FLASH);

// The notification is now visible.
ASSERT_TRUE(HasNotification());
Expand Down Expand Up @@ -150,7 +150,7 @@ TEST_F(UpdateNotificationControllerTest,

// Simulate an update that requires factory reset.
Shell::Get()->system_tray_model()->ShowUpdateIcon(
mojom::UpdateSeverity::LOW, true, mojom::UpdateType::SYSTEM);
mojom::UpdateSeverity::LOW, true, false, mojom::UpdateType::SYSTEM);

// The notification is now visible.
ASSERT_TRUE(HasNotification());
Expand All @@ -162,4 +162,23 @@ TEST_F(UpdateNotificationControllerTest,
EXPECT_EQ("Restart to update", GetNotificationButton(0));
}

TEST_F(UpdateNotificationControllerTest, VisibilityAfterRollback) {
// The system starts with no update pending, so the notification isn't
// visible.
EXPECT_FALSE(HasNotification());

// Simulate a rollback.
Shell::Get()->system_tray_model()->ShowUpdateIcon(
mojom::UpdateSeverity::LOW, false, true, mojom::UpdateType::SYSTEM);

// The notification is now visible.
ASSERT_TRUE(HasNotification());
EXPECT_EQ("Device will be rolled back", GetNotificationTitle());
EXPECT_EQ(
"Your administrator is rolling back your device. All data will"
" be deleted when the device is restarted.",
GetNotificationMessage());
EXPECT_EQ("Restart and reset", GetNotificationButton(0));
}

} // namespace ash
10 changes: 8 additions & 2 deletions chrome/app/settings_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@
<message name="IDS_SETTINGS_ABOUT_PAGE_RELAUNCH" desc="The label for the relaunch button that relaunches the browser once update is complete">
Restart
</message>
<message name="IDS_SETTINGS_ABOUT_PAGE_RELAUNCH_AND_POWERWASH" desc="The label for the button that relaunches and powerwashes the browser once update is complete">
Relaunch and Powerwash
<message name="IDS_SETTINGS_ABOUT_PAGE_RELAUNCH_AND_POWERWASH" desc="The label for the button that relaunches and powerwashes the browser once update is complete">
Restart and reset
</message>
<message name="IDS_SETTINGS_ABOUT_TPM_FIRMWARE_UPDATE_TITLE" desc="Title for the line item on the about page that allows the user to trigger a device hardware reset and request installation of a TPM firmware update.">
Powerwash for added security
Expand All @@ -83,6 +83,12 @@
<message name="IDS_SETTINGS_UPGRADE_SUCCESSFUL_CHANNEL_SWITCH" desc="Status label: Channel was successfully switched on ChromiumOS/ChromeOS">
Channel changed. Restart your device to apply changes.
</message>
<message name="IDS_SETTINGS_UPGRADE_ROLLBACK_IN_PROGRESS" desc="Status label: Rolling back ChromiumOS or Chrome OS.">
Your administrator is rolling back this device (<ph name="PROGRESS_PERCENT">$1<ex>90%</ex></ph>)
</message>
<message name="IDS_SETTINGS_UPGRADE_ROLLBACK_SUCCESS" desc="Status label: Successfully rolled back Chrome OS. All data on the device will be deleted during the next reboot.">
Your administrator rolled back this device. Please save important files, then restart. All data on the device will be deleted.
</message>
<message name="IDS_SETTINGS_UPGRADE_UP_TO_DATE" desc="Status label: Already up to date (ChromiumOS/ChromeOS)">
Your <ph name="DEVICE_TYPE">$1<ex>Chromebook</ex></ph> is up to date
</message>
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/upgrade_detector_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ void UpgradeDetectorChromeos::UpdateStatusChanged(
set_upgrade_detected_time(tick_clock()->NowTicks());

if (status.is_rollback) {
set_is_factory_reset_required(true);
set_is_rollback(true);
NotifyOnUpgrade();
} else {
// Determine whether powerwash is required based on the channel.
Expand Down
27 changes: 21 additions & 6 deletions chrome/browser/resources/settings/about_page/about_page.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ Polymer({
/** @private {?UpdateStatusChangedEvent} */
currentUpdateStatusEvent_: {
type: Object,
value: {message: '', progress: 0, status: UpdateStatus.DISABLED},
value: {
message: '',
progress: 0,
rollback: false,
status: UpdateStatus.DISABLED
},
},

// <if expr="chromeos">
Expand Down Expand Up @@ -298,8 +303,8 @@ Polymer({
this.showRelaunch_ = this.checkStatus_(UpdateStatus.NEARLY_UPDATED);
// </if>
// <if expr="chromeos">
this.showRelaunch_ = this.checkStatus_(UpdateStatus.NEARLY_UPDATED) &&
!this.isTargetChannelMoreStable_();
this.showRelaunch_ =
this.checkStatus_(UpdateStatus.NEARLY_UPDATED) && !this.isRollback_();
// </if>
},

Expand All @@ -324,6 +329,8 @@ Polymer({
// <if expr="chromeos">
if (this.currentChannel_ != this.targetChannel_)
return this.i18nAdvanced('aboutUpgradeSuccessChannelSwitch');
if (this.currentUpdateStatusEvent_.rollback)
return this.i18nAdvanced('aboutRollbackSuccess');
// </if>
return this.i18nAdvanced('aboutUpgradeRelaunch');
case UpdateStatus.UPDATED:
Expand All @@ -342,6 +349,11 @@ Polymer({
]
});
}
if (this.currentUpdateStatusEvent_.rollback) {
return this.i18nAdvanced('aboutRollbackInProgress', {
substitutions: [progressPercent],
});
}
// </if>
if (this.currentUpdateStatusEvent_.progress > 0) {
// NOTE(dbeam): some platforms (i.e. Mac) always send 0% while
Expand Down Expand Up @@ -442,9 +454,13 @@ Polymer({
* @return {boolean}
* @private
*/
isTargetChannelMoreStable_: function() {
isRollback_: function() {
assert(this.currentChannel_.length > 0);
assert(this.targetChannel_.length > 0);
if (this.currentUpdateStatusEvent_.rollback) {
return true;
}
// Channel switch to a more stable channel is also a rollback
return settings.isTargetChannelMoreStable(
this.currentChannel_, this.targetChannel_);
},
Expand All @@ -464,8 +480,7 @@ Polymer({
* @private
*/
computeShowRelaunchAndPowerwash_: function() {
return this.checkStatus_(UpdateStatus.NEARLY_UPDATED) &&
this.isTargetChannelMoreStable_();
return this.checkStatus_(UpdateStatus.NEARLY_UPDATED) && this.isRollback_();
},

/** @private */
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/ash/system_tray_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ void SystemTrayClient::HandleUpdateAvailable() {
: ash::mojom::UpdateType::FLASH;

system_tray_->ShowUpdateIcon(severity, detector->is_factory_reset_required(),
update_type);
detector->is_rollback(), update_type);
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
25 changes: 16 additions & 9 deletions chrome/browser/ui/webui/help/version_updater.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,22 @@ class VersionUpdater {
EolStatusCallback;
#endif

// Used to update the client of status changes. int parameter is the progress
// and should only be non-zero for the UPDATING state.
// std::string parameter is the version of the available update and should be
// empty string when update is not available.
// int64_t parameter is the size in bytes of the available update and should
// be 0 when update is not available.
// base::string16 parameter is a message explaining a failure.
typedef base::Callback<
void(Status, int, const std::string&, int64_t, const base::string16&)>
// Used to update the client of status changes.
// |status| is the current state of the update.
// |progress| should only be non-zero for the UPDATING state.
// |rollback| indicates whether the update is actually a rollback, which
// requires wiping the device upon reboot.
// |version| is the version of the available update and should be empty string
// when update is not available.
// |update_size| is the size of the available update in bytes and should be 0
// when update is not available.
// |message| is a message explaining a failure.
typedef base::Callback<void(Status status,
int progress,
bool rollback,
const std::string& version,
int64_t update_size,
const base::string16& message)>
StatusCallback;

// Used to show or hide the promote UI elements. Mac-only.
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/ui/webui/help/version_updater_basic.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ void VersionUpdaterBasic::CheckForUpdate(
const StatusCallback& status_callback,
const PromoteCallback&) {
if (UpgradeDetector::GetInstance()->notify_upgrade())
status_callback.Run(NEARLY_UPDATED, 0, std::string(), 0, base::string16());
status_callback.Run(NEARLY_UPDATED, 0, false, std::string(), 0,
base::string16());
else
status_callback.Run(DISABLED, 0, std::string(), 0, base::string16());
status_callback.Run(DISABLED, 0, false, std::string(), 0, base::string16());
}

VersionUpdater* VersionUpdater::Create(content::WebContents* web_contents) {
Expand Down
Loading

0 comments on commit 37ecdeb

Please sign in to comment.