Skip to content

fix(power): unify screen-off handling in session - #166

Merged
deepin-bot[bot] merged 1 commit into
masterfrom
task/x11power
Sep 9, 2026
Merged

fix(power): unify screen-off handling in session#166
deepin-bot[bot] merged 1 commit into
masterfrom
task/x11power

Conversation

@mhduiy

@mhduiy mhduiy commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

变更

  • 关屏逻辑收敛到电源会话服务 PowerManager::TurnOffScreen,dde-shortcut-tool 仅调用 Power1.TurnOffScreen
  • X11 下先锁屏再 DPMS 关屏,并用 KWin BlackScreen 遮罩遮盖锁屏绘制间隙
  • 屏幕已关闭时按电源键改为重新点亮;屏幕控制器不可用时忽略请求
  • 更新 plugin-power-session 翻译

关联

PMS: BUG-375787

Summary by Sourcery

Centralize screen power transitions in the power session service and delegate shortcut-triggered screen-off requests through Power1.

Bug Fixes:

  • Unify screen-off behavior so power-button presses wake an already-off screen and safely ignore requests when screen control is unavailable.

Enhancements:

  • Centralize screen locking, DPMS handling, and X11 black-screen masking in the power session service while reducing the shortcut tool to a Power1 TurnOffScreen call.

Chores:

  • Update plugin-power-session translations.

@sourcery-ai

sourcery-ai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Reviewer's Guide

Screen-off handling is consolidated behind PowerManager::TurnOffScreen, with controller availability checks, power-button wake toggling, and an X11 lock/BlackScreen/DPMS sequence; dde-shortcut-tool now makes only the Power1 D-Bus call, and translation file metadata is normalized.

Sequence diagram for unified screen-off handling

sequenceDiagram
    participant Shortcut as dde-shortcut-tool
    participant Power as PowerManager
    participant Screen as ScreenController
    participant KWin as KWinBlackScreen
    participant DPMS as DisplayPower

    Shortcut->>Power: TurnOffScreen()
    alt screen controller unavailable
        Power-->>Shortcut: ignore request
    else screen is off
        Power->>Power: TurnOnScreen()
    else Wayland session
        Power->>Power: doTurnOffScreen()
    else X11 session
        Power->>Screen: doLock(true)
        Power->>Power: SetPrepareSuspend(PS_ButtonClick)
        Power->>KWin: wmBlackScreenActive()
        opt mask not active
            Power->>KWin: setWmBlackScreenActive(true)
        end
        Power->>DPMS: setDPMSModeOff()
        opt mask was enabled
            Power->>KWin: setWmBlackScreenActive(false)
        end
        Power->>Power: SetPrepareSuspend(PS_Finish)
    end
Loading

File-Level Changes

Change Details Files
Centralize screen-off behavior in the PowerManager session service.
  • Move screen-controller validation and off-to-on toggle handling into the session service.
  • Delegate the shortcut tool’s screen-off action to the Power1 TurnOffScreen D-Bus method and remove duplicated transition logic.
  • Preserve error handling when the session service is unavailable or the D-Bus call fails.
src/plugin-qt/power/session/powermanager.cpp
src/plugin-qt/shortcut/tools/dde-shortcut-tool/powercontroller.cpp
src/plugin-qt/shortcut/tools/dde-shortcut-tool/powercontroller.h
Implement the X11 lock, masking, and DPMS transition sequence in the session service.
  • Keep Wayland on the existing direct screen-off path.
  • On X11, lock before DPMS off, notify prepare-suspend state, briefly enable KWin BlackScreen when needed, switch DPMS off, then clear the mask and finish the notification.
  • Add synchronous KWin BlackScreen getActive/setActive D-Bus proxy operations with failure logging.
src/plugin-qt/power/session/powermanager.cpp
src/plugin-qt/power/session/sessiondbusproxy.cpp
src/plugin-qt/power/session/sessiondbusproxy.h
Normalize the power-session translation file metadata and formatting.
  • Add UTF-8 XML declarations and ensure trailing newlines across localized translation files.
src/plugin-qt/power/session/translations/plugin-power-session_ca.ts
src/plugin-qt/power/session/translations/plugin-power-session_de.ts
src/plugin-qt/power/session/translations/plugin-power-session_fi.ts
src/plugin-qt/power/session/translations/plugin-power-session_ja.ts
src/plugin-qt/power/session/translations/plugin-power-session_pl.ts
src/plugin-qt/power/session/translations/plugin-power-session_pt.ts
src/plugin-qt/power/session/translations/plugin-power-session_pt_BR.ts
src/plugin-qt/power/session/translations/plugin-power-session_sq.ts
src/plugin-qt/power/session/translations/plugin-power-session_zh_CN.ts
src/plugin-qt/power/session/translations/plugin-power-session_zh_HK.ts
src/plugin-qt/power/session/translations/plugin-power-session_zh_TW.ts

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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/plugin-qt/power/session/powermanager.cpp" line_range="641-644" />
<code_context>
+        doLock(true);
+    SetPrepareSuspend(PS_ButtonClick);
+
+    const bool needMask = m_screenBlackLock && !m_proxy->wmBlackScreenActive();
+    if (needMask) {
+        m_proxy->setWmBlackScreenActive(true);
+        QThread::msleep(kBlackScreenMaskDelayMs);
+    }
+
</code_context>
<issue_to_address>
**issue (performance):** The session power service synchronously blocks its main thread for 100 ms after requesting the KWin mask, delaying all D-Bus handling, timers, and power-state signals during every X11 screen-off request.

**Triggers:** On X11 when the KWin BlackScreen mask is not already active.

**Suggested fix:** Replace the blocking sleep with an asynchronous timer/state machine that triggers DPMS after the compositor mask has had time to render.
</issue_to_address>

### Comment 2
<location path="src/plugin-qt/power/session/powermanager.cpp" line_range="641-646" />
<code_context>
+        doLock(true);
+    SetPrepareSuspend(PS_ButtonClick);
+
+    const bool needMask = m_screenBlackLock && !m_proxy->wmBlackScreenActive();
+    if (needMask) {
+        m_proxy->setWmBlackScreenActive(true);
+        QThread::msleep(kBlackScreenMaskDelayMs);
+    }
+
+    setDPMSModeOff();
+
</code_context>
<issue_to_address>
**nitpick (bug_risk):** When KWin BlackScreen is unavailable or its setActive call fails, wmBlackScreenActive returns false, needMask remains true, and TurnOffScreen still sleeps for 100 ms before DPMS; the requested mask is never installed and the failure is reduced to a warning.

**Triggers:** On X11 sessions without the KWin BlackScreen D-Bus interface, or when KWin rejects setActive.

**Suggested fix:** Check the KWin interface before enabling the mask and skip the delay when the mask cannot be activated; only wait after a successful setActive call.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread src/plugin-qt/power/session/powermanager.cpp Outdated
Comment thread src/plugin-qt/power/session/powermanager.cpp Outdated
1. Move the X11 lock-before-DPMS screen-off sequence out of dde-shortcut-tool
   and into the power session service's TurnOffScreen.
2. Turn the screen back on when the power button is pressed while the screen
   is already off, and skip the request when no screen controller is available.
3. Delegate dde-shortcut-tool's turn-off-screen action to Power1.TurnOffScreen
   instead of duplicating the sequence.

Log: Screen-off logic now lives in the power session service and the shortcut tool only calls Power1.TurnOffScreen.
Influence: Power-key screen-off and wake behave consistently.

fix(power): 统一屏幕关闭处理到会话服务

1. 将 dde-shortcut-tool 中「先锁屏再 DPMS 关屏」的 X11 处理移入电源会话服务的 TurnOffScreen。
2. 屏幕已关闭时再次按电源键改为重新点亮;屏幕控制器不可用时忽略请求。
3. dde-shortcut-tool 的关屏动作改为调用 Power1.TurnOffScreen,不再重复实现。

Log: 关屏逻辑收敛到电源会话服务,快捷工具只调用 Power1.TurnOffScreen。
PMS: BUG-375787
Influence: 电源键关屏与唤醒行为保持一致。
Change-Id: Ib0f330915ba419603aabb9039bd1fd8e393470c4
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

AI 代码审查报告

总体评分: 98 分 (通过阈值: 70分)

Pass


总体评价

项目 结果
审查结论 代码审查通过
PR 标题 fix(power): unify screen-off handling in session
提交 SHA 71e35b1
PMS 关联 BUG-375787
评分详情 本次 PR 将屏幕关闭逻辑统一到电源会话服务(PowerManager::TurnOffScreen)中,消除了 dde-shortcut-tool 中的代码重复。新增屏幕切换(toggle)功能和防御性检查,移除了阻塞调用(QThread::msleep)和子进程创建(QProcess::execute),提升了性能和安全性。代码结构清晰,注释完整,无新增安全漏洞。

漏洞对比统计

统计项 数量
当前漏洞总数 0
新增漏洞 0
减少漏洞 1
持平漏洞 0

注:安全扫描工具检测到 QProcess::execute("xset", {"dpms", "force", "off"}) 标记为 CWE-78 RCE,经人工审计确认为误报。该代码位于 diff 的删除行中(本次 PR 移除了该代码),且参数为硬编码字符串非用户输入。本次 PR 实际消除了该潜在风险面,属于安全改进。


详细分析

1. 语法逻辑 (25/25分) ✓ 通过

评价词: 语法正确,逻辑清晰

分析详情:

  1. TurnOffScreen() 方法逻辑流程清晰:控制器有效性检查 → 屏幕状态切换(toggle) → Wayland/X11 分支处理 → 锁屏 → DPMS关屏 → 完成挂起准备
  2. 所有引用的成员变量和方法均已验证存在:
    • m_screenCtrl (powermanager.h:321)
    • m_screenBlackLock (powermanager.h:353)
    • doLock(bool) (powermanager.h:216)
    • setDPMSModeOff() (powermanager.h:219)
    • isAllOff() (screencontroller.h:42)
    • PS_ButtonClick / PS_Finish (powermanager.h:55/52)
  3. 锁屏先于 DPMS 关屏的顺序正确(符合 bug-209669 的修复要求)
  4. systemTurnOffScreen() 简化为 DBus 调用,使用 QDBusReply<void> 检查返回值,错误处理正确
  5. 无编译错误风险,无边界条件问题

2. 代码质量 (23/25分) ✓ 通过

评价词: 代码结构清晰,注释完整

分析详情:

  1. 新增 TurnOffScreen() 中包含清晰的注释说明电源键切换屏幕的行为逻辑
  2. powercontroller.h 类文档已更新,反映新的委托设计模式("Power transitions are delegated to the owning session services.")
  3. powercontroller.cpp 移除约 80 行复杂的重复代码(shouldLockOnScreenBlackdoPrepareSuspendundoPrepareSuspendisWmBlackScreenActivesetWmBlackScreenActive
  4. systemTurnOffScreen() 从约 45 行简化为约 10 行,大幅提升可维护性
  5. 关屏逻辑收敛到单一位置(PowerManager::TurnOffScreen),消除了 dde-shortcut-tool 与 power session 之间的代码重复

潜在问题:

  1. constant.h 中的 KEY_SCREEN_BLACK_LOCK 常量在移除 shouldLockOnScreenBlack() 后可能变为未使用定义。建议确认该常量是否在其他地方使用,如不再使用可考虑清理。

3. 代码性能 (20/20分) ✓ 通过

评价词: 性能良好,资源使用合理

分析详情:

  1. 移除了 QThread::msleep(kBlackScreenMaskDelayMs) (100ms) 阻塞调用,消除了主线程 100ms 的阻塞等待
  2. 移除了 QProcess::execute("xset", {"dpms", "force", "off"}) 子进程创建,替换为 DBus IPC 通信,效率更高
  3. DBus 通信比子进程创建开销更小,响应更快
  4. 屏幕切换(toggle)行为避免了不必要的重复关屏操作

4. 代码安全 (30/30分) ✓ 通过

存在 0 个安全漏洞

漏洞对比统计:新增漏洞 0 个,减少漏洞 1 个,持平 0 个

分析详情:

  1. 安全扫描工具标记的 QProcess::execute("xset", ...) 为误报:该代码位于 diff 删除行中,本次 PR 移除了该代码,属于安全改进
  2. 新代码使用 DBus IPC 替代子进程执行,消除了潜在的命令注入风险面
  3. TurnOffScreen() 增加了 m_screenCtrl 有效性检查(!m_screenCtrl || !m_screenCtrl->isValid()),属于防御性编程改进
  4. systemTurnOffScreen() 增加了 DBus reply 错误处理(reply.isValid() 检查)
  5. 无硬编码密钥、无敏感信息泄露、无用户输入注入风险

代码变更概要

文件 变更类型 说明
powermanager.cpp 修改 TurnOffScreen() 从简单委托改为完整实现:增加控制器检查、屏幕toggle、Wayland/X11分支
powercontroller.cpp 修改/删除 systemTurnOffScreen() 简化为 DBus 调用;移除5个辅助方法和3个常量
powercontroller.h 修改/删除 移除5个方法声明,更新类文档注释

审查结论

本次 PR 是一次高质量的重构,将屏幕关闭逻辑从 dde-shortcut-tool 收敛到 PowerManager 会话服务中:

  • 消除了代码重复,建立了单一职责来源
  • 新增了屏幕 toggle 功能和防御性检查
  • 移除了阻塞调用和子进程创建,提升了性能
  • 无新增安全漏洞,且移除了潜在的命令执行风险面
  • 代码注释完整,结构清晰,符合 commit message 中描述的目标

建议: 可考虑清理 constant.h 中可能不再使用的 KEY_SCREEN_BLACK_LOCK 常量。


本报告由 AI 代码审查工具自动生成
审查时间: 2026-09-08 22:30:00

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@mhduiy

mhduiy commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot
deepin-bot Bot merged commit d173960 into master Sep 9, 2026
7 of 8 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.

3 participants