Skip to content

fix(dock): debounce tray plugin list logging to ensure stable state#1597

Open
Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:fix/debounce-plugin-log
Open

fix(dock): debounce tray plugin list logging to ensure stable state#1597
Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:fix/debounce-plugin-log

Conversation

@Ivy233
Copy link
Copy Markdown
Contributor

@Ivy233 Ivy233 commented May 14, 2026

Replace fixed 3-second delay with debounce mechanism for tray plugin list event logging. The original approach reported plugin state immediately upon tray applet connection and again after a fixed 3-second delay, which could capture incomplete plugin lists during login.

Use a 2-second debounce timer that resets on each
pluginsChanged signal, ensuring the log only records the final stable plugin list. Also adjust dde-application- manager packaging dependencies.

将托盘插件列表事件上报从固定3秒延迟改为防抖机制。
原始方案在托盘小程序连接后立即上报并延迟3秒再报一次,
在登录时可能捕获到不完整的插件列表。
改用2秒防抖定时器,每次插件列表变化时重置,
确保只记录最终稳定的插件列表。同时调整
dde-application-manager 打包依赖。

PMS: BUG-361067

PS: 打包依赖是dde-am修改的时候,依赖调整,如果需要单独提pr就挂个从comment。

Summary by Sourcery

Debounce logging of the dock tray plugin list so it records only the final stable state after plugin changes.

Bug Fixes:

  • Avoid logging incomplete tray plugin lists during startup by deferring logging until the plugin state stabilizes.

Enhancements:

  • Introduce a reusable debounced timer in the dock DBus proxy to trigger delayed plugin state logging instead of a fixed single-shot delay.

Build:

  • Adjust dde-application-manager packaging dependencies in the Debian control file.

@Ivy233 Ivy233 requested review from 18202781743, BLumia and mhduiy May 14, 2026 13:31
@deepin-ci-robot
Copy link
Copy Markdown

[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.

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
Copy Markdown

sourcery-ai Bot commented May 14, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Replaces the fixed 3‑second delayed logging of tray plugin lists with a debounced logging mechanism triggered by plugin change events, and adjusts dde-application-manager packaging dependencies.

Sequence diagram for debounced tray plugin list logging

sequenceDiagram
    participant TrayApplet
    participant DockDBusProxy
    participant DebounceTimer

    TrayApplet->>DockDBusProxy: pluginsChanged()
    DockDBusProxy->>DockDBusProxy: scheduleDebouncedLog()
    alt first_call_or_timer_not_created
        DockDBusProxy->>DebounceTimer: create QTimer
        DockDBusProxy->>DebounceTimer: setSingleShot(true)
        DockDBusProxy->>DebounceTimer: setInterval(2000)
        DebounceTimer-->>DockDBusProxy: timeout (connected to logInitialPluginState)
    end
    DockDBusProxy->>DebounceTimer: start()

    rect rgb(230,230,230)
        Note over DebounceTimer: 2s debounce window
    end

    DebounceTimer-->>DockDBusProxy: timeout
    DockDBusProxy->>DockDBusProxy: logInitialPluginState()
Loading

File-Level Changes

Change Details Files
Refactor tray plugin list logging from a fixed single-shot delay to an event-driven debounce timer to capture a stable plugin set.
  • Remove the post-init QTimer::singleShot(3000, ...) call that invoked logInitialPluginState after a fixed delay.
  • Connect the tray applet's pluginsChanged signal to the DockDBusProxy::pluginsChanged signal and then to a new lambda that schedules debounced logging.
  • Trigger an initial debounced logging schedule once the tray applet connects, instead of logging immediately plus a delayed second time.
panels/dock/dockdbusproxy.cpp
Introduce a reusable debounced logging helper that delays logInitialPluginState until plugins have been stable for 2 seconds.
  • Add a scheduleDebouncedLog() method that lazily creates a single-shot QTimer with a 2-second interval and connects its timeout to logInitialPluginState.
  • Start or restart the debounce timer on each scheduleDebouncedLog() call so rapid pluginsChanged bursts coalesce into a single log.
  • Declare scheduleDebouncedLog() and a QTimer* m_debounceTimer member initialized to nullptr in DockDBusProxy.
panels/dock/dockdbusproxy.cpp
panels/dock/dockdbusproxy.h
Update Debian packaging to adjust dde-application-manager dependencies.
  • Modify the dde-application-manager dependency list to reflect the dock changes and ensure correct runtime/build requirements.
debian/control

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
Copy Markdown

@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 extracting the hardcoded 2000 ms debounce interval into a named constant or configuration value so the debounce behavior is easier to adjust and understand in the future.
  • The QTimer timeout connection in scheduleDebouncedLog uses a lambda that simply calls logInitialPluginState; replacing this with a direct slot connection would simplify the code and avoid an extra indirection.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider extracting the hardcoded 2000 ms debounce interval into a named constant or configuration value so the debounce behavior is easier to adjust and understand in the future.
- The QTimer timeout connection in `scheduleDebouncedLog` uses a lambda that simply calls `logInitialPluginState`; replacing this with a direct slot connection would simplify the code and avoid an extra indirection.

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.

Replace fixed 3-second delay with debounce mechanism for
tray plugin list event logging. The original approach
reported plugin state immediately upon tray applet
connection and again after a fixed 3-second delay, which
could capture incomplete plugin lists during login.

Use a 2-second debounce timer that resets on each
pluginsChanged signal, ensuring the log only records
the final stable plugin list. Also adjust dde-application-
manager packaging dependencies.

将托盘插件列表事件上报从固定3秒延迟改为防抖机制。
原始方案在托盘小程序连接后立即上报并延迟3秒再报一次,
在登录时可能捕获到不完整的插件列表。
改用2秒防抖定时器,每次插件列表变化时重置,
确保只记录最终稳定的插件列表。同时调整
dde-application-manager 打包依赖。

PMS: BUG-361067
@Ivy233 Ivy233 force-pushed the fix/debounce-plugin-log branch from 248ac3d to f8b4b72 Compare May 14, 2026 13:44
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改主要涉及Debian打包依赖版本的调整,以及对Dock插件状态日志记录逻辑的重构(引入了防抖机制)。

总体而言,将原本的固定延时日志改为基于事件驱动的防抖日志是一个很好的优化,可以避免在插件频繁变动时产生冗余日志。但在代码逻辑、性能和安全方面,还有一些可以改进的空间。以下是详细的审查意见:

1. 语法与逻辑

  • Debian依赖版本逻辑变更风险 (>= 代替 >>)
    • debian/control 中,dde-application-manager-api 的依赖从 >> 1.2.51(大于)变为了 >= 1.2.48(大于等于)。
    • 问题:这不仅降低了最低版本要求,还改变了匹配逻辑。请确认 1.2.48 版本的 API 是否真的与当前代码兼容?如果代码中使用了 > 1.2.51 版本才引入的新特性,降级到 1.2.48 可能会导致运行时错误或编译失败。
  • 防抖计时器首次触发的逻辑
    • dockdbusproxy.cpp 中,当 m_trayApplet 准备就绪时,代码先 connectpluginsChanged 信号,然后直接调用了 scheduleDebouncedLog()
    • 潜在问题:如果在 m_trayApplet 创建完成的瞬间,底层立即发出了 pluginsChanged 信号,那么 scheduleDebouncedLog() 会被连续调用两次(一次由信号触发,一次由显式调用触发)。虽然 QTimer::start() 会重置计时器,不会导致崩溃或重复打日志,但从逻辑语义上看,首次加载的日志可能被不必要地延迟了2秒。
    • 建议:如果首次加载的日志非常重要且需要尽快输出,可以考虑首次直接调用 logInitialPluginState(),后续的变动再走防抖逻辑;或者确认当前的2秒延迟在业务上是可接受的。

2. 代码质量

  • Lambda 表达式的简化
    • scheduleDebouncedLog() 中,使用了如下代码:
      connect(m_debounceTimer, &QTimer::timeout, this, [this]() {
          logInitialPluginState();
      });
    • 建议:Lambda 表达式仅包装了对单个函数的调用,可以直接传递函数指针,使代码更简洁,编译器也更容易做内联优化:
      connect(m_debounceTimer, &QTimer::timeout, this, &DockDBusProxy::logInitialPluginState);
  • QTimer 的初始化方式
    • 当前在 scheduleDebouncedLog() 中使用了懒初始化(判断 if (!m_debounceTimer))来创建 QTimer。
    • 建议:虽然懒初始化没有问题,但由于 m_debounceTimer 的生命周期与 DockDBusProxy 一致,且 this 被传递为了 QTimer 的 parent,更推荐在 DockDBusProxy 的构造函数中直接创建并配置好 Timer,这样可以将初始化逻辑集中管理,降低方法的圈复杂度。如果坚持使用懒初始化,当前的写法也是安全可行的。

3. 代码性能

  • 信号连接的效率
    • dockdbusproxy.cpp 第84行:
      connect(m_trayApplet, SIGNAL(pluginsChanged()), this, SIGNAL(pluginsChanged()));
    • 问题:使用了旧式的 SIGNAL()SLOT() 宏。这种连接方式在运行时进行字符串比对,且不具备编译期类型检查,性能略差于新式语法。
    • 建议:如果 pluginsChanged 信号没有参数,建议使用新式 Qt5/Qt6 连接语法,性能更优且更安全:
      connect(m_trayApplet, &DS_NAMESPACE::DAppletProxy::pluginsChanged, this, &DockDBusProxy::pluginsChanged);

4. 代码安全

  • Debian 打包依赖的安全性
    • 新增了运行时依赖 dde-application-manager (>> 1.2.51),但编译时依赖 dde-application-manager-api 却降级到了 >= 1.2.48
    • 风险:这种编译时和运行时版本的不对称可能导致“编译通过但运行时行为不一致”的严重问题。如果 API 在 1.2.481.2.51 之间发生了行为变更或ABI不兼容,在低版本 API 编译的二进制文件放到高版本运行环境下运行时,可能会引发未定义行为或崩溃。
    • 建议:严格对齐编译时和运行时的依赖版本要求,确保编译环境和运行环境的基线一致。
  • 内存泄漏风险(当前安全,但需注意)
    • QTimer::singleShot 被替换前,原代码中存在 timer->deleteLater(),而新代码中 m_debounceTimer 通过 new QTimer(this) 指定了 parent,Qt 的对象树机制会保证其在 DockDBusProxy 析构时被正确销毁,因此当前是安全的。此条仅作提醒,无需修改。

改进后的代码建议

panels/dock/dockdbusproxy.cpp

// ... 前面代码保持不变 ...
    connect(m_trayApplet, &DS_NAMESPACE::DAppletProxy::pluginsChanged, this, &DockDBusProxy::pluginsChanged);
    
    // 采用防抖机制记录初始状态
    connect(this, &DockDBusProxy::pluginsChanged, this, [this]() {
        scheduleDebouncedLog();
    });
    scheduleDebouncedLog(); // 触发首次防抖计时
// ... 前面代码保持不变 ...

void DockDBusProxy::scheduleDebouncedLog()
{
    if (!m_debounceTimer) {
        m_debounceTimer = new QTimer(this);
        m_debounceTimer->setSingleShot(true);
        m_debounceTimer->setInterval(2000);
        // 使用函数指针替代 Lambda,更简洁高效
        connect(m_debounceTimer, &QTimer::timeout, this, &DockDBusProxy::logInitialPluginState);
    }
    m_debounceTimer->start();
}

debian/control (请务必复核版本号)

# 建议将 api 版本与 runtime 版本对齐,避免 ABI 兼容性问题
Build-Depends:
  ...
  dde-application-manager-api (>= 1.2.51), # 建议与运行时依赖保持一致
  ...

Depends:
  ...
  dde-application-manager (>= 1.2.51), # 建议统一使用 >= 避免歧义,或确认必须使用 >>
  ...

希望这些审查意见对你有所帮助!如果有任何疑问,欢迎随时提问。

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