GNOME Bugzilla – Bug 764773
[RFE] gnome-shell to forward all lock calls to logind
Last modified: 2016-05-12 13:28:23 UTC
In the example below, you can see that org.freedesktop.login1.Session.Unlock() was emitted but the Lock() was not. It seems to be an old issue [0]. Applications that are interested when session is locked should rely on the Lock signal. I believe that GDM is the one in GNOME that should emit this. [0] https://mail.gnome.org/archives/desktop-devel-list/2014-November/msg00011.html $gdbus monitor --system --dest org.freedesktop.login1 --object-path /org/freedesktop/login1/session/_31 Monitoring signals on object /org/freedesktop/login1/session/_31 owned by org.freedesktop.login1 The name org.freedesktop.login1 is owned by :1.5 /org/freedesktop/login1/session/_31: org.freedesktop.DBus.Properties.PropertiesChanged ('org.freedesktop.login1.Session', {'IdleHint': <true>, 'IdleSinceHint': <uint64 1460112031909357>, 'IdleSinceHintMonotonic': <uint64 18042981739>}, @as []) /org/freedesktop/login1/session/_31: org.freedesktop.login1.Session.Unlock () /org/freedesktop/login1/session/_31: org.freedesktop.DBus.Properties.PropertiesChanged ('org.freedesktop.login1.Session', {'IdleHint': <true>, 'IdleSinceHint': <uint64 1460094002164826>, 'IdleSinceHintMonotonic': <uint64 13237208>}, @as [])
I think the logind lock signal is only emitted when logind is used to lock the session. (logind is always used to unlock the session, internally by gdm). we could fix gnome-shell to forward all lock calls to logind. it already listens for the lock signal and performs a lock operation when it shows up. If we did that, there would be a race though where whatever client is listening for the lock signal may react before the lock finishes. that race is unavoidable with the logind api, so maybe the client should instead be using the gnome api ?
(In reply to Ray Strode [halfline] from comment #1) > I think the logind lock signal is only emitted when logind is used to lock > the session. (logind is always used to unlock the session, internally by > gdm). we could fix gnome-shell to forward all lock calls to logind. it > already listens for the lock signal and performs a lock operation when it > shows up. Right, thanks for clarifying > If we did that, there would be a race though where whatever client is > listening for the lock signal may react before the lock finishes. that race > is unavoidable with the logind api, so maybe the client should instead be > using the gnome api ? The reason to watch lock signal in logind (and also console-kit) is to not be dependent on the desktop environment... as I saw the UnLock coming from GDM I thought we could do the Lock with logind as well, if it makes sense. Which gnome api are you referring?
The org.gnome.ScreenSaver.ActiveChanged signal on the /org/gnome/ScreenSaver object of the org.gnome.ScreenSaver bus name. There's also a more cross desktop d-bus api called org.freedesktop.ScreenSaver. gnome-settings-daemon manages that and forwards calls from it to the gnome d-bus interface.
(In reply to Ray Strode [halfline] from comment #3) > The org.gnome.ScreenSaver.ActiveChanged signal on the /org/gnome/ScreenSaver > object of the org.gnome.ScreenSaver bus name. > > There's also a more cross desktop d-bus api called > org.freedesktop.ScreenSaver. gnome-settings-daemon manages that and forwards > calls from it to the gnome d-bus interface. To be honest, that was my first approach but it does not seem reliable. What if ScreenSaver is not installed or disabled? AFAIU, desktops either rely on logind or console-kit and those can provide this information. From my point of view, it seems better to integrate with them then ScreenSaver. But yeah! ScreenSaver works while logind Lock/Unlock doesn't. If this bug make sense we can fix that upstream ;) I'll be renaming this bug to match your comment #2
As another solution, we can now use LockedHint property from Session's object in Logind [0]. That should be possible from 230 onwards. [0] https://github.com/systemd/systemd/commit/42d35e1301928d08dd32ec51f0205252ae658ba5 Not sure if we should have some '#ifdef'-ish on javascript code to check systemd version but I'll be attaching the patch for gnome-shell that sets this property shortly.
Created attachment 327686 [details] [review] ScreenShield: set LockedHint property from systemd Logind recently got support for a hint property in Session Object to inform if session is Locked or not. It is up to desktop environments to keep this property up to date. This patch includes the method SetLockedHint and calls it when ScreenShield is active.
Review of attachment 327686 [details] [review]: (In reply to Victor Toso from comment #5) > Not sure if we should have some '#ifdef'-ish on javascript code to check > systemd version Well, we don't care about the systemd version at build time, we care about the runtime version. I don't see any version information in the login1 interface, but then we don't really need it anyway - if the version is too old, we'll try to call a non-existent DBus method that results in a (harmless) runtime error. If you want to avoid the warning, you could pass a callback and catch the error, but IMHO it's better not to - after all, there *is* an error, so keeping the warning may be useful. ::: js/ui/screenShield.js @@ +560,3 @@ this.emit('active-changed'); + this._loginSession.SetLockedHintRemote(active); Please check that this._loginSession is set before calling the method: if (this._loginSession) this._loginSession(SetLockedHintRemote(active);
Created attachment 327702 [details] [review] ScreenShield: set LockedHint property from systemd Logind recently got support for a hint property in Session Object to inform if session is Locked or not. It is up to desktop environments to keep this property up to date. This patch includes the method SetLockedHint and calls it when ScreenShield is active. ~changes~ v1->v2 * check if loginSession is set before calling its method
Review of attachment 327702 [details] [review]: LGTM (I'd leave out the last paragraph of the commit message, but OK)
(In reply to Florian Müllner from comment #9) > Review of attachment 327702 [details] [review] [review]: > > LGTM (I'd leave out the last paragraph of the commit message, but OK) Sure! (the link in the commit log gives enough background)
Attachment 327702 [details] pushed as ddea54a - ScreenShield: set LockedHint property from systemd
(In reply to Victor Toso from comment #10) > Sure! (the link in the commit log gives enough background) The code itself gives the background :-) The real background information that is not obvious from the code is in the first paragraph ...