Skip to content

fix(sni): monitor icon property changes for timely tray icon updates#441

Closed
add-uos wants to merge 1 commit intolinuxdeepin:masterfrom
add-uos:master
Closed

fix(sni): monitor icon property changes for timely tray icon updates#441
add-uos wants to merge 1 commit intolinuxdeepin:masterfrom
add-uos:master

Conversation

@add-uos
Copy link

@add-uos add-uos commented Mar 11, 2026

Add IconNameChanged and IconPixmapChanged signal handlers to detect icon property changes and ensure timely updates.

添加IconNameChanged和IconPixmapChanged信号处理,监听图标属性变化
并及时更新托盘图标。

Log: 修复SNI图标更新不及时的问题
PMS: BUG-352531
Influence: 修复后托盘图标能够及时响应SNI应用的图标属性变化,提升用户体验。

Summary by Sourcery

Bug Fixes:

  • Handle icon name and pixmap change signals from StatusNotifierItem so tray icons stay in sync with SNI applications.

Add IconNameChanged and IconPixmapChanged signal handlers to detect
icon property changes and ensure timely updates.

添加IconNameChanged和IconPixmapChanged信号处理,监听图标属性变化
并及时更新托盘图标。

Log: 修复SNI图标更新不及时的问题
PMS: BUG-352531
Influence: 修复后托盘图标能够及时响应SNI应用的图标属性变化,提升用户体验。
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: add-uos

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 11, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Extends the SNI tray protocol handler to also react to icon name and pixmap property change signals so the tray icon updates promptly when SNI applications change their icon properties.

Sequence diagram for updated SNI icon property change handling

sequenceDiagram
    participant App
    participant StatusNotifierItem
    participant SniTrayProtocolHandler
    participant TrayUI

    App->>StatusNotifierItem: Change icon name or pixmap
    StatusNotifierItem-->>SniTrayProtocolHandler: IconNameChanged
    SniTrayProtocolHandler->>SniTrayProtocolHandler: iconChanged
    SniTrayProtocolHandler-->>TrayUI: Update tray icon

    App->>StatusNotifierItem: Change icon pixmap
    StatusNotifierItem-->>SniTrayProtocolHandler: IconPixmapChanged
    SniTrayProtocolHandler->>SniTrayProtocolHandler: iconChanged
    SniTrayProtocolHandler-->>TrayUI: Update tray icon
Loading

File-Level Changes

Change Details Files
Update SNI tray protocol handler to listen to additional icon-related signals and trigger icon refreshes promptly.
  • Add connection from StatusNotifierItem::IconNameChanged to the existing iconChanged slot so icon name updates refresh the tray icon.
  • Add connection from StatusNotifierItem::IconPixmapChanged to the existing iconChanged slot so pixmap updates refresh the tray icon.
  • Reuse the existing iconChanged handler for these new signals to keep icon update logic centralized.
plugins/application-tray/sniprotocolhandler.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要是在 SniTrayProtocolHandler 的构造函数中增加了两个新的信号连接,用于监听图标名称和图标位图的变化。以下是对这段代码的审查意见,包括语法逻辑、代码质量、代码性能和代码安全方面的建议:

1. 语法逻辑

  • 逻辑正确性:代码逻辑是正确的。StatusNotifierItem (SNI) 协议通常支持通过名称 (IconName) 或位图数据 (IconPixmap) 来定义图标。原代码只监听了 NewIcon 信号(通常表示图标发生变化的通用信号),新增的代码补充了对具体属性变化(IconNameChangedIconPixmapChanged)的监听。这确保了无论应用程序是通过改变名称还是改变位图数据来更新图标,SniTrayProtocolHandler 都能捕获到变化并调用 iconChanged 槽函数。
  • 一致性:新增的两行代码风格与上下文保持一致,使用了 Qt 5 推荐的函数指针连接语法。

2. 代码质量

  • 可维护性与健壮性
    • 冗余性NewIcon 信号通常是 IconNameChangedIconPixmapChanged 的聚合信号。在某些实现中,监听 NewIcon 可能已经足够。不过,显式监听具体属性的变化通常更安全,因为不同的 SNI 服务端实现可能触发信号的时机略有不同(例如,有的可能只触发属性变化信号而不触发 NewIcon)。因此,增加这两个连接提高了兼容性。
    • 潜在问题:如果 StatusNotifierItem 接口定义中,NewIcon 信号和 IconNameChanged/IconPixmapChanged 信号在图标更新时会被同时触发,那么 iconChanged 槽函数可能会在短时间内被调用多次。
      • 建议:请检查 iconChanged 槽函数的实现。如果该函数涉及耗时的操作(如网络请求、复杂的图像处理)或频繁的 UI 刷新,建议考虑引入防抖机制或检查图标内容是否真的发生了变化再执行更新逻辑,以避免不必要的性能开销。

3. 代码性能

  • 信号槽开销:增加两个 connect 调用本身对性能影响微乎其微,因为它们只发生在构造阶段。
  • 运行时开销:主要风险在于信号触发频率。如上所述,如果 SNI 服务端频繁更新图标属性,或者同时触发多个信号,会导致 iconChanged 频繁执行。
    • 建议:确保 iconChanged 内部逻辑高效。如果涉及到图片加载或转换,确保有缓存机制。

4. 代码安全

  • 空指针检查:代码片段中未显示 m_sniInter 的初始化过程,但通常在 init() 或构造函数列表中完成。Qt 的 connect 在源对象为 nullptr 时会导致崩溃。请确保在执行这些 connect 之前,m_sniInter 已经被正确实例化。
  • 线程安全:Qt 的信号槽连接默认使用 Qt::AutoConnection。如果 m_sniInterthisSniTrayProtocolHandler)位于不同的线程,槽函数将在接收者的线程中执行。如果 iconChanged 涉及共享资源访问,请确保添加适当的锁机制,或者确保 iconChanged 是线程安全的(例如仅操作线程安全的 GUI 元素)。

总结与改进建议

这段代码在功能上是正确的,增强了协议处理的完整性。为了进一步优化,建议:

  1. 优化 iconChanged 逻辑
    iconChanged 槽函数内部,增加判断逻辑,只有当图标数据(名称或 Pixmap)确实发生了变化时,才进行后续的更新和重绘操作。

    // 伪代码示例
    void SniTrayProtocolHandler::iconChanged()
    {
        // 获取新图标
        QIcon newIcon = fetchIcon();
    
        // 比较新旧图标
        if (newIcon.cacheKey() != m_currentIcon.cacheKey()) {
            m_currentIcon = newIcon;
            updateUI(); // 执行实际的更新操作
        }
    }
  2. 信号去重(可选)
    如果确认 NewIconIconNameChanged/IconPixmapChanged 总是同时触发,可以考虑只连接具体属性变化的信号,去掉 NewIcon 的连接,以减少重复调用的可能性(但这取决于对 SNI 服务端行为的严格假设,保留原样通常更兼容)。

  3. 注释说明
    可以添加简短注释,说明为什么需要监听这些具体的属性变化信号,例如:

    // 监听具体属性变化以确保兼容不同实现的 SNI 服务端
    connect(m_sniInter, &StatusNotifierItem::IconNameChanged, this, &SniTrayProtocolHandler::iconChanged);
    connect(m_sniInter, &StatusNotifierItem::IconPixmapChanged, this, &SniTrayProtocolHandler::iconChanged);

@add-uos add-uos closed this Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants