Conversation
When an Electron app launches, its splash window closes and the main window may appear with a renderer subprocess PID. The AM Identify call fails because pidfd_open gets the wrong PID, and /proc/PID/cmdline fallback also cannot match the .desktop file. createByWindow returns nullptr, and handleWindowAdded dereferences it without a null check, causing the AppItem to never be created — the taskbar icon disappears. Fix at root-cause level with three changes: 1. taskmanager.cpp: add null check for desktopfile in handleWindowAdded before calling getAppItem(), preventing undefined behavior. 2. appitem.cpp: replace immediate deleteLater() in checkAppItemNeedDeleteAndDelete() with a 500ms grace period via QTimer::singleShot, allowing a new window (e.g. Electron main window after splash closes) to re-attach before the AppItem is destroyed. 3. desktopfileamparser.cpp: refactor identifyWindow() to extract tryAmIdentify() and findParentPid() helpers, and walk up the process tree (up to 5 levels) when the initial PID identification fails, so renderer subprocess PIDs can find their parent process for AM identification. Log: fix Electron app icon disappearing from taskbar Bug: https://pms.uniontech.com/bug-view-367743.html
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuidePrevents Electron taskbar icons from disappearing by safely handling failed desktop-file identification, retaining AppItems briefly during splash-to-main-window transitions, and resolving renderer windows through parent-process Application Manager identification. Sequence diagram for Electron window identification and AppItem retentionsequenceDiagram
participant Window as ElectronWindow
participant TaskManager
participant Parser as DesktopFileAMParser
participant AM as ApplicationManager
participant Proc as Procfs
participant AppItem
TaskManager->>Parser: identifyWindow(Window)
Parser->>AM: tryAmIdentify(Window.pid())
AM-->>Parser: identification fails
loop Up to 5 parent processes
Parser->>Proc: findParentPid(currentPid)
Proc-->>Parser: parentPid
Parser->>AM: tryAmIdentify(parentPid)
AM-->>Parser: desktopId or failure
end
Parser-->>TaskManager: desktopfile or null
alt desktopfile is null
TaskManager-->>TaskManager: skip window safely
else desktopfile found
TaskManager->>AppItem: getAppItem()
AppItem->>AppItem: checkAppItemNeedDeleteAndDelete()
AppItem->>AppItem: QTimer.singleShot(500ms)
Window->>AppItem: re-attach within grace period
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy 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 |
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
📝 代码变更概述本次PR修复了Electron应用（如思源笔记）图标从任务栏消失的问题，包含三处互补的修改：
🔍 详细分析1. 语法逻辑 ✅评价： 语法正确，逻辑清晰 ✅ 通过 评分： 25/25 分析：
潜在问题： 2. 代码质量 ✅评价： 代码结构清晰，注释完整 ✅ 通过 评分： 23/25 分析：
潜在问题：
建议：
3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 评分： 19/20 分析：
潜在问题：
建议：
4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过 评分： 30/30
安全分析：
漏洞对比统计：新增漏洞 0 个，减少漏洞 0 个，持平 0 个 安全漏洞详情： 📈 评分汇总
💡 改进建议代码示例// 修复文件描述符泄漏 - desktopfileamparser.cpp tryAmIdentify() 函数
static QString tryAmIdentify(pid_t pid)
{
auto pidfd = pidfd_open(pid, 0);
if (pidfd < 0) {
return QString();
}
auto res = DDBusSender().service("org.desktopspec.ApplicationManager1")
.interface("org.desktopspec.ApplicationManager1")
.path("/org/desktopspec/ApplicationManager1")
.method("Identify")
.arg(QDBusUnixFileDescriptor(pidfd))
.call();
close(pidfd); // 释放原始文件描述符，防止泄漏
if (res.isErrored()) {
qCDebug(amdesktopfileLog()) << "AM failed to identify pid" << pid
<< ", reason is:" << res.error().message();
return QString();
}
auto reply = res.value();
QList<QVariant> data = reply.arguments();
return data.first().toString();
}✅ 审查检查清单
本报告由 AI 代码审查工具自动生成 |
|
@mhduiy: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
TAG Bot New tag: 2.0.55 |
|
TAG Bot New tag: 2.0.56 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix: prevent Electron app icon disappearing from taskbar
When an Electron app launches, its splash window closes and the main
window appears. The actual root cause is an AppItem lifecycle race
condition:
deleteLater()incheckAppItemNeedDeleteAndDelete()isasynchronous, so an AppItem scheduled for deletion remains in
m_appitemsuntil the event loop processes the DeferredDelete event.If the main window is created during this window,
getAppItem()returns the pending-deletion AppItem,
appendWindowattaches the mainwindow to it, and the subsequent DeferredDelete still destroys the
AppItem — causing the taskbar icon to disappear.
This race is intermittent: it only triggers when the splash window is
destroyed before the main window is created. When the main window
appears first,
hasWindow()stays true and deletion is neverscheduled.
Fix with three changes:
appitem.cpp: replace immediate
deleteLater()incheckAppItemNeedDeleteAndDelete()with a 500ms grace period viaQTimer::singleShot, re-checkinghasWindow()before deletion.This is the core fix — it allows the main window to re-attach
during the grace period, cancelling the deferred deletion.
taskmanager.cpp: add null check for desktopfile in handleWindowAdded
before calling getAppItem(), as defensive programming.
desktopfileamparser.cpp: refactor identifyWindow() to extract
tryAmIdentify() and findParentPid() helpers, and walk up the process
tree (up to 5 levels) when the initial PID identification fails,
so renderer subprocess PIDs can find their parent process for AM
identification.
Log: fix Electron app icon disappearing from taskbar
Bug: https://pms.uniontech.com/bug-view-367743.html