Take file descriptor only instead of whole KWayland Display

Authored by romangg on Mar 16 2020, 7:51 PM.



The current KScreenLocker expects the Compositor to hand over the
KWayland::Server::Display pointer.

This has several disadvantages:

  • In KScreenLocker functionality is duplicated which the compositor probably has setup already (creating client connections manually).
  • The Display object has a way larger scope than what KScreenLocker actually needs.
  • Ownership of the Display is unclear in regards to memory but also what the compositor is still allowed to do in comparision to KScreenLocker with the Display.
  • KScreenLocker can only be integrated in compositors using KWayland for managing their wl_display protocol object.

Instead it is now enough to hand over a single file descriptor KScreenLocker
can use as its client endpoint to talk to the compositor.

Test Plan

Manually in Wayland session, autotest.

Diff Detail

R133 KScreenLocker
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
romangg requested review of this revision.Mar 16 2020, 7:51 PM
romangg created this revision.
apol added a subscriber: apol.Mar 17 2020, 12:33 AM

+1 It's cool how the code is much simpler like this, good stuff.


Isn't this redundant with KSldApp::userActivity()?

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

Could be. I remember I thought about removing it but I refrained because it comes from m_lockWindow and I wasn't sure if there are not other triggers of user activity besides the seats timestamps. But might very well be redundant.

romangg added inline comments.Mar 17 2020, 3:23 AM

Ah wait, now I remember. In the X11 case we get the user activity from the X11Locker class (there is an event filter installed). That's also why I kept it here in the if (m_isX11) path.

romangg added inline comments.Mar 17 2020, 3:26 AM

What could be an idea therefore is to remove the connect and instead from the event filter in X11Locker directly call the userActivity function of KSldApp like in the Wayland case the compositor does.

Would reduce the complexity definitely a bit more.

davidedmundson accepted this revision.Mar 18 2020, 9:39 PM
This revision is now accepted and ready to land.Mar 18 2020, 9:39 PM
This revision was automatically updated to reflect the committed changes.
Restricted Application added a project: Plasma. · View Herald TranscriptMar 28 2020, 9:38 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript