From 10b637631ccb888cd4809f00e708c3eb10623b86 Mon Sep 17 00:00:00 2001 From: Nick Diego Yamane Date: Wed, 25 Jan 2023 06:28:43 +0000 Subject: [PATCH] [M110] wayland: restrict fullscreen-only shortcuts inhibition to linux desktop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This implements an alternative behavior for 'keyboard shortcuts inhibition' feature in ozone/wayland's platform window impl for Linux Desktop, which restricts the inhibition to fullscreen state, as currently state in KeyboardLock web specification [1]. Lacros behavior is kept unchanged, which shouldn't be a problem since Exo supports zcr-keyboard-extension extension which allows unprocessed keys to be reported by the client to the compositor for further processing, when shortcuts inhibitor is in place. [1] https://wicg.github.io/keyboard-lock R=​hidehiko, tonikitoo@igalia.com (cherry picked from commit 92f7bfe890704705146d1b5e0ff6110c7c043328) Bug: 1338554 Change-Id: Ic79d5b67b7d765acd3dbcfddbb0e2b0387eb2391 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4179373 Reviewed-by: Antonio Gomes Commit-Queue: Nick Yamane Reviewed-by: Hidehiko Abe Cr-Original-Commit-Position: refs/heads/main@{#1094796} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4188482 Reviewed-by: Maksim Sisov Commit-Queue: Maksim Sisov Auto-Submit: Nick Yamane Cr-Commit-Position: refs/branch-heads/5481@{#640} Cr-Branched-From: 130f3e4d850f4bc7387cfb8d08aa993d288a67a9-refs/heads/main@{#1084008} --- .../wayland/host/wayland_connection.h | 3 ++ .../platform/wayland/host/wayland_surface.cc | 12 +++--- .../platform/wayland/host/wayland_surface.h | 7 ++-- .../wayland/host/wayland_toplevel_window.cc | 16 ++++++-- .../platform/wayland/host/wayland_window.cc | 41 +++++++++++++++++-- .../platform/wayland/host/wayland_window.h | 17 ++++++++ 6 files changed, 82 insertions(+), 14 deletions(-) diff --git a/ui/ozone/platform/wayland/host/wayland_connection.h b/ui/ozone/platform/wayland/host/wayland_connection.h index 6a4a866bb7cbea..73080d1ca13e1f 100644 --- a/ui/ozone/platform/wayland/host/wayland_connection.h +++ b/ui/ozone/platform/wayland/host/wayland_connection.h @@ -129,6 +129,9 @@ class WaylandConnection { } xdg_wm_base* shell() const { return shell_.get(); } wp_presentation* presentation() const { return presentation_.get(); } + zcr_keyboard_extension_v1* keyboard_extension_v1() const { + return keyboard_extension_v1_.get(); + } zwp_keyboard_shortcuts_inhibit_manager_v1* keyboard_shortcuts_inhibit_manager_v1() const { return keyboard_shortcuts_inhibit_manager_v1_.get(); diff --git a/ui/ozone/platform/wayland/host/wayland_surface.cc b/ui/ozone/platform/wayland/host/wayland_surface.cc index 363b324657262b..588d6210ccd9dd 100644 --- a/ui/ozone/platform/wayland/host/wayland_surface.cc +++ b/ui/ozone/platform/wayland/host/wayland_surface.cc @@ -723,14 +723,16 @@ void WaylandSurface::ForceImmediateStateApplication() { apply_state_immediately_ = true; } -void WaylandSurface::InhibitKeyboardShortcuts() { - if (auto* keyboard_shortcuts_inhibit_manager = - connection_->keyboard_shortcuts_inhibit_manager_v1()) { +void WaylandSurface::SetKeyboardShortcutsInhibition(bool enabled) { + if (!enabled) { + keyboard_shortcuts_inhibitor_.reset(); + return; + } + if (auto* manager = connection_->keyboard_shortcuts_inhibit_manager_v1()) { keyboard_shortcuts_inhibitor_ = wl::Object( zwp_keyboard_shortcuts_inhibit_manager_v1_inhibit_shortcuts( - keyboard_shortcuts_inhibit_manager, surface_.get(), - connection_->seat()->wl_object())); + manager, surface_.get(), connection_->seat()->wl_object())); } } diff --git a/ui/ozone/platform/wayland/host/wayland_surface.h b/ui/ozone/platform/wayland/host/wayland_surface.h index 9957bfc5ed0106..8452570dc3ad9d 100644 --- a/ui/ozone/platform/wayland/host/wayland_surface.h +++ b/ui/ozone/platform/wayland/host/wayland_surface.h @@ -215,9 +215,10 @@ class WaylandSurface { // effect immediately. void ForceImmediateStateApplication(); - // Requests to wayland compositor to send key events even if it matches - // with the compositor's accelerator keys. - void InhibitKeyboardShortcuts(); + // Asks the Wayland compositor to enable or disable the keyboard shortcuts + // inhibition for this surface. i.e: to receive key events even if they match + // compositor accelerators, e.g: Alt+Tab, etc. + void SetKeyboardShortcutsInhibition(bool enabled); private: FRIEND_TEST_ALL_PREFIXES(WaylandWindowTest, diff --git a/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc b/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc index 3be02c72b89181..9113d5e4be1ca4 100644 --- a/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc +++ b/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc @@ -498,9 +498,19 @@ void WaylandToplevelWindow::HandleAuraToplevelConfigure( // Thus, we must store previous bounds to restore later. SetOrResetRestoredBounds(); - if (old_state != state_ && !did_send_delegate_notification) { - previous_state_ = old_state; - delegate()->OnWindowStateChanged(previous_state_, state_); + if (old_state != state_) { + if (!did_send_delegate_notification) { + previous_state_ = old_state; + delegate()->OnWindowStateChanged(previous_state_, state_); + } + + if (keyboard_shortcuts_inhibition_mode() == + KeyboardShortcutsInhibitionMode::kFullscreenOnly && + (state_ == PlatformWindowState::kFullScreen || + old_state == PlatformWindowState::kFullScreen)) { + root_surface()->SetKeyboardShortcutsInhibition( + state_ == PlatformWindowState::kFullScreen); + } } if (did_active_change) diff --git a/ui/ozone/platform/wayland/host/wayland_window.cc b/ui/ozone/platform/wayland/host/wayland_window.cc index ba9a79fb0de420..e4256473e880c5 100644 --- a/ui/ozone/platform/wayland/host/wayland_window.cc +++ b/ui/ozone/platform/wayland/host/wayland_window.cc @@ -643,9 +643,6 @@ bool WaylandWindow::Initialize(PlatformWindowInitProperties properties) { return false; } - if (properties.inhibit_keyboard_shortcuts) - root_surface_->InhibitKeyboardShortcuts(); - // Update visual size in tests immediately if the test config is set. // Otherwise, such tests as interactive_ui_tests fail. if (!update_visual_size_immediately_for_testing_) { @@ -661,6 +658,10 @@ bool WaylandWindow::Initialize(PlatformWindowInitProperties properties) { opacity_ = properties.opacity; type_ = properties.type; + if (properties.inhibit_keyboard_shortcuts) { + InitKeyboardShortcutsInhibition(); + } + connection_->window_manager()->AddWindow(GetWidget(), this); if (!OnInitialize(std::move(properties))) @@ -686,6 +687,40 @@ bool WaylandWindow::Initialize(PlatformWindowInitProperties properties) { return true; } +// When upper layer requests to 'inhibit keyboard shortcuts', two different +// behaviors are currently implemented: +// +// 1. If `zcr_keyboard_extension_v1` extension is available (typically meaning +// it is running under Exo compositor), shortcuts are kept inhibited since the +// window initialization. That is required to keep Lacros behaving just like +// Ash Chrome's classic browser. +// +// 2. Otherwise, keyboard shortcuts will be inhibited only when in fullscreen. +// See KeyboardLock spec for more details: https://wicg.github.io/keyboard-lock +// +// The main technical difference here is that keyboard-extension-v1 extension +// allows ozone/wayland to report back to the Wayland compositor that a given +// key was not processed by the client, giving it a chance of processing global +// shortcuts (even with a shortcuts inhibitor in place), which is not currently +// possible with standard Wayland protocol and extensions. +// +// TODO(crbug.com/1338554): Revisit and update when/if this scenario changes. +void WaylandWindow::InitKeyboardShortcutsInhibition() { + DCHECK_EQ(keyboard_shortcuts_inhibition_mode_, + KeyboardShortcutsInhibitionMode::kDisabled); + if (!connection_->keyboard_extension_v1()) { + // Only set inhibition mode to kFullscreenOnly and defer the actual handling + // to the subsequent shell surface configure events, where window state is + // applied/updated. See WaylandToplevelWindow::HandleAuraToplevelConfigure. + keyboard_shortcuts_inhibition_mode_ = + KeyboardShortcutsInhibitionMode::kFullscreenOnly; + return; + } + keyboard_shortcuts_inhibition_mode_ = + KeyboardShortcutsInhibitionMode::kAlwaysEnabled; + root_surface()->SetKeyboardShortcutsInhibition(/*enabled=*/true); +} + void WaylandWindow::SetWindowGeometry(gfx::Size size_dip) {} gfx::Vector2d WaylandWindow::GetWindowGeometryOffsetInDIP() const { diff --git a/ui/ozone/platform/wayland/host/wayland_window.h b/ui/ozone/platform/wayland/host/wayland_window.h index 78ffeba9a525f7..a48aa0c9fc8f69 100644 --- a/ui/ozone/platform/wayland/host/wayland_window.h +++ b/ui/ozone/platform/wayland/host/wayland_window.h @@ -366,6 +366,12 @@ class WaylandWindow : public PlatformWindow, bool has_pending_configures() const { return !pending_configures_.empty(); } protected: + enum class KeyboardShortcutsInhibitionMode { + kDisabled, + kAlwaysEnabled, + kFullscreenOnly + }; + WaylandWindow(PlatformWindowDelegate* delegate, WaylandConnection* connection); @@ -401,6 +407,10 @@ class WaylandWindow : public PlatformWindow, const gfx::Size& restored_size_dip() const { return restored_size_dip_; } + KeyboardShortcutsInhibitionMode keyboard_shortcuts_inhibition_mode() const { + return keyboard_shortcuts_inhibition_mode_; + } + // Configure related: // Processes the pending bounds in dip. void ProcessPendingBoundsDip(uint32_t serial); @@ -445,6 +455,10 @@ class WaylandWindow : public PlatformWindow, // Additional initialization of derived classes. virtual bool OnInitialize(PlatformWindowInitProperties properties) = 0; + // Determines which keyboard shortcuts inhibition mode to be used and perform + // required initialization steps, if any. + void InitKeyboardShortcutsInhibition(); + // WaylandWindowDragController might need to take ownership of the wayland // surface whether the window that originated the DND session gets destroyed // in the middle of that session (e.g: when it is snapped into a tab strip). @@ -564,6 +578,9 @@ class WaylandWindow : public PlatformWindow, base::OnceClosure drag_loop_quit_closure_; + KeyboardShortcutsInhibitionMode keyboard_shortcuts_inhibition_mode_{ + KeyboardShortcutsInhibitionMode::kDisabled}; + #if DCHECK_IS_ON() bool disable_null_target_dcheck_for_test_ = false; #endif