From e1eec6874231c4405dcd786d56318d9df2780ed3 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Mon, 4 Feb 2019 16:34:52 +0100 Subject: [PATCH] Client xdg-shell: Fix crash when switching popups The call to flushWindowSystemEvents() sometimes caused new popups to be shown in the middle of hiding another. I.e. if multiple events show and hide surfaces, they would be shown/hidden in opposite order of their corresponding events. QMenus sometimes suffered from this (can be seen with the qopenglwidget example from qtbase). When the flush was added 5 years ago in 70c578cb5, it was to "reduce the chances of seeing a bad frame". I don't see any rendering artifacts, though, and I can't find any bug report on it. So let's hope it's safe to remove the hack. [ChangeLog][QPA plugin] Fixed a crash that sometimes happened when switching popups. Also adds more info to the workaround warning message that appears when a popup grab is attempted and there already is another grabbing popup that is not the parent. Fixes: QTBUG-73524 Change-Id: Ibfcbb48c4bbe295c2be1a30add2d7e05cad398c5 Reviewed-by: Paul Olav Tvete --- .../shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp | 9 ++++++++- .../shellintegration/xdg-shell/qwaylandxdgshell.cpp | 9 ++++++++- src/plugins/platforms/wayland/qwaylandwindow.cpp | 9 ++------- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp index 9e55e3e1680..2f729d9717b 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell-v6/qwaylandxdgshellv6.cpp @@ -335,7 +335,14 @@ void QWaylandXdgSurfaceV6::setGrabPopup(QWaylandWindow *parent, QWaylandInputDev auto *top = m_shell->m_topmostGrabbingPopup; if (top && top->m_xdgSurface != parentXdgSurface) { - qCWarning(lcQpaWayland) << "setGrabPopup called for a surface that was not the topmost popup, positions might be off."; + 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-v6 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-v6 mandate that child" + << "popups close before parents"; parent = top->m_xdgSurface->m_window; } setPopup(parent); diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp index 1310e340da2..d2452aa483d 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp @@ -369,7 +369,14 @@ void QWaylandXdgSurface::setGrabPopup(QWaylandWindow *parent, QWaylandInputDevic auto *top = m_shell->m_topmostGrabbingPopup; if (top && top->m_xdgSurface != parentXdgSurface) { - qCWarning(lcQpaWayland) << "setGrabPopup called for a surface that was not the topmost popup, positions might be off."; + 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); diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index 282179efb53..c46c49813fb 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -395,14 +395,9 @@ void QWaylandWindow::setVisible(bool visible) // QWaylandShmBackingStore::beginPaint(). } else { sendExposeEvent(QRect()); - // when flushing the event queue, it could contain a close event, in which - // case 'this' will be deleted. When that happens, we must abort right away. - QPointer deleteGuard(this); - QWindowSystemInterface::flushWindowSystemEvents(); - if (!deleteGuard.isNull() && window()->type() == Qt::Popup) + if (window()->type() == Qt::Popup) closePopups(this); - if (!deleteGuard.isNull()) - reset(); + reset(); } }