Skip to content

Commit

Permalink
glanceables: Refactor SystemTrayClient::RequestRestartForUpdate()
Browse files Browse the repository at this point in the history
Move the method to SessionControllerClient. This consolidates more of
the signout/restart logic in SessionControllerClient.

For glanceables we're experimenting with taking a screenshot of open
windows during signout/restart/shutdown. This consolidation means
there are will be fewer classes to modify to take the screenshot.

Bug: 1357058
Test: updated ash_unittests
Change-Id: I55b210f355c44ca142f51929816a5f65dcb615e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3868795
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1042342}
  • Loading branch information
James Cook authored and Chromium LUCI CQ committed Sep 1, 2022
1 parent 2100d29 commit 919cf20
Show file tree
Hide file tree
Showing 14 changed files with 34 additions and 13 deletions.
3 changes: 3 additions & 0 deletions ash/public/cpp/session/session_controller_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class ASH_PUBLIC_EXPORT SessionControllerClient {
// Requests signing out all users, ending the current session.
virtual void RequestSignOut() = 0;

// Requests to restart the system for OS update.
virtual void RequestRestartForUpdate() = 0;

// Attempts to restart the chrome browser.
virtual void AttemptRestartChrome() = 0;

Expand Down
3 changes: 0 additions & 3 deletions ash/public/cpp/system_tray_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,6 @@ class ASH_PUBLIC_EXPORT SystemTrayClient {
// Shows the Firmware update app.
virtual void ShowFirmwareUpdate() = 0;

// Attempts to restart the system for update.
virtual void RequestRestartForUpdate() = 0;

// Sets the UI locale to |locale_iso_code| and exit the session to take
// effect.
virtual void SetLocaleAndExit(const std::string& locale_iso_code) = 0;
Expand Down
2 changes: 0 additions & 2 deletions ash/public/cpp/test/test_system_tray_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,6 @@ void TestSystemTrayClient::ShowFirmwareUpdate() {
show_firmware_update_count_++;
}

void TestSystemTrayClient::RequestRestartForUpdate() {}

void TestSystemTrayClient::SetLocaleAndExit(
const std::string& locale_iso_code) {}

Expand Down
1 change: 0 additions & 1 deletion ash/public/cpp/test/test_system_tray_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ class ASH_PUBLIC_EXPORT TestSystemTrayClient : public SystemTrayClient {
void ShowNetworkSettings(const std::string& network_id) override;
void ShowMultiDeviceSetup() override;
void ShowFirmwareUpdate() override;
void RequestRestartForUpdate() override;
void SetLocaleAndExit(const std::string& locale_iso_code) override;
void ShowAccessCodeCastingDialog(
AccessCodeCastDialogOpenLocation open_location) override;
Expand Down
5 changes: 5 additions & 0 deletions ash/session/session_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ void SessionControllerImpl::ProceedWithSignOut() {
client_->RequestSignOut();
}

void SessionControllerImpl::RequestRestartForUpdate() {
if (client_)
client_->RequestRestartForUpdate();
}

void SessionControllerImpl::AttemptRestartChrome() {
if (client_)
client_->AttemptRestartChrome();
Expand Down
3 changes: 3 additions & 0 deletions ash/session/session_controller_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ class ASH_EXPORT SessionControllerImpl : public SessionController {
// should use LockStateController::RequestSignOut() instead.
void RequestSignOut();

// Requests a system restart to apply an OS update.
void RequestRestartForUpdate();

// Attempts to restart the chrome browser.
void AttemptRestartChrome();

Expand Down
4 changes: 4 additions & 0 deletions ash/session/test_session_controller_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,10 @@ void TestSessionControllerClient::RequestSignOut() {
++request_sign_out_count_;
}

void TestSessionControllerClient::RequestRestartForUpdate() {
++request_restart_for_update_count_;
}

void TestSessionControllerClient::AttemptRestartChrome() {
++attempt_restart_chrome_count_;
}
Expand Down
5 changes: 5 additions & 0 deletions ash/session/test_session_controller_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ class TestSessionControllerClient : public SessionControllerClient {
return attempt_restart_chrome_count_;
}
int request_sign_out_count() const { return request_sign_out_count_; }
int request_restart_for_update_count() const {
return request_restart_for_update_count_;
}

// Helpers to set SessionController state.
void SetCanLockScreen(bool can_lock);
Expand Down Expand Up @@ -129,6 +132,7 @@ class TestSessionControllerClient : public SessionControllerClient {
void RequestLockScreen() override;
void RequestHideLockScreen() override;
void RequestSignOut() override;
void RequestRestartForUpdate() override;
void AttemptRestartChrome() override;
void SwitchActiveUser(const AccountId& account_id) override;
void CycleActiveUser(CycleUserDirection direction) override;
Expand Down Expand Up @@ -160,6 +164,7 @@ class TestSessionControllerClient : public SessionControllerClient {

bool use_lower_case_user_id_ = true;
int request_sign_out_count_ = 0;
int request_restart_for_update_count_ = 0;
int attempt_restart_chrome_count_ = 0;

bool should_show_lock_screen_ = false;
Expand Down
2 changes: 1 addition & 1 deletion ash/system/update/update_notification_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ void UpdateNotificationController::RestartForUpdate() {
return;
}
// System updates require restarting the device.
Shell::Get()->system_tray_model()->client()->RequestRestartForUpdate();
Shell::Get()->session_controller()->RequestRestartForUpdate();
base::RecordAction(
base::UserMetricsAction("StatusArea_OS_Update_Default_Selected"));
}
Expand Down
8 changes: 8 additions & 0 deletions ash/system/update/update_notification_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,14 @@ TEST_F(UpdateNotificationControllerTest, VisibilityAfterUpdate) {
base::UTF16ToUTF8(system_app_name_) + " update",
GetNotificationMessage());
EXPECT_EQ("Restart to update", GetNotificationButton(0));

// Click the restart button.
message_center::MessageCenter::Get()->ClickOnNotificationButton(
kNotificationId, 0);

// Restart was requested.
EXPECT_EQ(1,
GetSessionControllerClient()->request_restart_for_update_count());
}

// Tests that the update icon becomes visible when an update becomes
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ui/ash/session_controller_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ void SessionControllerClientImpl::RequestSignOut() {
chrome::AttemptUserExit();
}

void SessionControllerClientImpl::RequestRestartForUpdate() {
browser_shutdown::NotifyAndTerminate(/*fast_path=*/true);
}

void SessionControllerClientImpl::AttemptRestartChrome() {
chrome::AttemptRestart();
}
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/ash/session_controller_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class SessionControllerClientImpl
void RequestLockScreen() override;
void RequestHideLockScreen() override;
void RequestSignOut() override;
void RequestRestartForUpdate() override;
void AttemptRestartChrome() override;
void SwitchActiveUser(const AccountId& account_id) override;
void CycleActiveUser(ash::CycleUserDirection direction) override;
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/ui/ash/system_tray_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#include "chrome/browser/browser_process_platform_part.h"
#include "chrome/browser/chromeos/extensions/vpn_provider/vpn_service_factory.h"
#include "chrome/browser/lifetime/application_lifetime.h"
#include "chrome/browser/lifetime/termination_notification.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/ash/system_web_apps/system_web_app_ui_utils.h"
#include "chrome/browser/ui/browser_navigator_params.h"
Expand Down Expand Up @@ -696,10 +695,6 @@ void SystemTrayClientImpl::ShowFirmwareUpdate() {
chrome::ShowFirmwareUpdatesApp(ProfileManager::GetActiveUserProfile());
}

void SystemTrayClientImpl::RequestRestartForUpdate() {
browser_shutdown::NotifyAndTerminate(/*fast_path=*/true);
}

void SystemTrayClientImpl::SetLocaleAndExit(
const std::string& locale_iso_code) {
ProfileManager::GetActiveUserProfile()->ChangeAppLocale(
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/ash/system_tray_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ class SystemTrayClientImpl : public ash::SystemTrayClient,
void ShowNetworkSettings(const std::string& network_id) override;
void ShowMultiDeviceSetup() override;
void ShowFirmwareUpdate() override;
void RequestRestartForUpdate() override;
void SetLocaleAndExit(const std::string& locale_iso_code) override;
void ShowAccessCodeCastingDialog(
AccessCodeCastDialogOpenLocation open_location) override;
Expand Down

0 comments on commit 919cf20

Please sign in to comment.