Skip to content

Commit

Permalink
Route accelerator keys to Lacros if desktop has focus.
Browse files Browse the repository at this point in the history
If Chrome OS Desktop has focus, following accelerator keys
are forwarded to Lacros, if Lacors is the primary browser.
- CTRL+N: Open a new window.
- CTRL+SHIFT+N: Open a new incognito window.
- CTRL+T: Open a new tab.
- CTRL+SHIFT+T: Restore the tab recently closed.

To support them, BrowserService (a.k.a. Lacros) starts to
support new IPC methods, NewTab and RestoreTab. Also its
NewWindow starts taking a bool "incognito".

BrowserManager in ash also starts to support those methods.
If Lacros is running, they are forwarded to Lacros via
Crosapi Mojo IPC above.
If not, they launches lacros with BrowserInitParam setting.

BUG=1188020
TEST=Ran those accelerator keys on DUT.

Change-Id: Ic5c9d824f5db8c04b336e749f3a4fc74fd0d00c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2781664
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#866471}
  • Loading branch information
Hidehiko Abe authored and Chromium LUCI CQ committed Mar 25, 2021
1 parent 65942a0 commit ee57c77
Show file tree
Hide file tree
Showing 19 changed files with 242 additions and 89 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/apps/app_service/publishers/lacros_apps.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ void LacrosApps::Launch(const std::string& app_id,
apps::mojom::LaunchSource launch_source,
apps::mojom::WindowInfoPtr window_info) {
DCHECK_EQ(extension_misc::kLacrosAppId, app_id);
crosapi::BrowserManager::Get()->NewWindow();
crosapi::BrowserManager::Get()->NewWindow(/*incognito=*/false);
}

void LacrosApps::GetMenuModel(const std::string& app_id,
Expand Down
131 changes: 84 additions & 47 deletions chrome/browser/ash/crosapi/browser_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,61 +238,43 @@ void BrowserManager::SetLoadCompleteCallback(LoadCompleteCallback callback) {
load_complete_callback_ = std::move(callback);
}

void BrowserManager::NewWindow() {
if (!browser_util::IsLacrosEnabled())
return;

if (!browser_util::IsLacrosAllowedToLaunch()) {
std::unique_ptr<message_center::Notification> notification =
ash::CreateSystemNotification(
message_center::NOTIFICATION_TYPE_SIMPLE,
kLacrosCannotLaunchNotificationID,
/*title=*/std::u16string(),
l10n_util::GetStringUTF16(
IDS_LACROS_CANNOT_LAUNCH_MULTI_SIGNIN_MESSAGE),
/* display_source= */ std::u16string(), GURL(),
message_center::NotifierId(
message_center::NotifierType::SYSTEM_COMPONENT,
kLacrosLauncherNotifierID),
message_center::RichNotificationData(),
base::MakeRefCounted<
message_center::HandleNotificationClickDelegate>(
base::RepeatingClosure()),
gfx::kNoneIcon,
message_center::SystemNotificationWarningLevel::NORMAL);

SystemNotificationHelper::GetInstance()->Display(*notification);
void BrowserManager::NewWindow(bool incognito) {
auto result = MaybeStart(
incognito ? browser_util::InitialBrowserAction::kOpenIncognitoWindow
: browser_util::InitialBrowserAction::kOpenWindow);
if (result != MaybeStartResult::kRunning)
return;
}

if (!IsReady()) {
LOG(WARNING) << "lacros component image not yet available";
if (!browser_service_.has_value()) {
LOG(ERROR) << "BrowserService was disconnected";
return;
}
DCHECK(!lacros_path_.empty());
browser_service_->service->NewWindow(incognito, base::DoNothing());
}

if (state_ == State::TERMINATING) {
LOG(WARNING) << "lacros-chrome is terminating, so cannot start now";
void BrowserManager::NewTab() {
auto result = MaybeStart(browser_util::InitialBrowserAction::kOpenWindow);
if (result != MaybeStartResult::kRunning)
return;
}

if (state_ == State::CREATING_LOG_FILE || state_ == State::STARTING) {
LOG(WARNING) << "lacros-chrome is in the process of launching";
if (!browser_service_.has_value()) {
LOG(ERROR) << "BrowserService was disconnected";
return;
}
browser_service_->service->NewTab(base::DoNothing());
}

if (state_ == State::STOPPED) {
// If lacros-chrome is not running, launch it.
Start();
void BrowserManager::RestoreTab() {
auto result =
MaybeStart(browser_util::InitialBrowserAction::kRestoreLastSession);
if (result != MaybeStartResult::kRunning)
return;
}

if (!browser_service_.has_value()) {
LOG(ERROR) << "BrowserService was disconnected";
return;
}

browser_service_->service->NewWindow(base::DoNothing());
browser_service_->service->RestoreTab(base::DoNothing());
}

bool BrowserManager::GetFeedbackDataSupported() const {
Expand Down Expand Up @@ -348,7 +330,61 @@ BrowserManager::BrowserServiceInfo::operator=(const BrowserServiceInfo&) =
default;
BrowserManager::BrowserServiceInfo::~BrowserServiceInfo() = default;

void BrowserManager::Start() {
BrowserManager::MaybeStartResult BrowserManager::MaybeStart(
browser_util::InitialBrowserAction initial_browser_action) {
if (!browser_util::IsLacrosEnabled())
return MaybeStartResult::kNotStarted;

if (!browser_util::IsLacrosAllowedToLaunch()) {
std::unique_ptr<message_center::Notification> notification =
ash::CreateSystemNotification(
message_center::NOTIFICATION_TYPE_SIMPLE,
kLacrosCannotLaunchNotificationID,
/*title=*/std::u16string(),
l10n_util::GetStringUTF16(
IDS_LACROS_CANNOT_LAUNCH_MULTI_SIGNIN_MESSAGE),
/* display_source= */ std::u16string(), GURL(),
message_center::NotifierId(
message_center::NotifierType::SYSTEM_COMPONENT,
kLacrosLauncherNotifierID),
message_center::RichNotificationData(),
base::MakeRefCounted<
message_center::HandleNotificationClickDelegate>(
base::RepeatingClosure()),
gfx::kNoneIcon,
message_center::SystemNotificationWarningLevel::NORMAL);

SystemNotificationHelper::GetInstance()->Display(*notification);
return MaybeStartResult::kNotStarted;
}

if (!IsReady()) {
LOG(WARNING) << "lacros component image not yet available";
return MaybeStartResult::kNotStarted;
}
DCHECK(!lacros_path_.empty());

if (state_ == State::TERMINATING) {
LOG(WARNING) << "lacros-chrome is terminating, so cannot start now";
return MaybeStartResult::kNotStarted;
}

if (state_ == State::CREATING_LOG_FILE || state_ == State::STARTING) {
LOG(WARNING) << "lacros-chrome is in the process of launching";
return MaybeStartResult::kStarting;
}

if (state_ == State::STOPPED) {
// If lacros-chrome is not running, launch it.
Start(initial_browser_action);
return MaybeStartResult::kStarting;
}

return MaybeStartResult::kRunning;
}

void BrowserManager::Start(
browser_util::InitialBrowserAction initial_browser_action) {
DCHECK_EQ(state_, State::STOPPED);
DCHECK(!lacros_path_.empty());
// Ensure we're not trying to open a window before the shelf is initialized.
Expand All @@ -359,10 +395,12 @@ void BrowserManager::Start() {
base::ThreadPool::PostTaskAndReplyWithResult(
FROM_HERE, {base::MayBlock()}, base::BindOnce(&CreateLogFile),
base::BindOnce(&BrowserManager::StartWithLogFile,
weak_factory_.GetWeakPtr()));
weak_factory_.GetWeakPtr(), initial_browser_action));
}

void BrowserManager::StartWithLogFile(base::ScopedFD logfd) {
void BrowserManager::StartWithLogFile(
browser_util::InitialBrowserAction initial_browser_action,
base::ScopedFD logfd) {
DCHECK_EQ(state_, State::CREATING_LOG_FILE);

std::string chrome_path = lacros_path_.MaybeAsASCII() + "/chrome";
Expand Down Expand Up @@ -451,8 +489,8 @@ void BrowserManager::StartWithLogFile(base::ScopedFD logfd) {
options.fds_to_remap.push_back(std::make_pair(logfd.get(), STDERR_FILENO));
}

base::ScopedFD startup_fd =
browser_util::CreateStartupData(environment_provider_.get());
base::ScopedFD startup_fd = browser_util::CreateStartupData(
environment_provider_.get(), initial_browser_action);
if (startup_fd.is_valid()) {
// Hardcoded to use FD 3 to make the ash-chrome's behavior more predictable.
// Lacros-chrome should not depend on the hardcoded value though. Instead
Expand Down Expand Up @@ -618,9 +656,8 @@ void BrowserManager::OnLoadComplete(const base::FilePath& path) {
std::move(load_complete_callback_).Run(success);
}

if (state_ == State::STOPPED && GetLaunchOnLoginPref()) {
Start();
}
if (state_ == State::STOPPED && GetLaunchOnLoginPref())
Start(browser_util::InitialBrowserAction::kOpenWindow);
}

void BrowserManager::NotifyMojoDisconnected() {
Expand Down
32 changes: 29 additions & 3 deletions chrome/browser/ash/crosapi/browser_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "base/time/time.h"
#include "chrome/browser/ash/crosapi/browser_manager_observer.h"
#include "chrome/browser/ash/crosapi/browser_service_host_observer.h"
#include "chrome/browser/ash/crosapi/browser_util.h"
#include "chrome/browser/ash/crosapi/crosapi_id.h"
#include "chrome/browser/ash/crosapi/environment_provider.h"
#include "chromeos/crosapi/mojom/crosapi.mojom.h"
Expand Down Expand Up @@ -87,7 +88,15 @@ class BrowserManager : public session_manager::SessionManagerObserver,
// class, so there's no way for callers to handle such error cases properly.
// This design often leads the flakiness behavior of the product and testing,
// so should be avoided.
void NewWindow();
void NewWindow(bool incognito);

// Similar to NewWindow(), but opens a tab, instead.
// See crosapi::mojom::BrowserService::NewTab for more details
void NewTab();

// Similar to NewWindow(), but restores a tab recently closed.
// See crosapi::mojom::BrowserService::RestoreTab for more details
void RestoreTab();

// Returns true if crosapi interface supports GetFeedbackData API.
bool GetFeedbackDataSupported() const;
Expand Down Expand Up @@ -177,12 +186,29 @@ class BrowserManager : public session_manager::SessionManagerObserver,
uint32_t interface_version;
};

enum class MaybeStartResult {
kNotStarted,
kStarting,
kRunning,
};
// Checks the precondition to start Lacros, and actually trigger to start
// if necessary.
// If the condition to start lacros is not met, kNotStarted is returned.
// If the condition to start lacros is met, and it is not yet started,
// or it is under starting, kStarting is returned.
// Otherwise, i.e., lacros is already running, kRunning is returned.
// |extra_args| will be passed to the argument to launch lacros.
MaybeStartResult MaybeStart(
browser_util::InitialBrowserAction initial_browser_action);

// Posts CreateLogFile() and StartWithLogFile() to the thread pooll.
void Start();
void Start(browser_util::InitialBrowserAction initial_browser_action);

// Starts the lacros-chrome process and redirects stdout/err to file pointed
// by logfd.
void StartWithLogFile(base::ScopedFD logfd);
void StartWithLogFile(
browser_util::InitialBrowserAction initial_browser_action,
base::ScopedFD logfd);

// BrowserServiceHostObserver:
void OnBrowserServiceConnected(CrosapiId id,
Expand Down
14 changes: 11 additions & 3 deletions chrome/browser/ash/crosapi/browser_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,8 @@ base::flat_map<base::Token, uint32_t> GetInterfaceVersions() {
}

mojom::BrowserInitParamsPtr GetBrowserInitParams(
EnvironmentProvider* environment_provider) {
EnvironmentProvider* environment_provider,
InitialBrowserAction initial_browser_action) {
auto params = mojom::BrowserInitParams::New();
params->crosapi_version = crosapi::mojom::Crosapi::Version_;
params->deprecated_ash_metrics_enabled_has_value = true;
Expand All @@ -321,11 +322,18 @@ mojom::BrowserInitParamsPtr GetBrowserInitParams(
params->device_account_policy = GetDeviceAccountPolicy(environment_provider);
params->idle_info = IdleServiceAsh::ReadIdleInfoFromSystem();

params->is_incognito =
initial_browser_action == InitialBrowserAction::kOpenIncognitoWindow;
params->restore_last_session =
initial_browser_action == InitialBrowserAction::kRestoreLastSession;

return params;
}

base::ScopedFD CreateStartupData(EnvironmentProvider* environment_provider) {
auto data = GetBrowserInitParams(environment_provider);
base::ScopedFD CreateStartupData(EnvironmentProvider* environment_provider,
InitialBrowserAction initial_browser_action) {
auto data =
GetBrowserInitParams(environment_provider, initial_browser_action);
std::vector<uint8_t> serialized =
crosapi::mojom::BrowserInitParams::Serialize(&data);

Expand Down
12 changes: 10 additions & 2 deletions chrome/browser/ash/crosapi/browser_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,17 @@ bool IsLacrosWindow(const aura::Window* window);
// Returns the UUID and version for all tracked interfaces. Exposed for testing.
base::flat_map<base::Token, uint32_t> GetInterfaceVersions();

enum class InitialBrowserAction {
kOpenWindow,
kOpenIncognitoWindow,
kRestoreLastSession,
};

// Returns the initial parameter to be passed to Crosapi client,
// such as lacros-chrome.
mojom::BrowserInitParamsPtr GetBrowserInitParams(
EnvironmentProvider* environment_provider);
EnvironmentProvider* environment_provider,
InitialBrowserAction initial_browser_action);

// Invite the lacros-chrome to the mojo universe.
// Queue messages to establish the mojo connection, so that the passed IPC is
Expand All @@ -108,7 +115,8 @@ mojo::Remote<crosapi::mojom::BrowserService> SendMojoInvitationToLacrosChrome(
// Creates a memory backed file containing the serialized |params|,
// and returns its FD.
base::ScopedFD CreateStartupData(
::crosapi::EnvironmentProvider* environment_provider);
::crosapi::EnvironmentProvider* environment_provider,
InitialBrowserAction initial_browser_action);

} // namespace browser_util
} // namespace crosapi
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ash/crosapi/crosapi_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ class CrosapiManager::LegacyInvitationFlow {
// This is for backward compatibility.
// TODO(crbug.com/1156033): Remove InitDeprecated() invocation when lacros
// becomes mature enough.
browser_service_->InitDeprecated(
browser_util::GetBrowserInitParams(environment_provider));
browser_service_->InitDeprecated(browser_util::GetBrowserInitParams(
environment_provider, browser_util::InitialBrowserAction::kOpenWindow));

browser_service_->RequestCrosapiReceiver(
base::BindOnce(&LegacyInvitationFlow::OnCrosapiReceiverReceived,
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/ash/crosapi/test_mojo_connection_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ void TestMojoConnectionManager::OnTestingSocketAvailable() {
LOG(WARNING) << "Mojo to lacros-chrome is disconnected";
}));

base::ScopedFD startup_fd =
browser_util::CreateStartupData(environment_provider_.get());
base::ScopedFD startup_fd = browser_util::CreateStartupData(
environment_provider_.get(),
browser_util::InitialBrowserAction::kOpenWindow);
if (!startup_fd.is_valid()) {
LOG(ERROR) << "Failed to create startup data";
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ class TestBrowserService : public crosapi::mojom::BrowserService {
std::move(callback).Run(crosapi_.BindNewPipeAndPassReceiver());
}

void NewWindow(NewWindowCallback callback) override {}
void NewWindow(bool incognito, NewWindowCallback callback) override {}
void NewTab(NewTabCallback callback) override {}
void RestoreTab(RestoreTabCallback callback) override {}
void GetFeedbackData(GetFeedbackDataCallback callback) override {}
void GetHistograms(GetHistogramsCallback callback) override {}
void GetActiveTabUrl(GetActiveTabUrlCallback callback) override {}
Expand Down
22 changes: 20 additions & 2 deletions chrome/browser/lacros/lacros_chrome_service_delegate_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,29 @@ void LacrosChromeServiceDelegateImpl::OnInitialized(
}
}

void LacrosChromeServiceDelegateImpl::NewWindow() {
void LacrosChromeServiceDelegateImpl::NewWindow(bool incognito) {
// TODO(crbug.com/1102815): Find what profile should be used.
Profile* profile = ProfileManager::GetLastUsedProfileAllowedByPolicy();
DCHECK(profile) << "No last used profile is found.";
chrome::NewEmptyWindow(profile);
chrome::NewEmptyWindow(incognito ? profile->GetPrimaryOTRProfile() : profile);
}

void LacrosChromeServiceDelegateImpl::NewTab() {
// TODO(crbug.com/1102815): Find what profile should be used.
Profile* profile = ProfileManager::GetLastUsedProfileAllowedByPolicy();
DCHECK(profile) << "No last used profile is found.";
Browser* browser = chrome::FindBrowserWithProfile(profile);
DCHECK(browser) << "No browser is found.";
chrome::NewTab(browser);
}

void LacrosChromeServiceDelegateImpl::RestoreTab() {
// TODO(crbug.com/1102815): Find what profile should be used.
Profile* profile = ProfileManager::GetLastUsedProfileAllowedByPolicy();
DCHECK(profile) << "No last used profile is found.";
Browser* browser = chrome::FindBrowserWithProfile(profile);
DCHECK(browser) << "No browser is found.";
chrome::RestoreTab(browser);
}

std::string LacrosChromeServiceDelegateImpl::GetChromeVersion() {
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/lacros/lacros_chrome_service_delegate_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ class LacrosChromeServiceDelegateImpl
// chromeos::LacrosChromeServiceDelegate:
void OnInitialized(
const crosapi::mojom::BrowserInitParams& init_params) override;
void NewWindow() override;
void NewWindow(bool incognito) override;
void NewTab() override;
void RestoreTab() override;
std::string GetChromeVersion() override;
void GetFeedbackData(GetFeedbackDataCallback callback) override;
void GetHistograms(GetHistogramsCallback callback) override;
Expand Down
Loading

0 comments on commit ee57c77

Please sign in to comment.