fix: improve window identity and tracking order #1310

wjyrich · 2025-10-23T05:07:27Z

Add desktop file ID to window identity for better uniqueness
Include AppItem desktopfileID in identity generation
Change window tracking order to emit signal before tracking
This prevents potential race conditions in window management

fix: 改进窗口标识和跟踪顺序

添加桌面文件ID到窗口标识以提高唯一性
在身份生成中包含AppItem的desktopfileID
更改窗口跟踪顺序，在跟踪前发出信号
这可以防止窗口管理中的潜在竞争条件

PMS: BUG-335801

Summary by Sourcery

Improve window identity uniqueness by appending desktop file identifiers and adjust the window tracking order to emit signals before tracking to avoid potential race conditions

Enhancements:

Include desktop file ID and AppItem desktopfileID in window identity generation to improve uniqueness
Emit windowAdded signal before invoking trackWindow to prevent race conditions in window management

sourcery-ai · 2025-10-23T05:07:43Z

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Enhance window identity uniqueness by incorporating desktop file IDs into identity generation and adjust the window tracking sequence by emitting the addition signal before actual tracking to avoid race conditions.

Sequence diagram for updated window tracking order in X11WindowMonitor

sequenceDiagram
    participant X11WindowMonitor
    participant "AbstractWindowMonitor"
    participant Window
    X11WindowMonitor->>Window: Create window object
    X11WindowMonitor->>"AbstractWindowMonitor": Emit windowAdded signal
    X11WindowMonitor->>Window: trackWindow(window)

Class diagram for updated window identity generation in X11Window

classDiagram
    class X11Window {
        +QStringList identity()
        -m_identity: QStringList
        -m_windowID: xcb_window_t
        +pid(): int
        +getAppItem(): AppItem*
    }
    class AppItem {
        +desktopfileID(): QString
    }
    X11Window --> AppItem: uses
    X11Window : identity() now appends getAppItem()->desktopfileID()

File-Level Changes

Change	Details	Files
Reorder window tracking in onWindowMapped to emit the windowAdded signal before tracking	Moved trackWindow call after emitting windowAdded signal Ensured signals are emitted prior to starting tracking to prevent racing	`panels/dock/taskmanager/x11windowmonitor.cpp`
Augment identity generation to include AppItem's desktopfileID	Added include of AppItem header Appended getAppItem()->desktopfileID() to the identity string	`panels/dock/taskmanager/x11window.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

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey there - 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 · 2025-10-23T05:08:28Z

deepin pr auto review

我来对这段代码进行审查，从语法逻辑、代码质量、性能和安全几个方面进行分析：

语法逻辑：

代码语法正确，没有明显的语法错误。
x11window.cpp中的identity()方法逻辑合理，先检查m_identity是否为空，如果为空则初始化。

代码质量：

在x11window.cpp中，identity()方法添加了新的依赖"appitem.h"，但没有检查getAppItem()返回的指针是否为空，这可能导致空指针解引用。
x11windowmonitor.cpp中调整了trackWindow()和windowAdded信号的顺序，这个改动是合理的，因为应该先通知外部有新窗口添加，再进行内部跟踪。

性能：

identity()方法每次调用都会检查m_identity是否为空，这是一个好的实践，避免重复计算。
使用QPointer来管理窗口对象是好的实践，可以防止悬空指针。

安全性：

主要的安全隐患在于getAppItem()可能返回空指针，建议添加空指针检查。

改进建议：

在x11window.cpp中添加空指针检查：

QStringList X11Window::identity()
{
    if (m_identity.isEmpty()) {
        m_identity = X11->getWindowWMClass(m_windowID);
        if (auto appItem = getAppItem()) {  // 添加空指针检查
            m_identity.append(appItem->desktopfileID());
        }
        m_identity.append(QString::number(pid()));
    }
    return m_identity;
}

在x11windowmonitor.cpp中，建议将window的创建和添加过程封装在一起，避免可能的内存泄漏：

void X11WindowMonitor::onWindowMapped(xcb_window_t xcb_window)
{
    auto window = std::make_unique<X11Window>(xcb_window);
    if (!window) {
        return;
    }

    uint32_t value_list[] = { XCB_EVENT_MASK_PROPERTY_CHANGE | XCB_EVENT_MASK_STRUCTURE_NOTIFY | XCB_EVENT_MASK_VISIBILITY_CHANGE};
    xcb_change_window_attributes(X11->getXcbConnection(), xcb_window, XCB_CW_EVENT_MASK, value_list);
    
    // 先添加到跟踪列表，确保不会丢失
    trackWindow(window.get());
    // 然后发送信号
    Q_EMIT AbstractWindowMonitor::windowAdded(static_cast<QPointer<AbstractWindow>>(window.get()));
}

考虑添加错误处理和日志记录，以便在出现问题时能够追踪：

void X11WindowMonitor::onWindowMapped(xcb_window_t xcb_window)
{
    try {
        auto window = std::make_unique<X11Window>(xcb_window);
        if (!window) {
            qWarning() << "Failed to create X11Window for xcb_window:" << xcb_window;
            return;
        }

        uint32_t value_list[] = { XCB_EVENT_MASK_PROPERTY_CHANGE | XCB_EVENT_MASK_STRUCTURE_NOTIFY | XCB_EVENT_MASK_VISIBILITY_CHANGE};
        xcb_change_window_attributes(X11->getXcbConnection(), xcb_window, XCB_CW_EVENT_MASK, value_list);
        
        trackWindow(window.get());
        Q_EMIT AbstractWindowMonitor::windowAdded(static_cast<QPointer<AbstractWindow>>(window.get()));
    } catch (const std::exception& e) {
        qCritical() << "Error in onWindowMapped:" << e.what();
    }
}

这些改进可以提高代码的健壮性和可维护性，同时保持原有功能不变。

18202781743 · 2025-10-23T05:47:16Z

panels/dock/taskmanager/x11window.cpp

 {
    if (m_identity.isEmpty()) {
        m_identity = X11->getWindowWMClass(m_windowID);
+        m_identity.append(getAppItem()->desktopfileID());


判断下是否为空吧，加个日志，
加个更新identity接口是不是更好，第一次就缓存，之后可能有问题，

1. Add desktop file ID to window identity for better uniqueness 2. Include AppItem desktopfileID in identity generation 3. Change window tracking order to emit signal before tracking 4. This prevents potential race conditions in window management fix: 改进窗口标识和跟踪顺序 1. 添加桌面文件ID到窗口标识以提高唯一性 2. 在身份生成中包含AppItem的desktopfileID 3. 更改窗口跟踪顺序，在跟踪前发出信号 4. 这可以防止窗口管理中的潜在竞争条件 PMS: BUG-335801

deepin-ci-robot · 2025-10-23T07:16:33Z

@wjyrich: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
github-trigger-obs-ci	`be25a00`	link	true	`/test github-trigger-obs-ci`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

deepin-ci-robot · 2025-10-23T07:17:26Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Details

Needs approval from an approver in each of these files:

debian/deepin/OWNERS

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

wjyrich · 2025-10-23T08:14:06Z

/forcemerge

deepin-bot · 2025-10-23T08:14:31Z

This pr force merged! (status: blocked)

BLumia

@tsic404 如果要加 am 应用识别的话，合理的加法是加到哪里？现在已经有一个来自 dde-apps 的 DesktopIdRole 了，是建议塞一个新 role 吗（比如... WindowMatchedDesktopId 什么的）？

BLumia · 2025-10-24T09:14:46Z

panels/dock/taskmanager/x11windowmonitor.cpp

    xcb_change_window_attributes(X11->getXcbConnection(), xcb_window, XCB_CW_EVENT_MASK, value_list);
-    trackWindow(window.get());
    Q_EMIT AbstractWindowMonitor::windowAdded(static_cast<QPointer<AbstractWindow>>(window.get()));
+    trackWindow(window.get());


这个顺序应该不能调，先track后发信号，不然可能信号接收到后尝试获取会拿不到（size不对）导致崩溃。

BLumia · 2025-10-24T09:16:45Z

panels/dock/taskmanager/x11window.cpp

+        if (auto appItem = getAppItem()) {
+            m_identity.append(appItem->desktopfileID());
+        } else {
+            qCWarning(x11windowLog) << "identify not found appitem." << id();
+        }


AppItem 是要废弃的，应当规避使用。

sourcery-ai bot reviewed Oct 23, 2025

View reviewed changes

18202781743 reviewed Oct 23, 2025

View reviewed changes

wjyrich force-pushed the fix-bug-335801 branch from d27c1b1 to be25a00 Compare October 23, 2025 07:14

18202781743 approved these changes Oct 23, 2025

View reviewed changes

deepin-bot bot merged commit fb5a9e9 into linuxdeepin:master Oct 23, 2025
7 of 9 checks passed

BLumia reviewed Oct 24, 2025

View reviewed changes

fix: improve window identity and tracking order #1310

fix: improve window identity and tracking order #1310

Uh oh!

Conversation

wjyrich commented Oct 23, 2025 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Oct 23, 2025

Reviewer's Guide

Sequence diagram for updated window tracking order in X11WindowMonitor

Class diagram for updated window identity generation in X11Window

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

deepin-ci-robot commented Oct 23, 2025

deepin pr auto review

Uh oh!

18202781743 Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepin-ci-robot commented Oct 23, 2025

Uh oh!

deepin-ci-robot commented Oct 23, 2025

Uh oh!

wjyrich commented Oct 23, 2025

Uh oh!

deepin-bot bot commented Oct 23, 2025

Uh oh!

Uh oh!

BLumia left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BLumia Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BLumia Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wjyrich commented Oct 23, 2025 •

edited by sourcery-ai bot

Loading

18202781743 Oct 23, 2025 •

edited

Loading

BLumia left a comment •

edited

Loading

BLumia Oct 24, 2025 •

edited

Loading