From 4a6badc22c88a30b10b4e288728981f225cba728 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 16 Jul 2020 10:17:19 +0300 Subject: [PATCH] Ignore setActive() for windows that are being deleted We may call setActive() on a window that is being deleted. We cannot guarantee that at that moment the X11 window or the Wayland surface is still valid. So, the best course of actions is to do nothing. BUG: 424255 --- abstract_client.cpp | 14 ++++++++++++++ abstract_client.h | 3 +++ internal_client.cpp | 1 + x11client.cpp | 15 ++++++--------- x11client.h | 1 - xdgshellclient.cpp | 17 ++++++----------- xdgshellclient.h | 2 -- 7 files changed, 30 insertions(+), 23 deletions(-) diff --git a/abstract_client.cpp b/abstract_client.cpp index 15c24c83e6..8d2a8ea7d8 100644 --- a/abstract_client.cpp +++ b/abstract_client.cpp @@ -201,6 +201,9 @@ void AbstractClient::setIcon(const QIcon &icon) void AbstractClient::setActive(bool act) { + if (isZombie()) { + return; + } if (m_active == act) { return; } @@ -235,6 +238,17 @@ void AbstractClient::doSetActive() { } +bool AbstractClient::isZombie() const +{ + return m_zombie; +} + +void AbstractClient::markAsZombie() +{ + Q_ASSERT(!m_zombie); + m_zombie = true; +} + Layer AbstractClient::layer() const { if (m_layer == UnknownLayer) diff --git a/abstract_client.h b/abstract_client.h index 3c5ae809d0..ca923e325a 100644 --- a/abstract_client.h +++ b/abstract_client.h @@ -358,6 +358,7 @@ public: return m_icon; } + bool isZombie() const; bool isActive() const { return m_active; } @@ -927,6 +928,7 @@ protected: void startAutoRaise(); void autoRaise(); bool isMostRecentlyRaised() const; + void markAsZombie(); /** * Whether the window accepts focus. * The difference to wantsInput is that the implementation should not check rules and return @@ -1237,6 +1239,7 @@ private: bool m_skipSwitcher = false; QIcon m_icon; bool m_active = false; + bool m_zombie = false; bool m_keepAbove = false; bool m_keepBelow = false; bool m_demandsAttention = false; diff --git a/internal_client.cpp b/internal_client.cpp index 4d94037e69..9e9e27c3e8 100644 --- a/internal_client.cpp +++ b/internal_client.cpp @@ -408,6 +408,7 @@ void InternalClient::showOnScreenEdge() void InternalClient::destroyClient() { + markAsZombie(); if (isMoveResize()) { leaveMoveResize(); } diff --git a/x11client.cpp b/x11client.cpp index f63ccc81c2..59e2f39852 100644 --- a/x11client.cpp +++ b/x11client.cpp @@ -146,7 +146,6 @@ X11Client::X11Client() info = nullptr; - deleting = false; m_fullscreenMode = FullScreenNone; hidden = false; noborder = false; @@ -214,8 +213,7 @@ void X11Client::deleteClient(X11Client *c) */ void X11Client::releaseWindow(bool on_shutdown) { - Q_ASSERT(!deleting); - deleting = true; + markAsZombie(); #ifdef KWIN_BUILD_TABBOX TabBox::TabBox *tabBox = TabBox::TabBox::self(); if (tabBox->isDisplayed() && tabBox->currentClient() == this) { @@ -288,8 +286,7 @@ void X11Client::releaseWindow(bool on_shutdown) */ void X11Client::destroyClient() { - Q_ASSERT(!deleting); - deleting = true; + markAsZombie(); #ifdef KWIN_BUILD_TABBOX TabBox::TabBox *tabBox = TabBox::TabBox::self(); if (tabBox && tabBox->isDisplayed() && tabBox->currentClient() == this) { @@ -1099,7 +1096,7 @@ void X11Client::destroyDecoration() move(grav); if (compositing()) discardWindowPixmap(); - if (!deleting) { + if (!isZombie()) { emit geometryShapeChanged(this, oldgeom); } } @@ -1533,7 +1530,7 @@ void X11Client::doSetShade(ShadeMode previousShadeMode) void X11Client::updateVisibility() { - if (deleting) + if (isZombie()) return; if (hidden) { info->setState(NET::Hidden, NET::Hidden); @@ -1579,7 +1576,7 @@ void X11Client::updateVisibility() void X11Client::exportMappingState(int s) { Q_ASSERT(m_client != XCB_WINDOW_NONE); - Q_ASSERT(!deleting || s == XCB_ICCCM_WM_STATE_WITHDRAWN); + Q_ASSERT(!isZombie() || s == XCB_ICCCM_WM_STATE_WITHDRAWN); if (s == XCB_ICCCM_WM_STATE_WITHDRAWN) { m_client.deleteProperty(atoms->wm_state); return; @@ -2202,7 +2199,7 @@ void X11Client::fetchIconicName() void X11Client::setClientShown(bool shown) { - if (deleting) + if (isZombie()) return; // Don't change shown status if this client is being deleted if (shown != hidden) return; // nothing to change diff --git a/x11client.h b/x11client.h index 817aa69e04..8bf6f4c4d8 100644 --- a/x11client.h +++ b/x11client.h @@ -491,7 +491,6 @@ private: xcb_window_t m_transientForId; xcb_window_t m_originalTransientForId; X11Client *shade_below; - uint deleting : 1; ///< True when doing cleanup and destroying the client Xcb::MotifHints m_motif; uint hidden : 1; ///< Forcibly hidden by calling hide() uint noborder : 1; diff --git a/xdgshellclient.cpp b/xdgshellclient.cpp index d8a1655cb4..5ca01e3074 100644 --- a/xdgshellclient.cpp +++ b/xdgshellclient.cpp @@ -188,7 +188,7 @@ bool XdgSurfaceClient::stateCompare() const void XdgSurfaceClient::scheduleConfigure() { - if (isClosing()) { + if (isZombie()) { return; } @@ -518,7 +518,7 @@ void XdgSurfaceClient::addDamage(const QRegion &damage) bool XdgSurfaceClient::isShown(bool shaded_is_shown) const { Q_UNUSED(shaded_is_shown) - return !isClosing() && !isHidden() && !isMinimized(); + return !isZombie() && !isHidden() && !isMinimized(); } bool XdgSurfaceClient::isHiddenInternal() const @@ -564,14 +564,9 @@ void XdgSurfaceClient::internalHide() emit windowHidden(this); } -bool XdgSurfaceClient::isClosing() const -{ - return m_isClosing; -} - void XdgSurfaceClient::destroyClient() { - m_isClosing = true; + markAsZombie(); m_configureTimer->stop(); if (isMoveResize()) { leaveMoveResize(); @@ -1076,7 +1071,7 @@ bool XdgToplevelClient::acceptsFocus() const return m_plasmaShellSurface->panelTakesFocus(); } } - return !isClosing() && readyForPainting(); + return !isZombie() && readyForPainting(); } Layer XdgToplevelClient::layerForDock() const @@ -1427,7 +1422,7 @@ void XdgToplevelClient::installServerDecoration(ServerSideDecorationInterface *d m_serverDecoration = decoration; connect(m_serverDecoration, &ServerSideDecorationInterface::destroyed, this, [this] { - if (!isClosing() && readyForPainting()) { + if (!isZombie() && readyForPainting()) { updateDecoration(/* check_workspace_pos */ true); } }); @@ -1449,7 +1444,7 @@ void XdgToplevelClient::installXdgDecoration(XdgToplevelDecorationV1Interface *d m_xdgDecoration = decoration; connect(m_xdgDecoration, &XdgToplevelDecorationV1Interface::destroyed, this, [this] { - if (!isClosing() && m_isInitialized) { + if (!isZombie() && m_isInitialized) { updateDecoration(/* check_workspace_pos */ true); } }); diff --git a/xdgshellclient.h b/xdgshellclient.h index 9eb216cea3..2e542f4aa5 100644 --- a/xdgshellclient.h +++ b/xdgshellclient.h @@ -82,7 +82,6 @@ public: QRect requestedClientGeometry() const; QSize requestedClientSize() const; QRect clientGeometry() const; - bool isClosing() const; bool isHidden() const; protected: @@ -121,7 +120,6 @@ private: QRect m_requestedFrameGeometry; QRect m_bufferGeometry; QRect m_requestedClientGeometry; - bool m_isClosing = false; bool m_isHidden = false; bool m_haveNextWindowGeometry = false; };