GNOME Bugzilla – Bug 693708
Regression: screen is not locked on suspend
Last modified: 2013-03-18 22:01:11 UTC
Since we landed the logind code to handle suspend, we removed the manual locking from the user menu. The problem with this is that fdo.login1.Lock causes us to lock without animation, which is a regression from previous behaviour, and in particular it behaves badly with g-s-d blanking the screen on lock. I see two ways to solve this problem: either we go back to the previous behavior of locking manually when suspended, or we just flip the animate boolean when locking from logind.
fdo.login1.Lock is not used during suspend, right ? We use a delay inhibitor to do the animation before suspend. What is the connection here ?
Oh, you're right, I misread the code. I need to investigate further why I doesn't work here, then...
Created attachment 238890 [details] [review] screenShield: Only release logind inhibitor on suspend To make sure that the screen shield is shown before suspending, we take a logind inhibitor and release it when the screen shield is shown. As the screen shield is not only shown on suspend, we can end up releasing the inhibitor independently from suspending (lock, idle), in which case the screen might not be locked when we do suspend. To fix, only release the inhibitor after showing the screen shield when we are about to suspend.
Review of attachment 238890 [details] [review]: The suspend inhibitor is taken in PrepareForSleep(true), and otherwise _inhibitor is null (so uninhibitSuspend does nothing). It should never be the case that such a call has an effect and we're not locked.
(In reply to comment #4) > Review of attachment 238890 [details] [review]: > > The suspend inhibitor is taken in PrepareForSleep(true), and otherwise > _inhibitor is null (so uninhibitSuspend does nothing). No, we take the inhibitor is _init() and PrepareForSleep(*false*). In other words, we should *always* have the inhibitor unless we are in the process of suspending and the screen shield is down.
Ah, my bad. Then the patch is correct.
Attachment 238890 [details] pushed as 4b3bf05 - screenShield: Only release logind inhibitor on suspend (In reply to comment #6) > Ah, my bad. Then the patch is correct. It's worth noting that the behavior you were assuming (only take the inhibitor when we are about to suspend and release it when we are ready) makes a lot of sense to me, but is not how the logind API is designed - the PrepareForSleep signal is only emitted if at least one inhibitor is taken, so always taking the inhibitor is what we are supposed to do (otherwise we'd need to rely on some other process inhibiting suspend).
Maybe worth for 3.6 too, in case distros want to backport it?