D6727: Destroy all kwayland objects created by registry when it is destroyed
This revision was automatically updated to reflect the committed changes. Closed by commit R127:b669d4148a39: Destroy all kwayland objects created by registry when it is destroyed (authored by davidedmundson). REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6727?vs=17272&id=17312 REVISION DETAIL https://phabricator.kde.org/D6727 AFFECTED FILES autotests/client/test_wayland_registry.cpp src/client/blur.h src/client/contrast.h src/client/datadevice.h src/client/datadevicemanager.h src/client/dataoffer.h src/client/datasource.h src/client/dpms.h src/client/event_queue.h src/client/fakeinput.h src/client/idle.h src/client/output.cpp src/client/output.h src/client/outputconfiguration.h src/client/outputdevice.cpp src/client/outputdevice.h src/client/outputmanagement.h src/client/plasmashell.h src/client/plasmawindowmanagement.h src/client/pointerconstraints.h src/client/pointergestures.h src/client/registry.cpp src/client/registry.h src/client/relativepointer.h src/client/seat.h src/client/server_decoration.h src/client/shadow.h src/client/shm_pool.h src/client/slide.h src/client/surface.h src/client/textinput.h src/client/xdgshell.h To: davidedmundson, graesslin Cc: graesslin, plasma-devel, #frameworks, leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas
D6727: Destroy all kwayland objects created by registry when it is destroyed
graesslin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R127 KWayland BRANCH davidedmundson/xdgv6 REVISION DETAIL https://phabricator.kde.org/D6727 To: davidedmundson, graesslin Cc: graesslin, plasma-devel, #frameworks, leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas
D6727: Destroy all kwayland objects created by registry when it is destroyed
davidedmundson updated this revision to Diff 17272. davidedmundson added a comment. Updated docs + unit test REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6727?vs=16758&id=17272 BRANCH davidedmundson/xdgv6 REVISION DETAIL https://phabricator.kde.org/D6727 AFFECTED FILES autotests/client/test_wayland_registry.cpp src/client/blur.h src/client/contrast.h src/client/datadevice.h src/client/datadevicemanager.h src/client/dataoffer.h src/client/datasource.h src/client/dpms.h src/client/event_queue.h src/client/fakeinput.h src/client/idle.h src/client/output.cpp src/client/output.h src/client/outputconfiguration.h src/client/outputdevice.cpp src/client/outputdevice.h src/client/outputmanagement.h src/client/plasmashell.h src/client/plasmawindowmanagement.h src/client/pointerconstraints.h src/client/pointergestures.h src/client/registry.cpp src/client/registry.h src/client/relativepointer.h src/client/seat.h src/client/server_decoration.h src/client/shadow.h src/client/shm_pool.h src/client/slide.h src/client/surface.h src/client/textinput.h src/client/xdgshell.h To: davidedmundson Cc: graesslin, plasma-devel, #frameworks, leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas
D6727: Destroy all kwayland objects created by registry when it is destroyed
graesslin added a comment. In https://phabricator.kde.org/D6727#125900, @davidedmundson wrote: > We still need https://phabricator.kde.org/D6571, with my proposed change. That's a special case where we delete the connection before the QPA. It might be that we don't need it. Also KWin destroys the QPA and maybe that's sufficient - Breeze has it's own ConnectionThread which is not shared with KWin's. > My added connect means that we get that signal regardless of whether we're in Kwin or not. > > What we wouldn't need is the ownership changes you did for Breeze (and would otherwise still need doing in Oxygen/QtCurve ) > >> May I ask for a small unit test for the new functionality? > > For you, a *big* unit test. Just extend the existing ones - that should do and is hopefully small ;-) REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D6727 To: davidedmundson Cc: graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas
D6727: Destroy all kwayland objects created by registry when it is destroyed
davidedmundson added a comment. We still need https://phabricator.kde.org/D6571, with my proposed change. That's a special case where we delete the connection before the QPA. My added connect means that we get that signal regardless of whether we're in Kwin or not. What we wouldn't need is the ownership changes you did for Breeze (and would otherwise still need doing in Oxygen/QtCurve ) > May I ask for a small unit test for the new functionality? For you, a *big* unit test. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D6727 To: davidedmundson Cc: graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas
D6727: Destroy all kwayland objects created by registry when it is destroyed
graesslin added a comment. I like the idea! That would basically allow us to abandon https://phabricator.kde.org/D6571 and would also solve the issues we see with the kwayland-integration plugin which can crash applications on exit. May I ask for a small unit test for the new functionality? REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D6727 To: davidedmundson Cc: graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas
D6727: Destroy all kwayland objects created by registry when it is destroyed
davidedmundson created this revision. Restricted Application added subscribers: Frameworks, plasma-devel. Restricted Application added projects: Plasma on Wayland, Frameworks. REVISION SUMMARY Currently one has to connect every object manually to connectionDied, which is something we can do for them. If the user also has a connection, the second will just no-op. Will add some awesome docs / unit tests if you're into the idea. TEST PLAN Commit 2: Emit connectionDied if the QPA owns the connection and is destroyed. We have a few objects that linger longer than the qApp. I've got a crash in plasmashell, and I've had a crash with breeze wrapping a dangly menu in konversation. This should fix those. REPOSITORY R127 KWayland BRANCH davidedmundson/xdgv6 REVISION DETAIL https://phabricator.kde.org/D6727 AFFECTED FILES src/client/connection_thread.cpp src/client/output.cpp src/client/output.h src/client/outputdevice.cpp src/client/outputdevice.h src/client/registry.cpp src/client/registry.h To: davidedmundson Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas
