From 70a4e1e265e896269670b2a62f99c6413b38923f Mon Sep 17 00:00:00 2001 From: Loren Burkholder Date: Tue, 19 Jan 2021 18:30:04 -0500 Subject: [PATCH 01/10] Keep DBUS from blocking --- src/notifications/ManagerLinux.cpp | 88 ++++++++++++++++-------------- 1 file changed, 48 insertions(+), 40 deletions(-) diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp index b5e9a6a4..ed5b60a2 100644 --- a/src/notifications/ManagerLinux.cpp +++ b/src/notifications/ManagerLinux.cpp @@ -2,9 +2,11 @@ #include #include -#include -#include -#include +#include +#include +#include +#include +#include NotificationsManager::NotificationsManager(QObject *parent) : QObject(parent) @@ -36,6 +38,12 @@ NotificationsManager::NotificationsManager(QObject *parent) SLOT(notificationReplied(uint, QString))); } +/** + * This function is based on code from + * https://github.com/rohieb/StratumsphereTrayIcon + * Copyright (C) 2012 Roland Hieber + * Licensed under the GNU General Public License, version 3 + */ void NotificationsManager::postNotification(const QString &roomid, const QString &eventid, @@ -44,50 +52,50 @@ NotificationsManager::postNotification(const QString &roomid, const QString &text, const QImage &icon) { - uint id = showNotification(roomname, sender + ": " + text, icon); - notificationIds[id] = roomEventId{roomid, eventid}; + Q_UNUSED(icon) + + QVariantMap hints; + hints["image-data"] = sender + ": " + text; + hints["sound-name"] = "message-new-instant"; + QList argumentList; + argumentList << "nheko"; // app_name + argumentList << (uint)0; // replace_id + argumentList << ""; // app_icon + argumentList << roomname; // summary + argumentList << text; // body + // The list of actions has always the action name and then a localized version of that + // action. Currently we just use an empty string for that. + // TODO(Nico): Look into what to actually put there. + argumentList << (QStringList("default") << "" + << "inline-reply" + << ""); // actions + argumentList << hints; // hints + argumentList << (int)-1; // timeout in ms + + static QDBusInterface notifyApp("org.freedesktop.Notifications", + "/org/freedesktop/Notifications", + "org.freedesktop.Notifications"); + auto call = + notifyApp.callWithArgumentList(QDBus::AutoDetect, "Notify", argumentList); + QDBusPendingCallWatcher watcher{QDBusPendingReply{call}}; + connect(&watcher, &QDBusPendingCallWatcher::finished, this, [&watcher, this, &roomid, &eventid]() { + if (watcher.reply().type() == QDBusMessage::ErrorMessage) + qDebug() << "D-Bus Error:" << watcher.reply().errorMessage(); + else + notificationIds[watcher.reply().arguments().first().toUInt()] = roomEventId{roomid, eventid}; + }); } -/** - * This function is based on code from - * https://github.com/rohieb/StratumsphereTrayIcon - * Copyright (C) 2012 Roland Hieber - * Licensed under the GNU General Public License, version 3 - */ + uint NotificationsManager::showNotification(const QString summary, const QString text, const QImage image) { - QVariantMap hints; - hints["image-data"] = image; - hints["sound-name"] = "message-new-instant"; - QList argumentList; - argumentList << "nheko"; // app_name - argumentList << (uint)0; // replace_id - argumentList << ""; // app_icon - argumentList << summary; // summary - argumentList << text; // body - // The list of actions has always the action name and then a localized version of that - // action. Currently we just use an empty string for that. - // TODO(Nico): Look into what to actually put there. - argumentList << (QStringList("default") << "" - << "inline-reply" - << ""); // actions - argumentList << hints; // hints - argumentList << (int)-1; // timeout in ms + Q_UNUSED(summary) + Q_UNUSED(text) + Q_UNUSED(image) - static QDBusInterface notifyApp("org.freedesktop.Notifications", - "/org/freedesktop/Notifications", - "org.freedesktop.Notifications"); - QDBusMessage reply = - notifyApp.callWithArgumentList(QDBus::AutoDetect, "Notify", argumentList); - if (reply.type() == QDBusMessage::ErrorMessage) { - qDebug() << "D-Bus Error:" << reply.errorMessage(); - return 0; - } else { - return reply.arguments().first().toUInt(); - } - return true; + return 0; } void From e2d89e093adcd8107f3e0ec891f28a25e400883f Mon Sep 17 00:00:00 2001 From: Loren Burkholder Date: Tue, 19 Jan 2021 18:46:25 -0500 Subject: [PATCH 02/10] Use async call --- src/notifications/ManagerLinux.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp index ed5b60a2..451e23b2 100644 --- a/src/notifications/ManagerLinux.cpp +++ b/src/notifications/ManagerLinux.cpp @@ -76,7 +76,7 @@ NotificationsManager::postNotification(const QString &roomid, "/org/freedesktop/Notifications", "org.freedesktop.Notifications"); auto call = - notifyApp.callWithArgumentList(QDBus::AutoDetect, "Notify", argumentList); + notifyApp.asyncCallWithArgumentList("Notify", argumentList); QDBusPendingCallWatcher watcher{QDBusPendingReply{call}}; connect(&watcher, &QDBusPendingCallWatcher::finished, this, [&watcher, this, &roomid, &eventid]() { if (watcher.reply().type() == QDBusMessage::ErrorMessage) From b04a7fbef6a6e8a8e2d588ac6770464e5d964f42 Mon Sep 17 00:00:00 2001 From: Loren Burkholder Date: Tue, 19 Jan 2021 18:47:18 -0500 Subject: [PATCH 03/10] Remove showNotification function --- src/notifications/Manager.h | 1 - src/notifications/ManagerLinux.cpp | 12 ------------ 2 files changed, 13 deletions(-) diff --git a/src/notifications/Manager.h b/src/notifications/Manager.h index b5347bd6..4c9852cc 100644 --- a/src/notifications/Manager.h +++ b/src/notifications/Manager.h @@ -47,7 +47,6 @@ public: private: QDBusInterface dbus; - uint showNotification(const QString summary, const QString text, const QImage image); void closeNotification(uint id); // notification ID to (room ID, event ID) diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp index 451e23b2..a52a43e9 100644 --- a/src/notifications/ManagerLinux.cpp +++ b/src/notifications/ManagerLinux.cpp @@ -86,18 +86,6 @@ NotificationsManager::postNotification(const QString &roomid, }); } -uint -NotificationsManager::showNotification(const QString summary, - const QString text, - const QImage image) -{ - Q_UNUSED(summary) - Q_UNUSED(text) - Q_UNUSED(image) - - return 0; -} - void NotificationsManager::closeNotification(uint id) { From 7727c0d2491e1fa6463720302e56b504e7e3d265 Mon Sep 17 00:00:00 2001 From: Loren Burkholder Date: Tue, 19 Jan 2021 18:47:44 -0500 Subject: [PATCH 04/10] make lint --- src/notifications/ManagerLinux.cpp | 67 ++++++++++++++++-------------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp index a52a43e9..b86c5223 100644 --- a/src/notifications/ManagerLinux.cpp +++ b/src/notifications/ManagerLinux.cpp @@ -1,12 +1,12 @@ #include "notifications/Manager.h" -#include -#include #include #include #include #include #include +#include +#include NotificationsManager::NotificationsManager(QObject *parent) : QObject(parent) @@ -52,38 +52,41 @@ NotificationsManager::postNotification(const QString &roomid, const QString &text, const QImage &icon) { - Q_UNUSED(icon) + Q_UNUSED(icon) - QVariantMap hints; - hints["image-data"] = sender + ": " + text; - hints["sound-name"] = "message-new-instant"; - QList argumentList; - argumentList << "nheko"; // app_name - argumentList << (uint)0; // replace_id - argumentList << ""; // app_icon - argumentList << roomname; // summary - argumentList << text; // body - // The list of actions has always the action name and then a localized version of that - // action. Currently we just use an empty string for that. - // TODO(Nico): Look into what to actually put there. - argumentList << (QStringList("default") << "" - << "inline-reply" - << ""); // actions - argumentList << hints; // hints - argumentList << (int)-1; // timeout in ms + QVariantMap hints; + hints["image-data"] = sender + ": " + text; + hints["sound-name"] = "message-new-instant"; + QList argumentList; + argumentList << "nheko"; // app_name + argumentList << (uint)0; // replace_id + argumentList << ""; // app_icon + argumentList << roomname; // summary + argumentList << text; // body + // The list of actions has always the action name and then a localized version of that + // action. Currently we just use an empty string for that. + // TODO(Nico): Look into what to actually put there. + argumentList << (QStringList("default") << "" + << "inline-reply" + << ""); // actions + argumentList << hints; // hints + argumentList << (int)-1; // timeout in ms - static QDBusInterface notifyApp("org.freedesktop.Notifications", - "/org/freedesktop/Notifications", - "org.freedesktop.Notifications"); - auto call = - notifyApp.asyncCallWithArgumentList("Notify", argumentList); - QDBusPendingCallWatcher watcher{QDBusPendingReply{call}}; - connect(&watcher, &QDBusPendingCallWatcher::finished, this, [&watcher, this, &roomid, &eventid]() { - if (watcher.reply().type() == QDBusMessage::ErrorMessage) - qDebug() << "D-Bus Error:" << watcher.reply().errorMessage(); - else - notificationIds[watcher.reply().arguments().first().toUInt()] = roomEventId{roomid, eventid}; - }); + static QDBusInterface notifyApp("org.freedesktop.Notifications", + "/org/freedesktop/Notifications", + "org.freedesktop.Notifications"); + auto call = notifyApp.asyncCallWithArgumentList("Notify", argumentList); + QDBusPendingCallWatcher watcher{QDBusPendingReply{call}}; + connect(&watcher, + &QDBusPendingCallWatcher::finished, + this, + [&watcher, this, &roomid, &eventid]() { + if (watcher.reply().type() == QDBusMessage::ErrorMessage) + qDebug() << "D-Bus Error:" << watcher.reply().errorMessage(); + else + notificationIds[watcher.reply().arguments().first().toUInt()] = + roomEventId{roomid, eventid}; + }); } void From ac36e924472582edbf99eeff879c638c708c5a41 Mon Sep 17 00:00:00 2001 From: Loren Burkholder Date: Wed, 20 Jan 2021 16:04:02 -0500 Subject: [PATCH 05/10] Make watcher a pointer so that it doesn't get destroyed too soon --- src/notifications/ManagerLinux.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp index b86c5223..ad9e2c21 100644 --- a/src/notifications/ManagerLinux.cpp +++ b/src/notifications/ManagerLinux.cpp @@ -76,16 +76,17 @@ NotificationsManager::postNotification(const QString &roomid, "/org/freedesktop/Notifications", "org.freedesktop.Notifications"); auto call = notifyApp.asyncCallWithArgumentList("Notify", argumentList); - QDBusPendingCallWatcher watcher{QDBusPendingReply{call}}; - connect(&watcher, + auto watcher = new QDBusPendingCall{QDBusPendingReply{call}}; + connect(watcher, &QDBusPendingCallWatcher::finished, this, - [&watcher, this, &roomid, &eventid]() { - if (watcher.reply().type() == QDBusMessage::ErrorMessage) + [watcher, this, &roomid, &eventid]() { + if (watcher->reply().type() == QDBusMessage::ErrorMessage) qDebug() << "D-Bus Error:" << watcher.reply().errorMessage(); else - notificationIds[watcher.reply().arguments().first().toUInt()] = + notificationIds[watcher->reply().arguments().first().toUInt()] = roomEventId{roomid, eventid}; + delete watcher; }); } From 1479743e702f2bfba4524ab4dae8cf1134547920 Mon Sep 17 00:00:00 2001 From: Loren Burkholder Date: Wed, 20 Jan 2021 16:09:25 -0500 Subject: [PATCH 06/10] Use async call in closeNotification --- src/notifications/ManagerLinux.cpp | 36 +++++++++++++++--------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp index ad9e2c21..d9d157fc 100644 --- a/src/notifications/ManagerLinux.cpp +++ b/src/notifications/ManagerLinux.cpp @@ -75,19 +75,17 @@ NotificationsManager::postNotification(const QString &roomid, static QDBusInterface notifyApp("org.freedesktop.Notifications", "/org/freedesktop/Notifications", "org.freedesktop.Notifications"); - auto call = notifyApp.asyncCallWithArgumentList("Notify", argumentList); - auto watcher = new QDBusPendingCall{QDBusPendingReply{call}}; - connect(watcher, - &QDBusPendingCallWatcher::finished, - this, - [watcher, this, &roomid, &eventid]() { - if (watcher->reply().type() == QDBusMessage::ErrorMessage) - qDebug() << "D-Bus Error:" << watcher.reply().errorMessage(); - else - notificationIds[watcher->reply().arguments().first().toUInt()] = - roomEventId{roomid, eventid}; - delete watcher; - }); + auto call = notifyApp.asyncCallWithArgumentList("Notify", argumentList); + auto watcher = new QDBusPendingCallWatcher{QDBusPendingCall{QDBusPendingReply{call}}}; + connect( + watcher, &QDBusPendingCallWatcher::finished, this, [watcher, this, &roomid, &eventid]() { + if (watcher->reply().type() == QDBusMessage::ErrorMessage) + qDebug() << "D-Bus Error:" << watcher->reply().errorMessage(); + else + notificationIds[watcher->reply().arguments().first().toUInt()] = + roomEventId{roomid, eventid}; + delete watcher; + }); } void @@ -99,11 +97,13 @@ NotificationsManager::closeNotification(uint id) static QDBusInterface closeCall("org.freedesktop.Notifications", "/org/freedesktop/Notifications", "org.freedesktop.Notifications"); - QDBusMessage reply = - closeCall.callWithArgumentList(QDBus::AutoDetect, "CloseNotification", argumentList); - if (reply.type() == QDBusMessage::ErrorMessage) { - qDebug() << "D-Bus Error:" << reply.errorMessage(); - } + auto call = closeCall.asyncCallWithArgumentList("CloseNotification", argumentList); + auto watcher = new QDBusPendingCallWatcher{QDBusPendingCall{QDBusPendingReply{call}}}; + connect(watcher, &QDBusPendingCallWatcher::finished, this, [watcher, this]() { + if (watcher->reply().type() == QDBusMessage::ErrorMessage) { + qDebug() << "D-Bus Error:" << watcher->reply().errorMessage(); + }; + }); } void From cf4f50dac854fd951066f56945f5d1adbbe679d4 Mon Sep 17 00:00:00 2001 From: Loren Burkholder Date: Wed, 20 Jan 2021 16:13:21 -0500 Subject: [PATCH 07/10] Use deleteLater() instead of delete --- src/notifications/ManagerLinux.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp index d9d157fc..36692d03 100644 --- a/src/notifications/ManagerLinux.cpp +++ b/src/notifications/ManagerLinux.cpp @@ -84,7 +84,7 @@ NotificationsManager::postNotification(const QString &roomid, else notificationIds[watcher->reply().arguments().first().toUInt()] = roomEventId{roomid, eventid}; - delete watcher; + watcher->deleteLater(); }); } @@ -103,6 +103,7 @@ NotificationsManager::closeNotification(uint id) if (watcher->reply().type() == QDBusMessage::ErrorMessage) { qDebug() << "D-Bus Error:" << watcher->reply().errorMessage(); }; + watcher->deleteLater(); }); } From 9c154e974747b699c62079c6d4edd2f81d7951ef Mon Sep 17 00:00:00 2001 From: Loren Burkholder Date: Wed, 20 Jan 2021 16:15:14 -0500 Subject: [PATCH 08/10] Fix error in assignment of image/text --- src/notifications/ManagerLinux.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp index 36692d03..2c15a712 100644 --- a/src/notifications/ManagerLinux.cpp +++ b/src/notifications/ManagerLinux.cpp @@ -55,14 +55,14 @@ NotificationsManager::postNotification(const QString &roomid, Q_UNUSED(icon) QVariantMap hints; - hints["image-data"] = sender + ": " + text; + hints["image-data"] = icon; hints["sound-name"] = "message-new-instant"; QList argumentList; - argumentList << "nheko"; // app_name - argumentList << (uint)0; // replace_id - argumentList << ""; // app_icon - argumentList << roomname; // summary - argumentList << text; // body + argumentList << "nheko"; // app_name + argumentList << (uint)0; // replace_id + argumentList << ""; // app_icon + argumentList << roomname; // summary + argumentList << sender + ": " + text; // body // The list of actions has always the action name and then a localized version of that // action. Currently we just use an empty string for that. // TODO(Nico): Look into what to actually put there. From 89304a5c6b1b1fd02bcecd887aabc2b164d6d86a Mon Sep 17 00:00:00 2001 From: Loren Burkholder Date: Wed, 20 Jan 2021 16:52:37 -0500 Subject: [PATCH 09/10] Fix crash --- src/notifications/ManagerLinux.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp index 2c15a712..326803b2 100644 --- a/src/notifications/ManagerLinux.cpp +++ b/src/notifications/ManagerLinux.cpp @@ -78,7 +78,7 @@ NotificationsManager::postNotification(const QString &roomid, auto call = notifyApp.asyncCallWithArgumentList("Notify", argumentList); auto watcher = new QDBusPendingCallWatcher{QDBusPendingCall{QDBusPendingReply{call}}}; connect( - watcher, &QDBusPendingCallWatcher::finished, this, [watcher, this, &roomid, &eventid]() { + watcher, &QDBusPendingCallWatcher::finished, this, [watcher, this, roomid, eventid]() { if (watcher->reply().type() == QDBusMessage::ErrorMessage) qDebug() << "D-Bus Error:" << watcher->reply().errorMessage(); else From 2605ce9a894217f5e6d51400498c0f67168187cd Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Wed, 20 Jan 2021 23:59:27 +0100 Subject: [PATCH 10/10] Clean up notification watching a bit --- src/notifications/ManagerLinux.cpp | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp index 326803b2..8f7261e6 100644 --- a/src/notifications/ManagerLinux.cpp +++ b/src/notifications/ManagerLinux.cpp @@ -52,8 +52,6 @@ NotificationsManager::postNotification(const QString &roomid, const QString &text, const QImage &icon) { - Q_UNUSED(icon) - QVariantMap hints; hints["image-data"] = icon; hints["sound-name"] = "message-new-instant"; @@ -75,8 +73,8 @@ NotificationsManager::postNotification(const QString &roomid, static QDBusInterface notifyApp("org.freedesktop.Notifications", "/org/freedesktop/Notifications", "org.freedesktop.Notifications"); - auto call = notifyApp.asyncCallWithArgumentList("Notify", argumentList); - auto watcher = new QDBusPendingCallWatcher{QDBusPendingCall{QDBusPendingReply{call}}}; + QDBusPendingCall call = notifyApp.asyncCallWithArgumentList("Notify", argumentList); + auto watcher = new QDBusPendingCallWatcher{call, this}; connect( watcher, &QDBusPendingCallWatcher::finished, this, [watcher, this, roomid, eventid]() { if (watcher->reply().type() == QDBusMessage::ErrorMessage) @@ -91,14 +89,11 @@ NotificationsManager::postNotification(const QString &roomid, void NotificationsManager::closeNotification(uint id) { - QList argumentList; - argumentList << (uint)id; // replace_id - static QDBusInterface closeCall("org.freedesktop.Notifications", "/org/freedesktop/Notifications", "org.freedesktop.Notifications"); - auto call = closeCall.asyncCallWithArgumentList("CloseNotification", argumentList); - auto watcher = new QDBusPendingCallWatcher{QDBusPendingCall{QDBusPendingReply{call}}}; + auto call = closeCall.asyncCall("CloseNotification", (uint)id); // replace_id + auto watcher = new QDBusPendingCallWatcher{call, this}; connect(watcher, &QDBusPendingCallWatcher::finished, this, [watcher, this]() { if (watcher->reply().type() == QDBusMessage::ErrorMessage) { qDebug() << "D-Bus Error:" << watcher->reply().errorMessage();