From c7b598cb65df920e9268b2780c6dbc99a738a8f7 Mon Sep 17 00:00:00 2001 From: Ivy233 Date: Tue, 11 Aug 2026 22:24:29 +0800 Subject: [PATCH] fix: expire notification countdown only after the bubble is displayed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Move the expire countdown from NotificationManager to the frontend ExpireTimer singleton, which tracks every displayed notification (bubble and staging area) with one shared QTimer and per-id absolute deadlines 2. Start the countdown in BubbleModel::insertBubble/replaceBubble and NotifyStagingModel::push/open, i.e. only when the bubble is actually shown, so queued notifications no longer expire before they are displayed 3. Let the bubble and the staging area share one countdown per id: push() of an already tracked id keeps the original deadline, and remove() suspends it via m_retired so switching between the two never restarts the countdown 4. Move hover handling into ExpireTimer::setBlockId, freezing the hovered bubble's countdown and resuming it with a short grace period afterwards 5. Make the server the single owner of the close on expiry: NotifyServerApplet forwards ExpireTimer::expired to NotificationManager::notificationClosed via Qt::QueuedConnection, whose idempotency check guards against duplicate closes 6. Add NotifyEntity::timeout()/urgency() accessors so ExpireTimer derives the effective timeout itself (0/Critical never expires, -1 defaults to 5000 ms) 7. Fix a race in NotifyStagingModel where a local remove+refill after expiry could re-insert the just-expired notification with a fresh countdown; the staging row is now dropped only via the server's stagingEntityClosed round trip 8. Remove the now-unused pending-timeout machinery and tests from NotificationManager and NotifyServerApplet Log: Defer the notification expire countdown until the bubble is displayed, and close expired notifications once through the server Influence: 1. Verify a notification disappears after the default 5 seconds when displayed normally 2. Verify a notification no longer expires before it is shown when many notifications are queued 3. Verify hovering over a bubble prevents it from expiring 4. Verify a notification shown in both the bubble and the staging area expires at its original deadline without restarting the countdown, and the server closes it only once 5. Run the notification server unit tests fix: 通知气泡显示后才开始过期计时 1. 将过期计时从 NotificationManager 迁移到前端 ExpireTimer 单例，用一个共享 QTimer 和每个 id 的绝对截止时间管理所有已显示通知（气泡与暂存区）的倒计时 2. 在 BubbleModel::insertBubble/replaceBubble 与 NotifyStagingModel::push/open 中启动倒计时，即气泡真正显示后才开始计时，排队中的通知不再提前过期 3. 气泡与暂存区对同一 id 共享一个倒计时：重复 push 保持原截止时间， remove() 通过 m_retired 挂起，切换显示位置不会重新计时 4. 悬停处理下沉到 ExpireTimer::setBlockId，冻结悬停气泡的倒计时，移开后 保留短暂宽限期再恢复 5. 由服务端统一负责过期关闭：NotifyServerApplet 通过 Qt::QueuedConnection 将 ExpireTimer::expired 转发给 NotificationManager::notificationClosed， 幂等检查防止重复关闭 6. 新增 NotifyEntity::timeout()/urgency() 访问器，由 ExpireTimer 自行推导 有效超时（0/Critical 永不过期，-1 默认 5000ms） 7. 修复 NotifyStagingModel 过期后本地 remove+补位可能把刚过期的通知重新插入 并重新计时的竞态，暂存区行改为仅通过服务端 stagingEntityClosed 回环移除 8. 移除 NotificationManager 与 NotifyServerApplet 中不再使用的挂起计时逻辑 及对应测试 Log: 将通知过期倒计时推迟到气泡显示之后，并由服务端统一关闭过期通知 Influence: 1. 验证正常显示的通知在默认 5 秒后消失 2. 验证大量通知排队时，通知不会在显示前提前过期 3. 验证鼠标悬停气泡时通知不会过期消失 4. 验证同时显示在气泡与暂存区的通知按原始截止时间过期、不重新计时， 且服务端只关闭一次 5. 运行通知服务端单元测试 PMS: BUG-372279 --- panels/notification/CMakeLists.txt | 2 + panels/notification/bubble/bubbleitem.cpp | 5 + panels/notification/bubble/bubbleitem.h | 1 + panels/notification/bubble/bubblemodel.cpp | 33 +-- panels/notification/bubble/bubblemodel.h | 3 +- panels/notification/bubble/bubblepanel.cpp | 3 +- .../center/notifystagingmodel.cpp | 25 +- panels/notification/common/expiretimer.cpp | 234 ++++++++++++++++++ panels/notification/common/expiretimer.h | 91 +++++++ panels/notification/common/notifyentity.cpp | 10 + panels/notification/common/notifyentity.h | 4 + .../server/notificationmanager.cpp | 134 +--------- .../notification/server/notificationmanager.h | 9 - .../server/notifyserverapplet.cpp | 18 +- .../notification/server/notifyserverapplet.h | 1 - .../server/notifyserverapplet_test.cpp | 22 -- 16 files changed, 411 insertions(+), 184 deletions(-) create mode 100644 panels/notification/common/expiretimer.cpp create mode 100644 panels/notification/common/expiretimer.h diff --git a/panels/notification/CMakeLists.txt b/panels/notification/CMakeLists.txt index b7c6414eb..f7a60ff03 100644 --- a/panels/notification/CMakeLists.txt +++ b/panels/notification/CMakeLists.txt @@ -22,6 +22,8 @@ add_library(ds-notification-shared SHARED ${CMAKE_SOURCE_DIR}/panels/notification/common/dbaccessor.cpp ${CMAKE_SOURCE_DIR}/panels/notification/common/notifysetting.h ${CMAKE_SOURCE_DIR}/panels/notification/common/notifysetting.cpp + ${CMAKE_SOURCE_DIR}/panels/notification/common/expiretimer.h + ${CMAKE_SOURCE_DIR}/panels/notification/common/expiretimer.cpp ) set_target_properties(ds-notification-shared PROPERTIES diff --git a/panels/notification/bubble/bubbleitem.cpp b/panels/notification/bubble/bubbleitem.cpp index 60d48a6d7..695a8d23b 100644 --- a/panels/notification/bubble/bubbleitem.cpp +++ b/panels/notification/bubble/bubbleitem.cpp @@ -51,6 +51,11 @@ qint64 BubbleItem::id() const return m_entity.id(); } +const NotifyEntity &BubbleItem::entity() const +{ + return m_entity; +} + uint BubbleItem::bubbleId() const { return m_entity.bubbleId(); diff --git a/panels/notification/bubble/bubbleitem.h b/panels/notification/bubble/bubbleitem.h index 1b28f0aa8..417bd5ba2 100644 --- a/panels/notification/bubble/bubbleitem.h +++ b/panels/notification/bubble/bubbleitem.h @@ -21,6 +21,7 @@ class BubbleItem : public QObject public: void setEntity(const NotifyEntity &entity); + const NotifyEntity &entity() const; public: qint64 id() const; diff --git a/panels/notification/bubble/bubblemodel.cpp b/panels/notification/bubble/bubblemodel.cpp index c9bbee6fb..be3639c6e 100644 --- a/panels/notification/bubble/bubblemodel.cpp +++ b/panels/notification/bubble/bubblemodel.cpp @@ -7,6 +7,7 @@ #include #include "bubbleitem.h" +#include "expiretimer.h" #include #include @@ -82,6 +83,10 @@ void BubbleModel::insertBubble(BubbleItem *bubble) beginInsertRows(QModelIndex(), 0, 0); m_bubbles.prepend(bubble); endInsertRows(); + + // A non-positive interval (Critical urgency or expireTimeout 0) means the + // bubble never expires on its own. + ExpireTimer::instance()->push(bubble->entity()); } bool BubbleModel::isReplaceBubble(const BubbleItem *bubble) const @@ -98,25 +103,12 @@ BubbleItem *BubbleModel::replaceBubble(BubbleItem *bubble) m_bubbles.replace(replaceIndex, bubble); Q_EMIT dataChanged(index(replaceIndex), index(replaceIndex)); - return oldBubble; -} - -void BubbleModel::clear() -{ - if (m_processPendingTimer) { - m_processPendingTimer->stop(); - } - qDeleteAll(m_pendingBubbles); - m_pendingBubbles.clear(); - - if (m_bubbles.count() <= 0) - return; - beginResetModel(); - qDeleteAll(m_bubbles); - m_bubbles.clear(); - endResetModel(); + // The replacement shares the bubble slot of the old bubble; ExpireTimer + // cancels the old countdown (transferring a hover block) and starts a new + // one, so just push it like insertBubble() does. + ExpireTimer::instance()->push(bubble->entity()); - m_updateTimeTipTimer->stop(); + return oldBubble; } QList BubbleModel::items() const @@ -131,9 +123,9 @@ void BubbleModel::remove(int index) beginRemoveRows(QModelIndex(), index, index); auto bubble = m_bubbles.takeAt(index); + ExpireTimer::instance()->remove(bubble->entity()); bubble->deleteLater(); endRemoveRows(); - } void BubbleModel::remove(const BubbleItem *bubble) @@ -298,4 +290,5 @@ void BubbleModel::updateContentRowCount(int rowCount) Q_EMIT dataChanged(index(0), index(m_bubbles.size() - 1), {BubbleModel::ContentRowCount}); } } -} + +} // notification diff --git a/panels/notification/bubble/bubblemodel.h b/panels/notification/bubble/bubblemodel.h index b9b8f6203..d5f74d5b4 100644 --- a/panels/notification/bubble/bubblemodel.h +++ b/panels/notification/bubble/bubblemodel.h @@ -8,6 +8,7 @@ #include "notifyentity.h" #include +#include #include class QTimer; @@ -49,7 +50,6 @@ class BubbleModel : public QAbstractListModel Q_INVOKABLE void remove(int index); void remove(const BubbleItem *bubble); BubbleItem *removeById(qint64 id); - void clear(); BubbleItem *bubbleItem(int bubbleIndex) const; @@ -68,7 +68,6 @@ class BubbleModel : public QAbstractListModel void updateBubbleTimeTip(); void updateContentRowCount(int rowCount); -private: QTimer *m_updateTimeTipTimer = nullptr; QTimer *m_processPendingTimer = nullptr; QList m_bubbles; diff --git a/panels/notification/bubble/bubblepanel.cpp b/panels/notification/bubble/bubblepanel.cpp index 469fe67f7..98f1f272a 100644 --- a/panels/notification/bubble/bubblepanel.cpp +++ b/panels/notification/bubble/bubblepanel.cpp @@ -6,6 +6,7 @@ #include "bubbleitem.h" #include "bubblemodel.h" #include "dataaccessorproxy.h" +#include "expiretimer.h" #include "pluginfactory.h" #include @@ -217,7 +218,7 @@ void BubblePanel::setEnabled(bool newEnabled) void BubblePanel::setHoveredId(qint64 id) { - QMetaObject::invokeMethod(m_notificationServer, "setBlockClosedId", Qt::DirectConnection, Q_ARG(qint64, id)); + ExpireTimer::instance()->setBlockId(id); } } diff --git a/panels/notification/center/notifystagingmodel.cpp b/panels/notification/center/notifystagingmodel.cpp index 303bfc75c..bacbdaf0f 100644 --- a/panels/notification/center/notifystagingmodel.cpp +++ b/panels/notification/center/notifystagingmodel.cpp @@ -8,6 +8,7 @@ #include #include "dataaccessorproxy.h" +#include "expiretimer.h" #include "notifyaccessor.h" #include "notifyentity.h" #include "notifyitem.h" @@ -25,6 +26,13 @@ NotifyStagingModel::NotifyStagingModel(QObject *parent) connect(NotifyAccessor::instance(), &NotifyAccessor::stagingEntityReceived, this, &NotifyStagingModel::doEntityReceived); connect(NotifyAccessor::instance(), &NotifyAccessor::stagingEntityClosed, this, &NotifyStagingModel::onEntityClosed); connect(NotifySetting::instance(), &NotifySetting::contentRowCountChanged, this, &NotifyStagingModel::updateContentRowCount); + // No direct reaction to ExpireTimer::expired here: the server is the single + // owner of the close on expiry (it listens to expired itself and marks the + // notification processed), and this model drops its row via the resulting + // stagingEntityClosed. Reacting locally would remove + refill while the + // server-side close is still queued on the worker thread, so the just-expired + // notification would still read as NotProcessed and be re-inserted with a + // fresh countdown. } void NotifyStagingModel::close() @@ -63,6 +71,10 @@ void NotifyStagingModel::push(const NotifyEntity &entity) updateOverlapCount(count); } + // A non-positive interval (Critical urgency or expireTimeout 0) means the + // notification never expires on its own. + ExpireTimer::instance()->push(entity); + if (m_refreshTimer < 0) { m_refreshTimer = startTimer(std::chrono::milliseconds(1000)); } @@ -90,6 +102,10 @@ void NotifyStagingModel::remove(qint64 id) { qDebug(notifyLog) << "Remove notify by id" << id; + const auto entity = notifyById(id); + if (entity.isValid()) + ExpireTimer::instance()->remove(entity); + int row = -1; for (int i = 0; i < m_appNotifies.size(); i++) { auto item = m_appNotifies[i]; @@ -146,6 +162,7 @@ void NotifyStagingModel::remove(qint64 id) auto notify = new AppNotifyItem(newEntity); m_appNotifies.insert(insertedIndex, notify); endInsertRows(); + ExpireTimer::instance()->push(newEntity); } } updateOverlapCount(entities.size()); @@ -172,6 +189,9 @@ void NotifyStagingModel::open() auto notify = new AppNotifyItem(entities.at(i)); m_appNotifies << notify; } + for (const auto &entity : entities) { + ExpireTimer::instance()->push(entity); + } updateOverlapCount(entities.size()); endResetModel(); @@ -250,8 +270,11 @@ void NotifyStagingModel::replace(const NotifyEntity &entity) { for (int i = 0; i < m_appNotifies.size(); i++) { auto item = m_appNotifies[i]; - if (item->id() == entity.bubbleId()) { + if (item->id() == entity.id()) { + // push() handles the replacement internally: it cancels the old + // countdown of the same bubble slot and starts the new one. item->setEntity(entity); + ExpireTimer::instance()->push(entity); const auto index = this->index(i, 0, {}); dataChanged(index, index); break; diff --git a/panels/notification/common/expiretimer.cpp b/panels/notification/common/expiretimer.cpp new file mode 100644 index 000000000..187cf0b18 --- /dev/null +++ b/panels/notification/common/expiretimer.cpp @@ -0,0 +1,234 @@ +// SPDX-FileCopyrightText: 2026 UnionTech Software Technology Co., Ltd. +// +// SPDX-License-Identifier: GPL-3.0-or-later + +#include "expiretimer.h" + +#include +#include + +#include + +namespace notification { + +static const int DefaultTimeoutMSecs = 5000; + +// Hover grace period: when the hover moves away from a bubble, its countdown +// resumes with at least this much time left so the bubble lingers briefly. +static const int BlockItemTimeout = 1000; + +// Upper bound on how many suspended deadlines remove() may remember. Ids that are +// removed and never pushed again (genuinely removed notifications) are +// evicted oldest-first once the cap is reached, so they cannot leak unboundedly. +static const int MaxRememberedIds = 64; + +// Effective expire timeout in milliseconds for a notification. +// Returns 0 for "never expire" (Critical urgency or expireTimeout == 0) and +// falls back to the server default of 5000 ms for expireTimeout == -1. +static int effectiveTimeout(const NotifyEntity &entity) +{ + if (entity.urgency() == NotifyEntity::Critical || entity.timeout() == 0) + return 0; + + return entity.timeout() == -1 ? DefaultTimeoutMSecs : entity.timeout(); +} + +ExpireTimer::ExpireTimer(QObject *parent) + : QObject(parent) + , m_timer(new QTimer(this)) +{ + m_timer->setSingleShot(true); + connect(m_timer, &QTimer::timeout, this, &ExpireTimer::onTimeout); +} + +ExpireTimer *ExpireTimer::instance() +{ + static ExpireTimer expireTimer; + return &expireTimer; +} + +void ExpireTimer::push(const NotifyEntity &entity) +{ + const auto id = entity.id(); + const int interval = effectiveTimeout(entity); + + // A replacement notification occupies the same bubble slot as the replaced + // one, so forget the old countdown (active or suspended) before starting + // the new one. If the replaced bubble is hovered, keep the block on the new + // id so hovering still keeps the replacement on screen. + if (entity.isReplace()) + cancelReplacement(entity); + + if (interval <= 0) { + // Never expire: cancel any pending, paused or suspended countdown. + if (m_paused.entity.isValid() && m_paused.entity.id() == id) { + m_paused = {}; + } + m_deadlines.remove(id); + m_retired.remove(id); + schedule(); + return; + } + + // Keep the paused countdown while the id is hovered. + if (m_paused.entity.isValid() && m_paused.entity.id() == id) + return; + + // A notification displayed in both the bubble and the staging area shares + // one deadline: starting an already tracked id keeps the original countdown + // instead of restarting it. + if (m_deadlines.contains(id)) + return; + + const auto now = QDateTime::currentMSecsSinceEpoch(); + + // Resume a countdown suspended by remove() instead of restarting it, so a + // notification that moved between the bubble and the staging area keeps + // its original expire deadline (or expires immediately if it already + // passed while suspended). + if (const auto it = m_retired.constFind(id); it != m_retired.cend()) { + m_deadlines.insert(id, {it->entity, qMax(now, it->point)}); + m_retired.erase(it); + schedule(); + return; + } + + m_deadlines.insert(id, {entity, now + interval}); + schedule(); +} + +void ExpireTimer::cancelReplacement(const NotifyEntity &entity) +{ + const auto id = entity.id(); + const auto bubbleId = entity.bubbleId(); + + const auto forgetMatching = [this, id, bubbleId, &entity](QHash &table) { + const QList matching = [&table, id, bubbleId] { + QList ids; + for (auto it = table.cbegin(); it != table.cend(); ++it) { + if (it.key() != id && it.value().entity.bubbleId() == bubbleId) + ids.append(it.key()); + } + return ids; + }(); + for (const auto &oldId : matching) { + table.remove(oldId); + // The replaced bubble is gone but the hover remains on the same + // slot, so freeze the replacement instead of the old id. + if (m_paused.entity.isValid() && m_paused.entity.id() == oldId) { + m_paused.entity = entity; + } + } + }; + + forgetMatching(m_deadlines); + forgetMatching(m_retired); + + // A hovered (paused) replaced bubble is in none of the tables; transfer the + // block to the replacement so hovering still keeps it on screen. + if (m_paused.entity.isValid() && m_paused.entity.id() != id && m_paused.entity.bubbleId() == bubbleId) { + m_paused.entity = entity; + } + schedule(); +} + +void ExpireTimer::remove(const NotifyEntity &entity) +{ + const auto id = entity.id(); + + // A hovered countdown that is removed keeps its remaining time too. + if (m_paused.entity.isValid() && m_paused.entity.id() == id) { + m_retired.insert(id, m_paused); + m_paused = {}; + } + + if (const auto it = m_deadlines.constFind(id); it != m_deadlines.cend()) { + // Suspend rather than forget so a later push() resumes the same + // countdown instead of restarting it. + m_retired.insert(id, it.value()); + m_deadlines.erase(it); + + // Bound the memory of ids that are removed and never pushed again. + if (m_retired.size() > MaxRememberedIds) { + const auto minIt = std::min_element(m_retired.constBegin(), m_retired.constEnd(), + [](const Deadline &lhs, const Deadline &rhs) { return lhs.point < rhs.point; }); + m_retired.erase(minIt); + } + schedule(); + } +} + +void ExpireTimer::setBlockId(qint64 id) +{ + if (m_paused.entity.isValid() && m_paused.entity.id() == id) + return; + + const qint64 now = QDateTime::currentMSecsSinceEpoch(); + + // The hover moved away from the previously blocked id: resume its + // countdown with at least BlockItemTimeout ms left so the bubble lingers + // briefly after the hover ends. + if (m_paused.entity.isValid()) { + m_deadlines.insert(m_paused.entity.id(), {m_paused.entity, qMax(m_paused.point, now + BlockItemTimeout)}); + schedule(); + } + + // Block the newly hovered id by freezing its deadline. If it has no running + // countdown (never-expire or not yet started) there is nothing to pause; + // clear the block so unhover does not later resume a ghost deadline. + const auto it = m_deadlines.constFind(id); + if (it == m_deadlines.cend()) { + m_paused = {}; + return; + } + + m_paused = Deadline{it->entity, it->point}; + m_deadlines.erase(it); + schedule(); +} + +void ExpireTimer::schedule() +{ + if (m_deadlines.isEmpty()) { + m_timer->stop(); + return; + } + + auto it = std::min_element(m_deadlines.cbegin(), m_deadlines.cend(), + [](const Deadline &lhs, const Deadline &rhs) { return lhs.point < rhs.point; }); + const qint64 remaining = qMax(0, it.value().point - QDateTime::currentMSecsSinceEpoch()); + m_timer->start(static_cast(remaining)); +} + +void ExpireTimer::onTimeout() +{ + const auto now = QDateTime::currentMSecsSinceEpoch(); + const QList expiredIds = [this, now] { + QList ids; + for (auto it = m_deadlines.cbegin(); it != m_deadlines.cend(); ++it) { + if (it.value().point <= now) + ids.append(it.key()); + } + return ids; + }(); + + for (const auto &id : expiredIds) { + const auto it = m_deadlines.find(id); + if (it == m_deadlines.cend()) + continue; + + // The id expired: its countdown is over for good, so forget it in every + // table (active, suspended and hovered). + const auto bubbleId = it->entity.bubbleId(); + m_deadlines.erase(it); + m_retired.remove(id); + if (m_paused.entity.isValid() && m_paused.entity.id() == id) { + m_paused = {}; + } + Q_EMIT expired(id, bubbleId); + } + + schedule(); +} + +} diff --git a/panels/notification/common/expiretimer.h b/panels/notification/common/expiretimer.h new file mode 100644 index 000000000..5a116bb4d --- /dev/null +++ b/panels/notification/common/expiretimer.h @@ -0,0 +1,91 @@ +// SPDX-FileCopyrightText: 2026 UnionTech Software Technology Co., Ltd. +// +// SPDX-License-Identifier: GPL-3.0-or-later + +#pragma once + +#include +#include + +#include "notifyentity.h" + +class QTimer; + +namespace notification { + +/** + * @brief Process-wide singleton that tracks the expire deadline of every + * displayed notification (bubble or staging area) with one shared + * single-shot QTimer. + * + * Each notification id keeps its own deadline and the nearest one drives the + * shared QTimer; when a deadline passes, expired() is emitted once with the + * notification id and its bubble id, and the id is forgotten. Pushing an + * already tracked id keeps the original deadline, and remove() suspends a + * countdown by remembering its absolute deadline, so a notification shown in + * both the bubble and the staging area (or moving between them) never restarts + * its countdown. The effective timeout is derived internally from the entity, + * so callers only describe what to do (push, remove, setBlockId) and never + * have to compute or query timeouts. No QTimer is allocated per id and nothing + * leaks when an id expires or is removed. Only one id can be blocked at a time + * (the hovered bubble); the hover handling is encapsulated in setBlockId so + * callers do not deal with pause/resume details. + */ +class ExpireTimer : public QObject +{ + Q_OBJECT +public: + static ExpireTimer *instance(); + + // Pushes the entity into the expire tracking so its id gets a countdown + // based on its urgency and expire timeout. A non-positive timeout (Critical + // urgency or expireTimeout 0) cancels any pending or paused countdown, i.e. + // the notification never expires. If the id was previously removed, the + // original countdown is resumed instead of being restarted. A replacement + // notification (isReplace()) occupies the same bubble slot as the replaced + // one, so its countdown is cancelled here before the new one is started; + // a hovered replaced bubble keeps its block on the new id. + void push(const NotifyEntity &entity); + // Removes the entity's id from tracking. The remaining countdown is + // suspended (not forgotten), so a later push() with the same id resumes it. + void remove(const NotifyEntity &entity); + // Blocks the hovered id from expiring, keeping its remaining time. Only one + // id is blocked at a time: switching to another id resumes the previous one + // with at least a short grace period left (keeping the bubble visible for a + // brief moment after the hover moves away). Passing InvalidId clears the + // block. + void setBlockId(qint64 id); + +Q_SIGNALS: + // Emitted once when the deadline of id passes. + void expired(qint64 id, uint bubbleId); + +private: + // A tracked countdown: the notification being timed and its absolute + // expire deadline. The bubble id used on expiry is read from the entity. + struct Deadline + { + NotifyEntity entity; + // Absolute expiry deadline in ms since the epoch. + qint64 point = 0; + }; + + explicit ExpireTimer(QObject *parent = nullptr); + + void schedule(); + void onTimeout(); + void cancelReplacement(const NotifyEntity &entity); + + QTimer *m_timer = nullptr; + // Active countdowns keyed by notification id. + QHash m_deadlines; + // Countdowns suspended by remove(); restored by push() so a context switch + // (bubble <-> staging) resumes the countdown. + QHash m_retired; + // The single hovered countdown whose deadline is frozen while the mouse + // stays over its bubble, and the state needed to restore it when the hover + // moves away. An invalid entity means no id is currently blocked. + Deadline m_paused; +}; + +} diff --git a/panels/notification/common/notifyentity.cpp b/panels/notification/common/notifyentity.cpp index 7c55cdc82..a7c832096 100644 --- a/panels/notification/common/notifyentity.cpp +++ b/panels/notification/common/notifyentity.cpp @@ -236,6 +236,16 @@ bool NotifyEntity::isReplace() const return d->replacesId != NoReplaceId; } +int NotifyEntity::timeout() const +{ + return d->expireTimeout; +} + +int NotifyEntity::urgency() const +{ + return d->hints.value("urgency").toInt(); +} + qint64 NotifyEntity::cTime() const { return d->cTime; diff --git a/panels/notification/common/notifyentity.h b/panels/notification/common/notifyentity.h index 967280ddb..f3f99200f 100644 --- a/panels/notification/common/notifyentity.h +++ b/panels/notification/common/notifyentity.h @@ -81,6 +81,10 @@ class NotifyEntity void setReplacesId(uint replacesId); bool isReplace() const; + // Expire timeout in milliseconds passed in by the client (-1 means server default). + int timeout() const; + int urgency() const; + qint64 cTime() const; void setCTime(qint64 cTime); diff --git a/panels/notification/server/notificationmanager.cpp b/panels/notification/server/notificationmanager.cpp index 74c564197..139edf7b8 100644 --- a/panels/notification/server/notificationmanager.cpp +++ b/panels/notification/server/notificationmanager.cpp @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -37,8 +36,6 @@ Q_DECLARE_LOGGING_CATEGORY(notifyLog) namespace notification { static const uint NoReplacesId = 0; -static const int DefaultTimeOutMSecs = 5000; -static const int BlockItemTimeout = 1000; static const QString NotificationsDBusService = "org.freedesktop.Notifications"; static const QString NotificationsDBusPath = "/org/freedesktop/Notifications"; static const QString DDENotifyDBusServer = "org.deepin.dde.Notification1"; @@ -50,11 +47,7 @@ NotificationManager::NotificationManager(QObject *parent) : QObject(parent) , m_persistence(DataAccessorProxy::instance()) , m_setting(new NotificationSetting(this)) - , m_pendingTimeout(new QTimer(this)) { - m_pendingTimeout->setSingleShot(true); - connect(m_pendingTimeout, &QTimer::timeout, this, &NotificationManager::onHandingPendingEntities); - DataAccessorProxy::instance()->setSource(DBAccessor::instance()); DAppletBridge bridge("org.deepin.ds.dde-apps"); @@ -164,6 +157,18 @@ void NotificationManager::actionInvoked(qint64 id, uint bubbleId, const QString void NotificationManager::notificationClosed(qint64 id, uint bubbleId, uint reason) { qDebug(notifyLog) << "Close notification id" << id << ", reason" << reason; + + const auto entity = m_persistence->fetchEntity(id); + // A notification can be tracked by more than one expire timer (the bubble + // frontend and the notification center staging model both schedule a timeout + // for the same id), so it may already be closed or removed by the time this + // is reached. Report the close only once to avoid emitting NotificationClosed + // twice for a single notification. An entity that was already processed is + // still stored (Expired keeps the row and only marks it Processed), so the + // validity check alone is not enough. + if (!entity.isValid() || entity.processedType() != NotifyEntity::NotProcessed) + return; + updateEntityProcessed(id, reason); Q_EMIT NotificationClosed(bubbleId, reason); @@ -296,22 +301,9 @@ uint NotificationManager::Notify(const QString &appName, uint replacesId, const return 0; } - if (entity.isReplace() && m_persistence->fetchLastEntity(entity.bubbleId()).isValid()) { - removePendingEntity(entity); - } - emitRecordCountChanged(); Q_EMIT NotificationStateChanged(entity.id(), entity.processedType()); - - bool critical = false; - if (auto iter = hints.find("urgency"); iter != hints.end()) { - critical = iter.value().toUInt() == NotifyEntity::Critical; - } - // 0: never expire. -1: DefaultTimeOutMSecs - if (expireTimeout != 0 && !critical) { - pushPendingEntity(entity, expireTimeout); - } } tryPlayNotificationSound(entity, appId, dndMode); @@ -372,29 +364,6 @@ QVariant NotificationManager::GetSystemInfo(uint configItem) return m_setting->systemValue(static_cast(configItem)); } -void NotificationManager::setBlockClosedId(qint64 id) -{ - if (id == m_blockClosedId) { - return; - } - - if(m_blockClosedId != NotifyEntity::InvalidId) { - auto findIter = std::find_if(m_pendingTimeoutEntities.begin(), m_pendingTimeoutEntities.end(), [this](const NotifyEntity &entity) { - return entity.id() == m_blockClosedId; - }); - - const auto current = QDateTime::currentMSecsSinceEpoch(); - if (findIter != m_pendingTimeoutEntities.end()) { - if (current > findIter.key() - BlockItemTimeout) { - qDebug(notifyLog) << "Delay close bubble id:" << m_blockClosedId << "for the new block bubble id:" << id; - m_pendingTimeoutEntities.insert(current + BlockItemTimeout, findIter.value()); - m_pendingTimeoutEntities.erase(findIter); - } - } - } - m_blockClosedId = id; - onHandingPendingEntities(); -} bool NotificationManager::isDoNotDisturb() const { @@ -498,21 +467,6 @@ void NotificationManager::emitRecordCountChanged() emit RecordCountChanged(count); } -void NotificationManager::pushPendingEntity(const NotifyEntity &entity, int expireTimeout) -{ - const int interval = expireTimeout == -1 ? DefaultTimeOutMSecs : expireTimeout; - - qint64 point = QDateTime::currentMSecsSinceEpoch() + interval; - m_pendingTimeoutEntities.insert(point, entity); - - if (m_lastTimeoutPoint > point) { - m_lastTimeoutPoint = point; - auto newInterval = m_lastTimeoutPoint - QDateTime::currentMSecsSinceEpoch(); - m_pendingTimeout->setInterval(newInterval); - m_pendingTimeout->start(); - } -} - void NotificationManager::updateEntityProcessed(qint64 id, uint reason) { auto entity = m_persistence->fetchEntity(id); @@ -546,7 +500,6 @@ void NotificationManager::updateEntityProcessed(const NotifyEntity &entity) Q_EMIT NotificationStateChanged(entity.id(), entity.processedType()); - removePendingEntity(entity); emitRecordCountChanged(); } @@ -707,69 +660,6 @@ void NotificationManager::initScreenLockedState() "Visible", this, SLOT(onScreenLockedChanged(bool))); } -void NotificationManager::onHandingPendingEntities() -{ - QList timeoutEntities; - - const auto current = QDateTime::currentMSecsSinceEpoch(); - for (auto iter = m_pendingTimeoutEntities.begin(); iter != m_pendingTimeoutEntities.end();) { - const auto point = iter.key(); - if (point > current) { - iter++; - continue; - } - - const auto entity = iter.value();; - timeoutEntities << entity; - iter = m_pendingTimeoutEntities.erase(iter); - } - - // update pendingTimeout to deal with m_pendingTimeoutEntities - if (!m_pendingTimeoutEntities.isEmpty()) { - auto points = m_pendingTimeoutEntities.keys(); - std::sort(points.begin(), points.end()); - // find last point to restart pendingTimeout - m_lastTimeoutPoint = points.first(); - auto newInterval = m_lastTimeoutPoint - current; - // let timer start in main thread - QMetaObject::invokeMethod(m_pendingTimeout, "start", Qt::QueuedConnection, Q_ARG(int, newInterval)); - } else { - // reset m_lastTimeoutPoint - m_lastTimeoutPoint = std::numeric_limits::max(); - } - - for (const auto &item : timeoutEntities) { - // Validate entity before processing timeout to prevent race conditions - if (!item.isValid()) { - qWarning(notifyLog) << "Skipping timeout processing for invalid entity id:" << item.id() << "appName:" << item.appName() - << "cTime:" << item.cTime(); - continue; - } - - if (item.id() == m_blockClosedId) { - qDebug(notifyLog) << "bubble id:" << item.bubbleId() << "entity id:" << item.id(); - m_pendingTimeoutEntities.insert(current, item); - continue; - } - - qDebug(notifyLog) << "Expired for the notification " << item.id() << item.appName(); - notificationClosed(item.id(), item.bubbleId(), NotifyEntity::Expired); - } -} - -void NotificationManager::removePendingEntity(const NotifyEntity &entity) -{ - for (auto iter = m_pendingTimeoutEntities.begin(); iter != m_pendingTimeoutEntities.end();) { - const auto item = iter.value(); - if (item == entity || (entity.isReplace() && item.bubbleId() == entity.bubbleId())) { - m_pendingTimeoutEntities.erase(iter); - onHandingPendingEntities(); - break; - } - ++iter; - } -} - void NotificationManager::onScreenLockedChanged(bool screenLocked) { m_screenLocked = screenLocked; diff --git a/panels/notification/server/notificationmanager.h b/panels/notification/server/notificationmanager.h index f2756669d..1d95b4530 100644 --- a/panels/notification/server/notificationmanager.h +++ b/panels/notification/server/notificationmanager.h @@ -7,7 +7,6 @@ #include #include -class QTimer; namespace notification { class NotifyEntity; @@ -68,14 +67,12 @@ public Q_SLOTS: void SetSystemInfo(uint configItem, const QVariant &value); QVariant GetSystemInfo(uint configItem); - void setBlockClosedId(qint64 id); private: bool isDoNotDisturb() const; bool recordNotification(NotifyEntity &entity); void tryPlayNotificationSound(const NotifyEntity &entity, const QString &appId, bool dndMode) const; void emitRecordCountChanged(); - void pushPendingEntity(const NotifyEntity &entity, int expireTimeout); void updateEntityProcessed(qint64 id, uint reason); void updateEntityProcessed(const NotifyEntity &entity); @@ -86,8 +83,6 @@ public Q_SLOTS: void initScreenLockedState(); private slots: - void onHandingPendingEntities(); - void removePendingEntity(const NotifyEntity &entity); void onScreenLockedChanged(bool); private: @@ -96,13 +91,9 @@ private slots: DataAccessor *m_persistence = nullptr; NotificationSetting *m_setting = nullptr; - QTimer *m_pendingTimeout = nullptr; - qint64 m_lastTimeoutPoint = std::numeric_limits::max(); - QMultiHash m_pendingTimeoutEntities; QStringList m_systemApps; QMap m_appNamesMap; int m_cleanupDays = 7; - qint64 m_blockClosedId = 0; }; } // notification diff --git a/panels/notification/server/notifyserverapplet.cpp b/panels/notification/server/notifyserverapplet.cpp index b4de43dfc..94708436b 100644 --- a/panels/notification/server/notifyserverapplet.cpp +++ b/panels/notification/server/notifyserverapplet.cpp @@ -5,6 +5,7 @@ #include "notifyserverapplet.h" #include "notificationmanager.h" #include "dbusadaptor.h" +#include "expiretimer.h" #include "pluginfactory.h" #include @@ -56,6 +57,15 @@ bool NotifyServerApplet::init() connect(m_manager, &NotificationManager::NotificationStateChanged, this, &NotifyServerApplet::notificationStateChanged); + // ExpireTimer tracks the countdown of every shown notification (bubble and + // staging area). When a deadline passes, this is the single place that tells + // the server to close the notification; the frontend views only react to the + // resulting NotificationStateChanged instead of closing on their own. + connect(ExpireTimer::instance(), &ExpireTimer::expired, this, [this](qint64 id, uint bubbleId) { + QMetaObject::invokeMethod(m_manager, "notificationClosed", Qt::QueuedConnection, + Q_ARG(qint64, id), Q_ARG(uint, bubbleId), Q_ARG(uint, NotifyEntity::Expired)); + }); + removeExpiredNotifications(); m_worker = new QThread(); @@ -76,7 +86,8 @@ void NotifyServerApplet::actionInvoked(qint64 id, const QString &actionKey) void NotifyServerApplet::notificationClosed(qint64 id, uint bubbleId, uint reason) { - QMetaObject::invokeMethod(m_manager, "notificationClosed", Qt::DirectConnection, Q_ARG(qint64, id), Q_ARG(uint, bubbleId), Q_ARG(uint, reason)); + // The manager lives on the worker thread, so deliver the close to it there. + QMetaObject::invokeMethod(m_manager, "notificationClosed", Qt::QueuedConnection, Q_ARG(qint64, id), Q_ARG(uint, bubbleId), Q_ARG(uint, reason)); } QVariant NotifyServerApplet::appValue(const QString &appId, int configItem) @@ -104,11 +115,6 @@ void NotifyServerApplet::removeExpiredNotifications() m_manager->removeExpiredNotifications(); } -void NotifyServerApplet::setBlockClosedId(qint64 id) -{ - m_manager->setBlockClosedId(id); -} - D_APPLET_CLASS(NotifyServerApplet) } diff --git a/panels/notification/server/notifyserverapplet.h b/panels/notification/server/notifyserverapplet.h index 20975e91d..ff8ea57e9 100644 --- a/panels/notification/server/notifyserverapplet.h +++ b/panels/notification/server/notifyserverapplet.h @@ -31,7 +31,6 @@ public Q_SLOTS: void removeNotifications(const QString &appName); void removeNotifications(); void removeExpiredNotifications(); - void setBlockClosedId(qint64 id); private: NotificationManager *m_manager = nullptr; diff --git a/tests/panels/notification/server/notifyserverapplet_test.cpp b/tests/panels/notification/server/notifyserverapplet_test.cpp index 9a0463167..64ba2b46d 100644 --- a/tests/panels/notification/server/notifyserverapplet_test.cpp +++ b/tests/panels/notification/server/notifyserverapplet_test.cpp @@ -34,7 +34,6 @@ class MockNotificationManager : public NotificationManager { MOCK_METHOD(void, removeNotifications, (const QString &appName)); MOCK_METHOD(void, removeNotifications, ()); MOCK_METHOD(void, removeExpiredNotifications, ()); - MOCK_METHOD(void, setBlockClosedId, (qint64 id)); }; // Test fixture for NotifyServerApplet @@ -244,17 +243,6 @@ TEST_F(NotifyServerAppletTest, RemoveExpiredNotificationsTest) { EXPECT_NO_THROW(applet->removeExpiredNotifications()); } -// Test setBlockClosedId -TEST_F(NotifyServerAppletTest, SetBlockClosedIdTest) { - // Initialize applet first - applet->init(); - - qint64 testId = 12345; - - // Test that setBlockClosedId doesn't crash - EXPECT_NO_THROW(applet->setBlockClosedId(testId)); -} - // Test notificationStateChanged signal TEST_F(NotifyServerAppletTest, NotificationStateChangedSignalTest) { // Initialize applet first @@ -315,16 +303,6 @@ TEST_F(NotifyServerAppletTest, NotificationClosedEdgeCasesTest) { EXPECT_NO_THROW(applet->notificationClosed(999999999, 999999, 3)); } -// Test edge cases for setBlockClosedId -TEST_F(NotifyServerAppletTest, SetBlockClosedIdEdgeCasesTest) { - applet->init(); - - // Test with various ID values - EXPECT_NO_THROW(applet->setBlockClosedId(0)); - EXPECT_NO_THROW(applet->setBlockClosedId(-1)); - EXPECT_NO_THROW(applet->setBlockClosedId(9223372036854775807LL)); // max qint64 -} - // Test that applet properly inherits from DApplet TEST_F(NotifyServerAppletTest, InheritanceTest) { EXPECT_TRUE(applet->inherits("ds::DApplet"));