Skip to content

Commit

Permalink
Lacros: Send ack_key on unhandled KeyEvent.
Browse files Browse the repository at this point in the history
Currently, WaylandKeyboard synchronously decides whether KeyEvent is
processed or not, and send ack_key.
However, KeyEvent can be sent to, e.g., RWHVA once, and it may return
the event is not handled.
If we already send ack_key, there's no way to cancel it, so we only
send "not-handled" ack_key only to compositor.
exo compositor handles this in a good way, and this is consistent
with ARC.

BUG=1301977
TEST=Ran on DUT.

Change-Id: Idb3ac28aaab85b3e4406ab43e17bc6309fce7c50
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3614807
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001089}
  • Loading branch information
Hidehiko Abe authored and Chromium LUCI CQ committed May 9, 2022
1 parent 6c6d3ad commit 3125229
Show file tree
Hide file tree
Showing 18 changed files with 165 additions and 36 deletions.
18 changes: 12 additions & 6 deletions ui/ozone/platform/wayland/host/wayland_event_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ bool HasAnyPointerButtonFlag(int flags) {
EF_FORWARD_MOUSE_BUTTON)) != 0;
}

std::vector<uint8_t> ToLittleEndianByteVector(uint32_t value) {
return {static_cast<uint8_t>(value), static_cast<uint8_t>(value >> 8),
static_cast<uint8_t>(value >> 16), static_cast<uint8_t>(value >> 24)};
}

// Number of fingers for scroll gestures.
constexpr int kGestureScrollFingerCount = 2;

Expand Down Expand Up @@ -121,6 +126,7 @@ uint32_t WaylandEventSource::OnKeyboardKeyEvent(
EventType type,
DomCode dom_code,
bool repeat,
absl::optional<uint32_t> serial,
base::TimeTicks timestamp,
int device_id,
WaylandKeyboard::KeyEventKind kind) {
Expand Down Expand Up @@ -164,14 +170,14 @@ uint32_t WaylandEventSource::OnKeyboardKeyEvent(
// expects, but GtkUiPlatformWayland::GetGdkKeyEventState() takes care of the
// conversion.
properties.emplace(kPropertyKeyboardState,
std::vector<uint8_t>{
static_cast<uint8_t>(state_before_event),
static_cast<uint8_t>(state_before_event >> 8),
static_cast<uint8_t>(state_before_event >> 16),
static_cast<uint8_t>(state_before_event >> 24),
});
ToLittleEndianByteVector(state_before_event));
#endif

if (serial.has_value()) {
properties.emplace(WaylandKeyboard::kPropertyWaylandSerial,
ToLittleEndianByteVector(serial.value()));
}

if (kind == WaylandKeyboard::KeyEventKind::kKey) {
// Mark that this is the key event which IME did not consume.
properties.emplace(kPropertyKeyboardImeFlag,
Expand Down
1 change: 1 addition & 0 deletions ui/ozone/platform/wayland/host/wayland_event_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class WaylandEventSource : public PlatformEventSource,
uint32_t OnKeyboardKeyEvent(EventType type,
DomCode dom_code,
bool repeat,
absl::optional<uint32_t> serial,
base::TimeTicks timestamp,
int device_id,
WaylandKeyboard::KeyEventKind kind) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ void WaylandInputMethodContext::OnKeysym(uint32_t keysym,
EventType type =
state == WL_KEYBOARD_KEY_STATE_PRESSED ? ET_KEY_PRESSED : ET_KEY_RELEASED;
key_delegate_->OnKeyboardKeyEvent(type, dom_code, /*repeat=*/false,
EventTimeForNow(), device_id,
absl::nullopt, EventTimeForNow(), device_id,
WaylandKeyboard::KeyEventKind::kKey);
#else
NOTIMPLEMENTED();
Expand Down
48 changes: 35 additions & 13 deletions ui/ozone/platform/wayland/host/wayland_keyboard.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,25 @@ WaylandKeyboard::~WaylandKeyboard() {
delegate_->OnKeyboardModifiersChanged(0);
}

void WaylandKeyboard::OnUnhandledKeyEvent(const KeyEvent& key_event) {
// No way to send ack_key.
if (!extended_keyboard_)
return;

// Obtain the serial from properties. See WaylandEventSource how to annotate.
const auto* properties = key_event.properties();
if (!properties)
return;
auto it = properties->find(kPropertyWaylandSerial);
if (it == properties->end())
return;
DCHECK_EQ(it->second.size(), 4u);
uint32_t serial = it->second[0] | (it->second[1] << 8) |
(it->second[2] << 16) | (it->second[3] << 24);

extended_keyboard_->AckKey(serial, false);
}

void WaylandKeyboard::Keymap(void* data,
wl_keyboard* obj,
uint32_t format,
Expand Down Expand Up @@ -238,8 +257,8 @@ void WaylandKeyboard::DispatchKey(unsigned int key,
int flags) {
// Key repeat is only triggered by wl_keyboard::key event,
// but not by extended_keyboard::peek_key.
DispatchKey(key, scan_code, down, repeat, timestamp, device_id, flags,
KeyEventKind::kKey);
DispatchKey(key, scan_code, down, repeat, absl::nullopt, timestamp, device_id,
flags, KeyEventKind::kKey);
}

void WaylandKeyboard::OnKey(uint32_t serial,
Expand Down Expand Up @@ -268,14 +287,16 @@ void WaylandKeyboard::OnKey(uint32_t serial,
return;
}

DispatchKey(key, 0 /*scan_code*/, down, false /*repeat*/, EventTimeForNow(),
device_id(), EF_NONE, kind);
DispatchKey(key, 0 /*scan_code*/, down, false /*repeat*/,
down ? absl::make_optional(serial) : absl::nullopt,
EventTimeForNow(), device_id(), EF_NONE, kind);
}

void WaylandKeyboard::DispatchKey(unsigned int key,
unsigned int scan_code,
bool down,
bool repeat,
absl::optional<uint32_t> serial,
base::TimeTicks timestamp,
int device_id,
int flags,
Expand All @@ -287,15 +308,16 @@ void WaylandKeyboard::DispatchKey(unsigned int key,
// Pass empty DomKey and KeyboardCode here so the delegate can pre-process
// and decode it when needed.
uint32_t result = delegate_->OnKeyboardKeyEvent(
down ? ET_KEY_PRESSED : ET_KEY_RELEASED, dom_code, repeat, timestamp,
device_id, kind);

if (extended_keyboard_) {
if (auto keypress_serial = connection_->serial_tracker().GetSerial(
wl::SerialType::kKeyPress)) {
bool handled = result & POST_DISPATCH_STOP_PROPAGATION;
extended_keyboard_->AckKey(keypress_serial->value, handled);
}
down ? ET_KEY_PRESSED : ET_KEY_RELEASED, dom_code, repeat, serial,
timestamp, device_id, kind);

if (extended_keyboard_ && !(result & POST_DISPATCH_STOP_PROPAGATION) &&
serial.has_value()) {
// Not handled, so send ack_key event immediately.
// If handled, we do not, because the event is at least sent to the client,
// but later on, it may be returned as unhandled. If we send ack_key to the
// compositor, there's no way to cancel it.
extended_keyboard_->AckKey(serial.value(), false);
}
}

Expand Down
10 changes: 10 additions & 0 deletions ui/ozone/platform/wayland/host/wayland_keyboard.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <cstdint>

#include "base/time/time.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/base/buildflags.h"
#include "ui/events/keycodes/dom/dom_code.h"
#include "ui/events/keycodes/keyboard_codes.h"
Expand All @@ -18,6 +19,7 @@
namespace ui {

class KeyboardLayoutEngine;
class KeyEvent;
class WaylandConnection;
class WaylandWindow;
#if BUILDFLAG(USE_XKBCOMMON)
Expand All @@ -34,6 +36,9 @@ class WaylandKeyboard : public EventAutoRepeatHandler::Delegate {
kKey, // Originated by wl_keyboard::key.
};

// Property key to annotate wayland serial to a KeyEvent.
static constexpr char kPropertyWaylandSerial[] = "_keyevent_wayland_serial_";

WaylandKeyboard(wl_keyboard* keyboard,
zcr_keyboard_extension_v1* keyboard_extension_v1,
WaylandConnection* connection,
Expand All @@ -44,6 +49,9 @@ class WaylandKeyboard : public EventAutoRepeatHandler::Delegate {
uint32_t id() const { return obj_.id(); }
int device_id() const { return obj_.id(); }

// Called when it turns out that KeyEvent is not handled.
void OnUnhandledKeyEvent(const KeyEvent& key_event);

private:
using LayoutEngine =
#if BUILDFLAG(USE_XKBCOMMON)
Expand Down Expand Up @@ -100,6 +108,7 @@ class WaylandKeyboard : public EventAutoRepeatHandler::Delegate {
unsigned int scan_code,
bool down,
bool repeat,
absl::optional<uint32_t> serial,
base::TimeTicks timestamp,
int device_id,
int flags,
Expand Down Expand Up @@ -138,6 +147,7 @@ class WaylandKeyboard::Delegate {
virtual uint32_t OnKeyboardKeyEvent(EventType type,
DomCode dom_code,
bool repeat,
absl::optional<uint32_t> serial,
base::TimeTicks timestamp,
int device_id,
WaylandKeyboard::KeyEventKind kind) = 0;
Expand Down
2 changes: 1 addition & 1 deletion ui/ozone/platform/wayland/ozone_platform_wayland.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ class OzonePlatformWayland : public OzonePlatform,
#endif

menu_utils_ = std::make_unique<WaylandMenuUtils>(connection_.get());
wayland_utils_ = std::make_unique<WaylandUtils>();
wayland_utils_ = std::make_unique<WaylandUtils>(connection_.get());

return true;
}
Expand Down
16 changes: 15 additions & 1 deletion ui/ozone/platform/wayland/wayland_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
#include "ui/ozone/platform/wayland/wayland_utils.h"

#include "ui/gfx/image/image_skia.h"
#include "ui/ozone/platform/wayland/host/wayland_connection.h"
#include "ui/ozone/platform/wayland/host/wayland_keyboard.h"
#include "ui/ozone/platform/wayland/host/wayland_seat.h"
#include "ui/ozone/platform/wayland/host/wayland_toplevel_window.h"

namespace ui {
Expand All @@ -25,7 +28,8 @@ class WaylandScopedDisableClientSideDecorationsForTest

} // namespace

WaylandUtils::WaylandUtils() = default;
WaylandUtils::WaylandUtils(WaylandConnection* connection)
: connection_(connection) {}

WaylandUtils::~WaylandUtils() = default;

Expand All @@ -43,4 +47,14 @@ WaylandUtils::DisableClientSideDecorationsForTest() {
return std::make_unique<WaylandScopedDisableClientSideDecorationsForTest>();
}

void WaylandUtils::OnUnhandledKeyEvent(const KeyEvent& key_event) {
auto* seat = connection_->seat();
if (!seat)
return;
auto* keyboard = seat->keyboard();
if (!keyboard)
return;
keyboard->OnUnhandledKeyEvent(key_event);
}

} // namespace ui
8 changes: 7 additions & 1 deletion ui/ozone/platform/wayland/wayland_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@

namespace ui {

class WaylandConnection;

class WaylandUtils : public PlatformUtils {
public:
WaylandUtils();
explicit WaylandUtils(WaylandConnection* connection);
WaylandUtils(const WaylandUtils&) = delete;
WaylandUtils& operator=(const WaylandUtils&) = delete;
~WaylandUtils() override;
Expand All @@ -20,6 +22,10 @@ class WaylandUtils : public PlatformUtils {
std::string GetWmWindowClass(const std::string& desktop_base_name) override;
std::unique_ptr<PlatformUtils::ScopedDisableClientSideDecorationsForTest>
DisableClientSideDecorationsForTest() override;
void OnUnhandledKeyEvent(const KeyEvent& key_event) override;

private:
WaylandConnection* const connection_;
};

} // namespace ui
Expand Down
4 changes: 4 additions & 0 deletions ui/ozone/platform/x11/x11_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,8 @@ X11Utils::DisableClientSideDecorationsForTest() {
return {};
}

void X11Utils::OnUnhandledKeyEvent(const KeyEvent& key_event) {
// Do nothing.
}

} // namespace ui
1 change: 1 addition & 0 deletions ui/ozone/platform/x11/x11_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class X11Utils : public PlatformUtils {
std::string GetWmWindowClass(const std::string& desktop_base_name) override;
std::unique_ptr<PlatformUtils::ScopedDisableClientSideDecorationsForTest>
DisableClientSideDecorationsForTest() override;
void OnUnhandledKeyEvent(const KeyEvent& key_event) override;
};

} // namespace ui
Expand Down
5 changes: 5 additions & 0 deletions ui/ozone/public/platform_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ class ImageSkia;

namespace ui {

class KeyEvent;

// Platform-specific general util functions that didn't find their way to any
// other existing utilities, but they are required to be accessed outside
// Ozone.
Expand Down Expand Up @@ -46,6 +48,9 @@ class COMPONENT_EXPORT(OZONE_BASE) PlatformUtils {
// affect tests.
virtual std::unique_ptr<ScopedDisableClientSideDecorationsForTest>
DisableClientSideDecorationsForTest() = 0;

// Called when it is found that a KeyEvent is not consumed.
virtual void OnUnhandledKeyEvent(const KeyEvent& key_event) = 0;
};

} // namespace ui
Expand Down
9 changes: 8 additions & 1 deletion ui/views/controls/webview/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//build/config/ozone.gni")

component("webview") {
sources = [
"unhandled_keyboard_event_handler.cc",
Expand Down Expand Up @@ -52,7 +54,12 @@ component("webview") {
]

if (is_linux || is_chromeos || is_android || is_fuchsia) {
sources += [ "unhandled_keyboard_event_handler_default.cc" ]
if (use_ozone) {
sources += [ "unhandled_keyboard_event_handler_ozone.cc" ]
deps += [ "//ui/ozone" ]
} else {
sources += [ "unhandled_keyboard_event_handler_default.cc" ]
}
}
}

Expand Down
5 changes: 3 additions & 2 deletions ui/views/controls/webview/unhandled_keyboard_event_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "ui/views/controls/webview/unhandled_keyboard_event_handler.h"

#include "content/public/browser/native_web_keyboard_event.h"
#include "ui/content_accelerators/accelerator_util.h"
#include "ui/views/focus/focus_manager.h"

Expand Down Expand Up @@ -50,8 +51,8 @@ bool UnhandledKeyboardEventHandler::HandleKeyboardEvent(
ignore_next_char_event_ = false;
}

if (event.os_event && !event.skip_in_browser)
return HandleNativeKeyboardEvent(event.os_event, focus_manager);
if (event.os_event)
return HandleNativeKeyboardEvent(event, focus_manager);

return false;
}
Expand Down
5 changes: 3 additions & 2 deletions ui/views/controls/webview/unhandled_keyboard_event_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ class WEBVIEW_EXPORT UnhandledKeyboardEventHandler {

private:
// Platform specific handling for unhandled keyboard events.
static bool HandleNativeKeyboardEvent(gfx::NativeEvent event,
FocusManager* focus_manager);
static bool HandleNativeKeyboardEvent(
const content::NativeWebKeyboardEvent& event,
FocusManager* focus_manager);

// Whether to ignore the next Char keyboard event.
// If a RawKeyDown event was handled as a shortcut key, then we're done
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,20 @@

#include "ui/views/controls/webview/unhandled_keyboard_event_handler.h"

#include "content/public/browser/native_web_keyboard_event.h"
#include "ui/events/event.h"
#include "ui/views/focus/focus_manager.h"

namespace views {

// static
bool UnhandledKeyboardEventHandler::HandleNativeKeyboardEvent(
gfx::NativeEvent event,
const content::NativeWebKeyboardEvent& event,
FocusManager* focus_manager) {
return !focus_manager->OnKeyEvent(*(event->AsKeyEvent()));
if (event.skip_in_browser)
return false;

return !focus_manager->OnKeyEvent(*(event.os_event->AsKeyEvent()));
}

} // namespace views

0 comments on commit 3125229

Please sign in to comment.