[Kde-hardware-devel] PowerDevil prepareForSuspend signal

[Oliver Henshaw]

On 16 April 2013 10:49, Martin Briza <mbriza at redhat.com> wrote:
> On Mon, 15 Apr 2013 19:49:44 +0200, Oliver Henshaw
> <oliver.henshaw at gmail.com> wrote:
>
>> Odd that polkit is asking for credentials in the first place - I guess
>> something goes wrong when you switch user and back? Anyway, locking
>> the screen before being sure we have polkit authentication is also
>> reported at https://bugs.kde.org/show_bug.cgi?id=294712
>
>
> This happens when more users are logged in and one tries to suspend the
> computer... but it depends on how restrictive the polkit rules are, of
> course.

OK. I've continued the polkit discussion in
https://bugzilla.redhat.com/show_bug.cgi?id=951174 (as you've seen)

>>> The solution to the bug itself would be calling inhibit() from the login1
>>> dbus interface before calling suspend and closing the returned file
>>> descriptor after the screensaver is started. This would allow us to
>>> handle
>>> the PrepareForSleep signal of login1 which is emitted to notify the
>>> inhibiting applications to do all the actions required before suspension.
>>> The change to overall behavior of any other backend would be just
>>> emitting
>>> the signal right before suspending and waiting for return from the slot.
>>
>>
>> Quite possibly. It might be even better to have ksmserver take the
>> delay lock and listen to prepareForSleep to active the screenlocker.
>
>
> In my opinion, it would be better to keep this in PowerDevil to have all the
> logic regarding power management in the same class, also allowing cleaner
> implementation for other backends. Anyway, I'm easy to be reasoned
> otherwise.

But it's ksmserver that actually locks the screen. Plus it may need to
hold a delay lock until the screenlocker is fully active to resolve
https://git.reviewboard.kde.org/r/109693/

Once ksmserver is listening to prepareForSleep it could use the signal
as a trigger to lock the screen, if so configured. Then powerdevil
wouldn't need to ask ksmserver to lock before suspending. And, as a
consequence,  the screen will stay unlocked until polkit is satisfied
that suspend can proceed.

I have some further ideas about shuffling responsibility between
ksmserver and powerdevil - I'll try to lay these out at the Solid
sprint.

>> We could go even further and
>> lock the screen whenever the session becomes inactive (i.e. when
>> switching VT) - see https://bugs.kde.org/show_bug.cgi?id=309051 - this
>> might be controversial though.
>
>
> This rather sounds like regular user switching detection to me - which is
> handled in kworkspace. The session would just listen to changes on its
> Active property on the login1 interface.

OK. I hadn't really taken much notice of kworkspace up till now.
That's a good point.

> I don't understand how it relates to suspending though.

True. Its just further musings on when to lock the screen.

>> This still leaves the problem of what to do when the lid is closed
>> though. With logind we can use the user_interaction boolean to specify
>> that the user can't enter a password with the lid closed. I think
>> something similar can be done with upower+polkit but it's less
>> elegant.
>
>
> Well, there's not only this problem with getting information about the user
> being able to interact but also if we use this to suspend the computer
> without authenticating him, it completely denies all need to enter the
> password on laptops - why would I authenticate when I can just close the
> lid?

I think this was a polkit policy of auth_admin_keep that was confusing
this, wasn't it? If you specify that the user can't enter
authentication then auth_admin actions will/should just fail, I think.

I think we should pass interactive = false (or do the upower + polkit
equivalent) in these situations: when the lid is closed [1], when the
screen is already locked[1], (maybe) when the session is inactive[2],
when powerdevil is taking an action due to idle[3]. Then the
powerdevil action would fail if authentication is required.

[1] In these cases we should probably play a sound or do something to
indicate that the action has failed.
[2] though I don't understand the point of an "allow_inactive" =
"auth_admin" policy - how can anyone provide authentication on an
inactive session?
[3] since if the user was around to give authentication they wouldn't
want the action to be taken


> But there could possibly be other things we want to do before the suspension
> and I think having a hook like this could come handy.
Perhaps. But it's just as easy to have powerdevil to take a delay lock
for itself if there are uses for it apart from locking the screen.