D5747: add pid to plasma window management protocol
sebas closed this revision. sebas added a comment. Issue has been fixed elsewhere, otherwise code is submitted. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, mart, hein, lukas
D5747: add pid to plasma window management protocol
hein added a comment. FWIW I did run make test on an earlier revision of the patch during the sprint while doing work on top of it; it must have regressed after. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, hein, lukas
D5747: add pid to plasma window management protocol
graesslin reopened this revision. graesslin added a comment. This revision is now accepted and ready to land. Can reproduce the failing test locally. Please fix ASAP! Broken tests is not acceptable in KWayland. This change has been on review so long, I find it quite disappointing that it was pushed without checking whether the tests work. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, hein, lukas
D5747: add pid to plasma window management protocol
hein added a comment. testPid is failing: https://build.kde.org/job/kwayland%20master%20kf5-qt5/204/PLATFORM=Linux,compiler=gcc/testReport/junit/(root)/TestSuite/kwayland_testPlasmaWindowModel/ REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, hein, lukas
D5747: add pid to plasma window management protocol
This revision was automatically updated to reflect the committed changes. Closed by commit R127:0b4d8a7fc1df: add pid to plasma window management protocol (authored by sebas). REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5747?vs=14442&id=14463 REVISION DETAIL https://phabricator.kde.org/D5747 AFFECTED FILES autotests/client/test_plasma_window_model.cpp autotests/client/test_wayland_windowmanagement.cpp src/client/plasmawindowmanagement.cpp src/client/plasmawindowmanagement.h src/client/plasmawindowmodel.cpp src/client/plasmawindowmodel.h src/client/protocols/plasma-window-management.xml src/server/plasmawindowmanagement_interface.cpp src/server/plasmawindowmanagement_interface.h To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, hein, lukas
D5747: add pid to plasma window management protocol
graesslin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R127 KWayland BRANCH sebas/processid REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, hein, lukas
D5747: add pid to plasma window management protocol
sebas updated this revision to Diff 14442. sebas added a comment. Restricted Application edited projects, added Plasma on Wayland; removed Plasma. - Update docs: the pid is just set, but doesn't logically change - ws-- REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5747?vs=14418&id=14442 BRANCH sebas/processid REVISION DETAIL https://phabricator.kde.org/D5747 AFFECTED FILES autotests/client/test_plasma_window_model.cpp autotests/client/test_wayland_windowmanagement.cpp src/client/plasmawindowmanagement.cpp src/client/plasmawindowmanagement.h src/client/plasmawindowmodel.cpp src/client/plasmawindowmodel.h src/client/protocols/plasma-window-management.xml src/server/plasmawindowmanagement_interface.cpp src/server/plasmawindowmanagement_interface.h To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, hein, lukas
D5747: add pid to plasma window management protocol
graesslin requested changes to this revision. graesslin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > plasmawindowmodel.cpp:84 > > + > QObject::connect(window, &PlasmaWindow::activeChanged, q, Unrelated newline > plasma-window-management.xml:263 > + > +This event will be sent when the process id of the application > owning the window has changed. > +The pid will initially be 0, meaning it hasn't been set. As the > compositor sets it, the client The pid will never change. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, lukas
D5747: add pid to plasma window management protocol
hein accepted this revision. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, lukas
D5747: add pid to plasma window management protocol
graesslin added a comment. In https://phabricator.kde.org/D5747#108916, @apol wrote: > Who is going to be setting this pid? Because note that containerized applications don't know their own pid (see kdbusaddons & knotifications patches for big shrug). It's KWin. Who gets the information from the socket. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, lukas
D5747: add pid to plasma window management protocol
apol added a comment. Who is going to be setting this pid? Because note that containerized applications don't know their own pid (see kdbusaddons & knotifications patches for big shrug). $ docker run ubuntu:17.04 pidof pidof 1 $ pidof pidof 6940 REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: apol, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, lukas
D5747: add pid to plasma window management protocol
hein added a dependent revision: D5818: Lift app identification heuristic out of XWindowTasksModel and share it with WaylandTasksModel.. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas
D5747: add pid to plasma window management protocol
sebas updated this revision to Diff 14418. sebas added a comment. Restricted Application edited projects, added Plasma; removed Plasma on Wayland. - Drop pid change handling in the model code - Extend model test --Eike on sebas' box REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5747?vs=14410&id=14418 BRANCH sebas/processid REVISION DETAIL https://phabricator.kde.org/D5747 AFFECTED FILES autotests/client/test_plasma_window_model.cpp autotests/client/test_wayland_windowmanagement.cpp src/client/plasmawindowmanagement.cpp src/client/plasmawindowmanagement.h src/client/plasmawindowmodel.cpp src/client/plasmawindowmodel.h src/client/protocols/plasma-window-management.xml src/server/plasmawindowmanagement_interface.cpp src/server/plasmawindowmanagement_interface.h To: sebas, #plasma, hein, graesslin Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas
D5747: add pid to plasma window management protocol
graesslin added inline comments.
INLINE COMMENTS
> test_plasma_window_model.cpp:215
>
> void PlasmaWindowModelTest::testRoleNames_data()
> {
Please extend this
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D5747
To: sebas, #plasma, hein, graesslin
Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff,
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein,
lukas
D5747: add pid to plasma window management protocol
graesslin added inline comments.
INLINE COMMENTS
> plasmawindowmodel.cpp:84-86
> +QObject::connect(window, &PlasmaWindow::pidChanged, q,
> +[window, this] { this->dataChanged(window, PlasmaWindowModel::Pid); }
> +);
I consider it as impossible that the PID changes.
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D5747
To: sebas, #plasma, hein, graesslin
Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff,
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein,
lukas
D5747: add pid to plasma window management protocol
sebas updated this revision to Diff 14410. sebas added a comment. Restricted Application edited projects, added Plasma on Wayland; removed Plasma. - also test default REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5747?vs=14407&id=14410 BRANCH sebas/processid REVISION DETAIL https://phabricator.kde.org/D5747 AFFECTED FILES autotests/client/test_plasma_window_model.cpp autotests/client/test_wayland_windowmanagement.cpp src/client/plasmawindowmanagement.cpp src/client/plasmawindowmanagement.h src/client/plasmawindowmodel.cpp src/client/plasmawindowmodel.h src/client/protocols/plasma-window-management.xml src/server/plasmawindowmanagement_interface.cpp src/server/plasmawindowmanagement_interface.h To: sebas, #plasma, hein, graesslin Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein, lukas
D5747: add pid to plasma window management protocol
sebas updated this revision to Diff 14407. sebas marked 3 inline comments as done. sebas added a comment. Restricted Application edited projects, added Plasma; removed Plasma on Wayland. Address review comments - Minor corrections in docs - Use uint for the pid - uint32 for pids instead of int32 - Fix @since in docs - send initial value when it's not 0 - Add autotest in TestWindowManagement as well REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5747?vs=14249&id=14407 BRANCH sebas/processid REVISION DETAIL https://phabricator.kde.org/D5747 AFFECTED FILES autotests/client/test_plasma_window_model.cpp autotests/client/test_wayland_windowmanagement.cpp src/client/plasmawindowmanagement.cpp src/client/plasmawindowmanagement.h src/client/plasmawindowmodel.cpp src/client/plasmawindowmodel.h src/client/protocols/plasma-window-management.xml src/server/plasmawindowmanagement_interface.cpp src/server/plasmawindowmanagement_interface.h To: sebas, #plasma, hein, graesslin Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas
D5747: add pid to plasma window management protocol
hein added a dependent revision: D5812: Add missing block to make sure initial pid is sent. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein, lukas
D5747: add pid to plasma window management protocol
hein added a dependent revision: D5756: Set pid on the ClientConnection backing the PlasmaWindow surface.. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, lukas
D5747: add pid to plasma window management protocol
hein added a dependent revision: D5755: Expose PlasmaWindow::pid through WaylandTasksModel.. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, lukas
D5747: add pid to plasma window management protocol
graesslin requested changes to this revision. graesslin added a comment. This revision now requires changes to proceed. Please also extend the autotests/client/test_wayland_windowmanagement.cpp INLINE COMMENTS > plasmawindowmanagement.h:385 > + * @returns The process id this window belongs to. > + * @see virtualDesktopChangeableChanged > + * @since 5.35 copy and paste error > plasmawindowmanagement.h:481 > + * @see pid > + **/ > +void pidChanged(); "@since" missing REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, lukas
D5747: add pid to plasma window management protocol
davidedmundson added inline comments. INLINE COMMENTS > plasma-window-management.xml:267 > + > + > + uint is a native type: https://wayland.freedesktop.org/docs/html/ch04.html#sect-Protocol-Wire-Format you don't need to do an implicit cast REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5747 To: sebas, #plasma, hein, graesslin Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, lukas
D5747: add pid to plasma window management protocol
sebas created this revision. Restricted Application added projects: Plasma on Wayland, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY This patch adds a pid event to the plasma window management protocol. It allows the compositor to tell allow a mapping between windows and processes. Bumps the version number of the interface to 8 to indicate this. TEST PLAN autotest added, passed REPOSITORY R127 KWayland BRANCH sebas/processid REVISION DETAIL https://phabricator.kde.org/D5747 AFFECTED FILES autotests/client/test_plasma_window_model.cpp src/client/plasmawindowmanagement.cpp src/client/plasmawindowmanagement.h src/client/plasmawindowmodel.cpp src/client/plasmawindowmodel.h src/client/protocols/plasma-window-management.xml src/server/plasmawindowmanagement_interface.cpp src/server/plasmawindowmanagement_interface.h To: sebas, #plasma, hein, graesslin Cc: plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, lukas
