dbus/dbusmenu: separate menu handles from status notifier items

No api changes yet.

src/core/qsmenu.hpp

@@ -111,6 +111,23 @@ private:
 	qsizetype refcount = 0;
 };
 
+class QsMenuHandle: public QObject {
+	Q_OBJECT;
+	QML_ELEMENT;
+	QML_UNCREATABLE("");
+
+public:
+	explicit QsMenuHandle(QObject* parent): QObject(parent) {}
+
+	virtual void ref() {};
+	virtual void unref() {};
+
+	[[nodiscard]] virtual QsMenuEntry* menu() const = 0;
+
+signals:
+	void menuChanged();
+};
+
 ///! Provides access to children of a QsMenuEntry
 class QsMenuOpener: public QObject {
 	Q_OBJECT;

src/dbus/dbusmenu/dbusmenu.cpp

@@ -512,4 +512,72 @@ DBusMenuPngImage::requestImage(const QString& /*unused*/, QSize* size, const QSi
 	return image;
 }
 
+void DBusMenuHandle::setAddress(const QString& service, const QString& path) {
+	if (service == this->service && path == this->path) return;
+	this->service = service;
+	this->path = path;
+	this->onMenuPathChanged();
+}
+
+void DBusMenuHandle::ref() {
+	this->refcount++;
+	qCDebug(logDbusMenu) << this << "gained a reference. Refcount is now" << this->refcount;
+
+	if (this->refcount == 1 || !this->mMenu) {
+		this->onMenuPathChanged();
+	} else {
+		// Refresh the layout when opening a menu in case a bad client isn't updating it
+		// and another ref is open somewhere.
+		this->mMenu->rootItem.updateLayout();
+	}
+}
+
+void DBusMenuHandle::unref() {
+	this->refcount--;
+	qCDebug(logDbusMenu) << this << "lost a reference. Refcount is now" << this->refcount;
+
+	if (this->refcount == 0) {
+		this->onMenuPathChanged();
+	}
+}
+
+void DBusMenuHandle::onMenuPathChanged() {
+	qCDebug(logDbusMenu) << "Updating" << this << "with refcount" << this->refcount;
+
+	if (this->mMenu) {
+		this->mMenu->deleteLater();
+		this->mMenu = nullptr;
+		this->loaded = false;
+		emit this->menuChanged();
+	}
+
+	if (this->refcount > 0 && !this->service.isEmpty() && !this->path.isEmpty()) {
+		this->mMenu = new DBusMenu(this->service, this->path);
+		this->mMenu->setParent(this);
+
+		QObject::connect(&this->mMenu->rootItem, &DBusMenuItem::layoutUpdated, this, [this]() {
+			this->loaded = true;
+			emit this->menuChanged();
+		});
+
+		this->mMenu->rootItem.setShowChildrenRecursive(true);
+	}
+}
+
+QsMenuEntry* DBusMenuHandle::menu() const {
+	return this->loaded ? &this->mMenu->rootItem : nullptr;
+}
+
+QDebug operator<<(QDebug debug, const DBusMenuHandle* handle) {
+	if (handle) {
+		auto saver = QDebugStateSaver(debug);
+		debug.nospace() << "DBusMenuHandle(" << static_cast<const void*>(handle)
+		                << ", service=" << handle->service << ", path=" << handle->path << ')';
+	} else {
+		debug << "DBusMenuHandle(nullptr)";
+	}
+
+	return debug;
+}
+
 } // namespace qs::dbus::dbusmenu

src/dbus/dbusmenu/dbusmenu.hpp

@@ -157,4 +157,35 @@ public:
 	QByteArray data;
 };
 
+class DBusMenuHandle;
+
+QDebug operator<<(QDebug debug, const DBusMenuHandle* handle);
+
+class DBusMenuHandle: public menu::QsMenuHandle {
+	Q_OBJECT;
+	QML_ELEMENT;
+	QML_UNCREATABLE("");
+
+public:
+	explicit DBusMenuHandle(QObject* parent): menu::QsMenuHandle(parent) {}
+
+	void setAddress(const QString& service, const QString& path);
+
+	void ref() override;
+	void unref() override;
+
+	[[nodiscard]] QsMenuEntry* menu() const override;
+
+private:
+	void onMenuPathChanged();
+
+	QString service;
+	QString path;
+	DBusMenu* mMenu = nullptr;
+	bool loaded = false;
+	quint32 refcount = 0;
+
+	friend QDebug operator<<(QDebug debug, const DBusMenuHandle* handle);
+};
+
 } // namespace qs::dbus::dbusmenu

src/services/status_notifier/item.cpp

@@ -232,48 +232,12 @@ void StatusNotifierItem::updateIcon() {
 	emit this->iconChanged();
 }
 
-DBusMenu* StatusNotifierItem::menu() const { return this->mMenu; }
+DBusMenuHandle* StatusNotifierItem::menuHandle() {
-
+	return this->menuPath.get().path().isEmpty() ? nullptr : &this->mMenuHandle;
-void StatusNotifierItem::refMenu() {
-	this->menuRefcount++;
-	qCDebug(logSniMenu) << "Menu of" << this << "gained a reference. Refcount is now"
-	                    << this->menuRefcount;
-
-	if (this->menuRefcount == 1) {
-		this->onMenuPathChanged();
-	} else {
-		// Refresh the layout when opening a menu in case a bad client isn't updating it
-		// and another ref is open somewhere.
-		this->mMenu->rootItem.updateLayout();
-	}
-}
-
-void StatusNotifierItem::unrefMenu() {
-	this->menuRefcount--;
-	qCDebug(logSniMenu) << "Menu of" << this << "lost a reference. Refcount is now"
-	                    << this->menuRefcount;
-
-	if (this->menuRefcount == 0) {
-		this->onMenuPathChanged();
-	}
 }
 
 void StatusNotifierItem::onMenuPathChanged() {
-	qCDebug(logSniMenu) << "Updating menu of" << this << "with refcount" << this->menuRefcount
+	this->mMenuHandle.setAddress(this->item->service(), this->menuPath.get().path());
-	                    << "path" << this->menuPath.get().path();
-
-	if (this->mMenu) {
-		this->mMenu->deleteLater();
-		this->mMenu = nullptr;
-	}
-
-	if (this->menuRefcount > 0 && !this->menuPath.get().path().isEmpty()) {
-		this->mMenu = new DBusMenu(this->item->service(), this->menuPath.get().path());
-		this->mMenu->setParent(this);
-		this->mMenu->rootItem.setShowChildrenRecursive(true);
-	}
-
-	emit this->menuChanged();
 }
 
 void StatusNotifierItem::onGetAllFinished() {

src/services/status_notifier/item.hpp

@@ -42,9 +42,7 @@ public:
 	[[nodiscard]] QString iconId() const;
 	[[nodiscard]] QPixmap createPixmap(const QSize& size) const;
 
-	[[nodiscard]] qs::dbus::dbusmenu::DBusMenu* menu() const;
+	[[nodiscard]] dbus::dbusmenu::DBusMenuHandle* menuHandle();
-	void refMenu();
-	void unrefMenu();
 
 	void activate();
 	void secondaryActivate();
@@ -73,7 +71,6 @@ public:
 signals:
 	void iconChanged();
 	void ready();
-	void menuChanged();
 
 private slots:
 	void updateIcon();
@@ -87,8 +84,7 @@ private:
 	TrayImageHandle imageHandle {this};
 	bool mReady = false;
 
-	dbus::dbusmenu::DBusMenu* mMenu = nullptr;
+	dbus::dbusmenu::DBusMenuHandle mMenuHandle {this};
-	quint32 menuRefcount = 0;
 
 	// bumped to inhibit caching
 	quint32 iconIndex = 0;

src/services/status_notifier/qml.cpp

@@ -20,6 +20,7 @@ using namespace qs::dbus;
 using namespace qs::dbus::dbusmenu;
 using namespace qs::service::sni;
 using namespace qs::menu::platform;
+using qs::menu::QsMenuHandle;
 
 SystemTrayItem::SystemTrayItem(qs::service::sni::StatusNotifierItem* item, QObject* parent)
     : QObject(parent)
@@ -108,25 +109,41 @@ void SystemTrayItem::scroll(qint32 delta, bool horizontal) const {
 }
 
 void SystemTrayItem::display(QObject* parentWindow, qint32 relativeX, qint32 relativeY) {
-	this->item->refMenu();
+	if (!this->item->menuHandle()) {
-	if (!this->item->menu()) {
-		this->item->unrefMenu();
 		qCritical() << "No menu present for" << this;
 		return;
 	}
 
-	auto* platform = new PlatformMenuEntry(&this->item->menu()->rootItem);
+	auto* handle = this->item->menuHandle();
+
+	auto onMenuChanged = [this, parentWindow, relativeX, relativeY, handle]() {
+		QObject::disconnect(handle, nullptr, this, nullptr);
+
+		if (!handle->menu()) {
+			handle->unref();
+			return;
+		}
+
+		auto* platform = new PlatformMenuEntry(handle->menu());
+
+		// clang-format off
+		QObject::connect(platform, &PlatformMenuEntry::closed, this, [=]() { platform->deleteLater(); });
+		QObject::connect(platform, &QObject::destroyed, this, [=]() { handle->unref(); });
+		// clang-format on
 
-	QObject::connect(&this->item->menu()->rootItem, &DBusMenuItem::layoutUpdated, platform, [=]() {
-		platform->relayout();
 		auto success = platform->display(parentWindow, relativeX, relativeY);
 
 		// calls destroy which also unrefs
 		if (!success) delete platform;
-	});
+	};
 
-	QObject::connect(platform, &PlatformMenuEntry::closed, this, [=]() { platform->deleteLater(); });
+	if (handle->menu()) {
-	QObject::connect(platform, &QObject::destroyed, this, [this]() { this->item->unrefMenu(); });
+		onMenuChanged();
+	} else {
+		QObject::connect(handle, &QsMenuHandle::menuChanged, this, onMenuChanged);
+	}
+
+	handle->ref();
 }
 
 SystemTray::SystemTray(QObject* parent): QObject(parent) {
@@ -162,7 +179,7 @@ SystemTrayItem* SystemTrayMenuWatcher::trayItem() const { return this->item; }
 
 SystemTrayMenuWatcher::~SystemTrayMenuWatcher() {
 	if (this->item != nullptr) {
-		this->item->item->unrefMenu();
+		this->item->item->menuHandle()->unref();
 	}
 }
 
@@ -170,20 +187,20 @@ void SystemTrayMenuWatcher::setTrayItem(SystemTrayItem* item) {
 	if (item == this->item) return;
 
 	if (this->item != nullptr) {
-		this->item->item->unrefMenu();
+		this->item->item->menuHandle()->unref();
 		QObject::disconnect(this->item, nullptr, this, nullptr);
 	}
 
 	this->item = item;
 
 	if (item != nullptr) {
-		this->item->item->refMenu();
+		this->item->item->menuHandle()->ref();
 
 		QObject::connect(item, &QObject::destroyed, this, &SystemTrayMenuWatcher::onItemDestroyed);
 
 		QObject::connect(
-		    item->item,
+		    item->item->menuHandle(),
-		    &StatusNotifierItem::menuChanged,
+		    &DBusMenuHandle::menuChanged,
 		    this,
 		    &SystemTrayMenuWatcher::menuChanged
 		);
@@ -194,7 +211,11 @@ void SystemTrayMenuWatcher::setTrayItem(SystemTrayItem* item) {
 }
 
 DBusMenuItem* SystemTrayMenuWatcher::menu() const {
-	return this->item ? &this->item->item->menu()->rootItem : nullptr;
+	if (this->item) {
+		return static_cast<DBusMenuItem*>(this->item->item->menuHandle()->menu()); // NOLINT
+	} else {
+		return nullptr;
+	}
 }
 
 void SystemTrayMenuWatcher::onItemDestroyed() {