fix: improve tray applet lifecycle management and model safety - #1687
Conversation
Reviewer's GuideRefactors tray applet handling to use a dedicated, safer retrieval method with QPointer and proper signal management, removes redundant timer polling, and hardens DockItemModel against invalid indices and null source models. Sequence diagram for tray applet lifecycle managementsequenceDiagram
participant DockDBusProxy
participant QTimer
participant DAppletBridge
participant TrayApplet
QTimer->>DockDBusProxy: timeout()
DockDBusProxy->>DockDBusProxy: trayApplet()
DockDBusProxy->>DAppletBridge: DAppletBridge("org.deepin.ds.dock.tray")
DAppletBridge-->>DockDBusProxy: applet()
alt [applet changed]
DockDBusProxy->>TrayApplet: QObject::disconnect(pluginsChanged)
DockDBusProxy->>TrayApplet: QObject::connect(pluginsChanged, Qt::UniqueConnection)
DockDBusProxy-->>DockDBusProxy: m_trayApplet = currentApplet
else [applet unchanged]
DockDBusProxy-->>DockDBusProxy: reuse m_trayApplet
end
DockDBusProxy-->>QTimer: stop()
DockDBusProxy-->>QTimer: deleteLater()
DockDBusProxy-->>DockDBusProxy: QTimer::singleShot(30000, logInitialPluginState())
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
| return m_trayApplet.data(); | ||
| } | ||
|
|
||
| if (m_trayApplet) { |
There was a problem hiding this comment.
当m_trayApplet被析构了,这个QPointer的值是个空的吧,不过为了以后可能多个applet,这个留着也行,
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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 |
1. Refactor tray applet retrieval into a dedicated `trayApplet()` method that accepts a `QPointer<QObject>` instead of raw pointers, safely managing the connection lifecycle and handling applet recreation 2. Remove the repeated timer-based polling logic that created a new bridge connection each time 3. Properly disconnect and reconnect the `pluginsChanged` signal when the tray applet instance changes, using `Qt::UniqueConnection` to avoid duplicate connections 4. Add null checks and index validation in `DockItemModel::data()` and `mapToSource()` to prevent potential crashes from invalid model indices or missing source models 5. Change the timer to be parented to `this` for proper cleanup Log: Fixed potential issues with tray applet lifecycle management and model access safety Influence: 1. Verify the dock tray area displays correctly with various applications running 2. Test applet updates when applications add/remove tray icons dynamically 3. Validate the dock list remains stable during frequent tray updates 4. Test with edge cases: empty model, invalid indices 5. Verify the initial plugin list logging still works after the timer refactor fix: 改进托盘小程序生命周期管理和模型安全性 1. 将托盘小程序获取重构为专用的 `trayApplet()` 方法,使用 `QPointer<QObject>` 替代原始指针,安全管理连接生命周期并处理小程序重建 2. 移除重复的基于定时器的轮询逻辑,该逻辑每次都会创建新的桥接连接 3. 当托盘小程序实例变化时正确断开和重新连接 `pluginsChanged` 信号,使用 `Qt::UniqueConnection` 避免重复连接 4. 在 `DockItemModel::data()` 和 `mapToSource()` 中添加空指针检查和索引 验证,防止无效模型索引或缺失源模型导致的潜在崩溃 5. 将定时器设置为 `this` 的子对象以确保正确的清理 Log: 修复托盘小程序生命周期管理和模型访问安全性的潜在问题 Influence: 1. 验证在各种应用运行时,dock 托盘区域显示是否正确 2. 测试应用动态添加/移除托盘图标时的小程序更新 3. 验证在频繁托盘更新期间 dock 列表是否保持稳定 4. 测试边界情况:空模型、无效索引 5. 验证定时器重构后初始插件列表日志记录功能是否正常 PMS: TASK-393637
22c9f85 to
0ea0748
Compare
deepin pr auto review★ 总体评分:95分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // dockdbusproxy.cpp —— 若后续引入 ITrayApplet 接口,可优化信号连接的编译期检查
// 假设定义了接口类 ITrayApplet : public QObject
QObject *DockDBusProxy::trayApplet()
{
DAppletBridge bridge("org.deepin.ds.dock.tray");
QObject *currentApplet = bridge.applet();
if (m_trayApplet.data() == currentApplet) {
return m_trayApplet.data();
}
if (m_trayApplet) {
QObject::disconnect(m_trayApplet.data(), SIGNAL(pluginsChanged()), this, SIGNAL(pluginsChanged()));
}
m_trayApplet = currentApplet;
if (m_trayApplet) {
// 理想情况:若能安全转换为目标接口类型,可使用函数指针替代字符串宏
// if (auto *tray = qobject_cast<ITrayApplet*>(m_trayApplet.data())) {
// connect(tray, &ITrayApplet::pluginsChanged, this, &DockDBusProxy::pluginsChanged, Qt::UniqueConnection);
// } else
{
QObject::connect(m_trayApplet.data(), SIGNAL(pluginsChanged()), this, SIGNAL(pluginsChanged()), Qt::UniqueConnection);
}
}
return m_trayApplet.data();
} |
trayApplet()method that accepts aQPointer<QObject>instead of raw pointers, safely managing the connection lifecycle and handling applet recreationpluginsChangedsignal when the tray applet instance changes, usingQt::UniqueConnectionto avoid duplicate connectionsDockItemModel::data()andmapToSource()to prevent potential crashes from invalid model indices or missing source modelsthisfor proper cleanupLog: Fixed potential issues with tray applet lifecycle management and model access safety
Influence:
fix: 改进托盘小程序生命周期管理和模型安全性
trayApplet()方法,使用QPointer<QObject>替代原始指针,安全管理连接生命周期并处理小程序重建pluginsChanged信号,使用Qt::UniqueConnection避免重复连接DockItemModel::data()和mapToSource()中添加空指针检查和索引 验证,防止无效模型索引或缺失源模型导致的潜在崩溃this的子对象以确保正确的清理Log: 修复托盘小程序生命周期管理和模型访问安全性的潜在问题
Influence:
PMS: TASK-393637
Summary by Sourcery
Improve tray applet lifecycle handling and dock model safety.
Bug Fixes:
Enhancements: