Skip to content

Conversation

@yixinshark
Copy link
Contributor

@yixinshark yixinshark commented Jan 29, 2026

emit plugins changed signal, then dde-control-center can obtain all plugin information in a timely manner.

Log: add tray plugins changed signal
Pms: BUG-311313

Summary by Sourcery

Add a new tray plugins-changed signal and expose it over DBus so external components can react to plugin updates.

New Features:

  • Introduce a pluginsChanged signal on the tray item and propagate it through the dock DBus proxy.
  • Expose a Q_INVOKABLE method to emit the pluginsChanged signal from QML when tray plugin surfaces are updated.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 29, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds a new tray pluginsChanged signal exposed from the TrayItem applet through DockDBusProxy and QML so external components (e.g., dde-control-center) can react when tray plugins change, and wires up emission from the tray QML when plugin surfaces are updated.

Sequence diagram for tray pluginsChanged signal propagation

sequenceDiagram
    actor User
    participant QML_Tray as QML_Tray
    participant TrayItem as TrayItem
    participant DockDBusProxy as DockDBusProxy
    participant DBusInterface as org_deepin_dde_dock1
    participant ExternalClient as dde_control_center

    User->>QML_Tray: Trigger action that updates tray plugins
    QML_Tray->>QML_Tray: onPluginSurfacesUpdated
    QML_Tray->>TrayItem: emitPluginsChanged()
    TrayItem->>TrayItem: Q_EMIT pluginsChanged
    TrayItem-->>DockDBusProxy: pluginsChanged (Qt signal connection)
    DockDBusProxy-->>DBusInterface: pluginsChanged (DBus signal)
    DBusInterface-->>ExternalClient: pluginsChanged (DBus broadcast)
    ExternalClient->>ExternalClient: Refresh tray plugin information
Loading

Class diagram for TrayItem and DockDBusProxy pluginsChanged integration

classDiagram
    class TrayItem {
        +Q_INVOKABLE DockItemInfos dockItemInfos()
        +Q_INVOKABLE void setItemOnDock(QString settingKey, QString itemKey, bool visible)
        +Q_INVOKABLE void emitPluginsChanged()
        +void trayPluginModelChanged()
        +void quickPluginModelChanged()
        +void fixedPluginModelChanged()
        +void pluginsChanged()
        -bool loopDockItemInfosModel(QAbstractItemModel model, std::function~bool(DockItemInfo)~ cb)
    }

    class DockDBusProxy {
        +DockDBusProxy(DockPanel parent)
        +void pluginVisibleChanged(QString pluginName, bool visible) const
        +void pluginsChanged()
        -DockPanel parent()
        -TrayItem m_trayApplet
    }

    class org_deepin_dde_dock1_interface {
        +signal pluginVisibleChanged(QString pluginName, bool visible)
        +signal pluginsChanged()
    }

    TrayItem <.. DockDBusProxy : uses
    DockDBusProxy ..> org_deepin_dde_dock1_interface : exposes_signals
Loading

File-Level Changes

Change Details Files
Expose a new pluginsChanged signal on TrayItem and make it invokable from QML.
  • Added a Q_INVOKABLE emitPluginsChanged() method on the TrayItem class that emits the pluginsChanged signal.
  • Declared a new pluginsChanged() signal in the TrayItem class API.
panels/dock/tray/trayitem.h
panels/dock/tray/trayitem.cpp
Expose the pluginsChanged event over the old dock D-Bus interface via DockDBusProxy.
  • Added a pluginsChanged signal definition to the org.deepin.dde.dock1.xml D-Bus interface description.
  • Added a pluginsChanged signal to DockDBusProxy and connected it to the TrayItem applet's pluginsChanged signal once the tray applet is available.
panels/dock/api/old/org.deepin.dde.dock1.xml
panels/dock/dockdbusproxy.h
panels/dock/dockdbusproxy.cpp
Trigger the pluginsChanged signal from the tray QML when plugin surfaces data changes.
  • Updated tray.qml to call Applet.emitPluginsChanged() in the onPluginSurfacesUpdated handler after updating availableSurfaces and logging the new length.
panels/dock/tray/package/tray.qml

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 left some high level feedback:

  • Consider whether pluginsChanged can be aligned or consolidated with the existing *PluginModelChanged signals (or emitted from the same code paths) to avoid having two parallel notification mechanisms that may diverge over time.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider whether `pluginsChanged` can be aligned or consolidated with the existing `*PluginModelChanged` signals (or emitted from the same code paths) to avoid having two parallel notification mechanisms that may diverge over time.

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

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, yixinshark

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

@deepin-bot
Copy link

deepin-bot bot commented Jan 29, 2026

TAG Bot

New tag: 2.0.28
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1415

emit plugins changed signal, then dde-control-center can obtain all plugin information in a timely manner.

Log: add tray plugins changed signal
Pms: BUG-311313
@yixinshark yixinshark force-pushed the feat-addpluginschanged branch from 19674ef to 11e270d Compare January 30, 2026 02:25
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查报告

本次代码变更主要涉及在 DDE Dock(任务栏)模块中添加一个 pluginsChanged 信号,用于在托盘插件表面更新时通知外部。以下是针对语法逻辑、代码质量、代码性能和代码安全的详细审查意见。

1. 语法逻辑

  • 总体评价:代码逻辑清晰,信号传递链路完整。

    • QML (tray.qml) 触发 -> C++ (TrayItem) 发射 -> C++ (DockDBusProxy) 转发 -> D-Bus XML (org.deepin.dde.dock1.xml) 定义。
    • 连接逻辑正确,符合 Qt 信号槽机制。
  • 潜在逻辑风险

    • dockdbusproxy.cpp 中,信号连接 connect(m_trayApplet, SIGNAL(pluginsChanged()), this, SIGNAL(pluginsChanged())) 被放置在 getOtherApplet() 的 lambda 回调中。
      • 问题:如果 getOtherApplet() 返回 true 多次(虽然通常情况下 timer 只会触发一次成功获取,但逻辑上存在重复连接的可能),会导致同一个信号被连接多次。如果外部有多个槽连接到 D-Bus 信号,这会导致通知被重复发送。
      • 建议:在连接前使用 QObject::disconnect() 断开旧连接,或者确保该 lambda 只执行一次(虽然代码中使用了 timer->stop()deleteLater(),这已经很大程度上避免了重复执行,但在代码健壮性上可以更明确)。

2. 代码质量

  • 命名规范

    • emitPluginsChanged() 函数名虽然语义准确,但通常在 Qt 中,信号发射函数由 moc 自动生成,直接调用 emit pluginsChanged() 即可。显式创建一个 emitPluginsChanged() 包装函数显得有些冗余。
    • 建议:可以直接在 QML 中调用 Applet.pluginsChanged()(如果信号被注册为 Q_INVOKABLE 或者通过 QML 信号机制连接),或者保持现状但注释说明这是为了方便 QML 调用的封装。
    • 修正:Qt 的信号不能直接被 QML 当作函数调用,除非它是 Q_INVOKABLE。这里定义一个 Q_INVOKABLE void emitPluginsChanged() 来包装 Q_EMIT pluginsChanged() 是为了在 QML 中显式触发信号,这种做法是合理的。
  • 代码一致性

    • trayitem.h 中,pluginsChanged 信号被添加在原有信号列表之后,位置合适,保持了代码风格一致。

3. 代码性能

  • 信号连接开销
    • 新增的信号连接发生在初始化阶段,且只连接一次,对运行时性能影响微乎其微。
    • onPluginSurfacesUpdated 在 QML 中被调用,触发了 console.log 和信号发射。这部分逻辑在插件表面更新时执行,属于正常业务逻辑,性能开销可接受。

4. 代码安全

  • 空指针检查
    • dockdbusproxy.cpp 中,连接信号前检查了 if (getOtherApplet()),这隐含了对 m_trayApplet 有效性的检查(假设 getOtherApplet 内部处理了指针初始化),这是良好的习惯。
  • D-Bus 接口变更
    • 修改了 org.deepin.dde.dock1.xml,这属于公共接口变更。
    • 风险:如果外部有其他进程依赖此 D-Bus 接口且未处理新增的 pluginsChanged 信号,通常不会有副作用(信号是单向的),但需确保该接口版本的兼容性策略允许这种新增。

改进建议汇总

  1. 优化 dockdbusproxy.cpp 中的连接逻辑
    为了防止潜在的重复连接(尽管概率极低),建议使用 Qt::UniqueConnection 标志,或者在连接前先断开。

    // panels/dock/dockdbusproxy.cpp
    // ...
    if (getOtherApplet()) {
        timer->stop();
        timer->deleteLater();
        // 使用 Qt::UniqueConnection 防止重复连接
        connect(m_trayApplet, &TrayItem::pluginsChanged, 
                this, &DockDBusProxy::pluginsChanged, 
                Qt::UniqueConnection);
    }
    // ...

    注意:将 SIGNAL/SLOT 宏替换为函数指针语法(如上所示)是现代 Qt 的推荐做法,因为它能在编译时检查信号槽的合法性,提高代码安全性。

  2. 关于 emitPluginsChanged 的注释
    trayitem.htrayitem.cpp 中添加注释,说明该函数是为了在 QML 端显式触发信号而设计的封装。

    // panels/dock/tray/trayitem.h
    // ...
    // Invokable method to emit pluginsChanged signal from QML
    Q_INVOKABLE void emitPluginsChanged();
    // ...
  3. QML 中的调用
    tray.qml 中的 Applet.emitPluginsChanged() 调用是正确的,因为 emitPluginsChanged 被声明为 Q_INVOKABLE

结论

该代码变更实现了预期的功能,整体逻辑正确,风险较低。主要建议是采用更现代的 Qt 信号槽连接语法(函数指针 + Qt::UniqueConnection)以增强编译时检查和防止重复连接,并适当增加注释以解释 QML 与 C++ 交互的意图。

@yixinshark
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Jan 30, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 1decd95 into linuxdeepin:master Jan 30, 2026
8 of 11 checks passed
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.

3 participants