[ksmserver] Signal session management state to kwin directly
ClosedPublic

Authored by davidedmundson on Oct 25 2019, 2:29 PM.

Details

Summary

Kwin had to have two ICE connections in order to track state
indepdendently of it's session saving.

This replaces that with a more direct DBus protocol allowing for both
simplification on the kwin side as well as comunicating the logout state
better for effects.

Whilst this code temporarily complicates things, now we have this
interface the next step is drop all the isWM() stuff and do kwin
specific session management also over this interface. See T11882

Test Plan

Added qdebug into kwin
started logging out with an unsaved file, cancelled shutdown
started logging out with, discarded file

Diff Detail

Repository
R120 Plasma Workspace
Branch
origin-master (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18169
Build 18187: arc lint + arc unit
davidedmundson created this revision.Oct 25 2019, 2:29 PM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 25 2019, 2:29 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidedmundson requested review of this revision.Oct 25 2019, 2:29 PM
apol added a subscriber: apol.Oct 25 2019, 3:35 PM

Looks good to me overall. It's better to use dbus.

ksmserver/org.kde.KWin.Session.xml
7

Here it recommends using uint for such things:
https://dbus.freedesktop.org/doc/dbus-api-design.html#interface-files

davidedmundson added inline comments.Oct 25 2019, 4:31 PM
ksmserver/org.kde.KWin.Session.xml
7

It also advocates for use of {sv} in your spec which is inherently string based.

I'm happy to use ints, as long as we then don't get into that trap of saying; "now we have an enum to sync in two places, we must create a header file and a dependency "

davidedmundson added inline comments.Oct 25 2019, 4:53 PM
ksmserver/org.kde.KWin.Session.xml
7

There was a third option I considered. Have N methods

startSave
startQuit
saveCancelled

and then make the state change an implicit part of the invokable action we need to do on save.

I went this way as I felt it was nice to have this acting more like a property independently, but what do you think.

apol added inline comments.Oct 26 2019, 12:35 AM
ksmserver/org.kde.KWin.Session.xml
7

I guess it doesn't matter ultimately. I won't advocate for a header with an enum, don't worry.
Logically, having an int is simpler than a string and we will have to document both and have an implicit tie in both implmentations.

Having 3 methods seems quite elegant as in it's a more semantic API.

As you put it (t, the 3rd seems simpler to grasp, not that everyone should be looking at it.

update to enum

Update to enum

broulik added inline comments.
ksmserver/logout.cpp
188

reply isn't actually used, remove?

apol accepted this revision.Nov 1 2019, 1:08 AM
This revision is now accepted and ready to land.Nov 1 2019, 1:08 AM
This revision was automatically updated to reflect the committed changes.