fix: correct instance request handling in dock model #1375
Merged
Conversation
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
1. Removed unused sourceModel and sourceRow variables that were causing incorrect behavior 2. Simplified logic by removing separate handling for active vs docked applications 3. Consolidated all action handling into a single code path using dde-am 4. Added debug logging for troubleshooting instance requests The previous implementation had separate code paths for active applications (using ActiveAppModel) and docked applications (using DBus). This caused inconsistencies in how applications were launched. Now all instance requests are handled uniformly through the dde-am command, ensuring consistent behavior regardless of whether the app is currently active or docked. Log: Fixed inconsistent application launching behavior in dock Influence: 1. Test clicking on dock icons to launch applications 2. Test right-click menu actions on dock icons 3. Verify new instance creation for multi-instance applications 4. Test dock/undock actions work correctly 5. Verify close window/close all actions function properly 6. Check debug logs for instance request tracking fix: 修复任务栏模型中实例请求处理问题 1. 移除了导致错误行为的未使用 sourceModel 和 sourceRow 变量 2. 通过移除活动应用和停靠应用的单独处理逻辑来简化代码 3. 将所有操作处理统一到使用 dde-am 的单一代码路径中 4. 添加了调试日志以便排查实例请求问题 之前的实现为活动应用（使用 ActiveAppModel）和停靠应用（使用 DBus）设置 了不同的代码路径，这导致应用程序启动行为不一致。现在所有实例请求都通过 dde-am 命令统一处理，确保无论应用当前是否活动，都能获得一致的行为。 Log: 修复了任务栏中应用程序启动行为不一致的问题 Influence: 1. 测试点击任务栏图标启动应用程序 2. 测试任务栏图标的右键菜单操作 3. 验证多实例应用程序的新实例创建 4. 测试停靠/取消停靠操作是否正常工作 5. 验证关闭窗口/关闭所有操作功能正常 6. 检查调试日志中的实例请求跟踪 PMS: BUG-343311
18202781743
requested review from
BLumia,
fly602,
mhduiy,
robertkill,
wjyrich and
yixinshark
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUnifies dock task manager instance request handling to always invoke dde-am, removing separate ActiveAppModel/DBus paths, cleaning up unused variables, and adding debug logging for better traceability of dock actions. Sequence diagram for unified dock instance request handlingsequenceDiagram
actor User
participant DockUI
participant DockGlobalElementModel
participant QProcess_dde_am
User->>DockUI: Click dock icon / select context menu action
DockUI->>DockGlobalElementModel: requestNewInstance(index, action)
alt action == DOCK_ACTION_DOCK
DockGlobalElementModel->>DockGlobalElementModel: requestDock(index, true)
DockGlobalElementModel-->>DockUI: return
else action == DOCK_ACTION_UNDOCK
DockGlobalElementModel->>DockGlobalElementModel: requestDock(index, false)
DockGlobalElementModel-->>DockUI: return
else action == DOCK_ACTION_CLOSEWINDOW or DOCK_ACTION_CLOSEALL
DockGlobalElementModel->>DockGlobalElementModel: requestClose(index, false)
DockGlobalElementModel-->>DockUI: return
else other action or empty action
DockGlobalElementModel->>DockGlobalElementModel: qDebug(Requesting new instance, id)
DockGlobalElementModel->>QProcess_dde_am: start("dde-am", [--by-user, id, action])
QProcess_dde_am-->>DockGlobalElementModel: waitForFinished()
DockGlobalElementModel-->>DockUI: return
end
Updated class diagram for DockGlobalElementModel instance handlingclassDiagram
class DockGlobalElementModel {
-m_data
+requestNewInstance(index, action)
+requestDock(index, dock)
+requestClose(index, closeAll)
}
class QProcess {
+setProcessChannelMode(mode)
+start(program, arguments)
+waitForFinished()
}
class DdeAmCommand {
+program : string
+arguments : list
+execute(by_user, id, action)
}
DockGlobalElementModel --> QProcess : uses
DockGlobalElementModel --> DdeAmCommand : invokes dde-am
class ActiveAppModel
class ApplicationManager1Application
DockGlobalElementModel ..> ActiveAppModel : removed_dependency
DockGlobalElementModel ..> ApplicationManager1Application : removed_dbus_launch_path
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来对这段代码变更进行审查：
新代码将后两种情况合并，统一使用dde-am命令处理。
a) 代码逻辑问题：
b) 错误处理：
c) 性能考虑：
d) 安全性：
void DockGlobalElementModel::requestNewInstance(const QModelIndex &index, const QString &action)
{
if (!index.isValid()) {
qWarning(dockGlobalElementModelLog) << "Invalid index provided";
return;
}
auto data = m_data.value(index.row());
if (data.empty()) {
qWarning(dockGlobalElementModelLog) << "No data found for index:" << index;
return;
}
auto id = std::get<0>(data);
if (id.isEmpty()) {
qWarning(dockGlobalElementModelLog) << "Empty application ID";
return;
}
qDebug(dockGlobalElementModelLog) << "Requesting new instance for index:" << index
<< "with action:" << action
<< "id:" << id;
// Handle special actions first
if (action == DOCK_ACTION_DOCK) {
requestDock(index);
return;
} else if (action == DOCK_ACTION_UNDOCK) {
requestUndock(index);
return;
} else if (action == DOCK_ACTION_CLOSEWINDOW || action == DOCK_ACTION_CLOSEALL) {
requestClose(index, false);
return;
}
// 使用QProcess::startDetached避免阻塞主线程
bool success = QProcess::startDetached("dde-am", {"--by-user", id, action});
if (!success) {
qWarning(dockGlobalElementModelLog) << "Failed to execute dde-am command";
// 可以考虑回退到原来的实现
handleFallbackLaunch(id);
}
}
void DockGlobalElementModel::handleFallbackLaunch(const QString &id)
{
QString dbusPath = QStringLiteral("/org/desktopspec/ApplicationManager1/") + escapeToObjectPath(id);
using Application = org::desktopspec::ApplicationManager1::Application;
Application appInterface(QStringLiteral("org.desktopspec.ApplicationManager1"),
dbusPath,
QDBusConnection::sessionBus());
if (appInterface.isValid()) {
appInterface.Launch(QString(), QStringList(), QVariantMap());
} else {
qWarning(dockGlobalElementModelLog) << "Failed to launch application via DBus:" << id;
}
}
这些改进可以提高代码的健壮性、安全性和可维护性，同时保持原有功能的完整性。 |
![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, and left some high level feedback:
- The new
qDebuginrequestNewInstancelogs the fullQModelIndexand fires on every instance request; consider reducing the verbosity or logging only the essential fields (row, id, action) to avoid noisy or expensive debug output in common code paths. - By routing all (including empty) actions through
dde-amwith a synchronouswaitForFinished, the UI thread may now block for simple launch/activate operations that previously used DBus/ActiveAppModel; consider using an asynchronous start (e.g.,startDetachedor avoidingwaitForFinished) to keep the dock responsive.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `qDebug` in `requestNewInstance` logs the full `QModelIndex` and fires on every instance request; consider reducing the verbosity or logging only the essential fields (row, id, action) to avoid noisy or expensive debug output in common code paths.
- By routing all (including empty) actions through `dde-am` with a synchronous `waitForFinished`, the UI thread may now block for simple launch/activate operations that previously used DBus/ActiveAppModel; consider using an asynchronous start (e.g., `startDetached` or avoiding `waitForFinished`) to keep the dock responsive.
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/dockglobalelementmodel.cpp:423` </location>
<code_context>
QProcess process;
process.setProcessChannelMode(QProcess::MergedChannels);
process.start("dde-am", {"--by-user", id, action});
process.waitForFinished();
- return;
- }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Synchronous `waitForFinished()` now potentially blocks the UI for common actions.
Previously, `waitForFinished()` only ran for non-empty actions; now it also runs for the default (empty) action, which is likely the most common path. This can block the UI thread until `dde-am` exits. Consider making this call asynchronous (e.g., use signals instead of `waitForFinished()`) or restrict the synchronous wait to non-interactive or rare actions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| QProcess process; | ||
| process.setProcessChannelMode(QProcess::MergedChannels); | ||
| process.start("dde-am", {"--by-user", id, action}); | ||
| process.waitForFinished(); |
There was a problem hiding this comment.
suggestion (bug_risk): Synchronous waitForFinished() now potentially blocks the UI for common actions.
Previously, waitForFinished() only ran for non-empty actions; now it also runs for the default (empty) action, which is likely the most common path. This can block the UI thread until dde-am exits. Consider making this call asynchronous (e.g., use signals instead of waitForFinished()) or restrict the synchronous wait to non-interactive or rare actions.
mhduiy
approved these changes
Dec 22, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
incorrect behavior
applications
The previous implementation had separate code paths for active
applications (using ActiveAppModel) and docked applications (using
DBus). This caused inconsistencies in how applications were launched.
Now all instance requests are handled uniformly through the dde-am
command, ensuring consistent behavior regardless of whether the app is
currently active or docked.
Log: Fixed inconsistent application launching behavior in dock
Influence:
fix: 修复任务栏模型中实例请求处理问题
之前的实现为活动应用（使用 ActiveAppModel）和停靠应用（使用 DBus）设置
了不同的代码路径，这导致应用程序启动行为不一致。现在所有实例请求都通过
dde-am 命令统一处理，确保无论应用当前是否活动，都能获得一致的行为。
Log: 修复了任务栏中应用程序启动行为不一致的问题
Influence:
PMS: BUG-343311
Summary by Sourcery
Unify dock task manager instance request handling through dde-am for consistent app launch behavior and action processing.
Bug Fixes:
Enhancements:
Tests: