Client: Move topmost grabbing popup tracking to QWaylandWindow

If the effective transient parent is different from
QWaylandWindow::transientParent(), then the popups may be closed in
wrong order and producing an xdg-shell protocol error.

This change lifts topmost popup tracking from the xdg-shell plugin to
QWaylandWindow so it can guess the correct transient parent and the
xdg-shell plugin doesn't have to pick a different parent behind our
back.

Fixes: QTBUG-119110
Change-Id: I7c5f780b7bd4c3362aa7b22762ff336ae908ff70
Reviewed-by: David Edmundson <davidedmundson@kde.org>
(cherry picked from commit cfaae5d910406ef38d124e8e2c9114e2bfe87cb3)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

4 files changed, 23 insertions, 22 deletions

diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp
index b81e00ef2..de2cafb6f 100644
--- a/src/client/qwaylandwindow.cpp
+++ b/src/client/qwaylandwindow.cpp
@@ -41,6 +41,7 @@ namespace QtWaylandClient {
 Q_LOGGING_CATEGORY(lcWaylandBackingstore, "qt.qpa.wayland.backingstore")
 
 QWaylandWindow *QWaylandWindow::mMouseGrab = nullptr;
+QWaylandWindow *QWaylandWindow::mTopPopup = nullptr;
 
 QWaylandWindow::QWaylandWindow(QWindow *window, QWaylandDisplay *display)
     : QPlatformWindow(window)
@@ -136,7 +137,22 @@ void QWaylandWindow::initWindow()
     } else if (shouldCreateShellSurface()) {
         Q_ASSERT(!mShellSurface);
         Q_ASSERT(mShellIntegration);
-        mTransientParent = closestTransientParent();
+        mTransientParent = guessTransientParent();
+        if (mTransientParent) {
+            if (window()->type() == Qt::Popup) {
+                if (mTopPopup && mTopPopup != mTransientParent) {
+                    qCWarning(lcQpaWayland) << "Creating a popup with a parent," << mTransientParent->window()
+                                            << "which does not match the current topmost grabbing popup,"
+                                            << mTopPopup->window() << "With some shell surface protocols, this"
+                                            << "is not allowed. The wayland QPA plugin is currently handling"
+                                            << "it by setting the parent to the topmost grabbing popup."
+                                            << "Note, however, that this may cause positioning errors and"
+                                            << "popups closing unxpectedly. Please fix the transient parent of the popup.";
+                    mTransientParent = mTopPopup;
+                }
+                mTopPopup = this;
+            }
+        }
 
         mShellSurface = mShellIntegration->createShellSurface(this);
         if (mShellSurface) {
@@ -271,6 +287,9 @@ void QWaylandWindow::reset()
 {
     closeChildPopups();
 
+    if (mTopPopup == this)
+        mTopPopup = mTransientParent && (mTransientParent->window()->type() == Qt::Popup) ? mTransientParent : nullptr;
+
     if (mSurface) {
         emit wlSurfaceDestroyed();
         QWriteLocker lock(&mSurfaceLock);
@@ -1120,7 +1139,7 @@ QWaylandWindow *QWaylandWindow::transientParent() const
     return mTransientParent;
 }
 
-QWaylandWindow *QWaylandWindow::closestTransientParent() const
+QWaylandWindow *QWaylandWindow::guessTransientParent() const
 {
     // Take the closest window with a shell surface, since the transient parent may be a
     // QWidgetWindow or some other window without a shell surface, which is then not able to
diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h
index fbf629066..862366ea5 100644
--- a/src/client/qwaylandwindow_p.h
+++ b/src/client/qwaylandwindow_p.h
@@ -349,7 +349,7 @@ private:
     void handleScreensChanged();
     void sendRecursiveExposeEvent();
 
-    QWaylandWindow *closestTransientParent() const;
+    QWaylandWindow *guessTransientParent() const;
     void addChildPopup(QWaylandWindow *child);
     void removeChildPopup(QWaylandWindow *child);
 
@@ -361,6 +361,7 @@ private:
     void handleFrameCallback(struct ::wl_callback* callback);
 
     static QWaylandWindow *mMouseGrab;
+    static QWaylandWindow *mTopPopup;
 
     friend class QWaylandSubSurface;
 };
diff --git a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp
index e5dde262c..f6e3e08a5 100644
--- a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp
+++ b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp
@@ -221,9 +221,6 @@ QWaylandXdgSurface::Popup::~Popup()
         destroy();
 
     if (m_grabbing) {
-        auto *shell = m_xdgSurface->m_shell;
-        Q_ASSERT(shell->m_topmostGrabbingPopup == this);
-        shell->m_topmostGrabbingPopup = m_parentXdgSurface ? m_parentXdgSurface->m_popup : nullptr;
         m_grabbing = false;
 
         // Synthesize Qt enter/leave events for popup
@@ -255,7 +252,6 @@ void QWaylandXdgSurface::Popup::resetConfiguration()
 
 void QWaylandXdgSurface::Popup::grab(QWaylandInputDevice *seat, uint serial)
 {
-    m_xdgSurface->m_shell->m_topmostGrabbingPopup = this;
     xdg_popup::grab(seat->wl_seat(), serial);
     m_grabbing = true;
 }
@@ -566,20 +562,6 @@ void QWaylandXdgSurface::setPopup(QWaylandWindow *parent)
 
 void QWaylandXdgSurface::setGrabPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial)
 {
-    auto parentXdgSurface = qobject_cast<QWaylandXdgSurface *>(parent->shellSurface());
-    auto *top = m_shell->m_topmostGrabbingPopup;
-
-    if (top && top->m_xdgSurface != parentXdgSurface) {
-        qCWarning(lcQpaWayland) << "setGrabPopup called with a parent," << parentXdgSurface
-                                << "which does not match the current topmost grabbing popup,"
-                                << top->m_xdgSurface << "According to the xdg-shell protocol, this"
-                                << "is not allowed. The wayland QPA plugin is currently handling"
-                                << "it by setting the parent to the topmost grabbing popup."
-                                << "Note, however, that this may cause positioning errors and"
-                                << "popups closing unxpectedly because xdg-shell mandate that child"
-                                << "popups close before parents";
-        parent = top->m_xdgSurface->m_window;
-    }
     setPopup(parent);
     m_popup->grab(device, serial);
 
diff --git a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h
index 951e8234f..e2dc12dd4 100644
--- a/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h
+++ b/src/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h
@@ -171,7 +171,6 @@ private:
     QScopedPointer<QWaylandXdgDecorationManagerV1> m_xdgDecorationManager;
     QScopedPointer<QWaylandXdgActivationV1> m_xdgActivation;
     QScopedPointer<QWaylandXdgExporterV2> m_xdgExporter;
-    QWaylandXdgSurface::Popup *m_topmostGrabbingPopup = nullptr;
 
     friend class QWaylandXdgSurface;
 };