Skip to content

Commit

Permalink
[ozone/wayland] Comply with wl_pointer::frame event in enter/leave
Browse files Browse the repository at this point in the history
This CL switches wl_pointer.enter and leave events to comply with
wl_pointer.frame.
Particularly for wl_pointer.leave, it comes accompanied with a TODO
that needs to be fixed prior to the switch over from kImmeadiate
to kOnFrame.

BUG=1298504
R=nickdiego@igalia.com

Change-Id: I3b10c0831f3eaf71535599725125ae7fe6a513d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3823229
Reviewed-by: Nick Yamane <nickdiego@igalia.com>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1035167}
  • Loading branch information
tonikitoo authored and Chromium LUCI CQ committed Aug 15, 2022
1 parent a8d50ff commit 94d8491
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,12 @@ void WaylandDataDragController::OnDragEnter(WaylandWindow* window,

if (pointer_grabber_for_window_drag_) {
DCHECK(drag_source_.has_value());
if (*drag_source_ == DragSource::kMouse)
pointer_delegate_->OnPointerFocusChanged(window, location);
else
if (*drag_source_ == DragSource::kMouse) {
pointer_delegate_->OnPointerFocusChanged(
window, location, wl::EventDispatchPolicy::kImmediate);
} else {
touch_delegate_->OnTouchFocusChanged(window);
}

pointer_grabber_for_window_drag_ =
window_manager_->GetCurrentPointerOrTouchFocusedWindow();
Expand Down
26 changes: 20 additions & 6 deletions ui/ozone/platform/wayland/host/wayland_event_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,26 +250,40 @@ uint32_t WaylandEventSource::OnKeyboardKeyEvent(
return DispatchEvent(&event);
}

void WaylandEventSource::OnPointerFocusChanged(WaylandWindow* window,
const gfx::PointF& location) {
void WaylandEventSource::OnPointerFocusChanged(
WaylandWindow* window,
const gfx::PointF& location,
wl::EventDispatchPolicy dispatch_policy) {
bool focused = !!window;
if (focused) {
// Save new pointer location.
pointer_location_ = location;
window_manager_->SetPointerFocusedWindow(window);
}

auto closure = focused ? base::NullCallback()
: base::BindOnce(
[](WaylandWindowManager* wwm) {
wwm->SetPointerFocusedWindow(nullptr);
},
window_manager_);

auto* target = window_manager_->GetCurrentPointerFocusedWindow();
if (target) {
EventType type = focused ? ET_MOUSE_ENTERED : ET_MOUSE_EXITED;
MouseEvent event(type, pointer_location_, pointer_location_,
EventTimeForNow(), pointer_flags_, 0);
SetTargetAndDispatchEvent(&event, target);
if (dispatch_policy == wl::EventDispatchPolicy::kImmediate) {
SetTargetAndDispatchEvent(&event, target);
} else {
pointer_frames_.push_back(
std::make_unique<PointerFrame>(event, std::move(closure)));
return;
}
}

if (!focused) {
window_manager_->SetPointerFocusedWindow(nullptr);
}
if (!closure.is_null())
std::move(closure).Run();
}

void WaylandEventSource::OnPointerButtonEvent(EventType type,
Expand Down
3 changes: 2 additions & 1 deletion ui/ozone/platform/wayland/host/wayland_event_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ class WaylandEventSource : public PlatformEventSource,

// WaylandPointer::Delegate
void OnPointerFocusChanged(WaylandWindow* window,
const gfx::PointF& location) override;
const gfx::PointF& location,
wl::EventDispatchPolicy dispatch_policy) override;
void OnPointerButtonEvent(EventType evtype,
int changed_button,
WaylandWindow* window = nullptr) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ TEST_P(WaylandEventSourceTest, CheckPointerButtonHandling) {
wl_resource* pointer_res = server_.seat()->pointer()->resource();

wl_pointer_send_enter(pointer_res, serial++, surface_res, 0, 0);
wl_pointer_send_frame(pointer_res);
wl_pointer_send_button(pointer_res, serial++, tstamp++, BTN_LEFT,
WL_POINTER_BUTTON_STATE_PRESSED);
EXPECT_CALL(delegate, DispatchEvent(_)).Times(2);
Expand Down
13 changes: 10 additions & 3 deletions ui/ozone/platform/wayland/host/wayland_pointer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "ui/events/types/event_type.h"
#include "ui/ozone/platform/wayland/common/wayland_util.h"
#include "ui/ozone/platform/wayland/host/wayland_connection.h"
#include "ui/ozone/platform/wayland/host/wayland_event_source.h"
#include "ui/ozone/platform/wayland/host/wayland_serial_tracker.h"
#include "ui/ozone/platform/wayland/host/wayland_window.h"

Expand All @@ -35,7 +36,8 @@ WaylandPointer::~WaylandPointer() {
// Even though, WaylandPointer::Leave is always called when Wayland destroys
// wl_pointer, it's better to be explicit as some Wayland compositors may have
// bugs.
delegate_->OnPointerFocusChanged(nullptr, {});
delegate_->OnPointerFocusChanged(nullptr, {},
wl::EventDispatchPolicy::kImmediate);
delegate_->OnResetPointerFlags();
}

Expand All @@ -55,7 +57,8 @@ void WaylandPointer::Enter(void* data,
static_cast<float>(wl_fixed_to_double(surface_y))};

pointer->delegate_->OnPointerFocusChanged(
window, pointer->connection_->MaybeConvertLocation(location, window));
window, pointer->connection_->MaybeConvertLocation(location, window),
wl::EventDispatchPolicy::kOnFrame);
}

// static
Expand All @@ -67,8 +70,12 @@ void WaylandPointer::Leave(void* data,
pointer->connection_->serial_tracker().ResetSerial(
wl::SerialType::kMouseEnter);

// TODO(https://crrev.com/c/1352584): Switch from kImmediate to kOnFrame when
// Exo comply with other compositors in how it isolates each
// wl_pointer.enter|leave event with their respective wl_pointer.frame.
pointer->delegate_->OnPointerFocusChanged(
nullptr, pointer->delegate_->GetPointerLocation());
nullptr, pointer->delegate_->GetPointerLocation(),
wl::EventDispatchPolicy::kImmediate);
}

// static
Expand Down
10 changes: 8 additions & 2 deletions ui/ozone/platform/wayland/host/wayland_pointer.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ class PointF;
class Vector2dF;
} // namespace gfx

namespace wl {
enum class EventDispatchPolicy;
} // namespace wl

namespace ui {

class WaylandConnection;
Expand Down Expand Up @@ -109,8 +113,10 @@ class WaylandPointer {

class WaylandPointer::Delegate {
public:
virtual void OnPointerFocusChanged(WaylandWindow* window,
const gfx::PointF& location) = 0;
virtual void OnPointerFocusChanged(
WaylandWindow* window,
const gfx::PointF& location,
wl::EventDispatchPolicy dispatch_policy) = 0;
virtual void OnPointerButtonEvent(EventType evtype,
int changed_button,
WaylandWindow* window = nullptr) = 0;
Expand Down
27 changes: 22 additions & 5 deletions ui/ozone/platform/wayland/host/wayland_pointer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ ACTION_P(CloneEvent, ptr) {

TEST_P(WaylandPointerTest, Enter) {
wl_pointer_send_enter(pointer_->resource(), 1, surface_->resource(), 0, 0);
wl_pointer_send_frame(pointer_->resource());

std::unique_ptr<Event> event;
EXPECT_CALL(delegate_, DispatchEvent(_)).WillOnce(CloneEvent(&event));
Expand Down Expand Up @@ -134,9 +135,15 @@ TEST_P(WaylandPointerTest, Leave) {
ASSERT_TRUE(other_surface);

wl_pointer_send_enter(pointer_->resource(), 1, surface_->resource(), 0, 0);
wl_pointer_send_frame(pointer_->resource());

wl_pointer_send_leave(pointer_->resource(), 2, surface_->resource());
wl_pointer_send_frame(pointer_->resource());

wl_pointer_send_enter(pointer_->resource(), 3, other_surface->resource(), 0,
0);
wl_pointer_send_frame(pointer_->resource());

wl_pointer_send_button(pointer_->resource(), 4, 1004, BTN_LEFT,
WL_POINTER_BUTTON_STATE_PRESSED);
EXPECT_CALL(delegate_, DispatchEvent(_)).Times(2);
Expand Down Expand Up @@ -205,6 +212,7 @@ TEST_P(WaylandPointerTest, MotionDragged) {
TEST_P(WaylandPointerTest, AxisSourceTypes) {
uint32_t time = 1001;
wl_pointer_send_enter(pointer_->resource(), 1, surface_->resource(), 0, 0);
wl_pointer_send_frame(pointer_->resource());
Sync(); // We're interested only in checking axis source types events in this
// test case, so skip Enter event here.

Expand Down Expand Up @@ -251,7 +259,7 @@ TEST_P(WaylandPointerTest, AxisSourceTypes) {
TEST_P(WaylandPointerTest, Axis) {
wl_pointer_send_enter(pointer_->resource(), 1, surface_->resource(),
wl_fixed_from_int(0), wl_fixed_from_int(0));

wl_pointer_send_frame(pointer_->resource());
Sync();

for (uint32_t axis :
Expand Down Expand Up @@ -290,6 +298,7 @@ TEST_P(WaylandPointerTest, Axis) {
TEST_P(WaylandPointerTest, SetBitmap) {
wl_pointer_send_enter(pointer_->resource(), 1, surface_->resource(),
wl_fixed_from_int(10), wl_fixed_from_int(10));
wl_pointer_send_frame(pointer_->resource());
Sync();

SkBitmap dummy_cursor;
Expand Down Expand Up @@ -331,6 +340,7 @@ TEST_P(WaylandPointerTest, SetBitmapAndScaleOnPointerFocus) {

wl_pointer_send_enter(pointer_->resource(), ++serial, surface_->resource(),
wl_fixed_from_int(10), wl_fixed_from_int(10));
wl_pointer_send_frame(pointer_->resource());
Sync();

// Set a cursor.
Expand All @@ -341,6 +351,7 @@ TEST_P(WaylandPointerTest, SetBitmapAndScaleOnPointerFocus) {
connection_->ScheduleFlush();

wl_pointer_send_leave(pointer_->resource(), ++serial, surface_->resource());
wl_pointer_send_frame(pointer_->resource());
Sync();
Mock::VerifyAndClearExpectations(pointer_);

Expand All @@ -353,6 +364,7 @@ TEST_P(WaylandPointerTest, SetBitmapAndScaleOnPointerFocus) {
EXPECT_CALL(*pointer_, SetCursor(Ne(nullptr), 5, 8));
wl_pointer_send_enter(pointer_->resource(), ++serial, surface_->resource(),
wl_fixed_from_int(50), wl_fixed_from_int(75));
wl_pointer_send_frame(pointer_->resource());
Sync();

connection_->ScheduleFlush();
Expand All @@ -362,6 +374,7 @@ TEST_P(WaylandPointerTest, SetBitmapAndScaleOnPointerFocus) {

// Reset the focus for the next iteration.
wl_pointer_send_leave(pointer_->resource(), ++serial, surface_->resource());
wl_pointer_send_frame(pointer_->resource());
Sync();
connection_->ScheduleFlush();
Sync();
Expand All @@ -375,9 +388,10 @@ TEST_P(WaylandPointerTest, FlingVertical) {
uint32_t time = 1001;
wl_pointer_send_enter(pointer_->resource(), ++serial, surface_->resource(),
wl_fixed_from_int(50), wl_fixed_from_int(75));
wl_pointer_send_frame(pointer_->resource());

wl_pointer_send_button(pointer_->resource(), ++serial, ++time, BTN_RIGHT,
WL_POINTER_BUTTON_STATE_PRESSED);

Sync();

std::unique_ptr<Event> event1, event2, event3;
Expand Down Expand Up @@ -429,9 +443,10 @@ TEST_P(WaylandPointerTest, FlingHorizontal) {
uint32_t time = 1001;
wl_pointer_send_enter(pointer_->resource(), ++serial, surface_->resource(),
wl_fixed_from_int(50), wl_fixed_from_int(75));
wl_pointer_send_frame(pointer_->resource());

wl_pointer_send_button(pointer_->resource(), ++serial, ++time, BTN_RIGHT,
WL_POINTER_BUTTON_STATE_PRESSED);

Sync();

std::unique_ptr<Event> event1, event2, event3;
Expand Down Expand Up @@ -483,9 +498,10 @@ TEST_P(WaylandPointerTest, FlingCancel) {
uint32_t time = 1001;
wl_pointer_send_enter(pointer_->resource(), ++serial, surface_->resource(),
wl_fixed_from_int(50), wl_fixed_from_int(75));
wl_pointer_send_frame(pointer_->resource());

wl_pointer_send_button(pointer_->resource(), ++serial, ++time, BTN_RIGHT,
WL_POINTER_BUTTON_STATE_PRESSED);

Sync();

std::unique_ptr<Event> event1, event2, event3, event4;
Expand Down Expand Up @@ -549,9 +565,10 @@ TEST_P(WaylandPointerTest, FlingDiagonal) {
uint32_t time = 1001;
wl_pointer_send_enter(pointer_->resource(), ++serial, surface_->resource(),
wl_fixed_from_int(50), wl_fixed_from_int(75));
wl_pointer_send_frame(pointer_->resource());

wl_pointer_send_button(pointer_->resource(), ++serial, ++time, BTN_RIGHT,
WL_POINTER_BUTTON_STATE_PRESSED);

Sync();

std::unique_ptr<Event> event1, event2, event3;
Expand Down
11 changes: 11 additions & 0 deletions ui/ozone/platform/wayland/host/wayland_screen_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,7 @@ TEST_P(WaylandScreenTest, GetCursorScreenPoint) {
uint32_t time = 1002;
wl_pointer_send_enter(pointer->resource(), ++serial, surface->resource(), 0,
0);
wl_pointer_send_frame(pointer->resource());
wl_pointer_send_motion(pointer->resource(), ++time, wl_fixed_from_int(10),
wl_fixed_from_int(20));

Expand All @@ -651,8 +652,10 @@ TEST_P(WaylandScreenTest, GetCursorScreenPoint) {
ASSERT_TRUE(second_surface);
// Now, leave the first surface and enter second one.
wl_pointer_send_leave(pointer->resource(), ++serial, surface->resource());
wl_pointer_send_frame(pointer->resource());
wl_pointer_send_enter(pointer->resource(), ++serial,
second_surface->resource(), 0, 0);
wl_pointer_send_frame(pointer->resource());
wl_pointer_send_motion(pointer->resource(), ++time, wl_fixed_from_int(20),
wl_fixed_from_int(10));

Expand All @@ -664,6 +667,7 @@ TEST_P(WaylandScreenTest, GetCursorScreenPoint) {
// Clear pointer focus.
wl_pointer_send_leave(pointer->resource(), ++serial,
second_surface->resource());
wl_pointer_send_frame(pointer->resource());

Sync();

Expand Down Expand Up @@ -694,6 +698,7 @@ TEST_P(WaylandScreenTest, GetCursorScreenPoint) {

wl_pointer_send_enter(pointer->resource(), ++serial, menu_surface->resource(),
0, 0);
wl_pointer_send_frame(pointer->resource());
wl_pointer_send_motion(pointer->resource(), ++time, wl_fixed_from_int(2),
wl_fixed_from_int(1));

Expand All @@ -709,8 +714,10 @@ TEST_P(WaylandScreenTest, GetCursorScreenPoint) {
// Leave the menu window and enter the top level window.
wl_pointer_send_leave(pointer->resource(), ++serial,
menu_surface->resource());
wl_pointer_send_frame(pointer->resource());
wl_pointer_send_enter(pointer->resource(), ++serial,
second_surface->resource(), 0, 0);
wl_pointer_send_frame(pointer->resource());
wl_pointer_send_motion(pointer->resource(), ++time, wl_fixed_from_int(1912),
wl_fixed_from_int(1071));

Expand All @@ -722,6 +729,7 @@ TEST_P(WaylandScreenTest, GetCursorScreenPoint) {

wl_pointer_send_leave(pointer->resource(), ++serial,
second_surface->resource());
wl_pointer_send_frame(pointer->resource());

// Now, create a nested menu window and make sure that the cursor screen point
// still has been correct. The location of the window is on the right side of
Expand All @@ -741,6 +749,7 @@ TEST_P(WaylandScreenTest, GetCursorScreenPoint) {

wl_pointer_send_enter(pointer->resource(), ++serial,
nested_menu_surface->resource(), 0, 0);
wl_pointer_send_frame(pointer->resource());
wl_pointer_send_motion(pointer->resource(), ++time, wl_fixed_from_int(2),
wl_fixed_from_int(3));

Expand All @@ -752,8 +761,10 @@ TEST_P(WaylandScreenTest, GetCursorScreenPoint) {
// point still must be reported correctly.
wl_pointer_send_leave(pointer->resource(), ++serial,
nested_menu_surface->resource());
wl_pointer_send_frame(pointer->resource());
wl_pointer_send_enter(pointer->resource(), ++serial, menu_surface->resource(),
0, 0);
wl_pointer_send_frame(pointer->resource());
wl_pointer_send_motion(pointer->resource(), ++time, wl_fixed_from_int(2),
wl_fixed_from_int(1));

Expand Down
13 changes: 8 additions & 5 deletions ui/ozone/platform/wayland/host/wayland_window_drag_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,12 @@ void WaylandWindowDragController::OnDragEnter(WaylandWindow* window,

DCHECK(drag_source_.has_value());
// Check if this is necessary.
if (*drag_source_ == DragSource::kMouse)
pointer_delegate_->OnPointerFocusChanged(window, location);
else
if (*drag_source_ == DragSource::kMouse) {
pointer_delegate_->OnPointerFocusChanged(
window, location, wl::EventDispatchPolicy::kImmediate);
} else {
touch_delegate_->OnTouchFocusChanged(window);
}

DVLOG(1) << "OnEnter. widget=" << window->GetWidget();

Expand Down Expand Up @@ -376,8 +378,9 @@ void WaylandWindowDragController::OnDataSourceFinish(bool completed) {
if (*drag_source_ == DragSource::kMouse) {
// TODO: check if this usage is correct.

pointer_delegate_->OnPointerFocusChanged(dragged_window_,
pointer_location_);
pointer_delegate_->OnPointerFocusChanged(
dragged_window_, pointer_location_,
wl::EventDispatchPolicy::kImmediate);
} else {
touch_delegate_->OnTouchFocusChanged(dragged_window_);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ TEST_P(WaylandWindowManagerTest, GetCurrentFocusedWindow) {
wl::MockSurface* surface = server_.GetObject<wl::MockSurface>(
window1->root_surface()->GetSurfaceId());
wl_pointer_send_enter(pointer->resource(), 1, surface->resource(), 0, 0);
wl_pointer_send_frame(pointer->resource());

Sync();

Expand All @@ -118,6 +119,7 @@ TEST_P(WaylandWindowManagerTest, GetCurrentFocusedWindow) {
EXPECT_TRUE(window1.get() == manager_->GetCurrentPointerFocusedWindow());

wl_pointer_send_leave(pointer->resource(), 2, surface->resource());
wl_pointer_send_frame(pointer->resource());

Sync();

Expand Down

0 comments on commit 94d8491

Please sign in to comment.