From 8c366c9bad9041eeee76215eaf8bb197a3e46f8a Mon Sep 17 00:00:00 2001 From: Ivy233 Date: Mon, 17 Aug 2026 13:31:08 +0800 Subject: [PATCH] fix: expire notification countdown only after it is displayed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Move the pending-timeout machinery from NotificationManager to the frontend ExpireTimer singleton, preserving one shared QTimer and absolute deadlines in a QMultiHash while removing the server-side timer state 2. Start a countdown only when a notification is inserted into the bubble model or the visible staging model, so queued, hidden and overflow notifications do not expire before they are displayed 3. Make push idempotent for the same entity so the bubble and staging views share the first deadline, and replace the old bubble-slot countdown when a new entity replaces it 4. Keep hover blocking inside ExpireTimer and give an unblocked notification a short grace period without spinning the timer at a zero interval 5. Keep the server authoritative for notification lifecycle: forward expiration through NotifyServerApplet with a queued call, and stop countdowns from server state changes instead of frontend model removal 6. Derive effective timeouts from NotifyEntity (0/Critical never expires and -1 uses the 5000 ms default) and carry bubbleId in the expiration signal 7. Remove the staging expiry race by waiting for the server close round trip before refilling rows; hidden-panel updates remain gated by NotifyAccessor 8. Remove the obsolete NotificationManager pending-timeout code and its applet tests 9. Match staging notification replacements by bubble id (the key preserved across a replace), exactly like BubbleModel::replaceBubbleIndex: a replacement may carry a fresh entity id (marked-processed then re-added) or the original entity id (replaceEntity), so matching on the entity id left the old staging row stale and inserted a duplicate instead of updating in place 10. Update the SPDX copyright year of notifystagingmodel.h to 2024-2026 Log: Start notification expiration after actual display and keep lifecycle cleanup owned by the server Influence: 1. Verify normal notifications disappear after their configured/default timeout 2. Verify queued and hidden notifications do not expire before being displayed 3. Verify opening the notification center starts only visible staging countdowns 4. Verify hovering prevents expiration and leaves a short grace period on exit 5. Verify replacement and shared bubble/staging notifications keep one countdown 6. Verify close, invoke and expiry paths stop countdowns through server state 7. Verify a replaced staging notification updates in place instead of leaving a stale row and inserting a duplicate fix: 通知实际显示后才开始过期计时 1. 将待超时机制从 NotificationManager 迁移到前端 ExpireTimer 单例，保留一个 共享 QTimer，并以 QMultiHash 保存绝对截止时间，同时移除服务端定时器状态 2. 仅在通知插入横幅模型或可见暂存区模型时启动倒计时，排队、隐藏及未展示的 重叠通知不会在实际显示前过期 3. 同一实体重复 push 时保持首次截止时间，使横幅与暂存区共享一个倒计时；新实体 替换通知时则移除同一气泡槽位的旧倒计时 4. 将悬停阻塞封装在 ExpireTimer 内，解除阻塞后保留短暂宽限期，并避免定时器以 零间隔空转 5. 由服务端统一决定通知生命周期：NotifyServerApplet 通过队列调用转发过期关闭， 并根据服务端状态变化停止倒计时，不再由前端模型移除操作清理 6. 由 NotifyEntity 推导有效超时（0/Critical 永不过期，-1 使用默认 5000ms）， 并在过期信号中携带 bubbleId 7. 暂存区等待服务端关闭回环后再补位，避免过期通知被重新插入；面板隐藏期间的 更新继续由 NotifyAccessor 统一屏蔽 8. 移除 NotificationManager 中废弃的待超时代码及对应 applet 测试 9. 暂存区通知替换改为按 bubbleId 匹配（与 BubbleModel::replaceBubbleIndex 一致）：替换可能携带全新实体 id（旧实体标记已处理后再新增）或原实体 id （replaceEntity），按实体 id 匹配会导致旧行残留并插入重复行，无法原地更新 10. 更新 notifystagingmodel.h 的 SPDX 版权年份为 2024-2026 Log: 通知实际显示后开始过期计时，并由服务端统一管理生命周期清理 Influence: 1. 验证普通通知按配置或默认超时时间消失 2. 验证排队及隐藏通知不会在显示前过期 3. 验证打开通知中心时仅启动实际可见暂存通知的倒计时 4. 验证悬停阻止通知过期，移开后保留短暂宽限期 5. 验证替换通知以及横幅/暂存区共享通知只保留一个倒计时 6. 验证关闭、调用动作及过期路径均通过服务端状态停止倒计时 7. 验证被替换的暂存区通知原地更新，而非残留旧行并插入重复行 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 | 32 ++-- panels/notification/bubble/bubblemodel.h | 3 +- panels/notification/bubble/bubblepanel.cpp | 3 +- .../center/notifystagingmodel.cpp | 52 ++++- .../notification/center/notifystagingmodel.h | 3 +- panels/notification/common/expiretimer.cpp | 179 ++++++++++++++++++ panels/notification/common/expiretimer.h | 76 ++++++++ panels/notification/common/notifyentity.cpp | 10 + panels/notification/common/notifyentity.h | 4 + .../server/notificationmanager.cpp | 134 ++----------- .../notification/server/notificationmanager.h | 9 - .../server/notifyserverapplet.cpp | 28 ++- .../notification/server/notifyserverapplet.h | 1 - .../server/notifyserverapplet_test.cpp | 22 --- 17 files changed, 370 insertions(+), 194 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..0feeef390 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 @@ -133,7 +125,6 @@ void BubbleModel::remove(int index) auto bubble = m_bubbles.takeAt(index); bubble->deleteLater(); endRemoveRows(); - } void BubbleModel::remove(const BubbleItem *bubble) @@ -298,4 +289,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..702da60d1 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)); } @@ -146,6 +158,7 @@ void NotifyStagingModel::remove(qint64 id) auto notify = new AppNotifyItem(newEntity); m_appNotifies.insert(insertedIndex, notify); endInsertRows(); + ExpireTimer::instance()->push(newEntity); } } updateOverlapCount(entities.size()); @@ -169,8 +182,10 @@ void NotifyStagingModel::open() const auto count = std::min(static_cast(entities.size()), BubbleMaxCount); for (int i = 0; i < count; i++) { - auto notify = new AppNotifyItem(entities.at(i)); + const auto &entity = entities.at(i); + auto notify = new AppNotifyItem(entity); m_appNotifies << notify; + ExpireTimer::instance()->push(entity); } updateOverlapCount(entities.size()); @@ -246,17 +261,33 @@ NotifyEntity NotifyStagingModel::notifyById(qint64 id) const return {}; } -void NotifyStagingModel::replace(const NotifyEntity &entity) +int NotifyStagingModel::rowByBubbleId(uint bubbleId) const { for (int i = 0; i < m_appNotifies.size(); i++) { - auto item = m_appNotifies[i]; - if (item->id() == entity.bubbleId()) { - item->setEntity(entity); - const auto index = this->index(i, 0, {}); - dataChanged(index, index); - break; - } + if (m_appNotifies[i]->entity().bubbleId() == bubbleId) + return i; } + return -1; +} + +void NotifyStagingModel::replace(const NotifyEntity &entity) +{ + // A replacement keeps the same bubble id as the replaced notification, so + // it is matched by bubble id, exactly like BubbleModel::replaceBubbleIndex: + // a replacement may carry a fresh entity id (marked-processed then re-added) + // or the original entity id (replaceEntity), so the entity id alone is not + // a reliable key. + const int row = rowByBubbleId(entity.bubbleId()); + if (row < 0) + return; + + auto item = m_appNotifies[row]; + // 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(row, 0, {}); + dataChanged(index, index); } QHash NotifyStagingModel::roleNames() const @@ -299,12 +330,13 @@ int NotifyStagingModel::overlapCount() const void NotifyStagingModel::doEntityReceived(qint64 id) { qDebug(notifyLog) << "Receive entity" << id; + auto entity = m_accessor->fetchEntity(id); if (!entity.isValid()) { qWarning(notifyLog) << "Received invalid entity:" << id << ", appName:" << entity.appName(); return; } - if (entity.isReplace() && notifyById(id).isValid()) { + if (entity.isReplace() && rowByBubbleId(entity.bubbleId()) >= 0) { replace(entity); } else { push(entity); diff --git a/panels/notification/center/notifystagingmodel.h b/panels/notification/center/notifystagingmodel.h index 7a71112fb..d41d4cce7 100644 --- a/panels/notification/center/notifystagingmodel.h +++ b/panels/notification/center/notifystagingmodel.h @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2024 UnionTech Software Technology Co., Ltd. +// SPDX-FileCopyrightText: 2024-2026 UnionTech Software Technology Co., Ltd. // // SPDX-License-Identifier: GPL-3.0-or-later @@ -64,6 +64,7 @@ private slots: void remove(qint64 id); void updateTime(); NotifyEntity notifyById(qint64 id) const; + int rowByBubbleId(uint bubbleId) const; private: QList m_appNotifies; diff --git a/panels/notification/common/expiretimer.cpp b/panels/notification/common/expiretimer.cpp new file mode 100644 index 000000000..d7956e18e --- /dev/null +++ b/panels/notification/common/expiretimer.cpp @@ -0,0 +1,179 @@ +// SPDX-FileCopyrightText: 2026 UnionTech Software Technology Co., Ltd. +// +// SPDX-License-Identifier: GPL-3.0-or-later + +#include "expiretimer.h" + +#include +#include +#include + +#include + +namespace notification { + +Q_DECLARE_LOGGING_CATEGORY(notifyLog) + +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; + +// 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); + + // The same entity can be presented by both frontends. Keep the first + // deadline, just as the server used to create only one pending entry. + for (auto iter = m_pendingEntities.cbegin(); iter != m_pendingEntities.cend(); ++iter) { + if (iter.value().id() == id && iter.value().cTime() == entity.cTime()) + return; + } + + // This is the former NotificationManager::removePendingEntity replacement + // path. A replacement occupies the old bubble slot, so remove that slot's + // pending entry before starting the replacement's countdown. + if (entity.isReplace()) { + for (auto iter = m_pendingEntities.begin(); iter != m_pendingEntities.end(); ++iter) { + if (iter.value().bubbleId() != entity.bubbleId()) + continue; + + const auto oldId = iter.value().id(); + m_pendingEntities.erase(iter); + if (m_blockId == oldId) + m_blockId = id; + onTimeout(); + break; + } + } + + if (interval <= 0) { + // Never expire: cancel any pending countdown for this id. + remove(id); + return; + } + + const auto current = QDateTime::currentMSecsSinceEpoch(); + const auto point = current + interval; + m_pendingEntities.insert(point, entity); + + if (m_lastPoint > point) { + m_lastPoint = point; + m_timer->start(static_cast(qMax(0, point - QDateTime::currentMSecsSinceEpoch()))); + } +} + +void ExpireTimer::remove(qint64 id) +{ + // This is the former NotificationManager::removePendingEntity lifecycle + // path, now keyed by id because the server owns the close operation. + for (auto iter = m_pendingEntities.begin(); iter != m_pendingEntities.end(); ++iter) { + if (iter.value().id() != id) + continue; + m_pendingEntities.erase(iter); + onTimeout(); + return; + } +} + +void ExpireTimer::setBlockId(qint64 id) +{ + if (id == m_blockId) + return; + + // 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_blockId != NotifyEntity::InvalidId) { + const auto current = QDateTime::currentMSecsSinceEpoch(); + for (auto iter = m_pendingEntities.begin(); iter != m_pendingEntities.end(); ++iter) { + if (iter.value().id() != m_blockId) + continue; + if (current > iter.key() - BlockItemTimeout) { + const auto blockedEntity = iter.value(); + m_pendingEntities.erase(iter); + m_pendingEntities.insert(current + BlockItemTimeout, blockedEntity); + } + break; + } + } + + m_blockId = id; + onTimeout(); +} + +void ExpireTimer::onTimeout() +{ + QList timeoutEntities; + + const auto current = QDateTime::currentMSecsSinceEpoch(); + for (auto iter = m_pendingEntities.begin(); iter != m_pendingEntities.end();) { + if (iter.key() > current) { + ++iter; + continue; + } + timeoutEntities << iter.value(); + iter = m_pendingEntities.erase(iter); + } + + for (const auto &item : timeoutEntities) { + if (!item.isValid()) { + qWarning(notifyLog) << "Skipping timeout processing for invalid entity id:" << item.id() + << "appName:" << item.appName() << "cTime:" << item.cTime(); + continue; + } + + // A hovered bubble must not expire on its own. Re-insert it one grace + // period ahead so the shared timer polls it at a low frequency while + // hovered instead of busy-looping at 0 ms (the server-side original + // re-inserted at `current`, which restarted the timer immediately). + if (item.id() == m_blockId) { + m_pendingEntities.insert(current + BlockItemTimeout, item); + continue; + } + qDebug(notifyLog) << "Expired for the notification" << item.id() << item.appName(); + Q_EMIT expired(item.id(), item.bubbleId()); + } + + // This is the former NotificationManager::onHandingPendingEntities timer + // scheduling path: the nearest absolute deadline drives the shared timer. + if (m_pendingEntities.isEmpty()) { + m_timer->stop(); + m_lastPoint = std::numeric_limits::max(); + return; + } + + auto points = m_pendingEntities.keys(); + std::sort(points.begin(), points.end()); + m_lastPoint = points.first(); + m_timer->start(static_cast(qMax(0, m_lastPoint - QDateTime::currentMSecsSinceEpoch()))); +} + +} diff --git a/panels/notification/common/expiretimer.h b/panels/notification/common/expiretimer.h new file mode 100644 index 000000000..7cf711ad5 --- /dev/null +++ b/panels/notification/common/expiretimer.h @@ -0,0 +1,76 @@ +// SPDX-FileCopyrightText: 2026 UnionTech Software Technology Co., Ltd. +// +// SPDX-License-Identifier: GPL-3.0-or-later + +#pragma once + +#include +#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. + * + * This is the notification server's pending-timeout machinery moved intact to + * the presentation side: entities are stored in a QMultiHash keyed by absolute + * deadline, the nearest deadline drives one single-shot QTimer, replacement + * removes the old bubble slot, and one hovered id is blocked with a short grace + * period after hover. The frontend-specific addition is idempotent push: when + * the bubble and staging area present the same entity, they share the first + * deadline instead of restarting it. + */ +class ExpireTimer : public QObject +{ + Q_OBJECT +public: + static ExpireTimer *instance(); + + // Starts the countdown of the entity's id based on its urgency and expire + // timeout. A non-positive timeout (Critical urgency or expireTimeout 0) + // cancels any pending countdown, i.e. the notification never expires. + // Starting an already tracked id keeps the original deadline instead of + // restarting it. A replacement notification (isReplace()) cancels the old + // countdown of the same bubble slot before starting the new one. + void push(const NotifyEntity &entity); + // Stops the countdown of the notification id. The server is the single + // owner of a notification's lifecycle: it calls this once the notification + // is closed or archived, so the frontend views don't touch the timer on + // their own. No-op when the id already expired on its own (its entry was + // dropped when expired() was emitted). + void remove(qint64 id); + // Blocks the hovered id from expiring. Only one id is blocked at a time: + // switching to another id (or passing InvalidId) unblocks the previous one, + // which then expires with at least a short grace period left so its bubble + // lingers briefly after the hover ends. + void setBlockId(qint64 id); + +Q_SIGNALS: + // Emitted once when the deadline of id passes. + void expired(qint64 id, uint bubbleId); + +private: + explicit ExpireTimer(QObject *parent = nullptr); + + void onTimeout(); + + QTimer *m_timer = nullptr; + // Pending entities keyed by their absolute expire deadline (ms since the + // epoch). A multi hash so several entities can share a deadline point. + QMultiHash m_pendingEntities; + // Nearest pending deadline; used to decide whether a new push must restart + // the timer. InvalidId-like sentinel when nothing is pending. + qint64 m_lastPoint = std::numeric_limits::max(); + // The single hovered id that must not expire. InvalidId means no block. + qint64 m_blockId = NotifyEntity::InvalidId; +}; + +} 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..0d77c45b1 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,25 @@ bool NotifyServerApplet::init() connect(m_manager, &NotificationManager::NotificationStateChanged, this, &NotifyServerApplet::notificationStateChanged); + // The server is the single owner of a notification's lifecycle: once it + // closes or archives a notification (Processed/Removed), stop its countdown + // here instead of letting each frontend view call ExpireTimer::remove on + // its own. remove() by id is a no-op when the entry already expired on its + // own (its deadline was dropped when expired() was emitted). + connect(m_manager, &NotificationManager::NotificationStateChanged, this, [](qint64 id, int processedType) { + if (processedType == NotifyEntity::Processed || processedType == NotifyEntity::Removed) + ExpireTimer::instance()->remove(id); + }, Qt::QueuedConnection); + + // 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 +96,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 +125,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"));