diff --git a/src/core/qsmenu.hpp b/src/core/qsmenu.hpp index b1c9b5a5..9c2f168d 100644 --- a/src/core/qsmenu.hpp +++ b/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; diff --git a/src/dbus/dbusmenu/dbusmenu.cpp b/src/dbus/dbusmenu/dbusmenu.cpp index ae68ecd2..1539500d 100644 --- a/src/dbus/dbusmenu/dbusmenu.cpp +++ b/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(handle) + << ", service=" << handle->service << ", path=" << handle->path << ')'; + } else { + debug << "DBusMenuHandle(nullptr)"; + } + + return debug; +} + } // namespace qs::dbus::dbusmenu diff --git a/src/dbus/dbusmenu/dbusmenu.hpp b/src/dbus/dbusmenu/dbusmenu.hpp index b49f666a..cbfa61f4 100644 --- a/src/dbus/dbusmenu/dbusmenu.hpp +++ b/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 diff --git a/src/services/status_notifier/item.cpp b/src/services/status_notifier/item.cpp index 9a82198b..7f990a9f 100644 --- a/src/services/status_notifier/item.cpp +++ b/src/services/status_notifier/item.cpp @@ -232,48 +232,12 @@ void StatusNotifierItem::updateIcon() { emit this->iconChanged(); } -DBusMenu* StatusNotifierItem::menu() const { return this->mMenu; } - -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(); - } +DBusMenuHandle* StatusNotifierItem::menuHandle() { + return this->menuPath.get().path().isEmpty() ? nullptr : &this->mMenuHandle; } void StatusNotifierItem::onMenuPathChanged() { - qCDebug(logSniMenu) << "Updating menu of" << this << "with refcount" << this->menuRefcount - << "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(); + this->mMenuHandle.setAddress(this->item->service(), this->menuPath.get().path()); } void StatusNotifierItem::onGetAllFinished() { diff --git a/src/services/status_notifier/item.hpp b/src/services/status_notifier/item.hpp index 04cceeff..efe31591 100644 --- a/src/services/status_notifier/item.hpp +++ b/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; - void refMenu(); - void unrefMenu(); + [[nodiscard]] dbus::dbusmenu::DBusMenuHandle* menuHandle(); 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; - quint32 menuRefcount = 0; + dbus::dbusmenu::DBusMenuHandle mMenuHandle {this}; // bumped to inhibit caching quint32 iconIndex = 0; diff --git a/src/services/status_notifier/qml.cpp b/src/services/status_notifier/qml.cpp index e9d1c0ea..854f4d27 100644 --- a/src/services/status_notifier/qml.cpp +++ b/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->menu()) { - this->item->unrefMenu(); + if (!this->item->menuHandle()) { 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(); }); - QObject::connect(platform, &QObject::destroyed, this, [this]() { this->item->unrefMenu(); }); + if (handle->menu()) { + 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, - &StatusNotifierItem::menuChanged, + item->item->menuHandle(), + &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(this->item->item->menuHandle()->menu()); // NOLINT + } else { + return nullptr; + } } void SystemTrayMenuWatcher::onItemDestroyed() {