Repository navigation
fix: guard launcher applet against unavailable dock panel and appeara… - #820
Conversation
Reviewer's GuideThe PR hardens launcher startup by handling unavailable appearance and dock services safely, retries wallpaper loading after appearance registration, and removes a blocking auto-start state read from context-menu construction by using the existing application model role. Sequence diagram for deferred wallpaper loading after appearance registrationsequenceDiagram
participant Appearance as Appearance service
participant Watcher as QDBusServiceWatcher
participant Launcher as Appearance
Launcher->>Appearance: wallpaperURls()
alt Empty reply
Launcher-->>Launcher: qCDebug and return
Appearance-->>Watcher: serviceRegistered
Watcher->>Launcher: updateAllWallpaper()
Launcher->>Appearance: wallpaperURls()
else Non-empty reply
Launcher->>Launcher: QJsonDocument::fromJson()
end
Sequence diagram for model-backed auto-start context menusequenceDiagram
participant User
participant Main as Main.qml
participant Model as AppsModel
participant Menu as AppItemMenu
participant Desktop as DesktopIntegration
User->>Main: Open context menu
Main->>Model: autoStart
Main->>Menu: Create with autoStart
Menu-->>User: Show Add/Remove from startup
User->>Menu: Trigger menu item
Menu->>Desktop: setAutoStart(desktopId, !autoStart)
Flow diagram for guarded launcher dock bindingsflowchart TD
Start[Launcher applet initializes] --> PanelReady{Panel.rootObject exists?}
PanelReady -->|No| Fallback[Use zero dimensions and scale]
PanelReady -->|Yes| Bind[Read dock sizes and geometry]
PanelReady -->|No| NoConnection[Do not instantiate left-edge Connections]
PanelReady -->|Yes| Connection[Instantiate left-edge Connections]
PanelReady -. rootObjectChanged .-> Reevaluate[Re-evaluate guarded bindings]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="shell-launcher-applet/package/launcheritem.qml" line_range="46-54" />
<code_context>
- function onLeftEdgeClicked(minOrder) {
- if (launcher.dockOrder == minOrder) {
- toggleLauncher()
+ Loader {
+ active: Panel.rootObject !== null
+ sourceComponent: Connections {
+ target: Panel
+ function onLeftEdgeClicked(minOrder) {
+ if (launcher.dockOrder == minOrder) {
+ toggleLauncher()
+ }
}
}
</code_context>
<issue_to_address>
**Left-edge toggle never connects**
When the launcher applet is loaded with `Panel.rootObject` available, `Loader` is given a `Connections` object as its `sourceComponent`; `Connections` is non-visual, so the loader cannot instantiate it and the `onLeftEdgeClicked` handler is not installed. The launcher therefore does not toggle from the dock’s left-edge click.
Gate a directly declared `Connections` object with a supported `enabled`/target condition, or create the handler through a mechanism that supports non-visual objects.
</issue_to_address>
|
/retest |
|
TAG Bot New tag: 2.0.49 |
78d7d8f to
d1e99b3
Compare
| // 1:4 the distance between app : dock height; get width/height≈0.8 | ||
| implicitWidth: useColumnLayout ? Panel.rootObject.dockSize : Panel.rootObject.dockItemMaxSize * 0.8 | ||
| implicitHeight: useColumnLayout ? Panel.rootObject.dockItemMaxSize * 0.8 : Panel.rootObject.dockSize | ||
| implicitWidth: useColumnLayout ? (Panel.rootObject ? Panel.rootObject.dockSize : 0) : (Panel.rootObject ? Panel.rootObject.dockItemMaxSize * 0.8 : 0) |
There was a problem hiding this comment.
Panel.rootObject不会为空吧？
There was a problem hiding this comment.
会为空。rootObject 是 QObject* + rootObjectChanged 的 NOTIFY 属性（dde-shell frame/applet.h:27），框架允许为空（dde-shell 自己的测试覆盖了 setRootObject(nullptr)/初始为空），且 applet 的 rootObject 是在组件 createFinished 回调里才 setRootObject() 的（shell/appletloader.cpp），存在求值窗口；dde-shell 框架自身也到处判空：dockhelper.cpp:300、waylanddockhelper.cpp:68/141/150、dockpanel.cpp:188。
实测：拿 2.0.48 原文件跑 dde-shell -p org.deepin.ds.dde-apps -p org.deepin.ds.dock.launcherapplet，本文件 :25/26（以及同批 :294/295/454）稳定打出 TypeError: Cannot read property 'dockItemMaxSize' of null（journal 里也有真实机器样本）；加判空后同一步骤 5 条全消，正向对照（未修的 FullscreenFrame.qml:116 同类裸读）照常触发，说明复现环境真实有效。判空成本为零：未就绪回退 0 与 TypeError 时 int 的默认值一致，面板就绪后由 rootObjectChanged 重算。所以这处保留。
There was a problem hiding this comment.
dde-shell -p org.deepin.ds.dde-apps -p org.deepin.ds.dock.launcherapplet 这个测试本身不合理，不应该连dock都不加载，就加载它的子applet，
There was a problem hiding this comment.
c++里判断是正常的，因为本身可能没有root.Object，它的子applet，尤其是qml里应该不需要判断才对，它qml如果没有这个对应的Panel，都不会显示，
There was a problem hiding this comment.
同意，之前用 dde-shell -p org.deepin.ds.dde-apps -p org.deepin.ds.dock.launcherapplet 单独拉子 applet 确实不是正常加载路径。
我又核对了正常会话 dde-shell@DDE.service 的 journal：dockItemMaxSize/dockSize/rootObject of null 和 onLeftEdgeClicked 告警只在手工 -p 启动时出现，正常会话 0 条。因此 launcheritem.qml 的判空和 Loader 已全部撤回。按后续决议，appearance 相关改动也已从 PR 撤回，当前 PR 只保留 autostart 读取修复（b3f7a9c）。感谢确认。
|
|
||
| // The appearance service may not be ready when the launcher starts | ||
| // (startup race); retry the wallpaper update once it registers. | ||
| auto serviceWatcher = new QDBusServiceWatcher(QStringLiteral("org.deepin.dde.Appearance1"), |
There was a problem hiding this comment.
m_dbusAppearanceIface 应该有对应的信号去处理吧，
要保证这个么，appearance不是拉起的么，它不应该获取就是正常的么，
另外，只需要处理壁纸么，其它的值都不需要么，
There was a problem hiding this comment.
-
信号：该用的都在用——
Changed构造时就订阅（allwallpaperuris更新），opacity 的OpacityChanged订阅已在最新提交补上（你评论时看的是d1e99b3，当时还没有）。服务不存在时发不出任何信号，所以"首次取值 + 注册后补一次"只能由注册事件触发（QDBusServiceWatcher 就是 NameOwnerChanged 的封装，不是轮询）。 -
不打算保证它有值，也不接管服务生命周期：Appearance1 确实是 D-Bus 激活的，调用会发出消息、可能触发激活；但激活是异步的且可能失败，服务未就绪时首次读取会失败/超时并回退到代理的缓存默认值——
wallpaperURls()拿到的就是空串（Get wallpapers failed: illegal value的来源：空串被直接丢进QJsonDocument::fromJson），且这次失败没有后续信号来纠正，blurhash 壁纸就一直不刷新。修复把空返回当可预期的未就绪分支（debug），注册后重试一次；非空但非法仍保留告警。 -
其它值：这条提得对——opacity 已并入同一注册重试（
initAppearanceValues()）；scaleFactor()按需读取、每次失败回退 1.0，唯一调用点是右键菜单构建（交互时服务早已就绪）；blurhash /GetCurrentWorkspaceBackgroundForMonitor之前就有isValid()静默守卫；其余属性没有读。
结论：不需要"保证"它，也不需要再额外处理其它值；保持"容忍未就绪窗口 + 注册后重试"即可。
bda8ef8 to
43051ae
Compare
|
/retest |
|
TAG Bot New tag: 2.0.50 |
43051ae to
88b5791
Compare
88b5791 to
4e06bb3
Compare
4e06bb3 to
1b088af
Compare
1. The context menu's "Add/Remove from startup" state was read through DesktopIntegration.isAutoStart() -> AppMgr::readProperty(), a synchronous D-Bus call on the GUI thread (25s default timeout). While ApplicationManager was restarting, every menu build could emit "Failed to read property AutoStart" and stall the GUI. 2. Add AppItemMenu.autoStart and pass model.autoStart from both menu creation sites: qml/Main.qml and the applet's launcheritem.qml showContextMenu, which applet views invoke via QML creation-context resolution. This is the existing autoStart model role refreshed through PropertiesChanged. 3. Remove the now-unused DesktopIntegration.isAutoStart() and AppMgr::autoStart(); the write path still uses setAutoStart() unchanged. Log: Avoid blocking D-Bus reads when building the launcher context menu. Influence: 1. Open an app context menu; verify "Add/Remove from startup" matches the emblem_autostart badge and toggles normally. 2. Restart ApplicationManager and open the context menu; verify no "Failed to read property AutoStart" warning and no GUI stall. 3. Reopen the context menu after toggling to verify the state refresh. fix: 右键菜单改从模型读取开机自启状态 1. 右键菜单的“开机自启”状态原先经 DesktopIntegration.isAutoStart() -> AppMgr::readProperty() 在 GUI 线程同步读 D-Bus（默认 25 秒超时）， ApplicationManager 重启窗口内会打出 "Failed to read property AutoStart" 并可能阻塞 GUI。 2. AppItemMenu 新增 autoStart 属性，qml/Main.qml 与 applet 的 launcheritem.qml showContextMenu 两处创建点传入 model.autoStart；即 emblem_autostart 已在使用的 autoStart 模型角色，随 PropertiesChanged 刷新。 3. 删除不再使用的 DesktopIntegration.isAutoStart() 与 AppMgr::autoStart()； 写入路径 setAutoStart() 保持不变。 Log: 避免构建启动器右键菜单时同步读取 D-Bus。 Influence: 1. 打开应用右键菜单，确认“开机自启”状态与 emblem_autostart 角标一致， 切换正常。 2. 重启 ApplicationManager 后打开右键菜单，确认无 "Failed to read property AutoStart" 告警且 GUI 不卡顿。 3. 切换后重新打开右键菜单，确认状态刷新。 PMS: TASK-394335
1b088af to
b3f7a9c
Compare
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
📋 PR 信息
🔍 详细分析1. 语法逻辑 ✅评价： 语法正确，逻辑清晰 ✅ 通过 评分： 25/25 分析：
潜在问题： 2. 代码质量 ✅评价： 代码结构清晰，注释完整 ✅ 通过 评分： 24/25 分析：
潜在问题：
3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 评分： 20/20 分析：
潜在问题： 4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过 评分： 30/30 分析：
安全漏洞详情： 📁 变更文件清单
🔎 OCR 审查结果
📝 审查总结本次 PR 将右键菜单中"开机自启"状态读取从 GUI 线程同步 D-Bus 调用改为使用应用模型中已缓存的 本报告由 AI 代码审查工具自动生成 | 2026-10-19 18:45 |
| // 1:4 the distance between app : dock height; get width/height≈0.8 | ||
| implicitWidth: useColumnLayout ? Panel.rootObject.dockSize : Panel.rootObject.dockItemMaxSize * 0.8 | ||
| implicitHeight: useColumnLayout ? Panel.rootObject.dockItemMaxSize * 0.8 : Panel.rootObject.dockSize | ||
| implicitWidth: useColumnLayout ? (Panel.rootObject ? Panel.rootObject.dockSize : 0) : (Panel.rootObject ? Panel.rootObject.dockItemMaxSize * 0.8 : 0) |
There was a problem hiding this comment.
c++里判断是正常的，因为本身可能没有root.Object，它的子applet，尤其是qml里应该不需要判断才对，它qml如果没有这个对应的Panel，都不会显示，
|
|
||
| // The appearance service may not be ready when the launcher starts | ||
| // (startup race); retry the wallpaper update once it registers. | ||
| auto serviceWatcher = new QDBusServiceWatcher(QStringLiteral("org.deepin.dde.Appearance1"), |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia, Ivy233 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
…nce service, and read auto-start state from the model
Guard empty wallpaperURls and retry when appearance service registers: updateAllWallpaper() fed the wallpaperURls() result straight into QJsonDocument::fromJson(), so when the appearance service is not ready yet (startup race) the empty string failed JSON parsing and printed a "Get wallpapers failed: illegal value" warning on every launcher start, and the blurhash wallpaper was never refreshed afterwards. Treat an empty reply as the expected not-ready case (debug output) and retry via a QDBusServiceWatcher once org.deepin.dde.Appearance1 registers; the allwallpaperuris change notification already covers a service that is up but not populated. Non-empty but malformed replies keep the warning.
Guard launcher item bindings against unavailable dock panel: the launcher applet reads Panel.rootObject (dockSize/dockItemMaxSize) in its implicitWidth/implicitHeight bindings while the dock panel may not be fully initialized yet, producing TypeError warnings, and the Connections handling onLeftEdgeClicked resolves the signal against a panel that is not always ready. Guard all Panel.rootObject dereferences in launcheritem.qml with a null check (the size bindings, the windowed launcher's dockThickness/dockReservedZone bindings and the DciIcon scale binding fall back to 0 until the panel is ready), and only instantiate the left-edge Connections once Panel.rootObject exists. The rootObjectChanged notify keeps every guarded binding re-evaluating once the panel appears.
Read the context menu's "Add/Remove from startup" state from the model instead of a blocking D-Bus call: the menu resolved the entry by calling DesktopIntegration.isAutoStart(), which reaches AppMgr::readProperty() and performs a synchronous QDBusConnection::sessionBus().call() on the GUI thread (default 25s reply timeout). While ApplicationManager is restarting, org.desktopspec.ApplicationManager1 has no owner, so every menu build emitted a "Failed to read property AutoStart" QDBusError warning and the GUI thread could stall on the blocking call. AppItemMenu gains an autoStart property, and its live creation site (qml/Main.qml) passes model.autoStart into it; the launcheritem.qml showContextMenu has no caller yet, so its pass-through was dropped to avoid dead code. That is the existing "autoStart" model role (AppsModel::IsAutoStartRole) already used by the emblem_autostart icon badge, filled from the dde-shell shared app model and refreshed through PropertiesChanged, so the menu no longer needs a live D-Bus read. The checked state and the toggle behaviour of the menu item are unchanged: the trigger only writes through setAutoStart() with the negated menu state, which is the value the label was computed from. DesktopIntegration.isAutoStart() is intentionally kept: it stays part of the QML API surface even though no QML consumer uses it now.
启动器告警与阻塞式 D-Bus 读取修复：
修复 appearance 服务未就绪时获取壁纸的告警与功能缺失：空返回值 按预期分支处理（qCDebug），并通过 QDBusServiceWatcher 在服务 注册后重试；非空但非法的 JSON 保留告警。
启动器图标绑定对 dock 面板未就绪场景加守卫：对 launcheritem.qml 全部 Panel.rootObject 直读加判空（就绪前回退 0），Connections 仅在 Panel.rootObject 存在时实例化（原 enabled 写法对连接建立无效）；rootObjectChanged 通知保证 面板就绪后各绑定自动重算。
右键菜单"开机自启"改从模型读取：此前经 DesktopIntegration.isAutoStart() 走到 AppMgr::readProperty()， 在 GUI 线程同步执行 sessionBus 调用（默认 25 秒超时）， ApplicationManager 重启窗口内每次构建菜单都会打 "Failed to read property AutoStart" 告警且可能阻塞 GUI。现为 AppItemMenu 增加 autoStart 属性，由使用中的创建点 qml/Main.qml 传入 model.autoStart（launcheritem.qml 的 showContextMenu 当前无调用 方，未在该路径传参，避免死代码）—— 即图标角标 emblem_autostart 已在使用的 "autoStart" 模型角色（AppsModel::IsAutoStartRole），由 dde-shell 共享应用模型填充并随 PropertiesChanged 自动刷新，菜单不再需要 实时读 D-Bus；勾选态与点击切换行为不变。DesktopIntegration .isAutoStart() 按计划保留接口，仅 QML 侧不再调用。
PMS: TASK-394335
Summary by Sourcery
Make launcher startup resilient to unavailable services and dock panels while removing synchronous D-Bus reads from the startup menu.
Bug Fixes:
Enhancements: