Details

Reviewers

None

Group Reviewers

Commits

R108:6e08fb2fa5f6: [xwl] Generic X selections translation mechanism with Clipboard support

Summary

In this patch an infrastructure is created to represent generic X selections
in a Wayland session and use them for data transfers between Xwayland windows
and Wayland native clients.

The central manager is the DataBridge class, in which Selection objects can be
created. This is hard-coded and such a Selection object persists until the end
of the session, so no arbitrary selections can be created on the fly. For now
the X Clipboard selection is supported, whose corresponding mechanism in the
Wayland protocol is just called Selection.

A Selection object listens for selection owner changes on the X side and for
similar events into the Wayland server interfaces. If a data provider is
available a selection source object is created by the Selection object. In case
data is requested on the other side, a data transfer is initialized by creating
a Transfer object. A Selection keeps track of all transfers and makes sure that
they are destroyed when they are finished or in case they idle because of
misbehaving clients.

The Clipboard class translates the X Clipboard via a proxy window. Selection
changes on the Wayland side are listened to through a new signal on the active
KWayland seat interface.

The previously used X clipboard syncer helper is disabled. The clipboard sync
autotest is changed to the new mechanism.

BUG: 394765
BUG: 395313

Test Plan

Manually and clipboard sync autotest.

Diff Detail

Branch

0clipboardXwl

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 2710
Build 2728: arc lint + arc unit

romangg requested review of this revision.Aug 24 2018, 7:34 PM

romangg created this revision.

Harbormaster completed remote builds in B2204: Diff 40377.Aug 24 2018, 7:34 PM

romangg set the repository for this revision to R108 KWin.Aug 24 2018, 7:35 PM

romangg added a project: KWin.

Restricted Application added a subscriber: kwin. · View Herald TranscriptAug 24 2018, 7:35 PM

romangg added dependencies: D15022: [autotests] Use Xwayland class in WaylandTestApplication, D15023: [server] Add selectionChanged signal.Aug 24 2018, 7:36 PM

romangg added a dependent revision: D15063: Remove X clipboard sync helper and rename its autotest.Aug 24 2018, 8:38 PM

Just wondering: why did you integrate directly into KWin, instead of adding this to the existing helper process? My motivation to use a dedicated process was to ensure that KWin cannot be attacked through the X clipboard (the data is hold by the helper process, not by KWin). If you think having it in KWin is better I suggest a follow up change to move the normal clipboard functionality also into the xwl directory.

In D15061#315063, @graesslin wrote:

Just wondering: why did you integrate directly into KWin, instead of adding this to the existing helper process? My motivation to use a dedicated process was to ensure that KWin cannot be attacked through the X clipboard (the data is hold by the helper process, not by KWin).

The patch is a preparation step for a not yet uploaded follow-up patch to allow Xwayland DND ( T4611 ). For that I orientated myself on the implementation / WIP-patches of Weston and wlroots, that both have X Selections translation inside the compositor process. Xwayland DND is difficult already and the other projects don't have full support for it yet, so putting the generic X selections support out-of-process was just another complication I wanted to avoid for now. I personally also don't see a particular attack vector when having it in the compositor process. X clients and KWin communicate through the X protocol when exchanging X selections like they always do in a Wayland session with running Xwayland and data received is interpreted as raw bytes and only forwarded to clients on the Wayland side. If there is benefit to having it out-of-process, then I would reassess this in the future, maybe then combined with a move to put all X11 interfacing out of process in a Wayland session. For now I opt for not reusing the helper process in order to reduce the overall complexity and have as a starting point a mechanism akin to other compositor projects.

If you think having it in KWin is better I suggest a follow up change to move the normal clipboard functionality also into the xwl directory.

Do you mean the X clipboard sync heper itself? It gets removed in follow-up patch D15063.

There is a small bug when the acitve window gets changed with a Wayland native selection: The Wayland source gets deleted on the later Xfixes notify and the selection request by the clients are not answered. Will update the diff with a fix.

Ignore Xfixes notify when we disown a selection

Harbormaster completed remote builds in B2287: Diff 40510.Aug 27 2018, 1:29 PM

Rebase on master.

Harbormaster completed remote builds in B2710: Diff 41388.Sep 11 2018, 10:48 AM

Rebase on master.

Harbormaster completed remote builds in B3042: Diff 42010.Sep 20 2018, 9:10 PM

zzag added a subscriber: zzag.Oct 22 2018, 5:24 PM

zzag added inline comments.

xwl/databridge.cpp
96–102	The lambda is superfluous.
xwl/selection.cpp
133	Is compiler happy about this?
235	Can't we do qobject_cast<Client *>(workspace()->activeClient()) == nullptr?
xwl/selection.h
81	Why explicit?

zzag added inline comments.Oct 22 2018, 6:59 PM

xwl/selection_source.h
34–35	Please follow coding style.
53	The filename is selection_source.h, so it makes sense to use the full name (SelectionSource).
136	Could you please explain why you use QVector<QString> instead of QStringList?

Rebase on master.

Harbormaster completed remote builds in B6550: Diff 48520.Jan 2 2019, 11:34 AM

Handle review comments

Harbormaster completed remote builds in B6552: Diff 48522.Jan 2 2019, 11:59 AM

romangg added inline comments.Jan 2 2019, 11:59 AM

xwl/databridge.cpp
96–102	See D15627
xwl/selection.cpp
133	Spills out warnings. It's similar to how Weston and wlroots does it though, so I don't want to increase difference to them here in this first version. Let's revisit it later though. Or do you have directly a simple solution for it?
235	Have to include client.h for that. But let's do it.
xwl/selection_source.h
136	In general if possible QVector should be preferred over QList according to Qt doc.

zzag added inline comments.Jan 8 2019, 12:27 PM

xwl/clipboard.cpp
99	Please use single line comments.
xwl/clipboard.h
25–30	Please follow coding style.
36–40	/**
44	Why explicit?
xwl/databridge.cpp
96–102	What exactly is the issue?
xwl/databridge.h
58	Please follow coding style.
60	Missing explicit?
xwl/selection.cpp
40	Please compare against QLatin1String
99	Please use a single line comment.
131	Please don't use c-style casts.
133	Or do you have directly a simple solution for it? http://doc.qt.io/qt-5/qtglobal.html#Q_FALLTHROUGH
309–311	Either add a TODO comment or delete it.
xwl/selection_source.h
120	/**
136	No short names

romangg edited the summary of this revision. (Show Details)Feb 19 2019, 8:21 AM

romangg edited the summary of this revision. (Show Details)

This revision was not accepted when it landed; it landed in state Needs Review.Feb 19 2019, 11:24 AM

Closed by commit R108:6e08fb2fa5f6: [xwl] Generic X selections translation mechanism with Clipboard support (authored by romangg). · Explain Why

This revision was automatically updated to reflect the committed changes.

romangg marked 16 inline comments as done.

		Path
M		CMakeLists.txt (24 lines)
M		atoms.h (8 lines)
M		atoms.cpp (8 lines)
M		autotests/integration/CMakeLists.txt (2 lines)
M		autotests/integration/kwin_wayland_test.cpp (4 lines)
M		autotests/integration/xclipboardsync_test.cpp (18 lines)
M		main_wayland.cpp (4 lines)
M		wayland_server.h (15 lines)
M		wayland_server.cpp (23 lines)
P		xwl/clipboard.h (51 lines) Copied from xwl/xwayland.h
A	M	xwl/clipboard.cpp (180 lines)
A	M	xwl/databridge.h (86 lines)
A	M	xwl/databridge.cpp (106 lines)
A	M	xwl/selection.h (138 lines)
A	M	xwl/selection.cpp (354 lines)
A	M	xwl/selection_source.h (151 lines)
A	M	xwl/selection_source.cpp (314 lines)
A	M	xwl/transfer.h (198 lines)
A	M	xwl/transfer.cpp (506 lines)
P		xwl/xwayland.h (17 lines) Copied to xwl/clipboard.h
M		xwl/xwayland.cpp (29 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	40377	179061d		Aug 24 2018, 7:34 PM	★	★
Diff 2	40510	179061d	- Ignore Xfixes notify when we disown a selection	Aug 27 2018, 1:29 PM	★	★
Diff 3	41388	28435b6	Rebase on master.	Sep 11 2018, 10:48 AM	★	★
Diff 4	42010	52a844a	Rebase on master.	Sep 20 2018, 9:10 PM	★	★
Diff 5	48520	db50e61	Rebase on master.	Jan 2 2019, 11:34 AM	★	★
Diff 6	48522	db50e61	- Handle review comments	Jan 2 2019, 11:59 AM	★	★
Diff 7	52052	608a89a	R108:6e08fb2fa5f6f2d03d9ad5ef74519295357c6ba2	Feb 19 2019, 11:24 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
58ab43b4ecc2	05a05894237a	3d2ea1c68c0a	Roman Gilg	Ignore Xfixes notify when we disown a selection	Aug 27 2018, 1:23 PM
3d2ea1c68c0a	cd0f9a791477	9fb78a1a337d	Roman Gilg	[xwl] Generic X selections translation mechanism with Clipboard support (Show More…)	Aug 24 2018, 7:23 PM
9fb78a1a337d	7577aed58867	b3ea819e0f3d	Roman Gilg	Omit setting file descriptor flags	Aug 24 2018, 4:54 PM
b3ea819e0f3d	9348ef44edf9	c5b702b62179	Roman Gilg	Socket notifier in Transfer and timeouts	Aug 24 2018, 4:14 PM
c5b702b62179	b2e982eb4f20	d564f6eff82b	Roman Gilg	Reuse source files in cmake	Aug 24 2018, 1:54 PM
d564f6eff82b	5371015b80bf	a7dcf9600af9	Roman Gilg	Comments and cleanup	Aug 24 2018, 11:29 AM
a7dcf9600af9	3febe600db62	62822a043872	Roman Gilg	Close fd only once in transfers	Aug 24 2018, 10:57 AM
62822a043872	2854adc852a1	731856e0ae6f	Roman Gilg	Simplify fd in Transfer classes	Aug 24 2018, 10:46 AM
731856e0ae6f	27f6e0ef3228	9cc1a4041f91	Roman Gilg	Clean up includes	Aug 23 2018, 10:32 PM
9cc1a4041f91	9e6f65e97530	0f41f370226b	Roman Gilg	Correct target type	Aug 23 2018, 10:10 PM
0f41f370226b	5c7f3a86a705	076b09133b9e	Roman Gilg	Remove unneeded properties in transfers	Aug 23 2018, 10:08 PM
076b09133b9e	e13f6c91fba4	1cbdde922529	Roman Gilg	Remove mime property for X to Wl transfer	Aug 23 2018, 10:00 PM
1cbdde922529	b92f3c6ae961	1e8a7fd5aa4a	Roman Gilg	Simplify trying to send to X	Aug 23 2018, 9:54 PM
1e8a7fd5aa4a	a64dc09f9faa	75d172b22f72	Roman Gilg	Separate window per X to Wl transfer	Aug 23 2018, 6:49 PM
75d172b22f72	5eeabb78afa1	cbc71486c09e	Roman Gilg	Start Wl to X transfers directly	Aug 23 2018, 5:53 PM
cbc71486c09e	bc79dd5fba71	99265aa31e52	Roman Gilg	No reset on wrong targets, always delete X source on owner change	Aug 23 2018, 10:34 AM
99265aa31e52	2c5291c66985	10277b6210ee	Roman Gilg	X source as parent of Kwayland DataSource	Aug 23 2018, 8:23 AM
10277b6210ee	ba0e2119a1b9	20b02b96ea94	Roman Gilg	Cleanup unused function	Aug 22 2018, 1:01 PM
20b02b96ea94	41ebd0260757	94d6a2f655fd	Roman Gilg	Include missing client headers	Aug 22 2018, 12:03 PM
94d6a2f655fd	fce031306424	fb589428c61c	Roman Gilg	Set wl source on active client change	Aug 22 2018, 11:51 AM
fb589428c61c	8783e9e2967f	2cb8860dd2ed	Roman Gilg	Adapt clipboard sync test	Aug 22 2018, 10:13 AM
2cb8860dd2ed	3c2a85fd4965	4170adeee98c	Roman Gilg	Fix seg faults	Aug 22 2018, 10:05 AM
4170adeee98c	2913fb25ba48	ee2b1d592d64	Roman Gilg	Fix test build	Aug 22 2018, 12:32 AM
ee2b1d592d64	0a7353be686e	8f229bd5a0f1	Roman Gilg	Disable xclipboard sync test	Aug 21 2018, 10:12 PM
8f229bd5a0f1	136ad1128f5b	28435b63c9fa	Roman Gilg	DataBridge, Selection, Clipboard, Transfer	Aug 21 2018, 8:06 PM

[xwl] Generic X selections translation mechanism with Clipboard support
ClosedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 41388

CMakeLists.txt

atoms.h

atoms.cpp

autotests/integration/CMakeLists.txt

autotests/integration/kwin_wayland_test.cpp

autotests/integration/xclipboardsync_test.cpp

main_wayland.cpp

wayland_server.h

wayland_server.cpp

xwl/clipboard.h

xwl/clipboard.cpp

xwl/databridge.h

xwl/databridge.cpp

xwl/selection.h

xwl/selection.cpp

xwl/selection_source.h

xwl/selection_source.cpp

xwl/transfer.h

xwl/transfer.cpp

xwl/xwayland.h

xwl/xwayland.cpp

[xwl] Generic X selections translation mechanism with Clipboard supportClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 41388

CMakeLists.txt

atoms.h

atoms.cpp

autotests/integration/CMakeLists.txt

autotests/integration/kwin_wayland_test.cpp

autotests/integration/xclipboardsync_test.cpp

main_wayland.cpp

wayland_server.h

wayland_server.cpp

xwl/clipboard.h

xwl/clipboard.cpp

xwl/databridge.h

xwl/databridge.cpp

xwl/selection.h

xwl/selection.cpp

xwl/selection_source.h

xwl/selection_source.cpp

xwl/transfer.h

xwl/transfer.cpp

xwl/xwayland.h

xwl/xwayland.cpp

[xwl] Generic X selections translation mechanism with Clipboard support
ClosedPublic
Actions

Revision Contents
Changeset List