From 3604aaf74cac587757b20daea108f80ade9973d5 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Sun, 23 Jun 2019 17:59:44 +0200 Subject: [PATCH] Close screen grabbing effect when screensaver starts Summary: The screenlock fails on X11 if it can't grab the keyboard. We can't nicely solve the generic case. We can solve the common case of a kwin effect being active. It's not critical, arguably not even desirable to have these effects persist after the screen is locked through an external trigger. We can just close the effect early. Key grabs have to be relased early before the close animation completes so that the locker doesn't have a race based on animation times. It's not ideal, but no worse than the current state for not much work. BUG: 234153 Test Plan: locked screen on a timer opened various effects Reviewers: #kwin, zzag Reviewed By: #kwin, zzag Subscribers: ngraham, zzag, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D20890 --- CMakeLists.txt | 1 + effects.cpp | 1 + effects/cube/cube.cpp | 10 ++++++++++ effects/desktopgrid/desktopgrid.cpp | 8 ++++++++ effects/flipswitch/flipswitch.cpp | 4 ++++ effects/presentwindows/presentwindows.cpp | 3 +++ libkwineffects/kwineffects.h | 7 +++++++ screenlockerwatcher.cpp | 9 +++++++-- screenlockerwatcher.h | 5 ++++- 9 files changed, 45 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d31a9abf67..132c42332c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -556,6 +556,7 @@ qt5_add_dbus_adaptor( kwin_KDEINIT_SRCS org.kde.kwin.OrientationSensor.xml orien qt5_add_dbus_adaptor( kwin_KDEINIT_SRCS org.kde.KWin.VirtualDesktopManager.xml dbusinterface.h KWin::VirtualDesktopManagerDBusInterface ) qt5_add_dbus_interface( kwin_KDEINIT_SRCS ${KSCREENLOCKER_DBUS_INTERFACES_DIR}/kf5_org.freedesktop.ScreenSaver.xml screenlocker_interface ) +qt5_add_dbus_interface( kwin_KDEINIT_SRCS ${KSCREENLOCKER_DBUS_INTERFACES_DIR}/org.kde.screensaver.xml kscreenlocker_interface ) qt5_add_dbus_interface( kwin_KDEINIT_SRCS org.kde.kappmenu.xml appmenu_interface ) diff --git a/effects.cpp b/effects.cpp index b4b7b82020..d269510074 100644 --- a/effects.cpp +++ b/effects.cpp @@ -212,6 +212,7 @@ EffectsHandlerImpl::EffectsHandlerImpl(Compositor *compositor, Scene *scene) #endif connect(ScreenEdges::self(), &ScreenEdges::approaching, this, &EffectsHandler::screenEdgeApproaching); connect(ScreenLockerWatcher::self(), &ScreenLockerWatcher::locked, this, &EffectsHandler::screenLockingChanged); + connect(ScreenLockerWatcher::self(), &ScreenLockerWatcher::aboutToLock, this, &EffectsHandler::screenAboutToLock); connect(kwinApp(), &Application::x11ConnectionChanged, this, [this] { diff --git a/effects/cube/cube.cpp b/effects/cube/cube.cpp index 1208f2e39d..986c373046 100644 --- a/effects/cube/cube.cpp +++ b/effects/cube/cube.cpp @@ -101,6 +101,16 @@ CubeEffect::CubeEffect() connect(effects, &EffectsHandler::tabBoxAdded, this, &CubeEffect::slotTabBoxAdded); connect(effects, &EffectsHandler::tabBoxClosed, this, &CubeEffect::slotTabBoxClosed); connect(effects, &EffectsHandler::tabBoxUpdated, this, &CubeEffect::slotTabBoxUpdated); + connect(effects, &EffectsHandler::screenAboutToLock, this, [this]() { + // Set active(false) does not release key grabs until the animation completes + // As we know the lockscreen is trying to grab them, release them early + // all other grabs are released in the normal way + setActive(false); + if (keyboard_grab) { + effects->ungrabKeyboard(); + keyboard_grab = false; + } + }); reconfigure(ReconfigureAll); } diff --git a/effects/desktopgrid/desktopgrid.cpp b/effects/desktopgrid/desktopgrid.cpp index 1946174a13..2430544fcd 100644 --- a/effects/desktopgrid/desktopgrid.cpp +++ b/effects/desktopgrid/desktopgrid.cpp @@ -88,6 +88,14 @@ DesktopGridEffect::DesktopGridEffect() connect(effects, &EffectsHandler::windowGeometryShapeChanged, this, &DesktopGridEffect::slotWindowGeometryShapeChanged); connect(effects, &EffectsHandler::numberScreensChanged, this, &DesktopGridEffect::setup); + connect(effects, &EffectsHandler::screenAboutToLock, this, [this]() { + setActive(false); + if (keyboardGrab) { + effects->ungrabKeyboard(); + keyboardGrab = false; + } + }); + // Load all other configuration details reconfigure(ReconfigureAll); } diff --git a/effects/flipswitch/flipswitch.cpp b/effects/flipswitch/flipswitch.cpp index 890b6d881e..b66f047975 100644 --- a/effects/flipswitch/flipswitch.cpp +++ b/effects/flipswitch/flipswitch.cpp @@ -75,6 +75,10 @@ FlipSwitchEffect::FlipSwitchEffect() connect(effects, &EffectsHandler::tabBoxClosed, this, &FlipSwitchEffect::slotTabBoxClosed); connect(effects, &EffectsHandler::tabBoxUpdated, this, &FlipSwitchEffect::slotTabBoxUpdated); connect(effects, &EffectsHandler::tabBoxKeyEvent, this, &FlipSwitchEffect::slotTabBoxKeyEvent); + connect(effects, &EffectsHandler::screenAboutToLock, this, [this]() { + setActive(false, AllDesktopsMode); + setActive(false, CurrentDesktopMode); + }); } FlipSwitchEffect::~FlipSwitchEffect() diff --git a/effects/presentwindows/presentwindows.cpp b/effects/presentwindows/presentwindows.cpp index cf92a5be69..b37c771e43 100644 --- a/effects/presentwindows/presentwindows.cpp +++ b/effects/presentwindows/presentwindows.cpp @@ -111,6 +111,9 @@ PresentWindowsEffect::PresentWindowsEffect() reCreateGrids(); } ); + connect(effects, &EffectsHandler::screenAboutToLock, this, [this]() { + setActive(false); + }); } PresentWindowsEffect::~PresentWindowsEffect() diff --git a/libkwineffects/kwineffects.h b/libkwineffects/kwineffects.h index b8feed6558..7b2f3d73b8 100644 --- a/libkwineffects/kwineffects.h +++ b/libkwineffects/kwineffects.h @@ -1661,6 +1661,13 @@ Q_SIGNALS: **/ void screenLockingChanged(bool locked); + /** + * This signal is emitted just before the screen locker tries to grab keys and lock the screen + * Effects should release any grabs immediately + * @since 5.17 + **/ + void screenAboutToLock(); + /** * This signels is emitted when ever the stacking order is change, ie. a window is risen * or lowered diff --git a/screenlockerwatcher.cpp b/screenlockerwatcher.cpp index 91c0fde841..0fc1f29481 100644 --- a/screenlockerwatcher.cpp +++ b/screenlockerwatcher.cpp @@ -24,6 +24,7 @@ along with this program. If not, see . #include // dbus generated #include "screenlocker_interface.h" +#include "kscreenlocker_interface.h" namespace KWin { @@ -34,7 +35,6 @@ static const QString SCREEN_LOCKER_SERVICE_NAME = QStringLiteral("org.freedeskto ScreenLockerWatcher::ScreenLockerWatcher(QObject *parent) : QObject(parent) - , m_interface(NULL) , m_serviceWatcher(new QDBusServiceWatcher(this)) , m_locked(false) { @@ -70,13 +70,18 @@ void ScreenLockerWatcher::serviceOwnerChanged(const QString &serviceName, const return; } delete m_interface; - m_interface = NULL; + m_interface = nullptr; + delete m_kdeInterface; + m_kdeInterface = nullptr; + m_locked = false; if (!newOwner.isEmpty()) { m_interface = new OrgFreedesktopScreenSaverInterface(newOwner, QStringLiteral("/ScreenSaver"), QDBusConnection::sessionBus(), this); + m_kdeInterface = new OrgKdeScreensaverInterface(newOwner, QStringLiteral("/ScreenSaver"), QDBusConnection::sessionBus(), this); connect(m_interface, SIGNAL(ActiveChanged(bool)), SLOT(setLocked(bool))); QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(m_interface->GetActive(), this); connect(watcher, SIGNAL(finished(QDBusPendingCallWatcher*)), SLOT(activeQueried(QDBusPendingCallWatcher*))); + connect(m_kdeInterface, &OrgKdeScreensaverInterface::AboutToLock, this, &ScreenLockerWatcher::aboutToLock); } } diff --git a/screenlockerwatcher.h b/screenlockerwatcher.h index 99bc503447..02aa68ba2e 100644 --- a/screenlockerwatcher.h +++ b/screenlockerwatcher.h @@ -25,6 +25,7 @@ along with this program. If not, see . #include class OrgFreedesktopScreenSaverInterface; +class OrgKdeScreensaverInterface; class QDBusServiceWatcher; class QDBusPendingCallWatcher; @@ -41,6 +42,7 @@ public: } Q_SIGNALS: void locked(bool locked); + void aboutToLock(); private Q_SLOTS: void setLocked(bool activated); void activeQueried(QDBusPendingCallWatcher *watcher); @@ -49,7 +51,8 @@ private Q_SLOTS: void serviceOwnerQueried(); private: void initialize(); - OrgFreedesktopScreenSaverInterface *m_interface; + OrgFreedesktopScreenSaverInterface *m_interface = nullptr; + OrgKdeScreensaverInterface *m_kdeInterface = nullptr; QDBusServiceWatcher *m_serviceWatcher; bool m_locked;