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>

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;