Commit fc28a126 authored by Nick Diego Yamane's avatar Nick Diego Yamane Committed by Commit Bot

ozone/wayland: Fix drag offset when a tab is detached through the top edge

Detaching a tab by dragging it upwards was not supported until
crrev.com/c/2362987, which modified wl_data_device::leave event
handling so that it dispatches fake motion events with [-1,-1] location,
so giving higher level GUI components (e.g Chrome's TabDragController) a
chance to detect when the pointer leaves that window. Such approach
works, but it turns out to cause an issue in the drag offset calculation
as can be seen in screen cast [1]. This issue can be avoided by keeping
the 'x' coordinate value and using -1 only for 'y'.

NOTE: This is a workaround targeted specifically to Chrome's tab drag
and should ideally be reworked in the future, with a broader change, in
higher level layers, such that they handle platforms that do not support
global screen coordinates appropriately.

Further context:

The aforementioned problem happens because //chrome code assumes global
screen coordinates are available in platform located events, in this
specific case, TabDragController uses a vertical "magnetism" constant to
determine when to detach the dragged tab selection [2]. Such constant
may result in coordinates outside the browser window boundaries, mainly
when dragging upwards, which is ok in a platform where screen positions
are available. That's not the case for Wayland, where DND is used for
tab dragging, where neither implicit grab is supported.

[1] https://youtu.be/NKv4h4Ut-gw
[2] https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/views/tabs/tab_drag_controller.cc;l=1105-1107;drc=6e8b402a6231405b753919029c9027404325ea00

R=msisov@igalia.com

Test: Covered by ozone_unittests
Bug: 896640
Change-Id: I4909acaa7e8a9fbb17968a4ec38632ac61915a0b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2388762Reviewed-by: default avatarMaksim Sisov (GMT+3) <msisov@igalia.com>
Commit-Queue: Nick Yamane <nickdiego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#804799}
parent a47e3a32
...@@ -155,6 +155,7 @@ void WaylandWindowDragController::OnDragEnter(WaylandWindow* window, ...@@ -155,6 +155,7 @@ void WaylandWindowDragController::OnDragEnter(WaylandWindow* window,
// Forward focus change event to the input delegate, so other components, such // Forward focus change event to the input delegate, so other components, such
// as WaylandScreen, are able to properly retrieve focus related info during // as WaylandScreen, are able to properly retrieve focus related info during
// window dragging sesstions. // window dragging sesstions.
pointer_location_ = location;
pointer_delegate_->OnPointerFocusChanged(window, location); pointer_delegate_->OnPointerFocusChanged(window, location);
VLOG(1) << "OnEnter. widget=" << window->GetWidget(); VLOG(1) << "OnEnter. widget=" << window->GetWidget();
...@@ -179,6 +180,7 @@ void WaylandWindowDragController::OnDragMotion(const gfx::PointF& location) { ...@@ -179,6 +180,7 @@ void WaylandWindowDragController::OnDragMotion(const gfx::PointF& location) {
VLOG(2) << "OnMotion. location=" << location.ToString(); VLOG(2) << "OnMotion. location=" << location.ToString();
// Forward cursor location update info to the input handling delegate. // Forward cursor location update info to the input handling delegate.
pointer_location_ = location;
pointer_delegate_->OnPointerMotionEvent(location); pointer_delegate_->OnPointerMotionEvent(location);
} }
...@@ -207,12 +209,15 @@ void WaylandWindowDragController::OnDragLeave() { ...@@ -207,12 +209,15 @@ void WaylandWindowDragController::OnDragLeave() {
// As Wayland clients are only aware of surface-local coordinates and there is // As Wayland clients are only aware of surface-local coordinates and there is
// no implicit grab during DND sessions, a fake motion event with negative // no implicit grab during DND sessions, a fake motion event with negative
// coordinates must be used here to make it possible for higher level UI // y coordinate is used here to allow higher level UI components to detect
// components to detect when a window should be detached. E.g: On Chrome, // when a window should be detached. E.g: On Chrome, dragging a tab all the
// dragging a tab all the way up to the top edge of the window won't work // way up to the top edge of the window won't work without this fake motion
// without this fake motion event upon wl_data_device::leave events. // event upon wl_data_device::leave events. This is a workaround and should
// ideally be reworked in the future, at higher level layers such that they
// properly handle platforms that do not support global screen coordinates,
// like Wayland.
if (state_ == State::kAttached) if (state_ == State::kAttached)
pointer_delegate_->OnPointerMotionEvent({-1, -1}); pointer_delegate_->OnPointerMotionEvent({pointer_location_.x(), -1});
} }
void WaylandWindowDragController::OnDragDrop() { void WaylandWindowDragController::OnDragDrop() {
......
...@@ -113,6 +113,9 @@ class WaylandWindowDragController : public WaylandDataDevice::DragDelegate, ...@@ -113,6 +113,9 @@ class WaylandWindowDragController : public WaylandDataDevice::DragDelegate,
State state_ = State::kIdle; State state_ = State::kIdle;
gfx::Vector2d drag_offset_; gfx::Vector2d drag_offset_;
// The last known pointer location in DIP.
gfx::PointF pointer_location_;
std::unique_ptr<WaylandDataSource> data_source_; std::unique_ptr<WaylandDataSource> data_source_;
std::unique_ptr<WaylandDataOffer> data_offer_; std::unique_ptr<WaylandDataOffer> data_offer_;
......
...@@ -548,12 +548,16 @@ TEST_P(WaylandWindowDragControllerTest, DragExitAttached) { ...@@ -548,12 +548,16 @@ TEST_P(WaylandWindowDragControllerTest, DragExitAttached) {
Sync(); Sync();
EXPECT_EQ(State::kAttached, drag_controller()->state()); EXPECT_EQ(State::kAttached, drag_controller()->state());
// Emulate a wl_data_device::leave and make sure a motion event is dispatched // Emulate a [motion => leave] event sequence and make sure the correct
// in response. // ui::Events are dispatched in response.
SendDndMotion({50, 50});
EXPECT_CALL(delegate_, DispatchEvent(_)).Times(1);
Sync();
SendDndLeave(); SendDndLeave();
EXPECT_CALL(delegate_, DispatchEvent(_)).WillOnce([&](Event* event) { EXPECT_CALL(delegate_, DispatchEvent(_)).WillOnce([&](Event* event) {
EXPECT_EQ(ET_MOUSE_DRAGGED, event->type()); EXPECT_EQ(ET_MOUSE_DRAGGED, event->type());
EXPECT_EQ(gfx::Point(-1, -1).ToString(), EXPECT_EQ(gfx::Point(50, -1).ToString(),
event->AsMouseEvent()->location().ToString()); event->AsMouseEvent()->location().ToString());
}); });
Sync(); Sync();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment