[wayland] Use the new plasma virtual desktop protocol
ClosedPublic

Authored by mart on Jul 4 2018, 4:03 PM.

Details

Summary

implement virtual desktop support for Wayland.
use the new virtual desktop protocol from D12820
The VirtualDesktopManager class needed some big change in order
to accomodate it, which is where most changes are.
Other than that, it's mostly connections to wire up
VirtualDesktopsManager and VirtualDesktopsManagement(the wayland protocol impl)

Depends on D12820
Other notable detail, is the client visibility updated to reflect the presence
of the client in the plasmavirtualdesktop.
(and the unSetDesktop concept)

Test Plan

used a bit a plasma session together with D12820, D13748 and D13746

Diff Detail

Repository
R108 KWin
Branch
mart/plasmavirtualdesktop
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1093
Build 1106: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
mart updated this revision to Diff 38121.Jul 20 2018, 8:46 AM
mart marked an inline comment as done.
  • use Application::OperationMode in a couple of places
  • get rid of desktopNumberFromId
  • get rid of new virtuals
  • get rid of m_desktop, use m_desktops for desktop()
  • setActiveDesktop has been removed
  • dragging windows between desktops works again
  • crash--
mart added a comment.Jul 20 2018, 9:08 AM

now also desktop grid drag and drop interaction works well.
One thing i would change is that holding ctrl used to move all the windows to a new desktop, but i would use ctrl instead to "copy" a window from a desktop to another, defaulting to a move to keep the behavior on x11. so moving all the windows may be done with shift or alt.

There is one bug remaining:
On wayland, the window being dragged disappears.
what i noted, is that the window gets drawn at desktopgrid.cpp line 210
if i remove line 208, where its WindowPaintData gets scaled, the window gets drawn correctly (unscaled) but if i scale it < 0.9 something in the lanczosfilter goes wrong (if i disable the filter completely it gets drawn correctly as well)

zzag added inline comments.Jul 20 2018, 11:09 AM
abstract_client.cpp
506

Use static_cast. :-)

587–592
const auto desks = desktops(); // is desks a good name?
QList<int> x11Ids;
x11Ids.reserve(desks.count());
std::transform(desks.constBegin(), desks.constEnd(),
    std::back_inserter(x11Ids),
    [](const VirtualDesktop *vd) {
        return vd->x11DesktopNumber();
    });
return x11Ids;
abstract_client.h
1100

Coding style nitpick:

QList<VirtualDesktop*> m_desktops;
activation.cpp
416 ↗(On Diff #38121)

Delete it?

autotests/test_window_paint_data.cpp
76
return {};
deleted.cpp
146
return {};
libkwineffects/kwineffects.h
1997–2014

It would be great to add a comment describing what's the difference between desktop and desktops. Maybe, deprecate desktop?

virtualdesktops.cpp
73

Unrelated change.

331–333

Would be great to reformat it.

336

IMHO, one shall not put else after return.

349

I suggest to put * and not deduce raw pointer types because

(a) it improves readability
(b) in some cases the type of a variable is not what you really expected, e.g.

int x = 42;

const auto foo = &x;
*foo = 3; // totally fine, foo is of type int *const

const auto *bar = &x;
*bar = 3; // compilation error, bar is of type const int *

Still, it would be great to get ack on this topic from Martin. :-)

virtualdesktops.h
362–374

Would be great to follow "Signal emitted ..." convention.

363

Typo: crated -> created.

370

Typo: to stil la valid. I think what you've meant was "It's guaranteed to be a valid ..."

mart updated this revision to Diff 38132.Jul 20 2018, 11:29 AM
  • assert--
  • better dnd visual feedback
mart updated this revision to Diff 38261.Jul 23 2018, 2:36 PM
mart marked an inline comment as done.
  • adress some comments
mart added inline comments.Jul 25 2018, 8:42 AM
abstract_client.h
1100

even for types inside templates, i've always assumed the normal variable declaration of ldelibs style (which kwin uses, tough still not everywhere as it was converted from a radically different style in the past)
https://community.kde.org/Policies/Kdelibs_Coding_Style#Variable_declaration

zzag added inline comments.Jul 25 2018, 9:22 AM
abstract_client.h
1100

Yeah, you're right about whitespace before '*'.

Looks like Qt does the same.

mart updated this revision to Diff 38431.Jul 25 2018, 3:04 PM
  • first rough non working prototype of a DBus interface
  • almost working dbus interface
  • move waypand protocol logic in VirtualDesktopManager
  • expose rows to the dbus interface
mart added a comment.Jul 25 2018, 3:11 PM

added a DBus interface to manage virtual desktops, the functionality of the kcm should be covered
(the kcm could be ported to this for both the x11 and wayland cases)
Still no support for orientation in wayland case.
not sure if the number of rows should be in the wayland protocol & having orientation as well.
The problem with orientation (even with current x11 implementation) is that is view-specific, so if you have 2 pagers, one in an horizontal panel and one in a vertical one, they will override the global orientation to each other, but one of the two should "win" and force the desktopgrid and other effects to conform to that layout.

hein added a subscriber: hein.Jul 26 2018, 9:06 AM

I've started work on a clean QML reimplementation of the Virtual Desktops KCM using the new DBus API.

mart updated this revision to Diff 38496.Jul 26 2018, 10:55 AM
  • new dbus protocol which maps more closely the wayland one
hein added a comment.Jul 31 2018, 5:15 PM

I've hit a wall trying to make the KCM work. I can't figure out how to make QDBusInterface connect to a signal that's using the struct as an argument. Qt can't figure out how to destructure it back into Qt types, falling back to QDBusRawType (which even crashes qDebug):

kcmshell5(12959)/default unknown: "desktopCreated(QString,QDBusRawType<0x2869737329>*)"

I'm not a fan of the struct for a few reasons:

  • It means the id is unnecessarily duplicated.
  • The "x11DesktopNumber" is weird. That sounds legacy, but I need an index in desktopCreated to map to QAIM API without a roundtrip to desktops() anyway.

Destructuring this stuff in the property getters is also rather maddening BTW. QDBusInterface::property() can't deal with it (David worked around the same upstream bug in 6f1c6dc84a3d in solid.git), so I had to resort to writing async method calls using org.kde.freedesktop.Properties.Get. That returns a terrible nested mess of QVariant and QDBusVariant and QDBusArgument etc. and while it's possible to do a lot of ugly legwork then, I haven't been able to shortcut it using qdbus_cast yet unfortunately.

The KCM code is otherwise more or less ready now, but this makes it ... well, not work due to missing data :D

hein added a comment.EditedJul 31 2018, 5:26 PM

Kai pointed me at plasma-workspace/gmenu-dbusmenu-proxy/gdbusmenutypes_p.h, it seems I really need to clone all this marshalling stuff on the client side too - I guess QtDbus ultimately just uses fdo.Introspect to provide a type name annotation and then does a lookup in the meta type registry on the other side, instead of really providing convenience stuff to work with DBus types ...

Still not a fan of the struct for the other reason though as mentioned :) Anyway, I will use it for now tomorrow.

hein added a comment.Aug 1 2018, 7:00 PM

KCM PoC is up. Please see review comments about the protocol here: D14542

mart added a comment.Aug 2 2018, 10:12 AM
In D13887#301460, @hein wrote:

Kai pointed me at plasma-workspace/gmenu-dbusmenu-proxy/gdbusmenutypes_p.h, it seems I really need to clone all this marshalling stuff on the client side too - I guess QtDbus ultimately just uses fdo.Introspect to provide a type name annotation and then does a lookup in the meta type registry on the other side, instead of really providing convenience stuff to work with DBus types ...

Still not a fan of the struct for the other reason though as mentioned :) Anyway, I will use it for now tomorrow.

yes, the marshaling stuff needs to be in both the client and the server poart.
in theory, properties should also be got with getall (an an async way)
the thing is done in a way do absolutely minimize roundtrips, which is the problem of dbus, so they are structs in order to avoid to have many getters to get the whole info for a single desktop, which would be difficult t oreliably reconstruct if it's called in an async way (and sync calls should be avoided at any cost, always)
so the concept is to ask async getall, then build all you data from what returns, then only use connections to signals to keep in sync from now on

davidedmundson requested changes to this revision.Aug 7 2018, 8:59 PM
davidedmundson added inline comments.
abstract_client.cpp
998

This seems wrong.

shellclient has already just emitted windowShown regardless before setting up the windowManagement iface.

X clients still have doSetDesktop(int,int) called much earlier which in turn calls updateVisibility and sets these.

This revision now requires changes to proceed.Aug 7 2018, 8:59 PM
mart updated this revision to Diff 40364.Aug 24 2018, 1:19 PM
  • Ctrl to "copy" windows
  • fix virtual_desktop_test
  • remove probably useless code
  • more tests
mart marked an inline comment as done.Aug 27 2018, 12:21 PM
davidedmundson added inline comments.Aug 30 2018, 8:15 PM
abstract_client.cpp
499

check it for what?

dbusinterface.cpp
378 ↗(On Diff #40364)

Why don't we have the adaptor directly on the VirtualDesktopManager?

The idea behind QDBusAbstractAdaptor is that they proxy all the signals/properties/slots from itself into the passed argument. We've come up with a manual layer of indirection which is just duplicating what VirtualDesktopManagerAdaptor does automatically.

If you do need to do mods or extra connects, you can subclass the VirtualDesktopManagerAdaptor rather than proxying.

453 ↗(On Diff #40364)

Do we want to expose setCount?

We have createDesktop/removeDesktop with IDs and names.
Having a version without that opens up a whole world of problems of desktops without IDs being out of sync.

Same for enterNewPlasmaVirtualDesktopRequested

virtualdesktops.cpp
560–564

Eww, definitely not.

What I assumed we would do would be have kwin start up with 1VD. (ksplash is on all anyway)
Then kactivitymanagerd (or even kcminit for the current mode) set everything on startup

in positive comments: I am now happy with all the libkwineffects changes

mart updated this revision to Diff 40746.Aug 31 2018, 10:31 AM
mart marked 2 inline comments as done.
  • don't expose setCount on dbus
mart added inline comments.Aug 31 2018, 10:32 AM
abstract_client.cpp
499

ouch, old comments before operationMode was used to check

dbusinterface.cpp
378 ↗(On Diff #40364)

the problem would be the custom types DBusDesktopDataStruct, DBusDesktopDataVector, would them be defined in the VirtualDesktopManagerAdaptor subclass?

mart updated this revision to Diff 42672.Oct 1 2018, 3:28 PM
  • take into account NET::OnAllDesktops
mart updated this revision to Diff 42842.Oct 4 2018, 11:25 AM
  • replace x11desktopnumber with position
mart updated this revision to Diff 42863.Oct 4 2018, 3:51 PM
  • createdesktop is 0 indexed as well
mart updated this revision to Diff 42922.Oct 5 2018, 2:11 PM
  • move the virtual desktop dbus types in own header/implementation
davidedmundson added inline comments.Oct 8 2018, 8:48 AM
deleted.cpp
146

Why aren't we copying and returning the full list?

If a window is on desktop 1 and 2 and we close the window we still want it to animate away on whichever desktop the user is on.

See void Deleted::copyToDeleted(Toplevel* c)

mart updated this revision to Diff 43404.Oct 11 2018, 4:28 PM
  • copy desktops over Deleted
mart marked an inline comment as done.Oct 11 2018, 4:29 PM

One minor comment left, then I think we should ship this.

abstract_client.cpp
562

set Desktop had a range check so there was a guard.

This has not guard that desktop is in bounds. Given it's invoked from effects, we should.

569

The add code has

if (m_desktops.isEmpty()) {
windowManagementInterface()->setOnAllDesktops(true);
}

this does not.

Either the windowManagementInterface should be responsible for it in both add and remove, or kwin should in both add and remove.

mart updated this revision to Diff 44469.Oct 29 2018, 10:38 PM
  • guard desktop number in unsetDesktop
  • Set initial value
  • Documentation
  • Fix first unit test
  • Fix assert in loading
  • Fix assert ID comparison
  • Don't update current desktop in setCount if desktop is undefined (Xid=0)
davidedmundson accepted this revision.Oct 30 2018, 12:32 PM
zzag retitled this revision from use the new plasma virtual desktop protocol to [wayland] Use the new plasma virtual desktop protocol.Oct 30 2018, 1:33 PM
This revision was not accepted when it landed; it landed in state Needs Review.Nov 1 2018, 3:35 PM
This revision was automatically updated to reflect the committed changes.

This caused a regression in the auto tests:

14353==ERROR: AddressSanitizer: heap-use-after-free on address 0x6070000ec588 at pc 0x7f82083b22f3 bp 0x7ffc5f33e060 sp 0x7ffc5f33e058

READ of size 8 at 0x6070000ec588 thread T0

#0 0x7f82083b22f2 in QString::QString(QString const&) /usr/include/qt5/QtCore/qstring.h:952
#1 0x7f82084dc781 in KWayland::Server::PlasmaVirtualDesktopInterface::id() const /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.11/kwayland/src/server/plasmavirtualdesktop_interface.cpp:350
#2 0x7f82084deafd in KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}::operator()(KWayland::Server::PlasmaVirtualDesktopInterface const*) const /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.11/kwayland/src/server/plasmavirtualdesktop_interface.cpp:115
#3 0x7f82084e9688 in bool __gnu_cxx::__ops::_Iter_pred<KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}>::operator()<QList<KWayland::Server::PlasmaVirtualDesktopInterface*>::iterator>(QList<KWayland::Server::PlasmaVirtualDesktopInterface*>::iterator) (/home/jenkins/install-prefix/lib64/libKF5WaylandServer.so.5+0x529688)
#4 0x7f82084e7d4d in QList<KWayland::Server::PlasmaVirtualDesktopInterface*>::iterator std::__find_if<QList<KWayland::Server::PlasmaVirtualDesktopInterface*>::iterator, __gnu_cxx::__ops::_Iter_pred<KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}> >(__gnu_cxx::__ops::_Iter_pred<KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}>, __gnu_cxx::__ops::_Iter_pred<KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}>, __gnu_cxx::__ops::_Iter_pred<KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}>, std::random_access_iterator_tag) /usr/include/c++/8/bits/stl_algo.h:144
#5 0x7f82084e38fe in QList<KWayland::Server::PlasmaVirtualDesktopInterface*>::iterator std::__find_if<QList<KWayland::Server::PlasmaVirtualDesktopInterface*>::iterator, __gnu_cxx::__ops::_Iter_pred<KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}> >(__gnu_cxx::__ops::_Iter_pred<KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}>, __gnu_cxx::__ops::_Iter_pred<KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}>, __gnu_cxx::__ops::_Iter_pred<KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}>) /usr/include/c++/8/bits/stl_algo.h:162
#6 0x7f82084df96a in QList<KWayland::Server::PlasmaVirtualDesktopInterface*>::iterator std::find_if<QList<KWayland::Server::PlasmaVirtualDesktopInterface*>::iterator, KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}>(KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}, KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}, KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&)::{lambda(KWayland::Server::PlasmaVirtualDesktopInterface const*)#1}) (/home/jenkins/install-prefix/lib64/libKF5WaylandServer.so.5+0x51f96a)
#7 0x7f82084dee63 in KWayland::Server::PlasmaVirtualDesktopManagementInterface::Private::findDesktop(QString const&) /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.11/kwayland/src/server/plasmavirtualdesktop_interface.cpp:115
#8 0x7f82084d8a27 in operator() /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.11/kwayland/src/server/plasmavirtualdesktop_interface.cpp:226
#9 0x7f82084de278 in call /usr/include/qt5/QtCore/qobjectdefs_impl.h:128
#10 0x7f82084de19f in call<QtPrivate::List<>, void> /usr/include/qt5/QtCore/qobjectdefs_impl.h:238
#11 0x7f82084de14b in impl /usr/include/qt5/QtCore/qobjectdefs_impl.h:421
#12 0x7f8205bc719f in QMetaObject::activate(QObject*, int, int, void**) (/usr/lib64/libQt5Core.so.5+0x2a019f)
#13 0x7f8205bc770e in QObject::destroyed(QObject*) (/usr/lib64/libQt5Core.so.5+0x2a070e)
#14 0x7f8205bcdfde in QObject::~QObject() (/usr/lib64/libQt5Core.so.5+0x2a6fde)
#15 0x7f82084dc6b6 in KWayland::Server::PlasmaVirtualDesktopInterface::~PlasmaVirtualDesktopInterface() /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.11/kwayland/src/server/plasmavirtualdesktop_interface.cpp:345
#16 0x7f82084dc6f5 in KWayland::Server::PlasmaVirtualDesktopInterface::~PlasmaVirtualDesktopInterface() /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.11/kwayland/src/server/plasmavirtualdesktop_interface.cpp:346
#17 0x7f8205bc791f in QObject::event(QEvent*) (/usr/lib64/libQt5Core.so.5+0x2a091f)
#18 0x7f82068644a0 in QApplicationPrivate::notify_helper(QObject*, QEvent*) (/usr/lib64/libQt5Widgets.so.5+0x1774a0)
#19 0x7f820686baef in QApplication::notify(QObject*, QEvent*) (/usr/lib64/libQt5Widgets.so.5+0x17eaef)
#20 0x7f8205b9de08 in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/usr/lib64/libQt5Core.so.5+0x276e08)
#21 0x7f8205ba0dfa in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/usr/lib64/libQt5Core.so.5+0x279dfa)
#22 0x7f8205becdaa in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (/usr/lib64/libQt5Core.so.5+0x2c5daa)
#23 0x7f81f7a1725c in QUnixEventDispatcherQPA::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) /home/abuild/rpmbuild/BUILD/qtbase-everywhere-src-5.11.2/src/platformsupport/eventdispatchers/qunixeventdispatcher.cpp:68
#24 0x7f8205b9cada in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (/usr/lib64/libQt5Core.so.5+0x275ada)
#25 0x484d72 in QTestEventLoop::enterLoopMSecs(int) (/home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/build/bin/testKeyboardLayout+0x484d72)
#26 0x48592e in QSignalSpy::wait(int) (/home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/build/bin/testKeyboardLayout+0x48592e)
#27 0x4c2700 in KWin::Test::destroyWaylandConnection() /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/autotests/integration/test_helpers.cpp:265
#28 0x41ac8e in KeyboardLayoutTest::cleanup() /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/autotests/integration/keyboard_layout_test.cpp:101
#29 0x47e6c4 in KeyboardLayoutTest::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/build/autotests/integration/testKeyboardLayout_autogen/include/keyboard_layout_test.moc:107
#30 0x7f8205bac6a4 in QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (/usr/lib64/libQt5Core.so.5+0x2856a4)
#31 0x7f82136a775c  (/usr/lib64/libQt5Test.so.5+0x1975c)
#32 0x7f82136a825a  (/usr/lib64/libQt5Test.so.5+0x1a25a)
#33 0x7f82136a8820  (/usr/lib64/libQt5Test.so.5+0x1a820)
#34 0x7f82136a8c2a in QTest::qRun() (/usr/lib64/libQt5Test.so.5+0x1ac2a)
#35 0x7f82136a8e3a in QTest::qExec(QObject*, int, char**) (/usr/lib64/libQt5Test.so.5+0x1ae3a)
#36 0x47e3fd in main /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/autotests/integration/keyboard_layout_test.cpp:503
#37 0x7f8205050fea in __libc_start_main (/lib64/libc.so.6+0x22fea)
#38 0x40f9d9 in _start (/home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/build/bin/testKeyboardLayout+0x40f9d9)

0x6070000ec588 is located 24 bytes inside of 72-byte region [0x6070000ec570,0x6070000ec5b8)
freed by thread T0 here:

#0 0x7f8212808670 in operator delete(void*) (/usr/lib64/libasan.so.5+0xee670)
#1 0x7f82084e676a in QScopedPointerDeleter<KWayland::Server::PlasmaVirtualDesktopInterface::Private>::cleanup(KWayland::Server::PlasmaVirtualDesktopInterface::Private*) /usr/include/qt5/QtCore/qscopedpointer.h:60
#2 0x7f82084e2dd5 in QScopedPointer<KWayland::Server::PlasmaVirtualDesktopInterface::Private, QScopedPointerDeleter<KWayland::Server::PlasmaVirtualDesktopInterface::Private> >::~QScopedPointer() (/home/jenkins/install-prefix/lib64/libKF5WaylandServer.so.5+0x522dd5)
#3 0x7f82084dc698 in KWayland::Server::PlasmaVirtualDesktopInterface::~PlasmaVirtualDesktopInterface() /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.11/kwayland/src/server/plasmavirtualdesktop_interface.cpp:345
#4 0x7f82084dc6f5 in KWayland::Server::PlasmaVirtualDesktopInterface::~PlasmaVirtualDesktopInterface() /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.11/kwayland/src/server/plasmavirtualdesktop_interface.cpp:346
#5 0x7f8205bc791f in QObject::event(QEvent*) (/usr/lib64/libQt5Core.so.5+0x2a091f)
#6 0x7f82068644a0 in QApplicationPrivate::notify_helper(QObject*, QEvent*) (/usr/lib64/libQt5Widgets.so.5+0x1774a0)

previously allocated by thread T0 here:

#0 0x7f8212807900 in operator new(unsigned long) (/usr/lib64/libasan.so.5+0xed900)
#1 0x7f82084dc5d6 in KWayland::Server::PlasmaVirtualDesktopInterface::PlasmaVirtualDesktopInterface(KWayland::Server::PlasmaVirtualDesktopManagementInterface*) /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.11/kwayland/src/server/plasmavirtualdesktop_interface.cpp:341
#2 0x7f82084d96f6 in KWayland::Server::PlasmaVirtualDesktopManagementInterface::createDesktop(QString const&, unsigned int) /home/jenkins/workspace/Administration/Dependency Build Plasma kf5-qt5 SUSEQt5.11/kwayland/src/server/plasmavirtualdesktop_interface.cpp:210
#3 0x7f8210cb2d71 in KWin::VirtualDesktopManager::setVirtualDesktopManagement(KWayland::Server::PlasmaVirtualDesktopManagementInterface*) /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/virtualdesktops.cpp:98
#4 0x7f8210eb246c in KWin::WaylandServer::initWorkspace() /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/wayland_server.cpp:400
#5 0x41a909 in KeyboardLayoutTest::initTestCase() /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/autotests/integration/keyboard_layout_test.cpp:91
#6 0x47e65a in KeyboardLayoutTest::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) /home/jenkins/workspace/Plasma/kwin/kf5-qt5 SUSEQt5.11/build/autotests/integration/testKeyboardLayout_autogen/include/keyboard_layout_test.moc:105
#7 0x7f8205bac6a4 in QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (/usr/lib64/libQt5Core.so.5+0x2856a4)
#8 0x7f82136a8740  (/usr/lib64/libQt5Test.so.5+0x1a740)
#9 0x7f82136a8c2a in QTest::qRun() (/usr/lib64/libQt5Test.so.5+0x1ac2a)
#10 0x7f8205050fea in __libc_start_main (/lib64/libc.so.6+0x22fea)

SUMMARY: AddressSanitizer: heap-use-after-free /usr/include/qt5/QtCore/qstring.h:952 in QString::QString(QString const&)
Shadow bytes around the buggy address:

0x0c0e80015860: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
0x0c0e80015870: 00 fa fa fa fa fa 00 00 00 00 00 00 00 00 00 fa
0x0c0e80015880: fa fa fa fa 00 00 00 00 00 00 00 00 00 fa fa fa
0x0c0e80015890: fa fa 00 00 00 00 00 00 00 00 00 fa fa fa fa fa
0x0c0e800158a0: 00 00 00 00 00 00 00 00 00 fa fa fa fa fa fd fd

>0x0c0e800158b0: fd[fd]fd fd fd fd fd fa fa fa fa fa 00 00 00 00

0x0c0e800158c0: 00 00 00 00 00 fa fa fa fa fa 00 00 00 00 00 00
0x0c0e800158d0: 00 00 00 fa fa fa fa fa 00 00 00 00 00 00 00 00
0x0c0e800158e0: 00 fa fa fa fa fa 00 00 00 00 00 00 00 00 00 fa
0x0c0e800158f0: fa fa fa fa 00 00 00 00 00 00 00 00 01 fa fa fa
0x0c0e80015900: fa fa 00 00 00 00 00 00 00 00 00 07 fa fa fa fa

Shadow byte legend (one shadow byte represents 8 application bytes):

Addressable:           00
Partially addressable: 01 02 03 04 05 06 07 
Heap left redzone:       fa
Freed heap region:       fd
Stack left redzone:      f1
Stack mid redzone:       f2
Stack right redzone:     f3
Stack after return:      f5
Stack use after scope:   f8
Global redzone:          f9
Global init order:       f6
Poisoned by user:        f7
Container overflow:      fc
Array cookie:            ac
Intra object redzone:    bb
ASan internal:           fe
Left alloca redzone:     ca
Right alloca redzone:    cb

14353==ABORTING

Also testKwinBindings started to regress with this change. Guys this has been in review for about half a year and it broke several unit tests? Seriously?

testVirtualDesktop is also broken since the change

The ASAN issue I have a patch on review.

testBindings poses an interesting question:

  • We have a window on desktop1
  • We press the key key to "window to desktop 5"
  • Should it then be on desktop [1,5] or just 5?

The ASAN issue I have a patch on review.

testBindings poses an interesting question:

  • We have a window on desktop1
  • We press the key key to "window to desktop 5"
  • Should it then be on desktop [1,5] or just 5?

Just 5. That's how the shortcut works on X11. If we want something different we need a new shortcut.