Skip to content

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Jul 23, 2025

PR 当前状态说明:

  1. 相比 WIP: refactor: migrate taskmanager model to the new DockItemModel #1148 ,此 PR 状态仅负责 model 切换以及切换后基础功能的相应变更/重新实现
  2. 原有代码关于“终端启动的应用的子进程”识别问题可以通过移除通过 AM 进行 cgroup 识别的调用来解决,但观察到有 360zip 这种不太对劲的 app 会在特定操作步骤后导致任务栏显示成齿轮。重构分支无此问题,在旧分支解决的意义可能不大。
  3. 原有代码关于非子进程识别问题的图标错误合并情况大概率是应用列表刷新时机的问题,旧的结构上只能绕过,不太方便根治。
  4. 此 PR 【目前】状态有一些功能未补全,包括关闭窗口时的预览状态等。这也是当前标记为 draft 的原因,挂一个 PR 旨在方便看 diff,不用在 worktree 之间切来切去。

切换 taskmanager 区域的 model 为新的 DockItemModel,以解决一系列问题:

  1. 部分场景下企业微信和微信图标合并/钉钉和微信合并等问题
  2. 终端执行 gitk/dde-dconfig-editor 时不会单独显示任务栏图标
  3. 没有 desktop-id 的带界面的可执行程序不会单独显示任务栏图标
  4. 开机后部分场景(AM启动晚于任务栏时)可能导致任务栏驻留全部消失

进展跟进:

状态说明:

  • 🚧 施工中
  • ✅ 相关修改已并入主线,并 rebase 当前分支至主干合入相关修改后的commit之上
  • 无 emoji 则必定提供 PR 链接,表示相关 PR 已就绪,待 review 或已合入但还未 rebase 此分支

Summary by Sourcery

Migrate the TaskManager from the old AppItem/ItemModel architecture to the new DockItemModel, refactor preview and activation APIs, and update QML bindings and tests accordingly.

New Features:

  • Replace AppItemWindowModel with DockItemWindowModel that drives window previews from QModelIndexList.
  • Implement AbstractTaskManagerInterface methods (requestActivate, requestClose, requestPreview, etc.) in window monitors.
  • Add fetchWindowPreview using KWin ScreenShot2 D-Bus interface for accurate window preview snapshots.

Bug Fixes:

  • Eliminate icon-merging problems (e.g. WeChat, Enterprise WeChat) by switching to the new dock model.

Enhancements:

  • Simplify QML by replacing legacy clickItem/drop signals with TaskManager.requestActivate and requestNewInstance using modelIndex.
  • Remove outdated docking drop-area hacks and delay window monitor startup for proper initialization.

Tests:

  • Include QAbstractItemModelTester in combine/group model tests to strengthen model validation.

@deepin-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@sourcery-ai
Copy link

sourcery-ai bot commented Jul 23, 2025

Reviewer's Guide

This PR migrates the TaskManager area to the new DockItemModel, refactoring window previews, monitors, TaskManager APIs, and QML delegates to operate on QModelIndexList and the AbstractTaskManagerInterface, while removing legacy AppItem/ItemModel code and updating tests.

Sequence diagram for window preview request using new model

sequenceDiagram
    participant User as actor User
    participant AppItem as AppItem (QML)
    participant TaskManagerQML as TaskManagerQML
    participant TaskManager as TaskManager
    participant AbstractWindowMonitor as AbstractWindowMonitor
    participant X11WindowPreviewContainer as X11WindowPreviewContainer
    User->>AppItem: Hover or click triggers preview
    AppItem->>TaskManagerQML: requestPreview([modelIndex], Panel.rootObject, xOffset, yOffset, position)
    TaskManagerQML->>TaskManager: requestPreview(...)
    TaskManager->>AbstractWindowMonitor: requestPreview(QModelIndexList, ...)
    AbstractWindowMonitor->>X11WindowPreviewContainer: showPreview(QModelIndexList, ...)
    X11WindowPreviewContainer->>DockItemWindowModel: setData(QModelIndexList)
    X11WindowPreviewContainer->>X11WindowPreviewContainer: updatePreviewIconFromBase64(...)
    X11WindowPreviewContainer->>X11WindowPreviewContainer: updatePreviewTitle(...)
    X11WindowPreviewContainer->>X11WindowPreviewContainer: updateSize(...)
Loading

Class diagram for TaskManager model migration to DockItemModel

classDiagram
    class TaskManager {
        +DockItemModel* dataModel() const
        +void dropFilesOnItem(QString, QStringList)
        +void hideItemPreview()
        ...
    }
    class DockItemModel {
        +void setSourceModel(QAbstractItemModel*)
        +void dumpItemInfo(QModelIndex)
        ...
    }
    class AbstractWindowMonitor {
        +QPointer<AbstractWindow> getWindowByWindowId(ulong)
        +void requestActivate(QModelIndex) const
        +void requestPreview(QModelIndexList, QObject*, int32_t, int32_t, uint32_t) const
        +void requestClose(QModelIndex, bool) const
        ...
    }
    class X11WindowMonitor {
        +void requestPreview(QModelIndexList, QObject*, int32_t, int32_t, uint32_t) const
        +void hideItemPreview()
        ...
    }
    class TreeLandWindowMonitor {
        +void requestPreview(QModelIndexList, QObject*, int32_t, int32_t, uint32_t) const
        +void hideItemPreview()
        ...
    }
    class DockItemWindowModel {
        +int rowCount(QModelIndex) const
        +QVariant data(QModelIndex, int) const
        +void setData(QModelIndexList)
        +void resetPreviewPixmap()
        ...
    }
    TaskManager --> DockItemModel : uses
    TaskManager --> AbstractWindowMonitor : owns
    AbstractWindowMonitor <|-- X11WindowMonitor
    AbstractWindowMonitor <|-- TreeLandWindowMonitor
    X11WindowMonitor --> DockItemWindowModel : uses
    TreeLandWindowMonitor --> DockItemWindowModel : uses
Loading

Class diagram for QML delegate and AppItem changes

classDiagram
    class AppItem {
        +string itemId
        +string name
        +string iconName
        +string icon
        +list<string> windows
        +int visualIndex
        +var modelIndex
        +signal dropFilesOnItem(itemId, files)
        +signal dragFinished()
        ...
    }
    class TaskManagerQML {
        +void requestActivate(var modelIndex)
        +void requestNewInstance(var modelIndex, string action)
        +void requestPreview(var indexes, QObject, int32_t, int32_t, uint32_t)
        ...
    }
    AppItem --> TaskManagerQML : calls methods
Loading

File-Level Changes

Change Details Files
Refactor window preview component to use DockItemModel and QModelIndexList
  • Renamed AppItemWindowModel to DockItemWindowModel keyed on QModelIndexList
  • Updated data(), setData(), and resetPreviewPixmap() to fetch roles from QModelIndex
  • Changed showPreview and updateSize signatures to accept QModelIndexList
  • Removed direct AppItem references and legacy signal connections
x11preview.cpp
x11preview.h
Replace ItemModel with DockItemModel in TaskManager
  • Changed dataModel() to return DockItemModel* and removed loadDockedAppItems
  • Removed clickItem and showItemPreview Q_INVOKABLEs, stubbed dropFilesOnItem
  • Deferred window monitor start via QTimer instead of immediate start
  • Guarded loadDockedItems() in TaskManagerSettings to avoid repeated loads
taskmanager.cpp
taskmanager.h
taskmanagersettings.cpp
Extend AbstractWindowMonitor with AbstractTaskManagerInterface and add request methods
  • Inherited AbstractTaskManagerInterface and removed showItemPreview virtual
  • Added requestActivate, requestClose, requestPreview, requestWindowsView, and other requestX methods
  • Adjusted dataChanged signals to include combined roles
abstractwindowmonitor.h
abstractwindowmonitor.cpp
Implement requestPreview in X11 and TreeLand window monitors
  • Replaced legacy showItemPreview with requestPreview forwarding indexes to preview container/context
  • Removed direct AppItem usage and updated preview lifecycle
  • Connected preview context and hid previews via requestPreview
treelandwindowmonitor.cpp
treelandwindowmonitor.h
x11windowmonitor.cpp
x11windowmonitor.h
Update QML delegates to use modelIndex and new request APIs
  • Added modelIndex property to TaskManager and AppItem delegates
  • Replaced clickItem with TaskManager.requestActivate and requestNewInstance
  • Rewired preview trigger to call requestPreview with indexes
  • Removed legacy DropArea drag handlers and simplified bindings
TaskManager.qml
AppItem.qml
Clean up combine models and tests
  • Enhanced DockCombineModel debug output and role handling
  • Invoked requestClose for CloseAll action in DockGlobalElementModel
  • Added QAbstractItemModelTester include to role model tests
  • Removed excess whitespace and formatting fixes
dockcombinemodel.cpp
dockglobalelementmodel.cpp
rolecombinemodeltests.cpp
rolegroupmodeltests.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

@BLumia BLumia force-pushed the taskmanager-model-switched branch 3 times, most recently from d243a0e to 0d7d711 Compare July 31, 2025 11:18
@deepin-bot
Copy link

deepin-bot bot commented Jul 31, 2025

TAG Bot

New tag: 2.0.4
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1210

@BLumia BLumia force-pushed the taskmanager-model-switched branch 4 times, most recently from 27eeb22 to 4af5797 Compare August 6, 2025 05:11
@deepin-bot
Copy link

deepin-bot bot commented Aug 7, 2025

TAG Bot

New tag: 2.0.5
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1217

@deepin-bot
Copy link

deepin-bot bot commented Aug 12, 2025

TAG Bot

New tag: 2.0.6
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1221

@BLumia
Copy link
Member Author

BLumia commented Aug 19, 2025

关联:

在此分支合入前,提供一个供开发者使用的选项以供手动关闭基于 cgroups 的应用识别,解决没有 desktop-id 的应用程序被显示在父进程上的问题。

注意:上述 PR 所提供的选项不面向最终用户,主动调整相关选项的值后所遇到的其他 bug 不受任何支持。最终用户应当等待此 PR 的合入、集成、推送。

@BLumia BLumia force-pushed the taskmanager-model-switched branch 2 times, most recently from 0159315 to 10ad22f Compare August 21, 2025 12:43
@deepin-bot
Copy link

deepin-bot bot commented Aug 22, 2025

TAG Bot

New tag: 2.0.7
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1228

BLumia added a commit to BLumia/dde-shell that referenced this pull request Aug 25, 2025
This patch is to continue prepare for linuxdeepin#1201,
by add previously missing changed signal for Docked and Menu role
when docked state is changed, and implementing actions for menu
entries.

Log:
BLumia added a commit to BLumia/dde-shell that referenced this pull request Aug 26, 2025
This patch is to continue prepare for linuxdeepin#1201,
by add previously missing changed signal for Docked and Menu role
when docked state is changed, and implementing actions for menu
entries.

This patch also make DnD on dock item works without using the
legacy ItemModel class by implementing requestOpenUrls().

Log:
BLumia added a commit that referenced this pull request Aug 26, 2025
This patch is to continue prepare for #1201,
by add previously missing changed signal for Docked and Menu role
when docked state is changed, and implementing actions for menu
entries.

This patch also make DnD on dock item works without using the
legacy ItemModel class by implementing requestOpenUrls().

Log:
@BLumia BLumia force-pushed the taskmanager-model-switched branch 2 times, most recently from 784fdaf to e0e5eb3 Compare August 28, 2025 05:17
@deepin-bot
Copy link

deepin-bot bot commented Aug 28, 2025

TAG Bot

New tag: 2.0.8
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1235

@BLumia BLumia force-pushed the taskmanager-model-switched branch 2 times, most recently from 25c2110 to 696ab2e Compare September 1, 2025 10:04
@BLumia BLumia changed the title WIP: refactor: migrate taskmanager model to the new DockItemModel refactor: migrate taskmanager model to the new DockItemModel Sep 1, 2025
@BLumia BLumia marked this pull request as ready for review September 1, 2025 10:05
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.

Sorry @BLumia, you have reached your weekly rate limit for Sourcery. Please try again later

@BLumia
Copy link
Member Author

BLumia commented Sep 1, 2025

@sourcery-ai review

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.

Sorry @BLumia, you have reached your weekly rate limit for Sourcery. Please try again later

Copy link

@SourceryAI SourceryAI left a comment

Choose a reason for hiding this comment

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

Sorry @BLumia, you have reached your weekly rate limit for Sourcery. Please try again later

@BLumia
Copy link
Member Author

BLumia commented Sep 2, 2025

@sourcery-ai review

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.

Sorry @BLumia, you have reached your weekly rate limit for Sourcery. Please try again later

Copy link

@SourceryAI SourceryAI left a comment

Choose a reason for hiding this comment

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

Sorry @BLumia, you have reached your weekly rate limit for Sourcery. Please try again later

@BLumia BLumia force-pushed the taskmanager-model-switched branch from 696ab2e to 5a3c52b Compare September 2, 2025 03:29
@BLumia BLumia force-pushed the taskmanager-model-switched branch from 5a3c52b to 0992f72 Compare September 3, 2025 05:39
切换 taskmanager 区域的 model 为新的 DockItemModel,以解决一系列问题:

1. 部分场景下企业微信和微信图标合并/钉钉和微信合并等问题
2. 终端执行 gitk/dde-dconfig-editor 时不会单独显示任务栏图标
3. 没有 desktop-id 的带界面的可执行程序不会单独显示任务栏图标

Log:
@BLumia BLumia force-pushed the taskmanager-model-switched branch from 0992f72 to 5d9ad80 Compare September 3, 2025 05:45
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查报告

1. 语法逻辑分析

  1. AbstractWindowMonitor类改进

    • 成功继承AbstractTaskManagerInterface,增强了类的功能
    • 新增的requestActivate、requestClose等方法实现合理
    • 但在requestClose方法中,force参数默认值设置为false可能导致意外的强制关闭行为
  2. TaskManager类重构

    • 移除了旧的clickItem方法,改用基于modelIndex的新API,这是一个好的改进
    • 新增了hoverPreviewModel,用于预览功能的代理
    • 但在init方法中,将m_windowMonitor的start延迟500ms可能不是最佳实践
  3. X11WindowMonitor改进

    • 重构了showItemPreview为requestPreview,使用sourceModel替代AppItem
    • 新增clearPreviewState方法用于清理预览状态
    • 但在previewWindow方法中缺少错误检查

2. 代码质量分析

  1. 命名规范

    • 整体命名规范良好,但部分变量名可以更具描述性
    • 例如:m_isDockPreviewCount可以改为m_activePreviewCount
  2. 代码复用

    • 在X11WindowPreviewContainer中,fetchWindowPreview函数被重复使用
    • 建议将其提取为独立的工具函数
  3. 错误处理

    • 部分方法缺少适当的错误处理
    • 例如:requestPreview方法中未检查sourceModel的有效性

3. 代码性能分析

  1. 内存管理

    • 使用QPointer进行内存管理是好的实践
    • 但在TreeLandWindowMonitor中,m_dockPreview的创建和销毁可能过于频繁
  2. 性能优化

    • 在DockGroupModel::all方法中,对每个子项都调用data()可能影响性能
    • 建议批量获取数据
  3. 预览功能

    • 预览图像的获取是同步的,可能会阻塞UI线程
    • 建议改为异步加载

4. 代码安全分析

  1. 边界检查

    • 在多个地方缺少适当的边界检查
    • 例如:AbstractWindowMonitor::data方法中未检查index的有效性
  2. 类型安全

    • 部分地方使用了原始指针而非智能指针
    • 例如:X11WindowMonitor中的xcb_window_t处理
  3. 资源管理

    • 在X11WindowPreviewContainer中,文件描述符的关闭时机可能不够安全
    • 建议使用RAII模式管理资源

改进建议

  1. 语法逻辑改进

    • 为requestClose的force参数添加明确的文档说明
    • 考虑将m_windowMonitor的start改为在所有初始化完成后立即执行
  2. 代码质量改进

    • 提取公共函数,如fetchWindowPreview
    • 为所有public方法添加参数有效性检查
    • 使用更具描述性的变量名
  3. 性能优化

    • 实现预览图像的异步加载
    • 优化DockGroupModel中的数据获取方式
    • 考虑使用对象池管理频繁创建销毁的对象
  4. 安全增强

    • 添加适当的边界检查
    • 使用智能指针替代原始指针
    • 实现RAII资源管理
  5. 其他建议

    • 添加单元测试覆盖关键功能
    • 考虑使用const引用传递大对象
    • 添加更多日志记录以帮助调试

总体而言,代码结构清晰,功能实现合理,但在健壮性和性能方面还有提升空间。建议优先处理边界检查和资源管理相关的问题。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, tsic404

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

@BLumia BLumia merged commit 3c507cd into linuxdeepin:master Sep 3, 2025
9 of 10 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.

4 participants