fix: start notification expire timer only after bubble is displayed - #1691
Open
Ivy233 wants to merge 1 commit into
Open
fix: start notification expire timer only after bubble is displayed#1691Ivy233 wants to merge 1 commit into
Ivy233 wants to merge 1 commit into
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ivy233 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideThis PR defers starting the notification expiration timer until a bubble is actually inserted into the UI model, wiring a new bubbleDisplayed signal through BubblePanel to NotificationManager, which now schedules timeouts based on the stored client expire timeout only when notifications are displayed, with thread-safe forwarding from the applet and added unit tests. Sequence diagram for deferred notification timeout start when bubble is displayedsequenceDiagram
participant BubbleModel
participant BubblePanel
participant NotifyServerApplet
participant NotificationManager
BubbleModel->>BubbleModel: insertBubble / replaceBubble
BubbleModel-->>BubblePanel: bubbleDisplayed(id)
BubblePanel->>NotifyServerApplet: notificationDisplayed(id)
NotifyServerApplet->>NotificationManager: notificationDisplayed(id)
NotificationManager->>NotificationManager: fetchEntity(id)
NotificationManager->>NotificationManager: pushPendingEntity(entity, entity.timeout())
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Ivy233
marked this pull request as ready for review
August 6, 2026 06:59
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Ivy233
force-pushed
the
fix/notification-expire-after-display
branch
from
August 6, 2026 11:52
430dd63 to
fcf70d8
Compare
18202781743
reviewed
Aug 6, 2026
| if (interval <= 0) | ||
| return; | ||
|
|
||
| auto *timer = new QTimer(this); |
18202781743
reviewed
Aug 6, 2026
| timer->setSingleShot(true); | ||
| timer->setInterval(interval); | ||
| connect(timer, &QTimer::timeout, this, [this, id, bubbleId = bubble->bubbleId()] { | ||
| m_timeoutTimers.remove(id); |
18202781743
reviewed
Aug 6, 2026
| } | ||
| } | ||
|
|
||
| void BubbleModel::setBlockedId(qint64 id) |
Contributor
There was a problem hiding this comment.
这样的话，进入了暂存区域的通知没有超时的机制了，
Ivy233
force-pushed
the
fix/notification-expire-after-display
branch
2 times, most recently
from
August 7, 2026 06:21
0b0eb8c to
2ae625f
Compare
Ivy233
force-pushed
the
fix/notification-expire-after-display
branch
from
August 7, 2026 08:07
2ae625f to
5ef467e
Compare
18202781743
reviewed
Aug 7, 2026
|
|
||
| Q_EMIT NotificationStateChanged(entity.id(), entity.processedType()); | ||
|
|
||
| bool critical = false; |
18202781743
reviewed
Aug 7, 2026
|
|
||
| bool contains(qint64 key) const; | ||
| // Milliseconds left for key, or 0 when it is not tracked. | ||
| int remaining(qint64 key) const; |
Contributor
There was a problem hiding this comment.
按操作来定义接口吧，不用搞这么通用的，然后让调用者去组合，
这里只有start，stop，clear吧，resume类似传递需要停止的entity，逻辑在内部封装，
18202781743
reviewed
Aug 7, 2026
| * that key. All bookkeeping lives in hash maps, so no QTimer is allocated per | ||
| * key and nothing leaks when a key expires or is stopped. | ||
| */ | ||
| class ExpireTimer : public QObject |
Contributor
There was a problem hiding this comment.
暂存区和通知横幅的是不是共用同一个定时器管理的呀，不然这里会不会一个通知有两个定时器在弄呀，
The notification expire timer previously ran in the notification server (worker thread) and was started when the notification was received, so a notification waiting in a long display queue could expire before its bubble was shown. The timeout is now owned by the bubble frontend and only starts once the bubble is actually displayed. 1. Add an ExpireTimer helper that manages each tracked key's deadline, paused remaining time and caller data (such as the bubble id) with a single shared single-shot QTimer. It exposes operation-based APIs (start/pause/resume/stop/clear) instead of lower-level get/set ones, so no QTimer is allocated per notification and nothing leaks on expiry. 2. BubbleModel starts a per-bubble expire timer when a bubble is shown (insert/replace) using the client expireTimeout (0: never expire, -1: 5000ms default, Critical urgency: never expire). On expiry it emits bubbleExpired(id, bubbleId). 3. BubblePanel closes the expired bubble and calls notificationClosed(id, bubbleId, Expired) on the server. 4. Move the hover block to the frontend: BubbleModel::setBlockedId pauses/resumes the expire timer so hovering keeps the bubble on screen, keeping the 1s grace after unhover. 5. Give the notification center staging model its own expire timer: notifications shown in the staging area also time out, because the bubble panel is disabled while the center window is open so the bubble-side timers are not running. 6. Make NotificationManager::notificationClosed idempotent. A notification is tracked by both the bubble and the staging expire timers, so the close is reported only while the entity still exists, preventing a notification from being closed twice. 7. Critical notifications never expire on their own, enforced both when computing the frontend timeout and on the server. 8. Remove the server-side timeout bookkeeping: notificationDisplayed, setBlockClosedId, pushPendingEntity, onHandingPendingEntities, removePendingEntity and the pending-timeout QTimer. 9. Expose BubbleItem::timeout() and NotifyEntity::timeout()/urgency() for the frontend timeout computation. 10. Remove the obsolete NotificationDisplayed/SetBlockClosedId tests. Log: Fixed notifications expiring before their bubble was displayed. Influence: 1. Send several notifications at once and verify each bubble stays for the full expire timeout. 2. Verify critical notifications and expireTimeout 0 never close. 3. Hover a bubble and verify it does not expire, then closes 1s after unhover. 4. Verify an expired notification is closed only once and moved to the notification center, and the DBus NotificationClosed(Expired) signal is emitted once. 5. Open the notification center and verify staged notifications expire after their timeout and leave the staging area. 6. Run the notification server unit tests. fix: 将通知过期计时迁移到气泡前端 通知过期计时此前在通知服务器（工作线程）中运行，收到通知时即启动，导致 长显示队列中的通知可能在显示前就已过期。现将超时逻辑交由气泡前端持有， 气泡真正显示后才开始计时。 1. 新增 ExpireTimer 工具类：用一个共享的单次 QTimer 管理每个键的过期 时间、暂停剩余时间及调用方数据（如气泡 ID）。提供 start/pause/resume/ stop/clear 操作化接口，而非 contains/remaining 这类底层读写操作，不再 为每个通知分配 QTimer，到期后也不会泄漏对象。 2. BubbleModel 在气泡显示（插入/替换）时根据客户端 expireTimeout 启动 过期计时（0：永不过期，-1：默认 5000ms，Critical 优先级：永不过期）， 到期时发出 bubbleExpired(id, bubbleId)。 3. 到期后 BubblePanel 关闭气泡并调用服务器 notificationClosed(id, bubbleId, Expired)。 4. 将悬停阻塞移到前端：BubbleModel::setBlockedId 暂停/恢复过期计时，使 悬停时气泡不关闭，并保留取消悬停后 1s 缓冲。 5. 为通知中心暂存模型增加过期计时：暂存区展示的通知同样会超时，因为 中心窗口打开时气泡面板被禁用，气泡侧的计时器不会运行。 6. 使 NotificationManager::notificationClosed 幂等：同一通知会被气泡与 暂存模型的过期计时同时跟踪，因此在实体仍存在时才上报关闭，避免通知 被关闭两次。 7. Critical 通知永不自动过期，前端超时计算与服务端均强制执行。 8. 移除服务端超时簿记：notificationDisplayed、setBlockClosedId、 pushPendingEntity、onHandingPendingEntities、removePendingEntity 以及 pending-timeout 定时器。 9. 为前端超时计算暴露 BubbleItem::timeout() 与 NotifyEntity::timeout()/ urgency()。 10. 移除过时的 NotificationDisplayed/SetBlockClosedId 测试。 Log: 修复通知在气泡显示前就过期的问题。 Influence: 1. 一次性发送多条通知，验证每个气泡都能保持完整的过期时间。 2. 验证 Critical 通知与 expireTimeout 为 0 的通知永不过期。 3. 悬停气泡验证其不关闭，取消悬停 1s 后关闭。 4. 验证过期通知只被关闭一次并进入通知中心，DBus NotificationClosed (Expired) 信号只发送一次。 5. 打开通知中心，验证暂存区的通知到期后超时并移出暂存区。 6. 运行通知服务器单元测试。 PMS: BUG-372279
Ivy233
force-pushed
the
fix/notification-expire-after-display
branch
from
August 10, 2026 03:27
5ef467e to
aa5a30c
Compare
deepin pr auto review★ 总体评分：100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 虽然当前代码无安全漏洞，但为了提升跨线程调用的绝对安全性，
// 建议将 BubblePanel 中的 Qt::DirectConnection 替换为 Qt::AutoConnection，
// 让 Qt 自行判断线程归属，防止未来架构调整时引发线程安全问题。
// 文件：panels/notification/bubble/bubblepanel.cpp
// 修改前：
connect(m_bubbles, &BubbleModel::bubbleExpired, this, [this](qint64 id, uint bubbleId) {
closeBubble(id);
QMetaObject::invokeMethod(m_notificationServer, "notificationClosed", Qt::DirectConnection,
Q_ARG(qint64, id), Q_ARG(uint, bubbleId), Q_ARG(uint, NotifyEntity::Expired));
});
// 修改后：
connect(m_bubbles, &BubbleModel::bubbleExpired, this, [this](qint64 id, uint bubbleId) {
closeBubble(id);
QMetaObject::invokeMethod(m_notificationServer, "notificationClosed", Qt::AutoConnection,
Q_ARG(qint64, id), Q_ARG(uint, bubbleId), Q_ARG(uint, NotifyEntity::Expired));
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pushPendingEntitycall fromNotify(), so the expire timer no longer starts as soon as a notification is receivedbubbleDisplayedsignal toBubbleModel, emitted when a bubble is actually inserted into the display modelbubbleDisplayedinBubblePaneland forward it to the notification server throughnotificationDisplayedtimeout()accessor toNotifyEntityto read the client provided expire timeoutNotificationManager::notificationDisplayedto schedule the timeout only when the bubble is shown, avoiding premature expiry while notifications are still queuednotificationDisplayedviaQt::QueuedConnectioninNotifyServerAppletso the pending timeout timer is started in the worker thread it belongs tonotificationDisplayedLog: Defer the notification expire timer until the bubble is actually displayed on screen
Influence:
fix: 通知气泡显示后才启动过期计时
Notify()中立即调用pushPendingEntity的逻辑，通知收到后不再 马上启动过期计时BubbleModel中新增bubbleDisplayed信号，在气泡实际插入显示模型 时发出BubblePanel中连接bubbleDisplayed，通过notificationDisplayed转发给通知服务端NotifyEntity新增timeout()访问器，用于读取客户端传入的过期时间NotificationManager::notificationDisplayed，仅在气泡显示时才调度 超时，避免通知在排队期间提前过期NotifyServerApplet中通过Qt::QueuedConnection转发notificationDisplayed，确保过期定时器在其所属的 worker 线程中启动notificationDisplayed补充单元测试Log: 将通知过期计时推迟到气泡真正显示之后
Influence:
PMS: BUG-372279
Summary by Sourcery
Defer starting notification expiry timers until bubbles are actually displayed on screen.
New Features:
Bug Fixes:
Enhancements:
Tests: