hyprland/focus_grab: handle destruction of tracked windows

changelog/next.md

@@ -60,6 +60,7 @@ set shell id.
 - Desktop action order is now preserved.
 - Fixed partial socket reads in greetd and hyprland on slow machines.
 - Worked around Qt bug causing crashes when plugging and unplugging monitors.
+- Fixed HyprlandFocusGrab crashing if windows were destroyed after being passed to it.
 
 ## Packaging Changes
 

src/core/platformmenu.cpp

@@ -18,7 +18,6 @@
 #include <qwindow.h>
 
 #include "../window/proxywindow.hpp"
-#include "../window/windowinterface.hpp"
 #include "iconprovider.hpp"
 #include "model.hpp"
 #include "platformmenu_p.hpp"
@@ -91,10 +90,8 @@ bool PlatformMenuEntry::display(QObject* parentWindow, int relativeX, int relati
 	} else if (parentWindow == nullptr) {
 		qCritical() << "Cannot display PlatformMenuEntry with null parent window.";
 		return false;
-	} else if (auto* proxy = qobject_cast<ProxyWindowBase*>(parentWindow)) {
+	} else if (auto* proxy = ProxyWindowBase::forObject(parentWindow)) {
 		window = proxy->backingWindow();
-	} else if (auto* interface = qobject_cast<WindowInterface*>(parentWindow)) {
-		window = interface->proxyWindow()->backingWindow();
 	} else {
 		qCritical() << "PlatformMenuEntry.display() must be called with a window.";
 		return false;

src/core/popupanchor.cpp

@@ -11,7 +11,6 @@
 #include <qwindow.h>
 
 #include "../window/proxywindow.hpp"
-#include "../window/windowinterface.hpp"
 #include "types.hpp"
 
 bool PopupAnchorState::operator==(const PopupAnchorState& other) const {
@@ -40,10 +39,8 @@ void PopupAnchor::setWindowInternal(QObject* window) {
 	}
 
 	if (window) {
-		if (auto* proxy = qobject_cast<ProxyWindowBase*>(window)) {
+		if (auto* proxy = ProxyWindowBase::forObject(window)) {
 			this->bProxyWindow = proxy;
-		} else if (auto* interface = qobject_cast<WindowInterface*>(window)) {
-			this->bProxyWindow = interface->proxyWindow();
 		} else {
 			qWarning() << "Tried to set popup anchor window to" << window
 			           << "which is not a quickshell window.";

src/wayland/hyprland/focus_grab/qml.cpp

@@ -9,7 +9,6 @@
 #include <qwindow.h>
 
 #include "../../../window/proxywindow.hpp"
-#include "../../../window/windowinterface.hpp"
 #include "grab.hpp"
 #include "manager.hpp"
 
@@ -38,8 +37,51 @@ QObjectList HyprlandFocusGrab::windows() const { return this->windowObjects; }
 
 void HyprlandFocusGrab::setWindows(QObjectList windows) {
 	if (windows == this->windowObjects) return;
+	if (this->grab) this->grab->startTransaction();
+
+	for (auto* obj: this->windowObjects) {
+		if (windows.contains(obj)) continue;
+		QObject::disconnect(obj, nullptr, this, nullptr);
+
+		auto* proxy = ProxyWindowBase::forObject(obj);
+		if (!proxy) continue;
+
+		QObject::disconnect(proxy, nullptr, this, nullptr);
+
+		if (this->grab && proxy->backingWindow()) {
+			this->grab->removeWindow(proxy->backingWindow());
+		}
+	}
+
+	for (auto it = windows.begin(); it != windows.end();) {
+		auto* proxy = ProxyWindowBase::forObject(*it);
+		if (!proxy) {
+			it = windows.erase(it);
+			continue;
+		}
+
+		if (this->windowObjects.contains(*it)) {
+			++it;
+			continue;
+		}
+
+		QObject::connect(*it, &QObject::destroyed, this, &HyprlandFocusGrab::onObjectDestroyed);
+		QObject::connect(
+		    proxy,
+		    &ProxyWindowBase::windowConnected,
+		    this,
+		    &HyprlandFocusGrab::onProxyConnected
+		);
+
+		if (this->grab && proxy->backingWindow()) {
+			this->grab->addWindow(proxy->backingWindow());
+		}
+
+		++it;
+	}
+
+	if (this->grab) this->grab->completeTransaction();
 	this->windowObjects = std::move(windows);
-	this->syncWindows();
 	emit this->windowsChanged();
 }
 
@@ -75,59 +117,18 @@ void HyprlandFocusGrab::tryActivate() {
 	QObject::connect(this->grab, &FocusGrab::cleared, this, &HyprlandFocusGrab::onGrabCleared);
 
 	this->grab->startTransaction();
-	for (auto* proxy: this->trackedProxies) {
+	for (auto* obj: this->windowObjects) {
-		if (proxy->backingWindow() != nullptr) {
+		auto* proxy = ProxyWindowBase::forObject(obj);
+		if (proxy && proxy->backingWindow()) {
 			this->grab->addWindow(proxy->backingWindow());
 		}
 	}
 	this->grab->completeTransaction();
 }
 
-void HyprlandFocusGrab::syncWindows() {
+void HyprlandFocusGrab::onObjectDestroyed(QObject* object) {
-	auto newProxy = QList<ProxyWindowBase*>();
+	this->windowObjects.removeOne(object);
-	for (auto* windowObject: this->windowObjects) {
+	emit this->windowsChanged();
-		auto* proxyWindow = qobject_cast<ProxyWindowBase*>(windowObject);
-
-		if (proxyWindow == nullptr) {
-			if (auto* iface = qobject_cast<WindowInterface*>(windowObject)) {
-				proxyWindow = iface->proxyWindow();
-			}
-		}
-
-		if (proxyWindow != nullptr) {
-			newProxy.push_back(proxyWindow);
-		}
-	}
-
-	if (this->grab) this->grab->startTransaction();
-
-	for (auto* oldWindow: this->trackedProxies) {
-		if (!newProxy.contains(oldWindow)) {
-			QObject::disconnect(oldWindow, nullptr, this, nullptr);
-
-			if (this->grab != nullptr && oldWindow->backingWindow() != nullptr) {
-				this->grab->removeWindow(oldWindow->backingWindow());
-			}
-		}
-	}
-
-	for (auto* newProxy: newProxy) {
-		if (!this->trackedProxies.contains(newProxy)) {
-			QObject::connect(
-			    newProxy,
-			    &ProxyWindowBase::windowConnected,
-			    this,
-			    &HyprlandFocusGrab::onProxyConnected
-			);
-
-			if (this->grab != nullptr && newProxy->backingWindow() != nullptr) {
-				this->grab->addWindow(newProxy->backingWindow());
-			}
-		}
-	}
-
-	this->trackedProxies = newProxy;
-	if (this->grab) this->grab->completeTransaction();
 }
 
 } // namespace qs::hyprland

src/wayland/hyprland/focus_grab/qml.hpp

@@ -96,15 +96,13 @@ private slots:
 	void onGrabActivated();
 	void onGrabCleared();
 	void onProxyConnected();
+	void onObjectDestroyed(QObject* object);
 
 private:
 	void tryActivate();
-	void syncWindows();
 
 	bool targetActive = false;
 	QObjectList windowObjects;
-	QList<ProxyWindowBase*> trackedProxies;
-	QList<QWindow*> trackedWindows;
 
 	focus_grab::FocusGrab* grab = nullptr;
 };

src/wayland/hyprland/surface/qml.cpp

@@ -14,7 +14,6 @@
 
 #include "../../../core/region.hpp"
 #include "../../../window/proxywindow.hpp"
-#include "../../../window/windowinterface.hpp"
 #include "manager.hpp"
 #include "surface.hpp"
 
@@ -23,13 +22,7 @@ using QtWaylandClient::QWaylandWindow;
 namespace qs::hyprland::surface {
 
 HyprlandWindow* HyprlandWindow::qmlAttachedProperties(QObject* object) {
-	auto* proxyWindow = qobject_cast<ProxyWindowBase*>(object);
+	auto* proxyWindow = ProxyWindowBase::forObject(object);
-
-	if (!proxyWindow) {
-		if (auto* iface = qobject_cast<WindowInterface*>(object)) {
-			proxyWindow = iface->proxyWindow();
-		}
-	}
 
 	if (!proxyWindow) return nullptr;
 	return new HyprlandWindow(proxyWindow);

src/wayland/idle_inhibit/inhibitor.cpp

@@ -6,7 +6,6 @@
 #include <qtmetamacros.h>
 
 #include "../../window/proxywindow.hpp"
-#include "../../window/windowinterface.hpp"
 #include "proto.hpp"
 
 namespace qs::wayland::idle_inhibit {
@@ -25,27 +24,13 @@ QObject* IdleInhibitor::window() const { return this->bWindowObject; }
 void IdleInhibitor::setWindow(QObject* window) {
 	if (window == this->bWindowObject) return;
 
-	auto* proxyWindow = qobject_cast<ProxyWindowBase*>(window);
+	auto* proxyWindow = ProxyWindowBase::forObject(window);
-
-	if (proxyWindow == nullptr) {
-		if (auto* iface = qobject_cast<WindowInterface*>(window)) {
-			proxyWindow = iface->proxyWindow();
-		}
-	}
-
 	this->bWindowObject = proxyWindow ? window : nullptr;
 }
 
 void IdleInhibitor::boundWindowChanged() {
 	auto* window = this->bBoundWindow.value();
-	auto* proxyWindow = qobject_cast<ProxyWindowBase*>(window);
+	auto* proxyWindow = ProxyWindowBase::forObject(window);
-
-	if (proxyWindow == nullptr) {
-		if (auto* iface = qobject_cast<WindowInterface*>(window)) {
-			proxyWindow = iface->proxyWindow();
-		}
-	}
-
 	if (proxyWindow == this->proxyWindow) return;
 
 	if (this->mWaylandWindow) {

src/wayland/shortcuts_inhibit/inhibitor.cpp

@@ -9,7 +9,6 @@
 #include <qwindow.h>
 
 #include "../../window/proxywindow.hpp"
-#include "../../window/windowinterface.hpp"
 #include "proto.hpp"
 
 namespace qs::wayland::shortcuts_inhibit {
@@ -48,14 +47,7 @@ ShortcutInhibitor::~ShortcutInhibitor() {
 
 void ShortcutInhibitor::onBoundWindowChanged() {
 	auto* window = this->bBoundWindow.value();
-	auto* proxyWindow = qobject_cast<ProxyWindowBase*>(window);
+	auto* proxyWindow = ProxyWindowBase::forObject(window);
-
-	if (!proxyWindow) {
-		if (auto* iface = qobject_cast<WindowInterface*>(window)) {
-			proxyWindow = iface->proxyWindow();
-		}
-	}
-
 	if (proxyWindow == this->proxyWindow) return;
 
 	if (this->proxyWindow) {

src/wayland/toplevel_management/qml.cpp

@@ -9,7 +9,6 @@
 #include "../../core/qmlscreen.hpp"
 #include "../../core/util.hpp"
 #include "../../window/proxywindow.hpp"
-#include "../../window/windowinterface.hpp"
 #include "../output_tracking.hpp"
 #include "handle.hpp"
 #include "manager.hpp"
@@ -73,13 +72,7 @@ void Toplevel::fullscreenOn(QuickshellScreenInfo* screen) {
 }
 
 void Toplevel::setRectangle(QObject* window, QRect rect) {
-	auto* proxyWindow = qobject_cast<ProxyWindowBase*>(window);
+	auto* proxyWindow = ProxyWindowBase::forObject(window);
-
-	if (proxyWindow == nullptr) {
-		if (auto* iface = qobject_cast<WindowInterface*>(window)) {
-			proxyWindow = iface->proxyWindow();
-		}
-	}
 
 	if (proxyWindow != this->rectWindow) {
 		if (this->rectWindow != nullptr) {

src/window/proxywindow.cpp

@@ -57,6 +57,12 @@ ProxyWindowBase::ProxyWindowBase(QObject* parent)
 
 ProxyWindowBase::~ProxyWindowBase() { this->deleteWindow(true); }
 
+ProxyWindowBase* ProxyWindowBase::forObject(QObject* obj) {
+	if (auto* proxy = qobject_cast<ProxyWindowBase*>(obj)) return proxy;
+	if (auto* iface = qobject_cast<WindowInterface*>(obj)) return iface->proxyWindow();
+	return nullptr;
+}
+
 void ProxyWindowBase::onReload(QObject* oldInstance) {
 	if (this->mVisible) this->window = this->retrieveWindow(oldInstance);
 	auto wasVisible = this->window != nullptr && this->window->isVisible();

src/window/proxywindow.hpp

@@ -66,6 +66,8 @@ public:
 	explicit ProxyWindowBase(QObject* parent = nullptr);
 	~ProxyWindowBase() override;
 
+	static ProxyWindowBase* forObject(QObject* obj);
+
 	ProxyWindowBase(ProxyWindowBase&) = delete;
 	ProxyWindowBase(ProxyWindowBase&&) = delete;
 	void operator=(ProxyWindowBase&) = delete;