Skip to content

sync: from linuxdeepin/dde-session-shell - #555

Merged
yixinshark merged 1 commit into
masterfrom
sync-pr-102-nosync
Oct 16, 2026
Merged

yixinshark merged 1 commit into
masterfrom
sync-pr-102-nosync

Conversation

@deepin-ci-robot

@deepin-ci-robot deepin-ci-robot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#102

Summary by Sourcery

Harden lock-screen initialization and fullscreen wallpaper handling against unavailable services, asynchronous failures, and multi-screen lifecycle changes.

Bug Fixes:

  • Prevent lock-screen startup from blocking or probing unavailable system information services.
  • Improve wallpaper loading so failures, timeouts, invalid images, screen changes, and stale asynchronous replies fall back safely and retry on the next display cycle.

Enhancements:

  • Centralize fullscreen background state and caching across multiple screens while preserving requested workspace wallpapers and avoiding redundant requests.

Tests:

  • Add isolated D-Bus regression coverage for background fallback, asynchronous loading, cache invalidation, multi-screen behavior, and system information initialization.

Chores:

  • Synchronize session shell source changes from the upstream project.

@sourcery-ai

sourcery-ai Bot commented Oct 16, 2026

Copy link
Copy Markdown

Reviewer's Guide

The pull request synchronizes the quick-login wallpaper fixes from dde-session-shell: it changes client-side background preparation to show only a validated blurred image or a solid fallback, performs later updates asynchronously with shared multi-screen state and stale-request protection, avoids synchronous SystemInfo probing, adds extensive isolated D-Bus regression tests, and documents the implementation and deferred wallpaper-service work.

Sequence diagram for quick-login wallpaper preparation and fallback

sequenceDiagram
    participant LockFrame
    participant FullScreenBackground
    participant WallpaperCache
    participant Renderer

    LockFrame->>FullScreenBackground: setScreen()
    FullScreenBackground->>WallpaperCache: Get(effect, originPath)
    alt Valid blurred image returned before timeout
        WallpaperCache-->>FullScreenBackground: blurPath
        FullScreenBackground->>FullScreenBackground: updateScreenBluBackground(blurPath)
        FullScreenBackground->>Renderer: Render validated blurred image
    else Timeout, D-Bus error, or invalid image
        WallpaperCache-->>FullScreenBackground: Error or invalid path
        FullScreenBackground->>Renderer: Render SOLID_BACKGROUND_COLOR
    end
    LockFrame->>FullScreenBackground: setVisible()
    FullScreenBackground->>WallpaperCache: async Get(effect, originPath)
    WallpaperCache-->>FullScreenBackground: blurPath or error
    FullScreenBackground->>Renderer: Update all screens or retain solid fallback
Loading

State diagram for shared quick-login wallpaper requests

stateDiagram-v2
    [*] --> Uninitialized
    Uninitialized --> Loading: updateBlurBackground(path)
    Loading --> Ready: onGetBlurBackgroundReply(valid blurPath)
    Loading --> Failed: onGetBlurBackgroundReply(error or invalid image)
    Ready --> Loading: updateBlurBackground(new path)
    Ready --> Ready: updateScreenBluBackground(blurPath)
    Failed --> Failed: updateBackground(same path)
    Failed --> Uninitialized: visibleChanged(false)
    Loading --> Uninitialized: visibleChanged(false)
    Ready --> Uninitialized: last frame destroyed
    Loading --> Uninitialized: newer backgroundRequestId
    Ready --> Failed: local image preparation fails
Loading

File-Level Changes

Change Details Files
Reworked quick-login wallpaper loading to guarantee a blurred first frame or solid-color fallback, then update asynchronously after display.
  • Added shared background lifecycle states, request IDs, cache invalidation, and stale-reply protection across screens.
  • Extended the effect-request timeout to 5 seconds and stopped falling back to the clear default wallpaper on errors or invalid results.
  • Reused same-source, same-size pixmaps and added failure handling for invalid, missing, or unreadable scaled images.
  • Kept workspace wallpaper selection across retries and coordinated behavior for hidden, removed, resized, and newly added screens.
src/widgets/fullscreenbackground.cpp
src/widgets/fullscreenbackground.h
Removed synchronous SystemInfo interface probing during lock-content initialization.
  • Issued the CPUHardware property request directly through an asynchronous D-Bus message.
  • Preserved PANGU CPU detection and error handling while allowing initialization to continue when the service is unavailable.
src/session-widgets/lockcontent.cpp
Added isolated regression coverage for wallpaper fallback, asynchronous updates, request races, multi-screen caching, and SystemInfo startup behavior.
  • Added a private D-Bus test runner and service stubs with controllable delays, errors, invalid paths, and scaled-cache responses.
  • Validated solid fallback, retry-on-next-display, stale-response rejection, cache reuse/invalidation, screen lifecycle handling, and delayed CPU property replies.
tests/dde-lock/run-background-tests.sh
tests/dde-lock/ut_backgroundfallback.cpp
tests/dde-lock/ut_systeminfo.cpp
Documented quick-login wallpaper behavior, known service-side limitations, validation procedures, and the distinction between greeter and fast-login processes.
  • Added development and wallpaper-flow documentation links to both README files and corrected the Developer Center URL.
  • Documented current D-Bus semantics, client fixes, deferred service optimizations, regression scenarios, and deployment results.
  • Clarified that fast login may be hosted by dde-lock rather than the LightDM greeter.
README.md
README.zh_CN.md
docs/development-guide.md
docs/quick-login-wallpaper.md
docs/wallpaper-service-review.md

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 reviewed your changes and they look great!


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

@deepin-ci-robot
deepin-ci-robot force-pushed the sync-pr-102-nosync branch 2 times, most recently from 545e42e to edd4505 Compare October 16, 2026 07:18
Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#102
@deepin-ci-robot

Copy link
Copy Markdown
Contributor Author

deepin pr auto review

AI 代码审查报告

基本信息

项目 内容
PR 链接 #555
PR 标题 sync: from linuxdeepin/dde-session-shell
作者 deepin-ci-robot
分支 sync-pr-102-nosync -> master
修改文件数 6
分析模式 全量分析

修改文件列表

文件路径 修改类型
src/session-widgets/lockcontent.cpp 修改
src/widgets/fullscreenbackground.cpp 修改
src/widgets/fullscreenbackground.h 修改
tests/dde-lock/run-background-tests.sh 新增
tests/dde-lock/ut_backgroundfallback.cpp 新增
tests/dde-lock/ut_systeminfo.cpp 新增

变更概述

本 PR 从 dde-session-shell 主仓库同步代码变更,主要内容包括:

  1. lockcontent.cpp:将 QDBusInterface 构造替换为直接使用 QDBusMessage::createMethodCall + QDBusConnection::asyncCall,避免 QDBusInterface 构造时对不可用服务进行同步内省(introspection)导致启动阻塞。
  2. fullscreenbackground.cpp/h:对背景加载逻辑进行重大重构,引入状态机模式(Uninitialized/Loading/Ready/Failed),使用异步 DBus 调用替代嵌套 GUI 事件循环(QDBus::BlockWithGui),增加请求 ID 跟踪防止过期回复覆盖新状态,添加超时处理机制。
  3. 测试文件:新增完整的背景回退逻辑测试和系统信息 DBus 调用测试。

四维度分析

维度1:语法逻辑(23/25)✓

语法正确,逻辑清晰

分析:

  1. lockcontent.cpp(第66-72行):QDBusMessage 的构造参数正确,createMethodCall 的四个参数(service、path、interface、method)与原 QDBusInterface 的参数一致。message << DSS_DBUS::systemInfoService << QStringLiteral("CPUHardware") 正确填充了 Get 方法的两个参数。asyncCall 返回 QDBusPendingCall,与 QDBusPendingReply<QDBusVariant> 兼容。

  2. fullscreenbackground.cpp 状态机逻辑:

    • updateBackground() 正确处理了 Loading/Failed 状态的短路返回,Ready 状态的缓存复用
    • updateBlurBackground() 设置状态为 Loading,递增 requestId,调用 clearBackground 清理旧状态
    • onGetBlurBackgroundReply() 通过 requestId 验证防止过期回复,正确处理错误和无效路径
    • 析构函数在最后一个 frame 移除时正确重置所有静态状态
  3. handleBackground()(第825-826行):新增 if (pixmap.isNull()) return; 空 pixmap 检查,防止后续 scaled/copy 操作崩溃,属于良好的防御性编程。

  4. updateScreenBluBackground():新增 trueSize().isEmpty() 检查,防止几何尺寸为 0 时产生无效 pixmap 操作。注释说明"Geometry updates may temporarily resize a screen to 0x0"。

  5. 测试文件中访问私有成员:ut_backgroundfallback.cpp 中直接访问 background->backgroundState(私有静态成员)和 FullScreenBackground::BackgroundState::Failed(私有枚举)。这需要测试代码具有类的友元访问权限或通过宏定义开放访问。由于是从主仓库同步的代码,推测构建系统已处理此问题,但需确认编译环境配置。

潜在问题:

  1. 静态成员变量 backgroundState、backgroundRequestId、backgroundDisplayStarted 未使用 std::atomic 限定。虽然 Qt DBus 回调通过事件循环在主线程交付,实际不存在竞态条件,但缺少显式的线程安全声明。

维度2:代码质量(23/25)✓

代码结构清晰,注释完整

分析:

  1. 注释质量:

    • lockcontent.cpp 中有清晰注释说明替换原因:"QDBusInterface construction can synchronously introspect an unavailable service"
    • fullscreenbackground.cpp 中多处注释解释设计决策,如"The request serves all screens. Keep it alive if its initiating screen is unplugged"
    • 测试文件中有注释说明测试策略和安全检查
  2. 代码结构:

    • 状态机模式的引入显著提升了代码可维护性,将原来散落的背景加载状态管理统一为明确的状态转换
    • clearBackground() 方法提取减少了重复代码
    • onGetBlurBackgroundReply() 作为静态方法处理回复,职责清晰
    • resizeEvent 中的逻辑简化为调用 updateScreenBluBackground(),消除了重复的 pixmap 加载代码
  3. 调试信息清理:

    • qCWarning 调用用于记录错误信息,属于合规的错误日志,非残留调试代码
    • 测试脚本中的 echo 输出用于使用说明,合理
  4. 代码重复:

    • 原 resizeEvent 和 updateScreenBluBackground 中有重复的 pixmap 加载逻辑,重构后统一为 updateScreenBluBackground 调用
    • clearBackground() 统一了清理逻辑

潜在问题:

  1. updateScreenBluBackground() 方法职责略有膨胀,同时处理缓存检查、pixmap 加载、错误处理和可见性更新,建议进一步拆分。
  2. updateBlurBackground() 方法中的同步/异步分支逻辑较为复杂,可增加更详细的状态转换注释。

维度3:代码性能(19/20)✓

性能良好,资源使用合理

分析:

  1. 重大性能改进:

    • 替换 QDBus::BlockWithGui(运行嵌套 GUI 事件循环,可能导致重入和界面冻结)为异步调用,显著改善用户体验
    • 避免 QDBusInterface 构造时对不可用服务的同步内省,减少启动延迟
    • BACKGROUND_REQUEST_TIMEOUT_MS = 5000 超时机制防止无限阻塞
  2. 请求 ID 跟踪:backgroundRequestId 机制使过期异步回复被快速丢弃(if (requestId != backgroundRequestId) return;),避免不必要的 pixmap 操作。

  3. 缓存策略:

    • 模糊背景缓存(blurBackgroundCacheMap)在 Ready 状态下复用,避免重复 DBus 调用
    • 失败时正确清理缓存,防止使用过期数据
  4. 资源管理:

    • QDBusPendingCallWatcher 通过 deleteLater() 正确释放
    • 析构函数在最后一个 frame 移除时清理所有静态资源

潜在问题:

  1. 首次显示时(!backgroundDisplayStarted)仍使用同步 DBus 调用(QDBus::Block),虽然是有意为之("Prepare the first frame without running a nested GUI event loop"),但在 DBus 服务响应缓慢时仍可能短暂阻塞。这是为了确保首帧即有背景图的合理权衡。

维度4:代码安全(30/30)✓

存在0个安全漏洞

分析:

  1. DBus 调用安全:所有 DBus 服务名、路径、接口均来自 DSS_DBUS 常量命名空间,无用户可控输入注入风险。

  2. 文件路径安全:

    • isPicture() 方法验证文件存在性和可读性
    • originBackgroundPath 来自系统配置/用户设置,非外部输入
    • 无路径遍历风险
  3. 资源管理安全:

    • QDBusPendingCallWatcher 通过 deleteLater() 在事件循环中安全释放
    • 析构函数正确清理静态状态,防止资源泄漏
    • handleBackground() 新增空 pixmap 检查,防止空指针解引用
  4. 测试脚本安全(run-background-tests.sh):

    • 使用 exec "$1" 正确引用参数,无命令注入风险
    • set -eu 确保错误即退出
    • dbus-run-session 创建隔离的 DBus 会话,不影响系统总线
  5. 无安全漏洞:

    • 无硬编码密钥或凭证
    • 无 SQL 注入、命令注入、路径遍历风险
    • 无不安全密码算法
    • 无敏感信息泄露
    • 无缓冲区溢出风险

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


评分总结

维度 得分 满分 标记 评价词
语法逻辑 23 25 ✓ 语法正确,逻辑清晰
代码质量 23 25 ✓ 代码结构清晰,注释完整
代码性能 19 20 ✓ 性能良好,资源使用合理
代码安全 30 30 ✓ 存在0个安全漏洞
总分 95 100 - 优秀

改进建议

  1. 线程安全声明:静态成员变量 backgroundState、backgroundRequestId、backgroundDisplayStarted 虽然实际只在主线程访问,建议添加注释说明线程安全假设,或使用 std::atomic 提供显式保证。

  2. 方法拆分:updateScreenBluBackground() 方法职责较多,建议将缓存检查、pixmap 加载、错误处理进一步拆分为独立方法,提升可读性。

  3. 状态转换文档:建议在头文件中添加状态机转换图或文字说明,明确各状态之间的转换条件和触发事件,方便后续维护。

  4. 测试私有成员访问:测试代码直接访问私有成员 backgroundState,建议通过友元声明或测试辅助接口明确授权,避免依赖编译环境特殊配置。

审查结论

本 PR 是从 dde-session-shell 主仓库同步的代码变更,核心改进是将锁屏背景加载从同步嵌套事件循环模式重构为异步状态机模式,同时优化了系统信息 DBus 调用方式避免启动阻塞。代码质量高,注释完整,测试覆盖全面(609行背景回退测试 + 116行系统信息测试)。未发现安全漏洞,性能有显著改善。建议合入。

@deepin-ci-robot

Copy link
Copy Markdown
Contributor Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot, 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

@yixinshark
yixinshark merged commit d33c339 into master Oct 16, 2026
25 of 26 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.

2 participants