Skip to content

Commit

Permalink
kiosk: deprecate RestartDevice mojom api
Browse files Browse the repository at this point in the history
Lacros can call PowerManagerClient::RequestRestart directly instead of
calling this mojom API.

Fixed: b/256160292
Tests: RestartDeviceTest.Restart
Change-Id: Ia4fbbef9071c28821ac169db82023ab486add35c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3990595
Reviewed-by: Ben Franz <bfranz@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: Sergii Bykov <sbykov@google.com>
Reviewed-by: Roland Bock <rbock@google.com>
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Irina Fedorova <irfedorova@google.com>
Cr-Commit-Position: refs/heads/main@{#1069067}
  • Loading branch information
FedorovaIra authored and Chromium LUCI CQ committed Nov 9, 2022
1 parent 963a081 commit 0e80baa
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 87 deletions.
14 changes: 4 additions & 10 deletions chrome/browser/apps/platform_apps/app_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "chrome/common/url_constants.h"
#include "chrome/test/base/test_switches.h"
#include "chrome/test/base/ui_test_utils.h"
#include "chromeos/login/login_state/login_state.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "components/services/app_service/public/cpp/app_launch_util.h"
Expand Down Expand Up @@ -1355,7 +1356,6 @@ class RestartDeviceTest : public PlatformAppBrowserTest {
void TearDownOnMainThread() override {
PlatformAppBrowserTest::TearDownOnMainThread();
user_manager_enabler_.reset();
fake_user_manager_ = nullptr;
}

protected:
Expand All @@ -1364,18 +1364,12 @@ class RestartDeviceTest : public PlatformAppBrowserTest {
}

void EnterKioskSession() {
fake_user_manager_ = new ash::FakeChromeUserManager();
user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
base::WrapUnique(fake_user_manager_));

const AccountId kiosk_account_id(
AccountId::FromUserEmail("kiosk@foobar.com"));
fake_user_manager_->AddKioskAppUser(kiosk_account_id);
fake_user_manager_->LoginUser(kiosk_account_id);
chromeos::LoginState::Get()->SetLoggedInState(
chromeos::LoginState::LoggedInState::LOGGED_IN_ACTIVE,
chromeos::LoginState::LoggedInUserType::LOGGED_IN_USER_KIOSK);
}

private:
ash::FakeChromeUserManager* fake_user_manager_ = nullptr;
std::unique_ptr<user_manager::ScopedUserManager> user_manager_enabler_;
};

Expand Down
14 changes: 4 additions & 10 deletions chrome/browser/ash/crosapi/kiosk_session_service_ash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,10 @@ void KioskSessionServiceAsh::AttemptUserExit() {
chrome::AttemptUserExit();
}

void KioskSessionServiceAsh::RestartDevice(const std::string& description,
RestartDeviceCallback callback) {
if (user_manager::UserManager::Get()->IsLoggedInAsKioskApp() ||
user_manager::UserManager::Get()->IsLoggedInAsWebKioskApp()) {
chromeos::PowerManagerClient::Get()->RequestRestart(
power_manager::REQUEST_RESTART_OTHER, description);
std::move(callback).Run(true);
} else {
std::move(callback).Run(false);
}
void KioskSessionServiceAsh::RestartDeviceDeprecated(
const std::string& description,
RestartDeviceDeprecatedCallback callback) {
NOTIMPLEMENTED();
}

} // namespace crosapi
5 changes: 3 additions & 2 deletions chrome/browser/ash/crosapi/kiosk_session_service_ash.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ class KioskSessionServiceAsh : public mojom::KioskSessionService {
// crosapi::mojom::KioskSessionService:
void AttemptUserExit() override;

void RestartDevice(const std::string& description,
RestartDeviceCallback callback) override;
void RestartDeviceDeprecated(
const std::string& description,
RestartDeviceDeprecatedCallback callback) override;

private:
// Any number of crosapi clients can connect to this class.
Expand Down
13 changes: 1 addition & 12 deletions chrome/browser/chromeos/app_mode/app_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/common/pref_names.h"
#include "chromeos/dbus/power/power_manager_client.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
Expand All @@ -36,14 +37,6 @@
#include "extensions/browser/guest_view/web_view/web_view_guest.h"
#include "ppapi/buildflags/buildflags.h"

#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "chromeos/dbus/power/power_manager_client.h"
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(IS_CHROMEOS_LACROS)
#include "chrome/browser/lacros/app_mode/kiosk_session_service_lacros.h"
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)

#if BUILDFLAG(ENABLE_PLUGINS)
#include "chrome/browser/chromeos/app_mode/kiosk_session_plugin_handler.h"
#include "chrome/browser/chromeos/app_mode/kiosk_session_plugin_handler_delegate.h"
Expand All @@ -67,12 +60,8 @@ bool IsPepperPlugin(const base::FilePath& plugin_path) {
#endif

void RebootDevice() {
#if BUILDFLAG(IS_CHROMEOS_ASH)
chromeos::PowerManagerClient::Get()->RequestRestart(
power_manager::REQUEST_RESTART_OTHER, "kiosk app session");
#elif BUILDFLAG(IS_CHROMEOS_LACROS)
KioskSessionServiceLacros::Get()->RestartDevice("kiosk app session");
#endif
}

// Sends a SIGFPE signal to plugin subprocesses that matches |child_ids|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/extensions/updater/extension_updater.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_navigator.h"
#include "chrome/browser/ui/browser_navigator_params.h"
Expand All @@ -38,16 +39,11 @@
#include "extensions/common/manifest.h"
#include "net/base/backoff_entry.h"

#if BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(IS_CHROMEOS)
#include "chromeos/dbus/power/power_manager_client.h"
#include "components/user_manager/user_manager.h"
#include "third_party/cros_system_api/dbus/service_constants.h"
#endif

#if BUILDFLAG(IS_CHROMEOS_LACROS)
#include "chrome/browser/lacros/app_mode/kiosk_session_service_lacros.h"
#endif

using extensions::Extension;
using extensions::ExtensionSystem;
using extensions::ExtensionUpdater;
Expand Down Expand Up @@ -332,19 +328,14 @@ bool ChromeRuntimeAPIDelegate::GetPlatformInfo(PlatformInfo* info) {
}

bool ChromeRuntimeAPIDelegate::RestartDevice(std::string* error_message) {
#if BUILDFLAG(IS_CHROMEOS_ASH)
if (user_manager::UserManager::Get()->IsLoggedInAsKioskApp() ||
user_manager::UserManager::Get()->IsLoggedInAsWebKioskApp()) {
#if BUILDFLAG(IS_CHROMEOS)
if (profiles::IsKioskSession()) {
chromeos::PowerManagerClient::Get()->RequestRestart(
power_manager::REQUEST_RESTART_API, "chrome.runtime API");
return true;
}
#endif
#if BUILDFLAG(IS_CHROMEOS_LACROS)
if (KioskSessionServiceLacros::Get()->RestartDevice("chrome.runtime API")) {
return true;
}
#endif

*error_message = "Function available only for ChromeOS kiosk mode.";
return false;
}
Expand Down
35 changes: 0 additions & 35 deletions chrome/browser/lacros/app_mode/kiosk_session_service_lacros.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ namespace {

static KioskSessionServiceLacros* g_kiosk_session_service = nullptr;

bool IsKioskSession(crosapi::mojom::SessionType session_type) {
return (session_type == crosapi::mojom::SessionType::kWebKioskSession) ||
(session_type == crosapi::mojom::SessionType::kAppKioskSession);
}

} // namespace

// static
Expand Down Expand Up @@ -83,33 +78,3 @@ void KioskSessionServiceLacros::AttemptUserExit() {

service->GetRemote<crosapi::mojom::KioskSessionService>()->AttemptUserExit();
}

bool KioskSessionServiceLacros::RestartDevice(const std::string& description) {
chromeos::LacrosService* service = chromeos::LacrosService::Get();
CHECK(service);

if (IsKioskSession(chromeos::BrowserParamsProxy::Get()->SessionType()) &&
service->IsAvailable<crosapi::mojom::KioskSessionService>()) {
int remote_version = service->GetInterfaceVersion(
crosapi::mojom::KioskSessionService::Uuid_);
if (remote_version >= 0 &&
static_cast<uint32_t>(remote_version) >=
crosapi::mojom::KioskSessionService::kRestartDeviceMinVersion) {
auto callback = base::BindOnce([](bool status) {
if (!status) {
LOG(ERROR) << "Restart device was called but failed";
}
});

auto& remote = service->GetRemote<crosapi::mojom::KioskSessionService>();
remote->RestartDevice(description, std::move(callback));
return true;
} else {
LOG(ERROR) << "Current KioskSessionService " << remote_version
<< " does not support RestartDevice";
}
} else {
LOG(ERROR) << "Kiosk session service is not available.";
}
return false;
}
3 changes: 0 additions & 3 deletions chrome/browser/lacros/app_mode/kiosk_session_service_lacros.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ class KioskSessionServiceLacros {
return app_session_.get();
}

// Tell the ash-chrome to restart device
virtual bool RestartDevice(const std::string& description);

protected:
// Tell the ash-chrome to end the kiosk session and return the current user
// to the login screen by calling the API provided by
Expand Down
4 changes: 3 additions & 1 deletion chromeos/crosapi/mojom/kiosk_session_service.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ interface KioskSessionService {
// to start actual exit process.
AttemptUserExit@0();

// Deprecated.
// Lacros can call `PowerManagerClient::RequestRestart` directly instead.
// Triggers a device restart initiated by extension runtime API
[MinVersion=1]
RestartDevice@1(string description) => (bool status);
RestartDeviceDeprecated@1(string description) => (bool status);
};

0 comments on commit 0e80baa

Please sign in to comment.