[ksmserver] Split xsession logout and shutdown into separate classes
ClosedPublic

Authored by davidedmundson on Oct 17 2018, 4:28 PM.

Details

Summary

This commit splits ksmserver's xsession shutdown logic from performing
the actual shutdown and running shutdown scripts and implement proposed
org.kde.Shutdown interface.

Intended longer term target is to move this to a separate executable.

KSMServer's existing logout dbus method still exists for compatibility
forwarding to the new interface.

There are 2 minor behavioural changes.

The shutdownMode property (which doesn't seem to do anything and is not
exposed in our UI) is not kept.

If you shutdown /whilst/ starting up somehow, previously we delayed
showing the logout prompt, we now delay performing the actual logout.

Test Plan

Logged out / shut down using the old API
Logged out / shut down using the new DBus API

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidedmundson created this revision.Oct 17 2018, 4:28 PM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 17 2018, 4:28 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidedmundson requested review of this revision.Oct 17 2018, 4:28 PM
apol requested changes to this revision.Oct 30 2018, 3:59 PM
apol added a subscriber: apol.
apol added inline comments.
ksmserver/shutdown.cpp
56–58

If you have to set it before calling startLogout(), maybe it would work better as an argument.

85

Is it really possible for ::locateAll to return directories that don't exist?

ksmserver/shutdown.h
2

It's missing the license.

This revision now requires changes to proceed.Oct 30 2018, 3:59 PM
davidedmundson marked 3 inline comments as done.Oct 31 2018, 5:12 PM

aleix's comments

romangg added a subscriber: romangg.Nov 2 2018, 3:41 PM

Stupid question, but is there a file missing? Because so much code is being removed.

ksmserver/shutdown.cpp
1–2

Missing license

Phab shows it weird.

Logout and shutdown are copies of a single file. One has one half removed the other has the other half removed.

apol accepted this revision.Nov 3 2018, 12:05 AM
This revision is now accepted and ready to land.Nov 3 2018, 12:05 AM

The functional split is kind of confusing. Some functions in logout.cpp still do shutdown related tasks when looking at their names, other in shutdown.cpp do logout things.

To me this whole distinction between shutdown and logout seems kind of artificial. In the end the main function is that the session quits, If afterwards the device shuts down or we go back to the login maanger should be only a side note.

Is my observation wrong? Or do you plan on working in this direction in the future?

https://phabricator.kde.org/T9779

Everything related to x is in logout. shutdown contains everything else and the new public interface.

romangg accepted this revision.Nov 4 2018, 10:06 AM

https://phabricator.kde.org/T9779

Everything related to x is in logout. shutdown contains everything else and the new public interface.

I know the task. The naming of the files isn't intuitive then. In logout.cpp the new public interface is also used. I would urge you to refine this with other patches later on. I tested this patch here (on Wayland) and login/logout works.

This revision was automatically updated to reflect the committed changes.