Adapt to new KScreenLocker API

Authored by romangg on Mar 16 2020, 10:02 PM.



KScreenLocker only anymore takes an FD when it tries to lock. The connection
is created and kept internally in KWin.

At the moment we do not do any further checks on the lock but directly hand
over an FD whenever KScreenLocker is about to lock.

Diff Detail

R108 KWin
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
romangg created this revision.Mar 16 2020, 10:02 PM
Restricted Application added a project: KWin. · View Herald TranscriptMar 16 2020, 10:02 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Mar 16 2020, 10:02 PM
apol added a subscriber: apol.Mar 17 2020, 12:40 AM

+1 overall


Sounds like if KSldApp wasn't a singleton much of the cleanup code would be much simpler. It could maybe make sense to revisit this?

romangg added inline comments.Mar 17 2020, 12:48 AM

Oh yea. I guess it would also make the autotest easier to maintain since you could just reset the screen locker.

(In regards to the autotest see also:

And actually I tried to make it a member of WaylandServer (and already replaced all the KSldApp::self() getters in the code base with some waylandServer()->screenLocker() getter). But crudely enough the screen locker is already queried before the WaylandServer singleton is created by classes like PointerInputRedirection.

What sounds kind of wrong. But to tackle this there would be a larger refactoring needed for the startup order or with which class the screen locker interfaces.

davidedmundson added inline comments.

not sure that's the right.

KSLDApp is the process that launches the screenlocker when needed, it has to have the lifespan of the session.


why do we do this on every lock?


Can this be done the other way round in the seat constructor

then we get rid of all these connects and disconnects


who deletes m_screenLockerClientConnection in this case?

romangg added inline comments.Mar 18 2020, 11:10 PM

Because the client connection is reset after every lock/unlock and I'm not sure it's possible to reuse the same socket for a new client. The connection is accessed by the greeter. And if the greeter exits the client connection is destroyed by callback.


Yea, when the KSldApp is a undestroyed singleton sure. Having it here improves locality imo but I can also put it in the constructors.


When the client goes down libwayland calls into the client connection destroy callback and that deletes the client here:

At this point trying to delete the connection from KWin's side would be a double-free.

If you think this API is not so intuitive don't look in the Client lib. :)

romangg added a comment.EditedMar 23 2020, 1:25 PM

Shall I put the connects in the constructor to move this forward?

davidedmundson accepted this revision.Mar 23 2020, 1:40 PM

Shall I put the connects in the constructor to move this forward?

You choose.


I don't like that client API :/

But this code makes sense then.

This revision is now accepted and ready to land.Mar 23 2020, 1:40 PM
This revision was automatically updated to reflect the committed changes.