Skip to content

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

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
Ivy233:fix/debounce-plugin-log
May 19, 2026
Merged

fix(dock): debounce tray plugin list logging to ensure stable state#1597
deepin-bot[bot] merged 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一下。调整pr原因:
版本号来源:linuxdeepin/dde-application-manager#354
依赖修改对应pr:#1590

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

@Ivy233 Ivy233 force-pushed the fix/debounce-plugin-log branch from 248ac3d to f8b4b72 Compare May 14, 2026 13:44
Comment thread panels/dock/dockdbusproxy.cpp Outdated
Comment thread panels/dock/dockdbusproxy.cpp Outdated
@Ivy233 Ivy233 force-pushed the fix/debounce-plugin-log branch from f8b4b72 to 2e5d44d Compare May 16, 2026 08:03
@Ivy233 Ivy233 requested a review from mhduiy May 16, 2026 08:51
@Ivy233 Ivy233 force-pushed the fix/debounce-plugin-log branch from 2e5d44d to 9c5da97 Compare May 19, 2026 06:32
延长托盘插件列表日志记录延时以等待插件完全加载

PMS: BUG-361067
@Ivy233 Ivy233 force-pushed the fix/debounce-plugin-log branch from 9c5da97 to 0e36585 Compare May 19, 2026 06:41
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff,本次修改主要涉及Debian打包控制文件和DDE Dock的DBus代理代码。

以下是我从语法逻辑、代码质量、代码性能和代码安全四个维度提出的详细审查意见和改进建议:

1. 语法与逻辑

debian/control 依赖版本逻辑冲突(高优先级)

debian/control 的修改中,存在一个明显的依赖版本逻辑冲突:

  • Build-Depends(编译依赖) 中,将 dde-application-manager-api>> 1.2.51(大于)改为了 >= 1.2.48(大于等于)。
  • Depends(运行依赖) 中,新增了 dde-application-manager (>> 1.2.51)(大于)。

问题:编译时允许 1.2.48 及以上版本,但运行时却强制要求大于 1.2.51。这在Debian包管理逻辑中是不合理的:如果一个用户安装了 1.2.49 版本的 dde-application-manager-api 进行编译,但运行时依赖却要求 dde-application-manager 必须大于 1.2.51,这会导致包依赖断裂或运行时找不到满足条件的版本。
建议:请确认实际的API兼容性版本。如果运行时必须大于 1.2.51,编译依赖通常也应保持一致(如 >= 1.2.51>> 1.2.50);如果向下兼容到 1.2.48,运行依赖也应放宽。

延时逻辑的可靠性

QTimer::singleShot 的延时从 3秒 改为 30秒。
问题:基于固定长时间的延时来等待数据加载完成是典型的“反模式”。在性能较差的机器上,30秒可能依然不够;而在性能极好的机器上,白白浪费30秒才记录日志是不必要的。
建议:如果 m_trayApplet 有加载完成的信号(如 loaded()initialized()),应改为信号驱动。如果必须使用定时器,建议增加安全检查:如果数据已加载完成则提前触发,而非死等30秒。

2. 代码质量

魔法数字

代码中使用了 30000 这样的魔法数字。
建议:应将其定义为具名常量,提高代码可读性和可维护性。例如:

static constexpr int INITIAL_PLUGIN_LOG_DELAY_MS = 30000; // 30 seconds
QTimer::singleShot(INITIAL_PLUGIN_LOG_DELAY_MS, this, [this]() {
    logInitialPluginState();
});

注释质量

新增的注释 // Log the initial plugin list after 30s delay to ensure the list has fully loaded 解释了“做什么”和“为什么延时”,这一点很好。但建议在注释中补充为什么是30秒(例如:某些极端环境下插件加载耗时最长可达XX秒),以便未来其他开发者理解这个经验值的来源。

3. 代码性能

启动耗时与资源占用

问题:30秒的延时虽然不阻塞主线程(因为用了 QTimer::singleShot),但这个Lambda对象以及相关的上下文会在内存中驻留30秒。更重要的是,如果 logInitialPluginState() 涉及到磁盘I/O或繁重的遍历逻辑,它在启动30秒后突然执行可能会引起UI卡顿或I/O峰值。
建议

  1. 确认 logInitialPluginState() 是否可以异步执行,或者是否可以拆分为更小的粒度。
  2. 如果这仅仅是为了调试/记录日志,考虑使用 Qt::VeryLowCoarseTimer 精度,或者在不影响用户体验的空闲时间执行。

4. 代码安全

本次修改在安全方面风险较低,但有两点需要注意:

时间盲点引发的竞态条件

问题:在30秒的等待期间,如果插件列表发生了变化(触发了 pluginsChanged 信号),30秒后 logInitialPluginState() 记录的将不再是“初始状态”,而是“中间状态”。
建议:如果严格要求记录的是“初始加载完成的状态”,应在定时器触发前,判断状态是否已经改变;或者在 pluginsChanged 信号触发时,取消或重置这个30秒的定时器。

Lambda生命周期安全

问题QTimer::singleShot(30000, this, [this]() { ... }) 中,this 指针被捕获。如果 DockDBusProxy 对象在30秒内被销毁,Qt的信号槽机制会自动断开,通常不会造成UAF(Use-After-Free),这是安全的。但需确保 m_trayApplet 在30秒后依然有效,如果 logInitialPluginState() 内部访问了 m_trayApplet,需增加空指针保护。


改进后的代码建议

针对 dockdbusproxy.cpp 的修改,建议优化如下:

// 在头文件或cpp文件顶部定义常量
static constexpr int INITIAL_PLUGIN_LOG_DELAY_MS = 30000; // 30s: 等待慢速机器插件列表完全加载

// ... 构造函数内部 ...
        timer->stop();
        timer->deleteLater();
        connect(m_trayApplet, SIGNAL(pluginsChanged()), this, SIGNAL(pluginsChanged()));
        
        // TODO: 未来如果 TrayApplet 提供了 loaded() 信号,应替换此定时器方案
        QTimer::singleShot(INITIAL_PLUGIN_LOG_DELAY_MS, this, [this]() {
            if (m_trayApplet) { // 确保 m_trayApplet 依然有效
                logInitialPluginState();
            }
        });

对于 debian/control,请务必重新核对 1.2.481.2.51 的版本约束,确保编译依赖和运行依赖的逻辑一致性。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Ivy233, mhduiy

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

@Ivy233
Copy link
Copy Markdown
Contributor Author

Ivy233 commented May 19, 2026

/forcemerge

@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented May 19, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit eac6591 into linuxdeepin:master May 19, 2026
9 of 12 checks passed
@Ivy233 Ivy233 deleted the fix/debounce-plugin-log branch May 19, 2026 06:59
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