From fe231be5e2d4811dc9132aecb931cf759d16ebf8 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Sun, 7 May 2017 01:40:48 +0200 Subject: [PATCH] Keep all touchpad QActions in the main thread Summary: Making QActions with the Connection as a parent is dangerous as it gets moved to a new thread, moving all child objects to that thread. KGlobalAccel and QAction aren't thread safe and we shouldn't be using it in two threads. This was notably seen when runnng invokeShortcut over DBus which then invokes it on the main thread. Something my laptop was doing when I closed the lid. This patch simply moves the code to the Input class, where we set up the libinput connection. Test Plan: Closed lid, kwin_wayland was still there when I resumed Set manual shortcut for toggling touchpad, that still worked Reviewers: #plasma Subscribers: plasma-devel, kwin, #kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D5733 --- input.cpp | 38 ++++++++++++++++++++++++++ input.h | 1 + libinput/connection.cpp | 59 +++++++++++------------------------------ libinput/connection.h | 2 ++ 4 files changed, 57 insertions(+), 43 deletions(-) diff --git a/input.cpp b/input.cpp index 2ffa05371f..8e7c4ffaab 100644 --- a/input.cpp +++ b/input.cpp @@ -50,6 +50,8 @@ along with this program. If not, see . #include #include #include +#include + //screenlocker #include // Qt @@ -1430,6 +1432,8 @@ public: KWIN_SINGLETON_FACTORY(InputRedirection) +static const QString s_touchpadComponent = QStringLiteral("kcm_touchpad"); + InputRedirection::InputRedirection(QObject *parent) : QObject(parent) , m_keyboard(new KeyboardInputRedirection(this)) @@ -1750,6 +1754,40 @@ void InputRedirection::setupLibInput() } ); } + setupTouchpadShortcuts(); +#endif +} + +void InputRedirection::setupTouchpadShortcuts() +{ + if (!m_libInput) { + return; + } +#ifdef HAVE_INPUT + QAction *touchpadToggleAction = new QAction(this); + QAction *touchpadOnAction = new QAction(this); + QAction *touchpadOffAction = new QAction(this); + + touchpadToggleAction->setObjectName(QStringLiteral("Toggle Touchpad")); + touchpadToggleAction->setProperty("componentName", s_touchpadComponent); + touchpadOnAction->setObjectName(QStringLiteral("Enable Touchpad")); + touchpadOnAction->setProperty("componentName", s_touchpadComponent); + touchpadOffAction->setObjectName(QStringLiteral("Disable Touchpad")); + touchpadOffAction->setProperty("componentName", s_touchpadComponent); + KGlobalAccel::self()->setDefaultShortcut(touchpadToggleAction, QList{Qt::Key_TouchpadToggle}); + KGlobalAccel::self()->setShortcut(touchpadToggleAction, QList{Qt::Key_TouchpadToggle}); + KGlobalAccel::self()->setDefaultShortcut(touchpadOnAction, QList{Qt::Key_TouchpadOn}); + KGlobalAccel::self()->setShortcut(touchpadOnAction, QList{Qt::Key_TouchpadOn}); + KGlobalAccel::self()->setDefaultShortcut(touchpadOffAction, QList{Qt::Key_TouchpadOff}); + KGlobalAccel::self()->setShortcut(touchpadOffAction, QList{Qt::Key_TouchpadOff}); +#ifndef KWIN_BUILD_TESTING + registerShortcut(Qt::Key_TouchpadToggle, touchpadToggleAction); + registerShortcut(Qt::Key_TouchpadOn, touchpadOnAction); + registerShortcut(Qt::Key_TouchpadOff, touchpadOffAction); +#endif + connect(touchpadToggleAction, &QAction::triggered, m_libInput, &LibInput::Connection::toggleTouchpads); + connect(touchpadOnAction, &QAction::triggered, m_libInput, &LibInput::Connection::enableTouchpads); + connect(touchpadOffAction, &QAction::triggered, m_libInput, &LibInput::Connection::disableTouchpads); #endif } diff --git a/input.h b/input.h index d44513d32c..01fddaab83 100644 --- a/input.h +++ b/input.h @@ -267,6 +267,7 @@ Q_SIGNALS: private: void setupLibInput(); + void setupTouchpadShortcuts(); void setupLibInputWithScreens(); void setupWorkspace(); void reconfigure(); diff --git a/libinput/connection.cpp b/libinput/connection.cpp index 4c73883f3e..d1a995462b 100644 --- a/libinput/connection.cpp +++ b/libinput/connection.cpp @@ -26,7 +26,6 @@ along with this program. If not, see . #include "libinput_logging.h" #include -#include #include #include @@ -146,7 +145,6 @@ Connection *Connection::create(QObject *parent) return s_self; } -static const QString s_touchpadComponent = QStringLiteral("kcm_touchpad"); Connection::Connection(Context *input, QObject *parent) : QObject(parent) @@ -156,47 +154,6 @@ Connection::Connection(Context *input, QObject *parent) , m_leds() { Q_ASSERT(m_input); - - // steal touchpad shortcuts - QAction *touchpadToggleAction = new QAction(this); - QAction *touchpadOnAction = new QAction(this); - QAction *touchpadOffAction = new QAction(this); - - touchpadToggleAction->setObjectName(QStringLiteral("Toggle Touchpad")); - touchpadToggleAction->setProperty("componentName", s_touchpadComponent); - touchpadOnAction->setObjectName(QStringLiteral("Enable Touchpad")); - touchpadOnAction->setProperty("componentName", s_touchpadComponent); - touchpadOffAction->setObjectName(QStringLiteral("Disable Touchpad")); - touchpadOffAction->setProperty("componentName", s_touchpadComponent); - KGlobalAccel::self()->setDefaultShortcut(touchpadToggleAction, QList{Qt::Key_TouchpadToggle}); - KGlobalAccel::self()->setShortcut(touchpadToggleAction, QList{Qt::Key_TouchpadToggle}); - KGlobalAccel::self()->setDefaultShortcut(touchpadOnAction, QList{Qt::Key_TouchpadOn}); - KGlobalAccel::self()->setShortcut(touchpadOnAction, QList{Qt::Key_TouchpadOn}); - KGlobalAccel::self()->setDefaultShortcut(touchpadOffAction, QList{Qt::Key_TouchpadOff}); - KGlobalAccel::self()->setShortcut(touchpadOffAction, QList{Qt::Key_TouchpadOff}); -#ifndef KWIN_BUILD_TESTING - InputRedirection::self()->registerShortcut(Qt::Key_TouchpadToggle, touchpadToggleAction); - InputRedirection::self()->registerShortcut(Qt::Key_TouchpadOn, touchpadOnAction); - InputRedirection::self()->registerShortcut(Qt::Key_TouchpadOff, touchpadOffAction); -#endif - connect(touchpadToggleAction, &QAction::triggered, this, &Connection::toggleTouchpads); - connect(touchpadOnAction, &QAction::triggered, this, - [this] { - if (m_touchpadsEnabled) { - return; - } - toggleTouchpads(); - } - ); - connect(touchpadOffAction, &QAction::triggered, this, - [this] { - if (!m_touchpadsEnabled) { - return; - } - toggleTouchpads(); - } - ); - // need to connect to KGlobalSettings as the mouse KCM does not emit a dedicated signal QDBusConnection::sessionBus().connect(QString(), QStringLiteral("/KGlobalSettings"), QStringLiteral("org.kde.KGlobalSettings"), QStringLiteral("notifyChange"), this, SLOT(slotKGlobalSettingsNotifyChange(int,int))); @@ -582,6 +539,22 @@ void Connection::toggleTouchpads() } } +void Connection::enableTouchpads() +{ + if (m_touchpadsEnabled) { + return; + } + toggleTouchpads(); +} + +void Connection::disableTouchpads() +{ + if (!m_touchpadsEnabled) { + return; + } + toggleTouchpads(); +} + void Connection::updateLEDs(Xkb::LEDs leds) { if (m_leds == leds) { diff --git a/libinput/connection.h b/libinput/connection.h index 41c2443ec0..f08b1d5bdf 100644 --- a/libinput/connection.h +++ b/libinput/connection.h @@ -80,6 +80,8 @@ public: void processEvents(); void toggleTouchpads(); + void enableTouchpads(); + void disableTouchpads(); QVector devices() const { return m_devices;