Review Request 124469: ConsoleKit2 support for screenlocker

[David Edmundson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124469/#review82970
-----------------------------------------------------------



ksmserver/screenlocker/logind.h (line 67)
<https://git.reviewboard.kde.org/r/124469/#comment57258>

    Can consolekit actually close and restart whilst a session is open?
    
    Is it ever possible to see both logind and consolekit? If so our connected=true/false can get potentially broken.



ksmserver/screenlocker/logind.h (line 70)
<https://git.reviewboard.kde.org/r/124469/#comment57261>

    our normal naming is m_lowerCamelCase
    
    also, can you set these to 0.
    
    const QString *m_service = 0;
    I know you set these before use but it makes debugging a backtrace easier if they do ever get used early somewhere in the future



ksmserver/screenlocker/logind.cpp (line 185)
<https://git.reviewboard.kde.org/r/124469/#comment57260>

    we should check m_connected here
    otherwise there's a code path where we have neither logind or console kit, m_service is invalid and we crash
    
    (ksldapp.cpp:280 doesn't check and will call inhibit regardless of connection state)


- David Edmundson


On July 26, 2015, 7:13 a.m., Eric Koegel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124469/
> -----------------------------------------------------------
> 
> (Updated July 26, 2015, 7:13 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> ConsoleKit2 has the same API as systemd-logind for Lock, Unlock,
> PrepareForSleep, and Inhibit. This patch adds the functionality
> for ConsoleKit2 while attempting to minimize code duplication.
> 
> 
> Diffs
> -----
> 
>   ksmserver/screenlocker/logind.h 9983673 
>   ksmserver/screenlocker/logind.cpp 5335b15 
> 
> Diff: https://git.reviewboard.kde.org/r/124469/diff/
> 
> 
> Testing
> -------
> 
> dbus-send --system --dest=org.freedesktop.ConsoleKit   --type=method_call --print-reply --reply-timeout=2000   /org/freedesktop/ConsoleKit/Manager   org.freedesktop.ConsoleKit.Manager.ListInhibitors
> method return sender=:1.1 -> dest=:1.80 reply_serial=2
>    array [
>       struct {
>          string "suspend"
>          string "NetworkManager"
>          string "NetworkManager needs to turn off networks"
>          string "delay"
>          uint32 0
>          uint32 3473
>       }
>       struct {
>          string "handle-power-key:handle-suspend-key:handle-hibernate-key:handle-lid-switch"
>          string "PowerDevil"
>          string "KDE handles power events"
>          string "block"
>          uint32 1000
>          uint32 9587
>       }
>       struct {
>          string "suspend"
>          string "Screen Locker"
>          string "Ensuring that the screen gets locked before going to sleep"
>          string "delay"
>          uint32 1000
>          uint32 9508
>       }
>    ]
> 
> Verified ConsoleKit2 does delay suspending until both delay locks are removed.
> 
> 
> Thanks,
> 
> Eric Koegel
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150726/d70b06e5/attachment-0001.html>