----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/109693/#review84754 -----------------------------------------------------------
just as a late update to the review request: in 5.x the ksldapp inhibits suspend on logind to ensure that the screen gets locked before we go to suspend. Also just this week I merged some work for 5.5 to improve notifying ksldapp about greeter being ready. Unfortunately I still get screen unlocked after suspend and I'm at the point where I think it's impossible without Wayland. Nevertheless: thanks for the approach to fixing the issue. - Martin Gräßlin On March 25, 2013, 4:23 p.m., Oliver Henshaw wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/109693/ > ----------------------------------------------------------- > > (Updated March 25, 2013, 4:23 p.m.) > > > Review request for Plasma and Solid. > > > Bugs: 313123 > http://bugs.kde.org/show_bug.cgi?id=313123 > > > Repository: kde-workspace > > > Description > ------- > > Here's an attempt to get the screen lock to fully activate before resume > starts. > > > This is a problem on systems that use systemd/logind to suspend as it > suspends immediately, so that there's unlikely to be time for the lock > process to hide the screen before the system suspends. There shouldn't be a > problem on systems that use upower, as it waits a second before suspending - > there's no guarantee that the screen locks in time, but it should in > practice. But there are reports of problems with upower (on ubuntu, at least) > - this needs further investigation to see if there's something else going on. > > It's also not clear why the lock process waits a further ten or so seconds > after resume before hiding the screen. > > > * Delay emitting org.freedesktop.ScreenSaver.ActiveChanged and replying to > org.freedesktop.ScreenSaver.Lock until the lock process is ready. We can't > have the ksmserver blocking in the .Lock call as this blocks the lock process > (as QApplication constructs a QSessionManager which attempts to communicate > with ksmserver over XSMP) so delay replying and complete the transaction when > the lock process signal its readiness. > > - The qml lockscreen still blocks on powerdevil as it queries supported > suspend methods using the synchronous Solid::PowerManagement API. So disable > this for now. This is harmless for the plasma desktop but does affect the > plasma active qml locker. > > - The plasma overlay locker seems to be broken. I think this is an unrelated > breakage on my test system but can't be sure. It should be fine in theory, > but might have a problem if there are any power-management plasmoids on it. > > > Other approaches to breaking the powerdevil <-> lock process deadlock are: > > * Have SuspendSession::lockScreenAndWait() call > face.call(QDBus::BlockWithGui, "Lock") to return to the event loop. > - But does this mean that the entire call-chain to this point would need to > be re-entrant with respect to the event loop? > * Make powerdevil Actions into transactions that are queued and signal their > completion (or failure) > - Tricky and fairly wide-ranging. And probably not suitable for 4.10. > * Have the kscreenlocker_greet lock process use asynchronous dbus calls to > query powerdevil rather than the synchronous API. I'm not sure if there's > already some prototype asynchronous API (for libsolid2) to cannibalise for > this. > * Have ksmserver or the lock process take a delay lock - > http://www.freedesktop.org/wiki/Software/systemd/inhibit - and return from > org.freedesktop.ScreenSaver.Lock as soon as the lock process is running, as > we do now. > - This would solve the problem on systems that use logind to suspend, but do > nothing on systems that suspend with upower. As mentioned above, it's unclear > what the cause of the problem is on upower systems. > > > The patch series is in my clone at > kde:clones/kde-workspace/oliverhenshaw/kde-workspace in the branch > review/screenlocker-waitforlock and comprises: > > > [1/5] Represent lock state with an enum > > > [2/5] Emit activeChanged() only when lock process ready > > * synchronously call desktopResized in the screen locker app > (this causes the lock process windows to be created) > * signal that the lock process is ready by calling > org.kde.screensaver.saverLockReady() > * introduce a Waiting LockState > * finish locking in lockProcessReady() when receive notification over > dbus - start locked timer and emit locked() here rather than earlier > > This is based on 62c2c398611b2e6ef6e1e38ed79bc9540fc02ec9 and the old > screensaver implementation (from 4.9 and earlier). But delaying the > lock() qt signal, and thus delaying the activeChanged() dbus signal, > until the lock prcess is ready is new behaviour. > > > [3/5] Delay reply from dbus calls until screen is locked > > This prevents e.g. suspending before the screen is properly locked. > > Based on the old screensaver implementation (from 4.9 and earlier). > > > [4/5] Print when lock process starts and when ready > > Useful for timing slow lock process creation > > > [5/5] Skip blocking Solid::PowerManagement call > > Solid::PowerManagement API calls powerdevil synchronously, causing > deadlock until dbus timeout. > > > Diffs > ----- > > ksmserver/screenlocker/greeter/CMakeLists.txt > 11905152321377aa3135424fa2e09bb991878da5 > ksmserver/screenlocker/greeter/greeterapp.h > f332bfc5c432b5b48b568c815904a12b4a74bd7a > ksmserver/screenlocker/greeter/greeterapp.cpp > b70c9d6c005aa66d2f85a42ef4f1dcb04ea44667 > ksmserver/screenlocker/greeter/main.cpp > 180d64d58b069e2d5b8362fb43c63ec2e834fadb > ksmserver/screenlocker/interface.h 018464cda6c1ea9511cd4553e615bee62a66949a > ksmserver/screenlocker/interface.cpp > 32acad1b8afc8f15403d9e7b92dd49c51d355bdc > ksmserver/screenlocker/ksldapp.h 3047c00558d664ffac864582a4cb2c4f725a3cf4 > ksmserver/screenlocker/ksldapp.cpp 7f2e6715f0c12baa1586e41fa8f767707c9f83c5 > plasma/screensaver/shell/CMakeLists.txt > 5c52da691acf24422efc420be9bdc71eb716b268 > plasma/screensaver/shell/main.cpp be151aa25dbff449d2e777ace9b43cac3de12f6d > > Diff: https://git.reviewboard.kde.org/r/109693/diff/ > > > Testing > ------- > > Not enough > > > Thanks, > > Oliver Henshaw > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
