fix: launchpad might be blank when starting a new treeland dde session#768
Merged
Conversation
修复treeland dde会话下，启动器应用列表可能完全空白的问题。 Log:
Reviewer's GuideRefactors AppMgr initialization to always wire up DBus object manager, DConfig, and app fetching (with service-watcher retry), and removes validity gating in AppsModel so launchpad updates correctly even when the application manager service appears late in a new treeland DDE session. Sequence diagram for app list initialization with late ApplicationManager1 servicesequenceDiagram
actor User
participant AppsModel as AppsModel
participant AppMgr as AppMgr
participant DConfig as DConfig
participant ServiceWatcher as QDBusServiceWatcher
participant AMService as ApplicationManager1Service
User ->> AppsModel: AppsModel::AppsModel()
AppsModel ->> AppMgr: AppMgr::instance()
activate AppMgr
AppMgr ->> AppMgr: AppMgr::AppMgr()
AppMgr ->> DConfig: DConfig::create()
DConfig -->> AppMgr: isValid()
alt DConfig is valid
AppMgr ->> DConfig: DConfig::value(AppsLaunchedTimes)
AppMgr ->> AppMgr: updateAppsLaunchedTimes()
DConfig ->> AppMgr: valueChanged(AppsLaunchedTimes)
AppMgr ->> AppMgr: updateAppsLaunchedTimes()
end
AppMgr ->> AppMgr: isValid()
alt AppMgr::isValid() true at startup
AppMgr ->> AMService: fetchAppItems()
AMService -->> AppMgr: app items
AppMgr ->> AppsModel: changed
end
AppMgr ->> ServiceWatcher: QDBusServiceWatcher::QDBusServiceWatcher()
ServiceWatcher ->> AppMgr: serviceRegistered
AppMgr ->> AMService: fetchAppItems()
AMService -->> AppMgr: app items
AppMgr ->> AppsModel: changed
AppsModel ->> AppsModel: connect(AppMgr::changed, QTimer::start)
AppsModel ->> AppsModel: connect(AppMgr::itemDataChanged, updateAppItemFromAM)
AppMgr ->> AppsModel: itemDataChanged(id)
AppsModel ->> AppsModel: itemFromDesktopId(id)
AppsModel ->> AppsModel: updateAppItemFromAM()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review你好！我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次代码变更主要将原本的 整体来看，代码的重构方向是好的，增强了动态监听能力，解决了服务延迟启动的痛点。但代码在逻辑严谨性、内存/生命周期管理、代码性能和安全性方面存在一些需要改进的地方。以下是详细的审查意见： 1. 逻辑缺陷问题：
2. 内存与生命周期管理问题：DConfig 在无效时成为悬空/无用对象 DConfig *config = DConfig::create("org.deepin.dde.application-manager", "org.deepin.dde.am", "", this);
if (!config->isValid()) {
qCWarning(logDdeIntegration) << "DConfig is invalid when getting launched times.";
// 这里没有 return，也没有 delete config
} else {
// ...
}
3. 代码性能问题：构造函数中的阻塞/耗时操作
4. 代码安全与健壮性问题： m_serviceWatcher = new QDBusServiceWatcher(QStringLiteral("org.desktopspec.ApplicationManager1"),
QDBusConnection::sessionBus(),
QDBusServiceWatcher::WatchForRegistration,
this);
5. 代码质量问题：Lambda 表达式参数按值传递 [this](const QDBusObjectPath &objPath, ObjectInterfaceMap interfacesAndProperties)
改进后的代码建议结合上述意见，我为你重构了构造函数的相关代码： AppMgr::AppMgr(QObject *parent)
{
m_checkTimer->setInterval(3000); // 3 second interval
connect(m_checkTimer, &QTimer::timeout, this, &AppMgr::checkPendingAppItems);
// 1. 优化：使用 const & 避免不必要的拷贝
connect(m_objectManager, &AppManager1ApplicationObjectManager::InterfacesAdded, this,
[this](const QDBusObjectPath &objPath, const ObjectInterfaceMap &interfacesAndProperties) {
const QString key(objPath.path());
qCDebug(logDdeIntegration) << "InterfacesAdded by AM, path:" << key;
if (m_appItems.contains(key)) { // 优化：直接使用 key，避免重复调用 objPath.path()
qWarning() << "App already exists for the path:" << key;
return;
}
m_checkCount = 0;
if (auto appItem = parseDBus2AppItem(interfacesAndProperties)) {
qCDebug(logDdeIntegration) << "App item added, desktopId" << appItem->id;
watchingAppItemAdded(key, appItem);
}
});
connect(m_objectManager, &AppManager1ApplicationObjectManager::InterfacesRemoved, this,
[this](const QDBusObjectPath &objPath, const QStringList &interfaces) {
Q_UNUSED(interfaces)
const QString key(objPath.path());
qCDebug(logDdeIntegration) << "InterfacesRemoved by AM, path:" << key;
watchingAppItemRemoved(key);
});
// 2. 改进：处理 DConfig 无效时的内存释放
DConfig *config = DConfig::create("org.deepin.dde.application-manager", "org.deepin.dde.am", "", this);
if (!config || !config->isValid()) {
qCWarning(logDdeIntegration) << "DConfig is invalid when getting launched times.";
delete config; // 防止无效对象常驻内存
} else {
static const QString AppsLaunchedTimes(u8"appsLaunchedTimes");
const auto &value = config->value(AppsLaunchedTimes).toMap();
updateAppsLaunchedTimes(value);
QObject::connect(config, &DConfig::valueChanged, this, [this, config](const QString &key) {
if (key != AppsLaunchedTimes) {
qCDebug(logDdeIntegration) << "Ignoring non-appsLaunchedTimes key:" << key;
return;
}
qCInfo(logDdeIntegration) << "appsLaunchedTimes of DConfig changed, updating";
const auto &value = config->value(AppsLaunchedTimes).toMap();
updateAppsLaunchedTimes(value);
});
}
if (isValid()) {
fetchAppItems();
}
// 3. 改进：增加 DBus 连接有效性检查
QDBusConnection bus = QDBusConnection::sessionBus();
if (!bus.isConnected()) {
qCWarning(logDdeIntegration) << "Session bus is not connected, service watcher disabled.";
} else {
m_serviceWatcher = new QDBusServiceWatcher(QStringLiteral("org.desktopspec.ApplicationManager1"),
bus,
QDBusServiceWatcher::WatchForRegistration,
this);
connect(m_serviceWatcher, &QDBusServiceWatcher::serviceRegistered, this, [this]() {
qCInfo(logDdeIntegration) << "AppManager1 service registered on bus, fetching app items";
// 注意：如果 m_objectManager 在服务重启后需要重新生成，应在此处处理
fetchAppItems();
});
}
}关于 |
![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 left some high level feedback:
- In the AppMgr constructor you now connect to m_objectManager signals unconditionally, but previously this was guarded by isValid() in initObjectManager(); if m_objectManager can be null or otherwise invalid, these new connects risk a crash and should be protected by the same validity check.
- AppsModel now connects to AppMgr::changed and itemDataChanged without checking AppMgr::instance()->isValid(); if those signals are only meaningful when the manager is valid, you may want to keep a similar guard or at least document that the model can handle a temporarily invalid manager.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the AppMgr constructor you now connect to m_objectManager signals unconditionally, but previously this was guarded by isValid() in initObjectManager(); if m_objectManager can be null or otherwise invalid, these new connects risk a crash and should be protected by the same validity check.
- AppsModel now connects to AppMgr::changed and itemDataChanged without checking AppMgr::instance()->isValid(); if those signals are only meaningful when the manager is valid, you may want to keep a similar guard or at least document that the model can handle a temporarily invalid manager.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
18202781743
approved these changes
May 29, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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 |
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
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.
修复treeland dde会话下，启动器应用列表可能完全空白的问题。
Log:
Summary by Sourcery
Ensure application items and launch statistics are initialized and updated when the DDE application manager service becomes available, preventing empty launchpad app lists in new sessions.
Bug Fixes:
Enhancements: