Skip to content

Commit

Permalink
ozone/wayland: Propagate tab drag status changes to platform window
Browse files Browse the repository at this point in the history
Wayland Ozone backend uses a platform drag-and-drop session to be able
to support the capabilities required by Chrome's tab dragging.
Particularly, keeping track of the focused window and the current mouse
cursor position while the tab/window is dragged around. Further context
can be found at the design doc [1].

So far, it has been relying only on {Run,End}MoveLoop API to trigger
such DND session. It works, but, as it is called after the (dragged)
windows creation and mapping, it is not possible to take the necessary
actions upon window creation or mapping (e.g: issue some dnd/extension
protocol request to the Wayland compositor), which makes it insufficient
to achieve glitches-free tab/window detaching implementation. Which has
been verified while addressing [2][3], and can be seen at video [4].

To fix it, this patch does:

- Add WaylandExtension platform window extension, with a single method
for now, that makes it possible for the upper layer components to start
the window dragging session explicitly;
- Plumbs up Chrome's Browser{Frame,DWTHLinux} such that it starts a
Wayland drag-and-drop session when the tab drag starts.

[1] https://docs.google.com/document/d/1s6OwTi_WC-pS21WLGQYI39yw2m42ZlVolUXBclljXB4/edit?usp=sharing
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=1099418
[3] https://chromium-review.googlesource.com/c/chromium/src/+/2307653
[4] https://www.youtube.com/watch?v=3sWVyA1OagA

R=sky@chromium.org

Bug: 896640
Test: Covered by ozone_unittests
Change-Id: I5e902caa7b495ae585330bc3e3b431bacb139d1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346545
Commit-Queue: Nick Yamane <nickdiego@igalia.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Maksim Sisov (GMT+3) <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#797778}
  • Loading branch information
nickdiego authored and Commit Bot committed Aug 13, 2020
1 parent 955c73f commit 7919ff1
Show file tree
Hide file tree
Showing 15 changed files with 163 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "chrome/browser/ui/views/tabs/tab_strip.h"
#include "ui/base/ui_base_features.h"
#include "ui/ozone/public/ozone_platform.h"
#include "ui/platform_window/extensions/wayland_extension.h"
#include "ui/platform_window/extensions/x11_extension.h"

#if defined(USE_DBUS_MENU)
Expand Down Expand Up @@ -78,16 +79,26 @@ bool BrowserDesktopWindowTreeHostLinux::UsesNativeSystemMenu() const {
return false;
}

void BrowserDesktopWindowTreeHostLinux::TabDraggingStatusChanged(
bool is_dragging) {
void BrowserDesktopWindowTreeHostLinux::TabDraggingKindChanged(
TabDragKind tab_drag_kind) {
// If there's no tabs left, the browser window is about to close, so don't
// call SetOverrideRedirect() to prevent the window from flashing.
if (!browser_view_->tabstrip()->GetModelCount())
return;

auto* x11_extension = GetX11Extension();
if (x11_extension && x11_extension->IsWmTiling())
x11_extension->SetOverrideRedirect(is_dragging);
if (x11_extension && x11_extension->IsWmTiling()) {
bool was_dragging_window =
browser_frame_->tab_drag_kind() == TabDragKind::kAllTabs;
bool is_dragging_window = tab_drag_kind == TabDragKind::kAllTabs;
if (is_dragging_window != was_dragging_window)
x11_extension->SetOverrideRedirect(is_dragging_window);
}

if (auto* wayland_extension = ui::GetWaylandExtension(*platform_window())) {
if (tab_drag_kind != TabDragKind::kNone)
wayland_extension->StartWindowDraggingSessionIfNeeded();
}
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ using DesktopWindowTreeHostLinuxImpl = views::DesktopWindowTreeHostLinux;

class BrowserFrame;
class BrowserView;
enum class TabDragKind;

namespace views {
class DesktopNativeWidgetAura;
Expand All @@ -33,8 +34,8 @@ class BrowserDesktopWindowTreeHostLinux
BrowserFrame* browser_frame);
~BrowserDesktopWindowTreeHostLinux() override;

// Called when the window starts or stops moving because of a tab drag.
void TabDraggingStatusChanged(bool is_dragging);
// Called when the tab drag status changes for this window.
void TabDraggingKindChanged(TabDragKind tab_drag_kind);

private:
// BrowserDesktopWindowTreeHost:
Expand Down
6 changes: 2 additions & 4 deletions chrome/browser/ui/views/frame/browser_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,8 @@ void BrowserFrame::SetTabDragKind(TabDragKind tab_drag_kind) {
if (tab_drag_kind_ == tab_drag_kind)
return;

bool was_dragging_window = tab_drag_kind_ == TabDragKind::kAllTabs;
bool is_dragging_window = tab_drag_kind == TabDragKind::kAllTabs;
if (was_dragging_window != is_dragging_window && native_browser_frame_)
native_browser_frame_->TabDraggingStatusChanged(is_dragging_window);
if (native_browser_frame_)
native_browser_frame_->TabDraggingKindChanged(tab_drag_kind);

bool was_dragging_any = tab_drag_kind_ != TabDragKind::kNone;
bool is_dragging_any = tab_drag_kind != TabDragKind::kNone;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,9 @@ bool DesktopBrowserFrameAuraLinux::UseCustomFrame() const {
return false;
}

void DesktopBrowserFrameAuraLinux::TabDraggingStatusChanged(bool is_dragging) {
host_->TabDraggingStatusChanged(is_dragging);
void DesktopBrowserFrameAuraLinux::TabDraggingKindChanged(
TabDragKind tab_drag_kind) {
host_->TabDraggingKindChanged(tab_drag_kind);
}

void DesktopBrowserFrameAuraLinux::OnUseCustomChromeFrameChanged() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class DesktopBrowserFrameAuraLinux : public DesktopBrowserFrameAura {
// Overridden from NativeBrowserFrame:
views::Widget::InitParams GetWidgetParams() override;
bool UseCustomFrame() const override;
void TabDraggingStatusChanged(bool is_dragging) override;
void TabDraggingKindChanged(TabDragKind tab_drag_kind) override;

private:
// Called when the preference changes.
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/ui/views/frame/native_browser_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "ui/views/widget/widget.h"

class BrowserFrame;
enum class TabDragKind;

namespace content {
struct NativeWebKeyboardEvent;
Expand Down Expand Up @@ -49,8 +50,8 @@ class NativeBrowserFrame {
virtual bool HandleKeyboardEvent(
const content::NativeWebKeyboardEvent& event) = 0;

// Called when the window starts or stops moving because of a tab drag.
virtual void TabDraggingStatusChanged(bool is_dragging) {}
// Called when the tab drag kind for this frame changes.
virtual void TabDraggingKindChanged(TabDragKind tab_drag_kind) {}

protected:
friend class BrowserFrame;
Expand Down
2 changes: 2 additions & 0 deletions ui/ozone/platform/wayland/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ source_set("wayland") {
"//ui/ozone/common",
"//ui/ozone/public/mojom/wayland:wayland_mojom",
"//ui/platform_window",
"//ui/platform_window/extensions",
"//ui/platform_window/wm",
]

Expand Down Expand Up @@ -352,6 +353,7 @@ source_set("wayland_unittests") {
"//ui/gfx/linux:test_support",
"//ui/ozone:platform",
"//ui/ozone:test_support",
"//ui/platform_window/extensions",
"//ui/platform_window/wm",
]

Expand Down
8 changes: 8 additions & 0 deletions ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
#include "ui/ozone/platform/wayland/host/wayland_connection.h"
#include "ui/ozone/platform/wayland/host/wayland_data_drag_controller.h"
#include "ui/ozone/platform/wayland/host/wayland_event_source.h"
#include "ui/ozone/platform/wayland/host/wayland_window.h"
#include "ui/ozone/platform/wayland/host/wayland_window_drag_controller.h"
#include "ui/platform_window/extensions/wayland_extension.h"
#include "ui/platform_window/wm/wm_drop_handler.h"

namespace ui {
Expand Down Expand Up @@ -338,6 +340,7 @@ bool WaylandToplevelWindow::OnInitialize(
#else
wm_class_class_ = properties.wm_class_class;
#endif
SetWaylandExtension(this, static_cast<WaylandExtension*>(this));
SetWmMoveLoopHandler(this, static_cast<WmMoveLoopHandler*>(this));
return true;
}
Expand All @@ -352,6 +355,11 @@ void WaylandToplevelWindow::EndMoveLoop() {
connection()->window_drag_controller()->StopDragging();
}

void WaylandToplevelWindow::StartWindowDraggingSessionIfNeeded() {
DCHECK(connection()->window_drag_controller());
connection()->window_drag_controller()->StartDragSession();
}

void WaylandToplevelWindow::TriggerStateChanges() {
if (!shell_surface_)
return;
Expand Down
7 changes: 6 additions & 1 deletion ui/ozone/platform/wayland/host/wayland_toplevel_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "build/lacros_buildflags.h"
#include "ui/gfx/geometry/vector2d.h"
#include "ui/ozone/platform/wayland/host/wayland_window.h"
#include "ui/platform_window/extensions/wayland_extension.h"
#include "ui/platform_window/wm/wm_drag_handler.h"
#include "ui/platform_window/wm/wm_move_loop_handler.h"
#include "ui/platform_window/wm/wm_move_resize_handler.h"
Expand All @@ -19,7 +20,8 @@ class ShellSurfaceWrapper;
class WaylandToplevelWindow : public WaylandWindow,
public WmMoveResizeHandler,
public WmDragHandler,
public WmMoveLoopHandler {
public WmMoveLoopHandler,
public WaylandExtension {
public:
WaylandToplevelWindow(PlatformWindowDelegate* delegate,
WaylandConnection* connection);
Expand Down Expand Up @@ -79,6 +81,9 @@ class WaylandToplevelWindow : public WaylandWindow,
bool RunMoveLoop(const gfx::Vector2d& drag_offset) override;
void EndMoveLoop() override;

// WaylandExtension:
void StartWindowDraggingSessionIfNeeded() override;

void TriggerStateChanges();
void SetWindowState(PlatformWindowState state);

Expand Down
74 changes: 36 additions & 38 deletions ui/ozone/platform/wayland/host/wayland_window_drag_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,48 @@ WaylandWindowDragController::WaylandWindowDragController(

WaylandWindowDragController::~WaylandWindowDragController() = default;

bool WaylandWindowDragController::Drag(WaylandToplevelWindow* window,
const gfx::Vector2d& offset) {
DCHECK_LE(state_, State::kAttached);
DCHECK(window);
bool WaylandWindowDragController::StartDragSession() {
if (state_ != State::kIdle)
return true;

if (!OfferWindow())
origin_window_ = window_manager_->GetCurrentFocusedWindow();
if (!origin_window_) {
LOG(ERROR) << "Failed to get origin window.";
return false;
}

VLOG(1) << "Starting DND session.";
state_ = State::kAttached;

DCHECK(!data_source_);
data_source_ = data_device_manager_->CreateSource(this);
data_source_->Offer({kMimeTypeChromiumWindow});
data_source_->SetAction(DragDropTypes::DRAG_MOVE);

// TODO(crbug.com/1099418): Use dragged window's surface as icon surface
// once "immediate drag" protocol extensions are available.
data_device_->StartDrag(*data_source_, *origin_window_,
/*icon_surface=*/nullptr, this);

pointer_grab_owner_ = origin_window_;

// Observe window so we can take ownership of the origin surface in case it
// is destroyed during the DND session.
window_manager_->AddObserver(this);
return true;
}

bool WaylandWindowDragController::Drag(WaylandToplevelWindow* window,
const gfx::Vector2d& offset) {
DCHECK_EQ(state_, State::kAttached);
DCHECK(window);
dragged_window_ = window;
drag_offset_ = offset;

RunLoop();

dragged_window_ = nullptr;

DCHECK(state_ == State::kAttached || state_ == State::kDropped);
bool dropped = state_ == State::kDropped;
if (dropped)
Expand Down Expand Up @@ -248,38 +277,6 @@ void WaylandWindowDragController::OnWindowRemoved(WaylandWindow* window) {
origin_surface_ = origin_window_->TakeWaylandSurface();
}

bool WaylandWindowDragController::OfferWindow() {
DCHECK_LE(state_, State::kAttached);

origin_window_ = window_manager_->GetCurrentFocusedWindow();
if (!origin_window_) {
LOG(ERROR) << "Failed to get origin window.";
return false;
}

if (!data_source_)
data_source_ = data_device_manager_->CreateSource(this);

if (state_ == State::kIdle) {
// Observe window so we can take ownership of the origin surface in case it
// is destroyed during the DND session.
window_manager_->AddObserver(this);

VLOG(1) << "Starting DND session.";
state_ = State::kAttached;
data_source_->Offer({kMimeTypeChromiumWindow});
data_source_->SetAction(DragDropTypes::DRAG_MOVE);

// TODO(crbug.com/1099418): Use dragged window's surface as icon surface
// once "immediate drag" protocol extensions are available.
data_device_->StartDrag(*data_source_, *origin_window_,
/*icon_surface=*/nullptr, this);
}

pointer_grab_owner_ = origin_window_;
return true;
}

void WaylandWindowDragController::HandleMotionEvent(MouseEvent* event) {
DCHECK_EQ(state_, State::kDetached);
DCHECK(dragged_window_);
Expand Down Expand Up @@ -324,7 +321,8 @@ void WaylandWindowDragController::RunLoop() {
DCHECK_EQ(state_, State::kAttached);
DCHECK(dragged_window_);

VLOG(1) << "Starting drag loop. widget=" << dragged_window_->GetWidget();
VLOG(1) << "Starting drag loop. widget=" << dragged_window_->GetWidget()
<< " offset=" << drag_offset_.ToString();

// TODO(crbug.com/896640): Handle cursor
auto old_dispatcher = std::move(nested_dispatcher_);
Expand Down
10 changes: 6 additions & 4 deletions ui/ozone/platform/wayland/host/wayland_window_drag_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ class WaylandWindowDragController : public WaylandDataDevice::DragDelegate,
delete;
~WaylandWindowDragController() override;

bool Drag(WaylandToplevelWindow* surface, const gfx::Vector2d& offset);
// Starts a new Wayland DND session for window dragging, if not done yet. A
// new data source is setup and the focused window is used as the origin
// surface.
bool StartDragSession();

bool Drag(WaylandToplevelWindow* window, const gfx::Vector2d& offset);
void StopDragging();

State state() const { return state_; }
Expand Down Expand Up @@ -87,9 +92,6 @@ class WaylandWindowDragController : public WaylandDataDevice::DragDelegate,
// WaylandWindowObserver:
void OnWindowRemoved(WaylandWindow* window) override;

// Offers the focused window as available to be dragged. A new data source is
// setup and the underlying DnD session is started, if not done yet.
bool OfferWindow();
// Handles drag/move mouse |event|, while in |kDetached| mode, forwarding it
// as a bounds change event to the upper layer handlers.
void HandleMotionEvent(MouseEvent* event);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "ui/ozone/platform/wayland/test/test_wayland_server_thread.h"
#include "ui/ozone/platform/wayland/test/wayland_test.h"
#include "ui/ozone/test/mock_platform_window_delegate.h"
#include "ui/platform_window/extensions/wayland_extension.h"
#include "ui/platform_window/platform_window_delegate.h"
#include "ui/platform_window/wm/wm_move_loop_handler.h"

Expand Down Expand Up @@ -209,10 +210,14 @@ TEST_P(WaylandWindowDragControllerTest, DragInsideWindowAndDrop) {
SendPointerPress(window_.get(), &delegate_, BTN_LEFT);
SendPointerMotion(window_.get(), &delegate_, {10, 10});

// Set up an "interaction flow" and RunMoveLoop:
// Set up an "interaction flow", start the drag session and run move loop:
// - Event dispatching and bounds changes are monitored
// - At each event, emulates a new event at server side and proceeds to the
// next test step.
auto* wayland_extension = GetWaylandExtension(*window_);
wayland_extension->StartWindowDraggingSessionIfNeeded();
EXPECT_EQ(State::kAttached, drag_controller()->state());

auto* move_loop_handler = GetWmMoveLoopHandler(*window_);
DCHECK(move_loop_handler);

Expand Down Expand Up @@ -296,10 +301,14 @@ TEST_P(WaylandWindowDragControllerTest, DragExitWindowAndDrop) {
SendPointerPress(window_.get(), &delegate_, BTN_LEFT);
SendPointerMotion(window_.get(), &delegate_, {10, 10});

// Sets up an "interaction flow" and RunMoveLoop:
// Sets up an "interaction flow", start the drag session and run move loop:
// - Event dispatching and bounds changes are monitored
// - At each event, emulates a new event on server side and proceeds to the
// next test step.
auto* wayland_extension = GetWaylandExtension(*window_);
wayland_extension->StartWindowDraggingSessionIfNeeded();
EXPECT_EQ(State::kAttached, drag_controller()->state());

auto* move_loop_handler = GetWmMoveLoopHandler(*window_);
DCHECK(move_loop_handler);

Expand Down Expand Up @@ -408,10 +417,14 @@ TEST_P(WaylandWindowDragControllerTest, DragToOtherWindowSnapDragDrop) {
SendPointerPress(source_window, &delegate_, BTN_LEFT);
SendPointerMotion(source_window, &delegate_, {10, 10});

// Sets up an "interaction flow" and RunMoveLoop:
// Sets up an "interaction flow", start the drag session and run move loop:
// - Event dispatching and bounds changes are monitored
// - At each event, emulates a new event on server side and proceeds to the
// next test step.
auto* wayland_extension = GetWaylandExtension(*window_);
wayland_extension->StartWindowDraggingSessionIfNeeded();
EXPECT_EQ(State::kAttached, drag_controller()->state());

auto* move_loop_handler = GetWmMoveLoopHandler(*window_);
DCHECK(move_loop_handler);

Expand Down
2 changes: 2 additions & 0 deletions ui/platform_window/extensions/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ source_set("extensions") {

if (is_linux || is_chromeos) {
sources += [
"wayland_extension.cc",
"wayland_extension.h",
"x11_extension.cc",
"x11_extension.h",
"x11_extension_delegate.h",
Expand Down
Loading

0 comments on commit 7919ff1

Please sign in to comment.