diff options
author | Johan Klokkhammer Helsing <johan.helsing@qt.io> | 2019-02-14 13:41:30 +0100 |
---|---|---|
committer | Johan Helsing <johan.helsing@qt.io> | 2019-02-20 14:13:05 +0000 |
commit | eb66211ea9b58537a21630893229c7d3c86a10b3 (patch) | |
tree | d3597596dbde8538727792b179453f9233a70725 | |
parent | cf378b581009b80510cf54c8b56d4c2d950c1153 (diff) |
Client: Don't leak wl_data_offers
[ChangeLog][QPA plugin] Fixed a leak of wl_data_offers.
Data offers would previously leak when a surface with keyboard focus was
destroyed... or if it was hidden... or if it changed type, and so on.
Make keyboard focus follow a wl_surface instead of a QWaylandWindow.
This also fixes a couple of other minor issues, such as the mRepeatTimer not
stopping when a wl_surface was destroyed.
Ideally, we would have a QWaylandSurface class separate from the
QWaylandWindow, but that's out of scope for this fix.
Fixes: QTBUG-73825
Change-Id: I56e502512c3959e3fcdb63744adc4a7698e3d53d
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>
-rw-r--r-- | src/client/qwaylanddisplay.cpp | 1 | ||||
-rw-r--r-- | src/client/qwaylandinputdevice.cpp | 77 | ||||
-rw-r--r-- | src/client/qwaylandinputdevice_p.h | 7 | ||||
-rw-r--r-- | src/client/qwaylandwindow.cpp | 23 | ||||
-rw-r--r-- | src/client/qwaylandwindow_p.h | 4 |
5 files changed, 62 insertions, 50 deletions
diff --git a/src/client/qwaylanddisplay.cpp b/src/client/qwaylanddisplay.cpp index 3675806b2..491b66e15 100644 --- a/src/client/qwaylanddisplay.cpp +++ b/src/client/qwaylanddisplay.cpp @@ -50,6 +50,7 @@ #endif #if QT_CONFIG(wayland_datadevice) #include "qwaylanddatadevicemanager_p.h" +#include "qwaylanddatadevice_p.h" #endif #if QT_CONFIG(cursor) #include <wayland-cursor.h> diff --git a/src/client/qwaylandinputdevice.cpp b/src/client/qwaylandinputdevice.cpp index cb26def68..43ca6dbd0 100644 --- a/src/client/qwaylandinputdevice.cpp +++ b/src/client/qwaylandinputdevice.cpp @@ -168,9 +168,9 @@ QWaylandInputDevice::Keyboard::~Keyboard() wl_keyboard_destroy(object()); } -void QWaylandInputDevice::Keyboard::stopRepeat() +QWaylandWindow *QWaylandInputDevice::Keyboard::focusWindow() const { - mRepeatTimer.stop(); + return mFocus ? QWaylandWindow::fromWlSurface(mFocus) : nullptr; } QWaylandInputDevice::Pointer::Pointer(QWaylandInputDevice *seat) @@ -421,12 +421,6 @@ QWaylandInputDevice::Touch *QWaylandInputDevice::createTouch(QWaylandInputDevice return new Touch(device); } -void QWaylandInputDevice::handleWindowDestroyed(QWaylandWindow *window) -{ - if (mKeyboard && window == mKeyboard->mFocus) - mKeyboard->stopRepeat(); -} - void QWaylandInputDevice::handleEndDrag() { if (mTouch) @@ -470,7 +464,7 @@ QWaylandWindow *QWaylandInputDevice::pointerFocus() const QWaylandWindow *QWaylandInputDevice::keyboardFocus() const { - return mKeyboard ? mKeyboard->mFocus : nullptr; + return mKeyboard ? mKeyboard->focusWindow() : nullptr; } QWaylandWindow *QWaylandInputDevice::touchFocus() const @@ -767,12 +761,19 @@ void QWaylandInputDevice::Keyboard::keyboard_enter(uint32_t time, struct wl_surf Q_UNUSED(time); Q_UNUSED(keys); - if (!surface) + if (!surface) { + // Ignoring wl_keyboard.enter event with null surface. This is either a compositor bug, + // or it's a race with a wl_surface.destroy request. In either case, ignore the event. return; + } + if (mFocus) { + qCWarning(lcQpaWayland()) << "Unexpected wl_keyboard.enter event. Keyboard already has focus"; + disconnect(focusWindow(), &QWaylandWindow::wlSurfaceDestroyed, this, &Keyboard::handleFocusDestroyed); + } - QWaylandWindow *window = QWaylandWindow::fromWlSurface(surface); - mFocus = window; + mFocus = surface; + connect(focusWindow(), &QWaylandWindow::wlSurfaceDestroyed, this, &Keyboard::handleFocusDestroyed); mParent->mQDisplay->handleKeyboardFocusChanged(mParent); } @@ -780,18 +781,20 @@ void QWaylandInputDevice::Keyboard::keyboard_enter(uint32_t time, struct wl_surf void QWaylandInputDevice::Keyboard::keyboard_leave(uint32_t time, struct wl_surface *surface) { Q_UNUSED(time); - Q_UNUSED(surface); - if (surface) { - QWaylandWindow *window = QWaylandWindow::fromWlSurface(surface); - window->unfocus(); + if (!surface) { + // Either a compositor bug, or a race condition with wl_surface.destroy, ignore the event. + return; } - mFocus = nullptr; - - mParent->mQDisplay->handleKeyboardFocusChanged(mParent); - - mRepeatTimer.stop(); + if (surface != mFocus) { + qCWarning(lcQpaWayland) << "Ignoring unexpected wl_keyboard.leave event." + << "wl_surface argument does not match the current focus" + << "This is most likely a compositor bug"; + return; + } + disconnect(focusWindow(), &QWaylandWindow::wlSurfaceDestroyed, this, &Keyboard::handleFocusDestroyed); + handleFocusLost(); } static void sendKey(QWindow *tlw, ulong timestamp, QEvent::Type type, int key, Qt::KeyboardModifiers modifiers, @@ -816,7 +819,7 @@ static void sendKey(QWindow *tlw, ulong timestamp, QEvent::Type type, int key, Q void QWaylandInputDevice::Keyboard::keyboard_key(uint32_t serial, uint32_t time, uint32_t key, uint32_t state) { - QWaylandWindow *window = mFocus; + auto *window = focusWindow(); if (!window) { // We destroyed the keyboard focus surface, but the server didn't get the message yet... // or the server didn't send an enter event first. In either case, ignore the event. @@ -896,14 +899,15 @@ void QWaylandInputDevice::Keyboard::keyboard_key(uint32_t serial, uint32_t time, void QWaylandInputDevice::Keyboard::repeatKey() { - if (!mFocus) { + auto *window = focusWindow(); + if (!window) { // We destroyed the keyboard focus surface, but the server didn't get the message yet... // or the server didn't send an enter event first. return; } mRepeatTimer.setInterval(mRepeatRate); - sendKey(mFocus->window(), mRepeatTime, QEvent::KeyRelease, mRepeatKey, modifiers(), mRepeatCode, + sendKey(window->window(), mRepeatTime, QEvent::KeyRelease, mRepeatKey, modifiers(), mRepeatCode, #if QT_CONFIG(xkbcommon) mRepeatSym, mNativeModifiers, #else @@ -911,7 +915,7 @@ void QWaylandInputDevice::Keyboard::repeatKey() #endif mRepeatText, true); - sendKey(mFocus->window(), mRepeatTime, QEvent::KeyPress, mRepeatKey, modifiers(), mRepeatCode, + sendKey(window->window(), mRepeatTime, QEvent::KeyPress, mRepeatKey, modifiers(), mRepeatCode, #if QT_CONFIG(xkbcommon) mRepeatSym, mNativeModifiers, #else @@ -920,6 +924,27 @@ void QWaylandInputDevice::Keyboard::repeatKey() mRepeatText, true); } +void QWaylandInputDevice::Keyboard::handleFocusDestroyed() +{ + // The signal is emitted by QWaylandWindow, which is not necessarily destroyed along with the + // surface, so we still need to disconnect the signal + auto *window = qobject_cast<QWaylandWindow *>(sender()); + disconnect(window, &QWaylandWindow::wlSurfaceDestroyed, this, &Keyboard::handleFocusDestroyed); + Q_ASSERT(window->object() == mFocus); + handleFocusLost(); +} + +void QWaylandInputDevice::Keyboard::handleFocusLost() +{ + mFocus = nullptr; +#if QT_CONFIG(clipboard) + if (auto *dataDevice = mParent->dataDevice()) + dataDevice->invalidateSelectionOffer(); +#endif + mParent->mQDisplay->handleKeyboardFocusChanged(mParent); + mRepeatTimer.stop(); +} + void QWaylandInputDevice::Keyboard::keyboard_modifiers(uint32_t serial, uint32_t mods_depressed, uint32_t mods_latched, @@ -1021,7 +1046,7 @@ void QWaylandInputDevice::handleTouchPoint(int id, double x, double y, Qt::Touch if (!win && mPointer) win = mPointer->mFocus; if (!win && mKeyboard) - win = mKeyboard->mFocus; + win = mKeyboard->focusWindow(); if (!win || !win->window()) return; diff --git a/src/client/qwaylandinputdevice_p.h b/src/client/qwaylandinputdevice_p.h index e57d7f4fd..cb382da31 100644 --- a/src/client/qwaylandinputdevice_p.h +++ b/src/client/qwaylandinputdevice_p.h @@ -113,7 +113,6 @@ public: #if QT_CONFIG(cursor) void setCursor(const QCursor *cursor, const QSharedPointer<QWaylandBuffer> &cachedBuffer = {}, int fallbackOutputScale = 1); #endif - void handleWindowDestroyed(QWaylandWindow *window); void handleEndDrag(); #if QT_CONFIG(wayland_datadevice) @@ -193,7 +192,7 @@ public: Keyboard(QWaylandInputDevice *p); ~Keyboard() override; - void stopRepeat(); + QWaylandWindow *focusWindow() const; void keyboard_keymap(uint32_t format, int32_t fd, @@ -213,7 +212,7 @@ public: void keyboard_repeat_info(int32_t rate, int32_t delay) override; QWaylandInputDevice *mParent = nullptr; - QPointer<QWaylandWindow> mFocus; + ::wl_surface *mFocus = nullptr; #if QT_CONFIG(xkbcommon) xkb_context *mXkbContext = nullptr; xkb_keymap *mXkbMap = nullptr; @@ -238,6 +237,8 @@ public: private slots: void repeatKey(); + void handleFocusDestroyed(); + void handleFocusLost(); private: #if QT_CONFIG(xkbcommon) diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp index ae4fc5772..ca4d176fe 100644 --- a/src/client/qwaylandwindow.cpp +++ b/src/client/qwaylandwindow.cpp @@ -51,11 +51,6 @@ #include "qwaylanddecorationfactory_p.h" #include "qwaylandshmbackingstore_p.h" -#if QT_CONFIG(wayland_datadevice) -#include "qwaylanddatadevice_p.h" -#endif - - #include <QtCore/QFileInfo> #include <QtCore/QPointer> #include <QtCore/QRegularExpression> @@ -95,10 +90,6 @@ QWaylandWindow::~QWaylandWindow() if (isInitialized()) reset(false); - QList<QWaylandInputDevice *> inputDevices = mDisplay->inputDevices(); - for (int i = 0; i < inputDevices.size(); ++i) - inputDevices.at(i)->handleWindowDestroyed(this); - const QWindow *parent = window(); foreach (QWindow *w, QGuiApplication::topLevelWindows()) { if (w->transientParent() == parent) @@ -236,8 +227,10 @@ void QWaylandWindow::reset(bool sendDestroyEvent) mShellSurface = nullptr; delete mSubSurfaceWindow; mSubSurfaceWindow = nullptr; - if (isInitialized()) + if (isInitialized()) { + emit wlSurfaceDestroyed(); destroy(); + } mScreens.clear(); if (mFrameCallback) { @@ -972,16 +965,6 @@ void QWaylandWindow::requestActivateWindow() qCWarning(lcQpaWayland) << "Wayland does not support QWindow::requestActivate()"; } -void QWaylandWindow::unfocus() -{ -#if QT_CONFIG(clipboard) - QWaylandInputDevice *inputDevice = mDisplay->currentInputDevice(); - if (inputDevice && inputDevice->dataDevice()) { - inputDevice->dataDevice()->invalidateSelectionOffer(); - } -#endif -} - bool QWaylandWindow::isExposed() const { if (mShellSurface) diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h index 0e573f350..8999682d9 100644 --- a/src/client/qwaylandwindow_p.h +++ b/src/client/qwaylandwindow_p.h @@ -153,7 +153,6 @@ public: void requestActivateWindow() override; bool isExposed() const override; bool isActive() const override; - void unfocus(); QWaylandAbstractDecoration *decoration() const; @@ -200,6 +199,9 @@ public: public slots: void applyConfigure(); +signals: + void wlSurfaceDestroyed(); + protected: void surface_enter(struct ::wl_output *output) override; void surface_leave(struct ::wl_output *output) override; |