D28455: add dbus interface to the greeter

David Edmundson noreply at phabricator.kde.org
Tue Mar 31 09:46:31 BST 2020


davidedmundson added a comment.


  I'd say we go for the simplest solution +++
  
  Given we have MPRIS on here, if we were to do any DBus lockdowns we would almost certainly want to use xdg-dbus-proxy, at which point this would continue to work.

INLINE COMMENTS

> greeterapp.cpp:139
> +    (void) new GreeterAdapter(this);
> +    QDBusConnection::sessionBus().registerService(QStringLiteral("org.kde.screensaver.Greeter"));
> +    QDBusConnection::sessionBus().registerObject(QStringLiteral("/Greeter"), this);

Register objects before registering a service.

DBus receiving is in another thread so you could hypothetically get a message between the two events.

Doesn't really matter if we lose a notification in this case, but it's a good practice

> greeterapp.cpp:497
> +        // we invoke the notify method of the greeter root object
> +        QMetaObject::invokeMethod(rootObject, "Notify", Qt::QueuedConnection,
> +                                  Q_ARG(uint, id),

This pattern of fetching and poking the root object of a scene isn't one I like, it's better to expose an object to the view that then has normal signals and slots - but this does follow the convention already here, so I can't really object.

> greeterapp.h:51
>      Q_OBJECT
> +    Q_CLASSINFO("D-Bus Interface", "org.kde.screensaver.GreeterApp")
>  public:

You don't need this if you're using an adaptor

REPOSITORY
  R133 KScreenLocker

REVISION DETAIL
  https://phabricator.kde.org/D28455

To: bshah
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200331/411ebb6a/attachment.html>


More information about the Plasma-devel mailing list