Skip to content

Conversation

@Ivy233
Copy link
Contributor

@Ivy233 Ivy233 commented Jan 4, 2026

概述

新增基于应用类别(Categories)的 cgroups 分组跳过列表功能,解决终端模拟器中启动的图形程序被错误归类到终端应用的问题。

问题描述

目前任务栏基于 cgroup 识别应用时,终端中启动的图形程序会继承父进程(终端)的 cgroup 信息,导致窗口被错误识别为归属终端应用。虽然存在白名单 cgroupsBasedGroupingSkipAppIds,但需要逐个手动添加终端模拟器的 appId。

解决方案

  1. 新增 DConfig 配置项 cgroupsBasedGroupingSkipCategories

    • 默认值:["TerminalEmulator"]
    • 支持基于 desktop 文件的 Categories 字段进行匹配
  2. 实现类别检查函数 shouldSkipCgroupsByCategories()

    • 通过 DBus 查询应用的 Categories
    • 检查是否在跳过列表中
  3. 集成到窗口匹配流程

    • 在 cgroups 匹配前进行类别检查
    • 如果应用属于跳过类别,则跳过 cgroups 匹配

修改文件

  • panels/dock/taskmanager/dconfig/org.deepin.ds.dock.taskmanager.json - 新增配置项
  • panels/dock/taskmanager/globals.h - 新增配置键常量
  • panels/dock/taskmanager/taskmanager.cpp - 实现类别检查逻辑
  • panels/dock/taskmanager/taskmanagersettings.cpp - 读取配置
  • panels/dock/taskmanager/taskmanagersettings.h - 添加接口

测试验证

  • 安装 Alacritty 终端模拟器
  • 在 Alacritty 中运行 dde-dconfig-editor
  • 验证 dde-dconfig-editor 显示为独立图标(不被归类到 Alacritty)
  • 验证深度终端等原有白名单应用仍正常工作
  • 确认非终端类应用的分组行为不受影响

Summary by Sourcery

Add configurable category-based skip list for cgroup-based taskbar grouping to avoid mis-grouping terminal-launched GUI apps.

New Features:

  • Introduce a DConfig option to define application categories that should be excluded from cgroup-based window grouping.
  • Use desktop file Categories retrieved via DBus to decide whether to skip cgroup-based grouping for a window.

Enhancements:

  • Extend task manager settings and matching logic to combine app ID and category-based skip rules when applying cgroup-based grouping.

@deepin-ci-robot
Copy link

[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

@github-actions
Copy link

github-actions bot commented Jan 4, 2026

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 4, 2026

Reviewer's Guide

Adds a configurable, category-based skip list to the task manager’s cgroups-based window grouping, so that apps in specified desktop Categories (default: TerminalEmulator) bypass cgroup-based grouping and avoid being incorrectly grouped under terminal emulators.

Sequence diagram for cgroups-based grouping with category skip check

sequenceDiagram
    actor User
    participant TerminalEmulator
    participant GUIApp
    participant TaskManager
    participant Settings
    participant ApplicationManager
    participant Application

    User->>TerminalEmulator: Start terminal emulator
    User->>TerminalEmulator: Run graphical app command
    TerminalEmulator->>GUIApp: Spawn GUI process
    GUIApp->>TaskManager: New window detected

    TaskManager->>Settings: cgroupsBasedGrouping()
    alt cgroups based grouping enabled
        TaskManager->>TaskManager: getDesktopIdByPid(identifies)
        alt desktopId is empty
            TaskManager->>TaskManager: Fallback grouping (no desktopId)
        else desktopId is not empty
            TaskManager->>Settings: cgroupsBasedGroupingSkipIds()
            alt desktopId in skip id list
                TaskManager->>TaskManager: Skip cgroups grouping (whitelisted by id)
            else desktopId not in skip id list
                TaskManager->>TaskManager: shouldSkipCgroupsByCategories(desktopId)
                TaskManager->>Settings: cgroupsBasedGroupingSkipCategories()
                TaskManager->>ApplicationManager: Create Application proxy with desktopId
                alt Application proxy is valid
                    TaskManager->>Application: categories()
                    alt category in skip category list
                        TaskManager->>TaskManager: Skip cgroups grouping (whitelisted by category)
                    else no matching category
                        TaskManager->>TaskManager: Perform cgroups based grouping
                    end
                else Application proxy invalid
                    TaskManager->>TaskManager: Perform cgroups based grouping
                end
            end
        end
    else cgroups based grouping disabled
        TaskManager->>TaskManager: Use existing non cgroups grouping
    end

    TaskManager-->>User: Window appears in taskbar (grouped or independent depending on checks)
Loading

Class diagram for updated TaskManagerSettings and Application interface usage

classDiagram
    class TaskManagerSettings {
        -bool m_cgroupsBasedGrouping
        -QStringList m_dockedElements
        -QStringList m_cgroupsBasedGroupingSkipAppIds
        -QStringList m_cgroupsBasedGroupingSkipCategories
        +TaskManagerSettings(QObject parent)
        +bool cgroupsBasedGrouping()
        +QStringList cgroupsBasedGroupingSkipIds()
        +QStringList cgroupsBasedGroupingSkipCategories()
        +QStringList dockedElements()
        +void setDockedElements(QStringList elements)
        +void toggleDockedElement(QString element)
    }

    class TaskManager {
        +bool init()
    }

    class Application {
        +QStringList categories()
        +bool isValid()
    }

    class Settings {
        +bool cgroupsBasedGrouping()
        +QStringList cgroupsBasedGroupingSkipIds()
        +QStringList cgroupsBasedGroupingSkipCategories()
    }

    TaskManager --> Settings : uses
    TaskManager --> Application : queries categories via DBus
    Settings --> TaskManagerSettings : wraps persistent config
    TaskManagerSettings ..> QStringList : stores skip lists
Loading

File-Level Changes

Change Details Files
Add DConfig-backed setting for cgroups grouping skip categories and expose it via TaskManagerSettings.
  • Introduce TASKMANAGER_CGROUPS_BASED_GROUPING_SKIP_CATEGORIES key in globals.
  • Load cgroupsBasedGroupingSkipCategories from DConfig with default ["TerminalEmulator"].
  • Add cgroupsBasedGroupingSkipCategories() accessor and backing member on TaskManagerSettings.
panels/dock/taskmanager/globals.h
panels/dock/taskmanager/taskmanagersettings.h
panels/dock/taskmanager/taskmanagersettings.cpp
panels/dock/taskmanager/dconfig/org.deepin.ds.dock.taskmanager.json
Integrate category-based skip logic into the cgroups-based window matching flow using ApplicationManager DBus API.
  • Include applicationinterface.h in taskmanager.cpp to talk to ApplicationManager over DBus.
  • Implement shouldSkipCgroupsByCategories(desktopId) that queries the app’s Categories via DBus and checks them against the configured skip categories list.
  • Extend the cgroups-based matching condition to also require that desktopId is not in the skip categories (using shouldSkipCgroupsByCategories).
panels/dock/taskmanager/taskmanager.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

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:

  • The per-window DBus lookup in shouldSkipCgroupsByCategories may add noticeable overhead in the hot path of window matching; consider caching categories by desktopId or precomputing the skip decision to avoid repeated DBus calls.
  • When checking skipCategories.contains(category), you might want to normalize case or use a QSet for skipCategories to avoid subtle mismatches and to make the membership checks more efficient.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The per-window DBus lookup in shouldSkipCgroupsByCategories may add noticeable overhead in the hot path of window matching; consider caching categories by desktopId or precomputing the skip decision to avoid repeated DBus calls.
- When checking skipCategories.contains(category), you might want to normalize case or use a QSet for skipCategories to avoid subtle mismatches and to make the membership checks more efficient.

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.

Comment on lines 80 to 90
QString dbusPath = QStringLiteral("/org/desktopspec/ApplicationManager1/") + escapeToObjectPath(desktopId);

// 创建 Application 接口
Application appInterface(QStringLiteral("org.desktopspec.ApplicationManager1"),
dbusPath,
QDBusConnection::sessionBus());

if (!appInterface.isValid()) {
qCDebug(taskManagerLog) << "Failed to create Application interface for:" << desktopId;
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

不再绕一次 dbus 了,在 AppItem 一层新增一个 CategoryRole(可以参考现有的 DDECategoryRole),然后直接使用这个新增的 role 即可。

@Ivy233
Copy link
Contributor Author

Ivy233 commented Jan 5, 2026

recheck

@Ivy233 Ivy233 force-pushed the feature/cgroups-skip-categories branch from a0e2e66 to 3b87e6d Compare January 6, 2026 15:42
if (activeAppModel) {
auto existingItem = activeAppModel->match(activeAppModel->index(0, 0), roleNames.key("desktopId"), desktopId, 1, Qt::MatchFixedString | Qt::MatchWrap).value(0);
if (existingItem.isValid()) {
categories = activeAppModel->data(existingItem, roleNames.key("categories")).toStringList();
Copy link
Member

Choose a reason for hiding this comment

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

不用再从 roleNames.key("categories") 获取 role 了吧?已经有 TaskManager::CategoriesRole 了。

}

// 检查是否有任何 category 在跳过列表中
for (const QString &category : categories) {
Copy link
Member

Choose a reason for hiding this comment

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

是不是反了,不应该是迭代 skipCategories 然后判断 categories 是否包含 skipCategoriy 吗?

skipCategories 配置里预期不会有太多值,默认只有终端,也不期望用户自己随便加。

}

// 检查应用的 Categories 是否在跳过列表中
static bool shouldSkipCgroupsByCategories(const QString &desktopId, QAbstractItemModel *activeAppModel, const QHash<int, QByteArray> &roleNames)
Copy link
Member

Choose a reason for hiding this comment

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

倒不如直接把 “尝试通过AM(Application Manager)匹配应用程序” 那块提出来。只提目前这一部分有点怪怪的。

@deepin-bot
Copy link

deepin-bot bot commented Jan 7, 2026

TAG Bot

New tag: 2.0.25
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1394

@Ivy233
Copy link
Contributor Author

Ivy233 commented Jan 13, 2026

recheck

@deepin-bot
Copy link

deepin-bot bot commented Jan 14, 2026

TAG Bot

New tag: 2.0.26
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1396

1. 新增 DConfig 配置项 cgroupsBasedGroupingSkipCategories
2. 默认跳过 TerminalEmulator 类别的应用,避免终端中启动的程序被错误识别
3. 在 dde-apps 的 AppItem 中添加 categories 属性
4. 在 AppItemModel 中添加 CategoriesRole
5. 在 AMAppItem 构造函数中保存从 AM 获取的 categories
6. 实现 shouldSkipCgroupsByCategories() 函数,从 dde-apps 缓存读取 categories
7. 提取 tryMatchByApplicationManager() 函数,优化代码结构
8. 使用 TaskManager::CategoriesRole 枚举常量替代运行时查找
9. 优化迭代逻辑,迭代 skipCategories 而非 categories,提升性能
10. 在窗口匹配流程中集成类别检查逻辑

Log: 新增基于应用类别的 cgroups 分组跳过列表,解决终端中启动的图形程序被错误归类的问题

Influence:
1. 测试在 Alacritty 等终端模拟器中启动图形应用
2. 验证新启动的应用在任务栏显示独立图标
3. 验证 categories 从 dde-apps 缓存读取,无额外 DBus 调用
4. 检查 DConfig 配置项可正常读取和修改
5. 确认不影响非终端类应用的正常分组行为
6. 验证深度终端等原有白名单应用仍正常工作
7. 确认代码结构优化不影响功能正确性

PMS: TASK-384865
@Ivy233 Ivy233 force-pushed the feature/cgroups-skip-categories branch from 3b87e6d to 62a5b07 Compare January 14, 2026 10:23
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查报告

概述

这段代码主要是为任务管理器添加基于应用类别的分组跳过功能。当应用属于特定类别时,可以跳过基于cgroups的应用分组。下面是对代码的详细审查:

语法逻辑

  1. 新增函数命名和实现

    • shouldSkipCgroupsByCategories() 函数逻辑清晰,正确实现了检查应用类别是否在跳过列表中的功能。
    • tryMatchByApplicationManager() 函数将原有的AM匹配逻辑提取出来,并增加了类别检查,逻辑合理。
  2. 数据流

    • 从 AM(AppItem) 获取 categories → 存储到 AppItemModel → 在 TaskManager 中读取并检查,数据流向清晰。
  3. 空值检查

    • shouldSkipCgroupsByCategories() 中有适当的空值检查:
      if (skipCategories.isEmpty() || desktopId.isEmpty()) {
          return false;
      }

代码质量

  1. 代码组织

    • 将原有逻辑提取为独立函数 tryMatchByApplicationManager() 提高了代码可读性和可维护性。
    • 相关功能集中在一起,符合单一职责原则。
  2. 命名规范

    • 函数和变量命名清晰,如 shouldSkipCgroupsByCategories()cgroupsBasedGroupingSkipCategories
  3. 注释

    • 添加了中文注释,如 // 检查应用的 Categories 是否在跳过列表中,提高了代码可读性。
  4. 一致性

    • 新增的 CategoriesRole 与现有的 DDECategoryRole 保持一致的设计模式。

代码性能

  1. 潜在性能问题

    • shouldSkipCgroupsByCategories() 中,每次调用都需要从模型中查找应用并获取其类别:
      auto existingItem = activeAppModel->match(activeAppModel->index(0, 0), roleNames.key("desktopId"), desktopId, 1, Qt::MatchFixedString | Qt::MatchWrap).value(0);
      如果频繁调用,可能会影响性能。
  2. 优化建议

    • 考虑缓存应用类别信息,避免重复查询模型。
    • 如果模型很大,可以考虑使用哈希表来加速查找。

代码安全

  1. 输入验证

    • 对输入参数进行了基本的空值检查,但可以进一步加强:
      if (!activeAppModel || roleNames.isEmpty()) {
          return false;
      }
  2. 边界条件

    • 考虑了 categories 为空的情况,返回 false 是合理的。
  3. 配置安全性

    • 新增的配置项 cgroupsBasedGroupingSkipCategories 有默认值 ["TerminalEmulator"],避免配置缺失导致的问题。

改进建议

  1. 性能优化

    // 在 TaskManager 类中添加缓存成员
    QHash<QString, QStringList> m_categoriesCache;
    
    // 修改 shouldSkipCgroupsByCategories 函数
    static bool shouldSkipCgroupsByCategories(const QString &desktopId, QAbstractItemModel *activeAppModel, 
                                              const QHash<int, QByteArray> &roleNames,
                                              QHash<QString, QStringList> &cache)
    {
        auto skipCategories = Settings->cgroupsBasedGroupingSkipCategories();
        if (skipCategories.isEmpty() || desktopId.isEmpty()) {
            return false;
        }
        
        // 先检查缓存
        if (cache.contains(desktopId)) {
            const auto &categories = cache[desktopId];
            for (const QString &skipCategory : skipCategories) {
                if (categories.contains(skipCategory)) {
                    qCDebug(taskManagerLog) << "Skipping cgroups grouping for" << desktopId
                                           << "due to category:" << skipCategory;
                    return true;
                }
            }
            return false;
        }
        
        // 从模型获取并缓存
        QStringList categories;
        if (activeAppModel) {
            auto existingItem = activeAppModel->match(activeAppModel->index(0, 0), roleNames.key("desktopId"), 
                                                      desktopId, 1, Qt::MatchFixedString | Qt::MatchWrap).value(0);
            if (existingItem.isValid()) {
                categories = activeAppModel->data(existingItem, TaskManager::CategoriesRole).toStringList();
                cache[desktopId] = categories;
            }
        }
        
        if (categories.isEmpty()) {
            return false;
        }
        
        for (const QString &skipCategory : skipCategories) {
            if (categories.contains(skipCategory)) {
                qCDebug(taskManagerLog) << "Skipping cgroups grouping for" << desktopId
                                       << "due to category:" << skipCategory;
                return true;
            }
        }
        
        return false;
    }
  2. 错误处理

    • 添加更多错误处理,例如当模型查询失败时记录警告日志。
  3. 测试建议

    • 添加单元测试,验证以下场景:
      • 应用类别在跳过列表中
      • 应用类别不在跳过列表中
      • 应用类别为空
      • 跳过列表为空
      • 模型查询失败的情况

总体评价

这段代码实现了基于应用类别的分组跳过功能,逻辑清晰,与现有代码集成良好。主要改进点在于性能优化和错误处理。建议实施上述优化措施,以提高代码的健壮性和性能。

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