[Notifications] Still compress "immediate" show, and delay individually

This ensures that the first `update()` call with the summary will be shown
atomically to the user rather than show an empty job popup briefly.

Also handle the delay on a per-job basis, previously there was a common
timer, which made no sense.

libnotificationmanager/job_p.cpp

@@ -42,6 +42,9 @@ JobPrivate::JobPrivate(uint id, QObject *parent)
     : QObject(parent)
     , m_id(id)
 {
+    m_showTimer.setSingleShot(true);
+    connect(&m_showTimer, &QTimer::timeout, this, &JobPrivate::requestShow);
+
     m_objectPath.setPath(QStringLiteral("/org/kde/notificationmanager/jobs/JobView_%1").arg(id));
 
     // TODO also v1? it's identical to V2 except it doesn't have setError method so supporting it should be easy
@@ -53,6 +56,14 @@ JobPrivate::JobPrivate(uint id, QObject *parent)
 
 JobPrivate::~JobPrivate() = default;
 
+void JobPrivate::requestShow()
+{
+    if (!m_showRequested) {
+        m_showRequested = true;
+        Q_EMIT showRequested();
+    }
+}
+
 QDBusObjectPath JobPrivate::objectPath() const
 {
     return m_objectPath;
@@ -223,6 +234,15 @@ QString JobPrivate::text() const
     return QString();
 }
 
+void JobPrivate::delayedShow(std::chrono::milliseconds delay, ShowConditions showConditions)
+{
+    m_showConditions = showConditions;
+
+    if (showConditions.testFlag(ShowCondition::OnTimeout)) {
+        m_showTimer.start(delay);
+    }
+}
+
 void JobPrivate::kill()
 {
     emit cancelRequested();
@@ -393,6 +413,12 @@ void JobPrivate::terminate(uint errorCode, const QString &errorMessage, const QV
     Job *job = static_cast<Job *>(parent());
     job->setError(errorCode);
     job->setErrorText(errorMessage);
+
+    // Request show just before changing state to stopped, so we're not discarded
+    if (m_showConditions.testFlag(ShowCondition::OnTermination)) {
+        requestShow();
+    }
+
     job->setState(Notifications::JobStateStopped);
     finish();
 }
@@ -453,4 +479,8 @@ void JobPrivate::update(const QVariantMap &properties)
     }
 
     updateHasDetails();
+
+    if (!m_summary.isEmpty() && m_showConditions.testFlag(ShowCondition::OnSummary)) {
+        requestShow();
+    }
 }

libnotificationmanager/job_p.h

@@ -28,11 +28,13 @@
 #include <QSharedPointer>
 #include <QString>
 #include <QUrl>
+#include <QTimer>
+
+#include <chrono>
 
 #include "job.h"
 #include "notifications.h"
 
-class QTimer;
 class KFilePlacesModel;
 
 namespace NotificationManager
@@ -45,10 +47,18 @@ public:
     JobPrivate(uint id, QObject *parent);
     ~JobPrivate() override;
 
+    enum class ShowCondition {
+        OnTimeout = 1 << 0,
+        OnSummary = 1 << 1,
+        OnTermination = 1 << 2
+    };
+    Q_DECLARE_FLAGS(ShowConditions, ShowCondition)
+
     QDBusObjectPath objectPath() const;
     QUrl descriptionUrl() const;
     QString text() const;
 
+    void delayedShow(std::chrono::milliseconds delay, ShowConditions showConditions);
     void kill();
 
     // DBus
@@ -70,6 +80,7 @@ public:
     void update(const QVariantMap &properties);
 
 Q_SIGNALS:
+    void showRequested();
     void closed();
 
     void infoMessageChanged();
@@ -111,12 +122,18 @@ private:
 
     static QUrl localFileOrUrl(const QString &stringUrl);
 
+    void requestShow();
+
     QUrl destUrl() const;
     QString prettyUrl(const QUrl &url) const;
     void updateHasDetails();
 
     void finish();
 
+    QTimer m_showTimer;
+    ShowConditions m_showConditions = {};
+    bool m_showRequested = false;
+
     QTimer *m_killTimer = nullptr;
 
     uint m_id = 0;
@@ -168,3 +185,5 @@ private:
 };
 
 } // namespace NotificationManager
+
+Q_DECLARE_OPERATORS_FOR_FLAGS(NotificationManager::JobPrivate::ShowConditions)

libnotificationmanager/jobsmodel_p.cpp

@@ -43,14 +43,15 @@
 #include <kio/global.h>
 
 #include <algorithm>
+#include <chrono>
 
 using namespace NotificationManager;
+using namespace std::literals::chrono_literals;
 
 JobsModelPrivate::JobsModelPrivate(QObject *parent)
     : QObject(parent)
     , m_serviceWatcher(new QDBusServiceWatcher(this))
     , m_compressUpdatesTimer(new QTimer(this))
-    , m_pendingJobViewsTimer(new QTimer(this))
 {
     m_serviceWatcher->setConnection(QDBusConnection::sessionBus());
     m_serviceWatcher->setWatchMode(QDBusServiceWatcher::WatchForUnregistration);
@@ -77,29 +78,6 @@ JobsModelPrivate::JobsModelPrivate(QObject *parent)
 
         m_pendingDirtyRoles.clear();
     });
-
-    m_pendingJobViewsTimer->setInterval(500);
-    m_pendingJobViewsTimer->setSingleShot(true);
-    connect(m_pendingJobViewsTimer, &QTimer::timeout, this, [this] {
-        const auto pendingJobs = m_pendingJobViews;
-        for (Job *job : pendingJobs) {
-            if (job->state() == Notifications::JobStateStopped) {
-                // Stop finished or canceled in the meantime, remove
-                qCDebug(NOTIFICATIONMANAGER) << "By the time we wanted to show JobView" << job->id() << "from" << job->applicationName()
-                                             << ", it was already stopped";
-                remove(job);
-                continue;
-            }
-
-            const int newRow = m_jobViews.count();
-            emit jobViewAboutToBeAdded(newRow, job);
-            m_jobViews.append(job);
-            emit jobViewAdded(newRow, job);
-            updateApplicationPercentage(job->desktopEntry());
-        }
-
-        m_pendingJobViews.clear();
-    });
 }
 
 JobsModelPrivate::~JobsModelPrivate()
@@ -317,6 +295,42 @@ QDBusObjectPath JobsModelPrivate::requestView(const QString &desktopEntry, int c
     job->setSuspendable(capabilities & KJob::Suspendable);
     job->setKillable(capabilities & KJob::Killable);
 
+    connect(job->d, &JobPrivate::showRequested, this, [this, job] {
+        if (job->state() == Notifications::JobStateStopped) {
+            // Stop finished or canceled in the meantime, remove
+            qCDebug(NOTIFICATIONMANAGER) << "By the time we wanted to show JobView" << job->id() << "from" << job->applicationName()
+                                         << ", it was already stopped";
+            remove(job);
+            return;
+        }
+
+        const int pendingRow = m_pendingJobViews.indexOf(job);
+        Q_ASSERT(pendingRow > -1);
+        m_pendingJobViews.removeAt(pendingRow);
+
+        const int newRow = m_jobViews.count();
+        Q_EMIT jobViewAboutToBeAdded(newRow, job);
+        m_jobViews.append(job);
+        Q_EMIT jobViewAdded(newRow, job);
+        updateApplicationPercentage(job->desktopEntry());
+    });
+
+    m_pendingJobViews.append(job);
+
+    if (hints.value(QStringLiteral("immediate")).toBool()) {
+        // Slightly delay showing the job so that the first update() call with a
+        // summary will be shown atomically to the user.
+        job->d->delayedShow(50ms, JobPrivate::ShowCondition::OnTimeout
+                               | JobPrivate::ShowCondition::OnSummary
+                               | JobPrivate::ShowCondition::OnTermination);
+    } else {
+        // Delay showing a job view to avoid showing really short stat jobs and other useless stuff.
+        job->d->delayedShow(500ms, JobPrivate::ShowCondition::OnTimeout);
+    }
+
+    m_jobServices.insert(job, serviceName);
+    m_serviceWatcher->addWatchedService(serviceName);
+
     // Apply initial properties
     job->d->update(hints);
 
@@ -360,21 +374,6 @@ QDBusObjectPath JobsModelPrivate::requestView(const QString &desktopEntry, int c
         remove(job);
     });
 
-    // Delay showing a job view by 500ms to avoid showing really short stat jobs and other useless stuff
-    if (hints.value(QStringLiteral("immediate")).toBool()) {
-        const int newRow = m_jobViews.count();
-        emit jobViewAboutToBeAdded(newRow, job);
-        m_jobViews.append(job);
-        emit jobViewAdded(newRow, job);
-        updateApplicationPercentage(job->desktopEntry());
-    } else {
-        m_pendingJobViews.append(job);
-        m_pendingJobViewsTimer->start();
-    }
-
-    m_jobServices.insert(job, serviceName);
-    m_serviceWatcher->addWatchedService(serviceName);
-
     if (!connection().interface()->isServiceRegistered(serviceName)) {
         qCWarning(NOTIFICATIONMANAGER) << "Service that requested the view wasn't registered anymore by the time the request was being processed";
         QMetaObject::invokeMethod(

libnotificationmanager/jobsmodel_p.h

@@ -95,7 +95,6 @@ private:
     QTimer *m_compressUpdatesTimer = nullptr;
     QHash<Job *, QVector<int>> m_pendingDirtyRoles;
 
-    QTimer *m_pendingJobViewsTimer = nullptr;
     QVector<Job *> m_pendingJobViews;
 };