From 3df458677d542fea23e00cdf19a1e8d77647a51e Mon Sep 17 00:00:00 2001 From: Kai Uwe Broulik Date: Thu, 1 Mar 2018 09:28:50 +0100 Subject: [PATCH] Split Actions handling into a dedicated class --- gmenu-dbusmenu-proxy/CMakeLists.txt | 1 + gmenu-dbusmenu-proxy/menu.cpp | 284 +++++----------------------- gmenu-dbusmenu-proxy/menu.h | 16 +- 3 files changed, 60 insertions(+), 241 deletions(-) diff --git a/gmenu-dbusmenu-proxy/CMakeLists.txt b/gmenu-dbusmenu-proxy/CMakeLists.txt index 583c51c01..c0f5a6782 100644 --- a/gmenu-dbusmenu-proxy/CMakeLists.txt +++ b/gmenu-dbusmenu-proxy/CMakeLists.txt @@ -14,6 +14,7 @@ set(GMENU_DBUSMENU_PROXY_SRCS main.cpp menuproxy.cpp menu.cpp + actions.cpp gdbusmenutypes_p.cpp icons.cpp ../libdbusmenuqt/dbusmenutypes_p.cpp diff --git a/gmenu-dbusmenu-proxy/menu.cpp b/gmenu-dbusmenu-proxy/menu.cpp index c576eeebf..d815d7610 100644 --- a/gmenu-dbusmenu-proxy/menu.cpp +++ b/gmenu-dbusmenu-proxy/menu.cpp @@ -34,6 +34,7 @@ #include +#include "actions.h" #include "dbusmenuadaptor.h" #include "icons.h" @@ -42,6 +43,10 @@ static const QString s_orgGtkActions = QStringLiteral("org.gtk.Actions"); static const QString s_orgGtkMenus = QStringLiteral("org.gtk.Menus"); +static const QString s_applicationActionsPrefix = QStringLiteral("app."); +static const QString s_unityActionsPrefix = QStringLiteral("unity."); +static const QString s_windowActionsPrefix = QStringLiteral("win."); + Menu::Menu(const QString &serviceName) : QObject() , m_serviceName(serviceName) @@ -78,72 +83,49 @@ void Menu::init() qCWarning(DBUSMENUPROXY) << "Failed to subscribe to menu bar changes on" << m_serviceName << "at" << m_menuBarObjectPath; } - if (!m_applicationObjectPath.isEmpty() && !QDBusConnection::sessionBus().connect(m_serviceName, - m_applicationObjectPath, - s_orgGtkActions, - QStringLiteral("Changed"), - this, - SLOT(onApplicationActionsChanged(QStringList,StringBoolMap,QVariantMap,GMenuActionMap)))) { - qCWarning(DBUSMENUPROXY) << "Failed to subscribe to application action changes on" << m_serviceName << "at" << m_applicationObjectPath; - } - - if (!m_unityObjectPath.isEmpty() && !QDBusConnection::sessionBus().connect(m_serviceName, - m_unityObjectPath, - s_orgGtkActions, - QStringLiteral("Changed"), - this, - SLOT(onUnityActionsChanged(QStringList,StringBoolMap,QVariantMap,GMenuActionMap)))) { - qCWarning(DBUSMENUPROXY) << "Failed to subscribe to Unity action changes on" << m_serviceName << "at" << m_applicationObjectPath; - } - - if (!m_windowObjectPath.isEmpty() && !QDBusConnection::sessionBus().connect(m_serviceName, - m_windowObjectPath, - s_orgGtkActions, - QStringLiteral("Changed"), - this, - SLOT(onWindowActionsChanged(QStringList,StringBoolMap,QVariantMap,GMenuActionMap)))) { - qCWarning(DBUSMENUPROXY) << "Failed to subscribe to window action changes on" << m_serviceName << "at" << m_windowObjectPath; - } - - // TODO share application actions between menus of the same app? if (!m_applicationObjectPath.isEmpty()) { - getActions(m_applicationObjectPath, [this](const GMenuActionMap &actions, bool ok) { - if (ok) { - // TODO just do all of this in getActions instead of copying it thrice - if (m_menuInited) { - onApplicationActionsChanged({}, {}, {}, actions); - } else { - m_applicationActions = actions; - initMenu(); - } + m_applicationActions = new Actions(m_serviceName, m_applicationObjectPath); + connect(m_applicationActions, &Actions::actionsChanged, this, [this](const QStringList &dirtyActions) { + actionsChanged(dirtyActions, s_applicationActionsPrefix); + }); + connect(m_applicationActions, &Actions::loaded, this, [this] { + if (m_menuInited) { + actionsChanged(m_applicationActions->getAll().keys(), s_applicationActionsPrefix); + } else { + initMenu(); } }); + m_applicationActions->load(); } if (!m_unityObjectPath.isEmpty()) { - getActions(m_unityObjectPath, [this](const GMenuActionMap &actions, bool ok) { - if (ok) { - if (m_menuInited) { - onUnityActionsChanged({}, {}, {}, actions); - } else { - m_unityActions = actions; - initMenu(); - } + m_unityActions = new Actions(m_serviceName, m_unityObjectPath); + connect(m_unityActions, &Actions::actionsChanged, this, [this](const QStringList &dirtyActions) { + actionsChanged(dirtyActions, s_unityActionsPrefix); + }); + connect(m_unityActions, &Actions::loaded, this, [this] { + if (m_menuInited) { + actionsChanged(m_unityActions->getAll().keys(), s_unityActionsPrefix); + } else { + initMenu(); } }); + m_unityActions->load(); } if (!m_windowObjectPath.isEmpty()) { - getActions(m_windowObjectPath, [this](const GMenuActionMap &actions, bool ok) { - if (ok) { - if (m_menuInited) { - onWindowActionsChanged({}, {}, {}, actions); - } else { - m_windowActions = actions; - initMenu(); - } + m_windowActions = new Actions(m_serviceName, m_windowObjectPath); + connect(m_windowActions, &Actions::actionsChanged, this, [this](const QStringList &dirtyActions) { + actionsChanged(dirtyActions, s_windowActionsPrefix); + }); + connect(m_windowActions, &Actions::loaded, this, [this] { + if (m_menuInited) { + actionsChanged(m_windowActions->getAll().keys(), s_windowActionsPrefix); + } else { + initMenu(); } }); + m_windowActions->load(); } } @@ -469,208 +451,48 @@ void Menu::menuChanged(const GMenuChangeList &changes) } } -void Menu::onApplicationActionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added) -{ - if (!m_menuInited) { - return; - } - actionsChanged(removed, enabledChanges, stateChanges, added, m_applicationActions, QStringLiteral("app.")); -} - -void Menu::onUnityActionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added) +bool Menu::getAction(const QString &name, GMenuAction &action) const { - if (!m_menuInited) { - return; - } - actionsChanged(removed, enabledChanges, stateChanges, added, m_unityActions, QStringLiteral("unity.")); -} + QString lookupName; + Actions *actions = getActionsForAction(name, lookupName); -void Menu::onWindowActionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added) -{ - if (!m_menuInited) { - return; + if (!actions) { + return false; } - actionsChanged(removed, enabledChanges, stateChanges, added, m_windowActions, QStringLiteral("win.")); -} - -void Menu::getActions(const QString &path, const std::function &cb) -{ - QDBusMessage msg = QDBusMessage::createMethodCall(m_serviceName, - path, - s_orgGtkActions, - QStringLiteral("DescribeAll")); - QDBusPendingReply reply = QDBusConnection::sessionBus().asyncCall(msg); - QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(reply, this); - connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, path, cb](QDBusPendingCallWatcher *watcher) { - QDBusPendingReply reply = *watcher; - if (reply.isError()) { - qCWarning(DBUSMENUPROXY) << "Failed to get actions from" << m_serviceName << "at" << path << reply.error(); - cb({}, false); - } else { - cb(reply.value(), true); - } - }); + return actions->get(lookupName, action); } -bool Menu::getAction(const QString &name, GMenuAction &action) const +void Menu::triggerAction(const QString &name, uint timestamp) { QString lookupName; - const GMenuActionMap *actionMap = nullptr; - - if (name.startsWith(QLatin1String("app."))) { - lookupName = name.mid(4); - actionMap = &m_applicationActions; - } else if (name.startsWith(QLatin1String("unity."))) { - lookupName = name.mid(6); - actionMap = &m_unityActions; - } else if (name.startsWith(QLatin1String("win."))) { - lookupName = name.mid(4); - actionMap = &m_windowActions; - } + Actions *actions = getActionsForAction(name, lookupName); - if (!actionMap) { - return false; - } - - auto it = actionMap->constFind(lookupName); - if (it == actionMap->constEnd()) { - return false; + if (!actions) { + return; } - action = *it; - return true; + actions->trigger(lookupName, timestamp); } -void Menu::triggerAction(const QString &name, uint timestamp) +Actions *Menu::getActionsForAction(const QString &name, QString &lookupName) const { - QString lookupName; - QString path; - - // TODO avoid code duplication with getAction if (name.startsWith(QLatin1String("app."))) { lookupName = name.mid(4); - path = m_applicationObjectPath; + return m_applicationActions; } else if (name.startsWith(QLatin1String("unity."))) { lookupName = name.mid(6); - path = m_unityObjectPath; + return m_unityActions; } else if (name.startsWith(QLatin1String("win."))) { lookupName = name.mid(4); - path = m_windowObjectPath; - } - - if (path.isEmpty()) { - return; - } - - GMenuAction action; - if (!getAction(name, action)) { - return; + return m_windowActions; } - QDBusMessage msg = QDBusMessage::createMethodCall(m_serviceName, - path, - s_orgGtkActions, - QStringLiteral("Activate")); - msg << lookupName; - // TODO use the arguments provided by "target" in the menu item - msg << QVariant::fromValue(QVariantList()); - - QVariantMap platformData; - - if (timestamp) { - // From documentation: - // If the startup notification id is not available, this can be just "_TIMEtime", where - // time is the time stamp from the event triggering the call. - // see also gtkwindow.c extract_time_from_startup_id and startup_id_is_fake - platformData.insert(QStringLiteral("desktop-startup-id"), QStringLiteral("_TIME") + QString::number(timestamp)); - } - - msg << platformData; - - QDBusPendingReply reply = QDBusConnection::sessionBus().asyncCall(msg); - QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(reply, this); - connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, path, name](QDBusPendingCallWatcher *watcher) { - QDBusPendingReply reply = *watcher; - if (reply.isError()) { - qCWarning(DBUSMENUPROXY) << "Failed to invoke action" << name << "on" << m_serviceName << "at" << path << reply.error(); - } - }); + return nullptr; } -void Menu::actionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added, GMenuActionMap &actions, const QString &prefix) +void Menu::actionsChanged(const QStringList &dirtyActions, const QString &prefix) { - // Collect the actions that we removed, altered, or added, so we can eventually signal changes for all menus that contain one of those actions - QSet dirtyActions; - - // TODO I bet for most of the loops below we could use a nice short std algorithm - - for (const QString &removedAction : removed) { - if (actions.remove(removedAction)) { - dirtyActions.insert(removedAction); - } - } - - for (auto it = enabledChanges.constBegin(), end = enabledChanges.constEnd(); it != end; ++it) { - const QString &actionName = it.key(); - const bool enabled = it.value(); - - auto actionIt = actions.find(actionName); - if (actionIt == actions.end()) { - qCInfo(DBUSMENUPROXY) << "Got enabled changed for action" << actionName << "which we don't know"; - continue; - } - - GMenuAction &action = *actionIt; - if (action.enabled != enabled) { - action.enabled = enabled; - dirtyActions.insert(actionName); - } else { - qCInfo(DBUSMENUPROXY) << "Got enabled change for action" << actionName << "which didn't change it"; - } - } - - for (auto it = stateChanges.constBegin(), end = stateChanges.constEnd(); it != end; ++it) { - const QString &actionName = it.key(); - const QVariant &state = it.value(); - - auto actionIt = actions.find(actionName); - if (actionIt == actions.end()) { - qCInfo(DBUSMENUPROXY) << "Got state changed for action" << actionName << "which we don't know"; - continue; - } - - GMenuAction &action = *actionIt; - - if (action.state.isEmpty()) { - qCDebug(DBUSMENUPROXY) << "Got new state for action" << actionName << "that didn't have any state before"; - action.state.append(state); - dirtyActions.insert(actionName); - } else { - // Action state is a list but the state change only sends us a single variant, so just overwrite the first one - QVariant &firstState = action.state.first(); - if (firstState != state) { - firstState = state; - dirtyActions.insert(actionName); - } else { - qCInfo(DBUSMENUPROXY) << "Got state change for action" << actionName << "which didn't change it"; - } - } - } - - // unite() will result in keys being present multiple times, do it manually and overwrite existing ones - for (auto it = added.constBegin(), end = added.constEnd(); it != end; ++it) { - const QString &actionName = it.key(); - - if (actions.contains(actionName)) { // TODO check isInfoEnabled - qCInfo(DBUSMENUPROXY) << "Got new action" << actionName << "that we already have, overwriting existing one"; - } - - actions.insert(actionName, it.value()); - - dirtyActions.insert(actionName); - } - auto forEachMenuItem = [this](const std::function &cb) { for (auto it = m_menus.constBegin(), end = m_menus.constEnd(); it != end; ++it) { const int subscription = it.key(); @@ -696,8 +518,6 @@ void Menu::actionsChanged(const QStringList &removed, const StringBoolMap &enabl return; }; - //qDebug() << "The following actions changed" << dirtyActions; - // now find in which menus these actions are and emit a change accordingly DBusMenuItemList dirtyItems; diff --git a/gmenu-dbusmenu-proxy/menu.h b/gmenu-dbusmenu-proxy/menu.h index 718a5ff6d..46dac8cec 100644 --- a/gmenu-dbusmenu-proxy/menu.h +++ b/gmenu-dbusmenu-proxy/menu.h @@ -32,6 +32,8 @@ class QDBusVariant; +class Actions; + class Menu : public QObject, protected QDBusContext { Q_OBJECT @@ -94,9 +96,6 @@ signals: private slots: void onApplicationMenuChanged(const GMenuChangeList &changes); void onMenuBarChanged(const GMenuChangeList &changes); - void onApplicationActionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added); - void onUnityActionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added); - void onWindowActionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added); private: void initMenu(); @@ -105,13 +104,12 @@ private: bool registerDBusObject(); - void getActions(const QString &path, const std::function &cb); bool getAction(const QString &name, GMenuAction &action) const; void triggerAction(const QString &name, uint timestamp = 0); + Actions *getActionsForAction(const QString &name, QString &lookupName) const; void menuChanged(const GMenuChangeList &changes); - void actionsChanged(const QStringList &removed, const StringBoolMap &enabledChanges, const QVariantMap &stateChanges, const GMenuActionMap &added, - GMenuActionMap &actions, const QString &prefix); + void actionsChanged(const QStringList &dirty, const QString &prefix); static int treeStructureToInt(int subscription, int section, int index); static void intToTreeStructure(int source, int &subscription, int §ion, int &index); @@ -142,9 +140,9 @@ private: QHash m_pendingGetLayouts; - GMenuActionMap m_applicationActions; - GMenuActionMap m_windowActions; - GMenuActionMap m_unityActions; + Actions *m_applicationActions = nullptr; + Actions *m_unityActions = nullptr; + Actions *m_windowActions = nullptr; bool m_menuInited = false;