Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixup! Implements Drag and Drop for Ozone/X11 #498

Open
wants to merge 17 commits into
base: ozone-wayland-dev
Choose a base branch
from

Conversation

jkim-julie
Copy link
Member

It adds APIs to handle drop action at WmDropHandler, implements
them at DesktopDragDropClientOzone and connects them to Ozone/X11.

msisov and others added 17 commits October 15, 2018 11:02
… with system libgbm""

This reverts commit 816f0e1.

This is a temporary revert of the above mentioned commit to make
it possible to compile chromium with a system libgbm.

Before relanding this, we have to fix ChromeOS.
This patch enables native gpu memory buffers support by fixing compile
time assumptions in client_native_pixmap_factory_dmabuf.cc.

Now, the supports_import_from_dmabuf_ field is added to the
client_native_pixmap_factory.h interface, and it is used to identify
whether *cpu_read_write buffer usages can be supported.

Bug: 864914
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ifd7d3c6d3906990a79521867ee4e6a826ce4c618
It introduces DesktopDragDropClientOzone class to manage
drag-and-drop. 'DesktopWindowTreeHostPlatform::CreateDragDropClient'
creates DesktopDragDropClientOzone and it's connected to platform
windows and has an internal runloop like other DragDropClients.

It adds WmDragHandler and sets the platform window as a WmDragHandler.
When DesktopWindowTreeHostPlatform starts dragging, it gets
WmDragHandler through the platform window and sends the dragging
request to the platform layer.
It also adds WmDropHandler and sets DesktopDragDropClientOzone as
a WmDropHandler. When each platform receives messages related to the
drop operation, it delivers them to DesktopDragDropClientOzone.

It implements 'StartDragAndDrop' of 'DragDropClient' for Ozone
and adds 'OnDragSessionClosed' to DesktopDragDropClientOzone to
finish dragging session.

As Wayland platform window has the drag and drop implementation
at the platform layer already, it extends WaylandWindow interfaces,
StartDrag.

With this change, Ozone/Wayland build can start dragging from
Chromium to other applications.

Test=DesktopDragDropClientOzoneTest.StartDrag
BUG=875164

Change-Id: I4ebaedfae5f6affb80b4d6c7c6d8ed3ba86ef4ca
This patch enables hardware assisted video decoding via the
Chromium V4L2VDA. Including changes when Linux is used. In
order to use this, use_linux_v4l2_only flag should be set
to true.

Signed-off-by: Ryo Kodama <[email protected]>

fixup! avoid building not declared formats

"FRAME", "_SLICE",  "V4L2_PIX_FMT_VP9" are not defined in mainline
 Linux headers. This patch avoids building these formats.

Signed-off-by: Ryo Kodama <[email protected]>

Issue Igalia#437
This patch sets XSetWindowAttributes::override_redirect to
True, which removes different properties from windows.
At this stage, all windows with width() less than 600 are treated
as menus and override_redirect is set to True.

It's almost same as we had with mus except services.

Issue Igalia#38
Issue Igalia#417
This commit adds a popup window type to ozone windows.
Popup windows are drop down windows, for example.

Issue Igalia#417

fixup! Add popup window type support.

It added case handling on WindowTreeHostPlatform::GetWindowType().
This patch adds an initial support to start a move loop and create
a new window by dragging a tab. To start with, this commit
extends WindowTree::PerformWindowMove, which calls
WindowServer::StartMoveLoop -> PlatformWindow::RunMoveLoop.

X11 Ozone implementation of PlatformWindow uses
WindowMoveLoopClient, which instantiates the move loop and
WholeScreenMoveLoop, which creates an invisible window
and intercepts all the events from it. Then the system screen
location is taken and sent to WindowMoveLoopClient, which
updates actual bounds of X11WindowBase.

The way it works is precisly the same as in stock X11, but
further work to share the code is needed, because there are
some difference between stock x11 and ozone x11 event
handlings, bounds set and etc.

Issue Igalia#264

fixup! [ozone/wayland] Add inital support for a tab drag window.

adapt to
https://crrev.com/c/774778

CanDispatchEvent is modified in such a way that it also tests if
the client in a move loop right now. If so, it must dispatch events
further once whole_screen_move_loop processes them in order to update
mouse locations on aura side.

fixup! [ozone/wayland] Add inital support for a tab drag window.

It removed changes at services from previous change to make it
available without mus and made WindowFinder with ozone work without
mus with creating GetLocalProcessWindowAtPointOzone to
get gfx::NativeWindow at input position.

Issue Igalia#430

fixup! Add inital support for a tab drag window.

Remove the non-existing code and fix ozone/drm build.

Change-Id: I9d3a94a9e47ed2bddaef0253de193d92e01768b0
This is a work-in-progress, which allows to fetch display data
and unblock start of ozone/x11, which crashes now.

Idieally, X11NativeDisplayDelegate should communicate with
DesktopScreenX11 (to be copied and modified and renamed to
X11DesktopScreenOzone).

fixup! [ozone/x11] Add initial display data fetching mechanism.

Compilation fixes.
X11NativeDisplayDelegate owns X11DisplayManager and it's
registered as an X11DisplayManager's observer and it starts to
communicate with it. When X11DisplayManager is created, it
gathers display information and create DisplaySnapshot.
DesktopScreenOzone gets updated display through
DesktopScreenOzone::OnConfigurationChanged,
X11NativeDisplayDelegate::GetDisplays and
DesktopScreenOzone::OnHostDisplaysReady.

fixup! Add X11DisplayManager to get display information

This patch removed OwnedDisplaySnapshot because it doesn't need to
own DisplayMode. DisplaySnapshot is created with
DisplaySnapshot::DisplayModeList, which is the list has DisplayModes.
Whenever DisplayMode is added to the list, it's cloned and kept in
the list and every DisplayMode should be in the list.
…ing.

That is, after we've added ime imeplementation for Wayland, other
ozone platforms started to crash. To avoid this, use fake factory.

fixup! [ozone/x11/headless] Use FakeInputMethodContextFactory to avoid crashing.

Added condition before setting LinuxInputMethodContextFactory.

fixup! [ozone/x11/headless] Use FakeInputMethodContextFactory to avoid crashing.

It needs to skip DCHEK at InitializeInputMethodForTesting() as
LinuxInputMethodContextFactory's already set at
OzonePlatform::InitializeUI().
It introduces X11DragSource and X11DragContext to handle DnD
on Ozone/X11. They are from DesktopDragDropClientAuraX11 which
includes implementation for platform layer.

X11DragSource handles event and deliver the data at a server side
and X11DragContext handles events and read the data at a client
side.

It splits OSExchangeDataProviderAuraX11 to OSExchangeDataProviderAuraX11
and OSExchangeDataProviderAuraX11Base to resue it.
OSExchangeDataProviderAuraX11 keeps only platform event specific part.
Though minigbm gbm_bo_get_plane_count returns a size_t, libgbm is
returning an int. And a return value of -1 is possible (it
reports the operation failed).

So modify DCHECK that would assume both are size_t for the
case build is using system GBM.
This patch adds a move/resize support to the Ozone/X11 platform.

Change-Id: Ie5b2a059f0024f12b3235272f43876379be041b6
… in WaylandScreen.

- GetDisplayForAcceleratedWidget:
* This CL implements wl_surface_listener, which says what
 wl_output a wl_surface enters. That information is then
 used to identify on what display an accelerated widget is shown.

 To give more details, a WaylandWindow sets a wl_surface_listener now.
 As soon as that specific window enters a certain display,
 wl_surface_listener::enter event is called, and the WaylandWindow
 receives a pointer to that specific wl_output where it is shown.

 Then, the window uses WaylandOutputManager to identify the id of
 that display/output and stores it.

 Once the WaylandScreen::GetDisplayForAcceleratedWidget call comes,
 a WaylandWindow, which corresponds to that specific widget, is found
 and the id of the output it is shown on is taken.

 If there are two outputs used to show the window, the very first one
 is always used.

- GetAcceleratedWidgetAtScreenPoint
* This is a tricky one. To ensure right functionality, a widget under a
 cursor must be returned. But, Wayland clients cannot know where the windows
 are located in the global space coordinate system. Instead, it's possible
 to know widgets located on a surface local coordinate system (remember that
 clients cannot also know the position of the pointer in the global space
 coordinate system, but rather on a local surface coordinate system). That
 is, we will have to pretend that a single surface is a "display", where
 other widgets (child widgets are located in the surface local coordinate
 system, where the main surface has 0,0 origin) are shown. Whenever that
 surface is focused (the cursor is located under that widget), we will use
 it to determine if the point is on that main surface, a menu surface and
 etc.

 Note that bounds where widgets are shown will be fixed in the follow-up cl.
 Now, the widgets are not shown properly if the returned display has origin
 different from 0,0. That is, remember that wayland requires placing them
 relatively to the parent surface, which Chromium does not honor, but rather
 uses global space coordinate system.

- GetDisplayNearestPoint and GetDisplayMatching:
* Nothing special. Both of them use the existing helper methods.

Bug: 875161, 890271, 890272
Change-Id: I7d6d8a7d6827cf639df2c3f0992deece156b2962
It adds APIs to handle drop action at WmDropHandler, implements
them at DesktopDragDropClientOzone and connects them to Ozone/X11.
@jkim-julie jkim-julie requested a review from msisov October 15, 2018 07:18
Copy link
Member

@msisov msisov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the patch is unclear. can you explain why you extend the WmDropHandler interface? Wayland looks completely different here.

DesktopDragDropClientOzone::CreateDropTargetEvent() const {
gfx::Point root_location(point_.x(), point_.y());
root_window_->GetHost()->ConvertScreenInPixelsToDIP(&root_location);
return CreateDropTargetEvent(gfx::PointF(root_location));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need a point with float?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this change, https://chromium-review.googlesource.com/c/chromium/src/+/1153433, DropTargetEvent uses PointF.

GetRootTransform().TransformPoint(&point_3f);
*point = gfx::ToFlooredPoint(point_3f.AsPointF());
*point = point_3f.AsPointF();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't you use the above method? why float is needed here?

NOTIMPLEMENTED_LOG_ONCE();
WmDropHandler* drop_handler = GetWmDropHandler(*delegate());
if (!drop_handler) {
LOG(ERROR) << "Failed to get drag handler.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this LOG message is needed et all.

NOTIMPLEMENTED_LOG_ONCE();
WmDropHandler* drop_handler = GetWmDropHandler(*delegate());
if (!drop_handler) {
LOG(ERROR) << "Failed to get drag handler.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here.


WmDropHandler* drop_handler = GetWmDropHandler(*delegate());
if (!drop_handler) {
LOG(ERROR) << "Failed to get drag handler.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here.

NOTIMPLEMENTED_LOG_ONCE();
WmDropHandler* drop_handler = GetWmDropHandler(*delegate());
if (!drop_handler) {
LOG(ERROR) << "Failed to get drag handler.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here.

@jkim-julie
Copy link
Member Author

Wayland changes also has WmDrophandler. During rebase, i think we missed the part to handle drop action for wayland from somewhere because there is only start dragging at upstream gerrit now. For receiving drop, it'll be handled after landing start dragging patch. If you look at DesktopDragDropClientOzone ctor,

SetWmDropHandler(delegate, AsWmDropHandler());
, it's registered as WmDropWindowHanlder and receives messages from platform when drag is finished. The drag and drop has two way communication. So, without window handlers from previous change, it extended interfaces at PlatformWindow, from views to platform, for starting drag from chromium, and PlatformWindowDelegate, from platform to views, for receiving drop from platform.
This patch has both of them, starting dragging and receiving drop.

@msisov
Copy link
Member

msisov commented Oct 18, 2018

Ok, then using callbacks is not a good idea, right?

@jkim-julie
Copy link
Member Author

For the change at upstream, https://chromium-review.googlesource.com/c/chromium/src/+/1218446, I removed drop handler as I agreed we can use callback for starting dragging. When dragging is started from external applications, we need to consider how PlatformWindow sends messages to DesktopDragDropClientOzone.

  • My first trial was using PlatformWindowDelegate but it causes adding new interfaces to PlatformWindowDelegate and it affects all Ozone Platform.
  • Second trial is using WmDropHandler.

If Window Handler is not fit for handling it,

  • We could create another interface like 'aura::client::SetDragDropClient'.
    In case of 'SetDragDropClient', we can only access with AURA window. We need something to access using PlatformWindow or some information from PlatformLayer and some interfaces like OnDragEnter, OnDragMotion... I think it could be similar to WmDropHandler.

@msisov
Copy link
Member

msisov commented Oct 19, 2018

Ok, I need to check more carefully.

@msisov msisov force-pushed the ozone-wayland-dev branch from 5ff151d to 37f1bc5 Compare October 22, 2018 10:43
@jkim-julie jkim-julie force-pushed the ozone-wayland-dev branch 2 times, most recently from 3a61571 to f15ed30 Compare November 5, 2018 01:30
@jkim-julie jkim-julie force-pushed the ozone-wayland-dev branch 2 times, most recently from bc20ff6 to 076da08 Compare November 26, 2018 04:44
@msisov msisov force-pushed the ozone-wayland-dev branch from 8bdadf8 to 087ad68 Compare December 3, 2018 08:17
@msisov msisov force-pushed the ozone-wayland-dev branch from 087ad68 to 3460de8 Compare December 3, 2018 09:23
msisov pushed a commit that referenced this pull request Dec 23, 2018
… on a non-UI thread

A D-Bus method was being called from the blocking pool instead of the UI
thread which is a no-go.

Additionally, rename a proto field since the autogenerated accessor was
name-colliding with Java proto bindings.

Bug: 893245
Change-Id: I5f6aa08d9df4a90d507b1e27ed3da90872f4d0ff
Reviewed-on: https://chromium-review.googlesource.com/c/1297422
Reviewed-by: Maksim Ivanov <[email protected]>
Commit-Queue: Ivan Šandrk <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#602679}(cherry picked from commit 08685b9)
Reviewed-on: https://chromium-review.googlesource.com/c/1317851
Reviewed-by: Ivan Šandrk <[email protected]>
Cr-Commit-Position: refs/branch-heads/3578@{#498}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
@msisov msisov force-pushed the ozone-wayland-dev branch from 0275091 to ea913bd Compare January 7, 2019 08:58
@msisov msisov force-pushed the ozone-wayland-dev branch 2 times, most recently from 21f4a33 to f4c0623 Compare January 21, 2019 06:04
@msisov msisov force-pushed the ozone-wayland-dev branch from f4c0623 to c10fc7b Compare January 28, 2019 08:14
msisov pushed a commit that referenced this pull request Jan 31, 2019
This histogram is for all web apps in contrast with the other WebRTC text logging histograms.

Bug: 915679
Change-Id: I7f90cc6438db9a9e25f5c64e60ce9a4d4cf0f7a1
Reviewed-on: https://chromium-review.googlesource.com/c/1379957
Reviewed-by: Brian White <[email protected]>
Reviewed-by: Tommi <[email protected]>
Reviewed-by: Oskar Sundbom <[email protected]>
Commit-Queue: Henrik Grunell <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#618221}(cherry picked from commit 019922b)
Reviewed-on: https://chromium-review.googlesource.com/c/1388442
Reviewed-by: Henrik Grunell <[email protected]>
Cr-Commit-Position: refs/branch-heads/3626@{#498}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
@msisov msisov force-pushed the ozone-wayland-dev branch 2 times, most recently from 27f5faa to 5c0e21a Compare February 11, 2019 09:15
@msisov msisov force-pushed the ozone-wayland-dev branch 2 times, most recently from cdb2e63 to 0ddb056 Compare February 25, 2019 10:56
@msisov msisov force-pushed the ozone-wayland-dev branch 2 times, most recently from 92c1adf to 14e8e28 Compare March 4, 2019 06:50
@msisov msisov force-pushed the ozone-wayland-dev branch from 14e8e28 to d436cea Compare March 11, 2019 07:44
msisov pushed a commit that referenced this pull request Mar 14, 2019
[email protected]

Change-Id: I089064fe555b6bd1e397c62a5b46e5766261a2c4
Reviewed-on: https://chromium-review.googlesource.com/c/1477994
Reviewed-by: Mustafa Emre Acer <[email protected]>
Cr-Commit-Position: refs/branch-heads/3683@{#498}
Cr-Branched-From: e510299-refs/heads/master@{#625896}
@msisov msisov force-pushed the ozone-wayland-dev branch from d436cea to 7da975a Compare March 18, 2019 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants