fix: possible proxy model desync bug on model reset - #1695
Open
BLumia wants to merge 1 commit into
Open
Conversation
Bug 1 (RoleCombineModel 缺少 modelReset 转发) — 可触发 AbstractWindowMonitor::clearTrackedWindows() 会调用 beginResetModel() / endResetModel()，在以下时机被调用： treelandwindowmonitor.cpp:105 和 x11windowmonitor.cpp:94 — 当显示服务器重连或 compositor 重启时，窗口监视器会清空所有跟踪窗口并复位模型 这个 modelReset 会穿过 BoolFilterModel 到达 RoleCombineModel 的 major model。修复前，复位后 m_indexMap 残留旧映射且下游收不到 reset 信号，导致新旧数据冲突。这是一个生产环境可触发的 bug。 Bug 2 (RoleGroupModel rowsRemoved 跳过后续分组) — 当前不可触发，属于防御性修复 RoleGroupModel 的源模型是 DockGlobalElementModel，其所有删除路径（destroyWindow 单窗口关闭、DockGlobalElementModel 内部循环）都是逐个调用 beginRemoveRows(pos, pos) / endRemoveRows()，每次只有 first == last。因此 range 分支在当前代码中永远不会被执行，--i 缺失不会造成问题。但 handler 的接口签名支持 first..last 范围，LargeDataMemoryStabilityTest 测试也直接传入了范围，若未来有代码路径直接传入多行范围，就会触发。这是一个防御性修复。 Log:
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
Reviewer's GuideAdds reset handling and defensive mapping fixes to proxy models used by the dock task manager, plus targeted test coverage to reproduce and guard against two bugs in RoleCombineModel and RoleGroupModel. Sequence diagram for modelReset handling in RoleCombineModelsequenceDiagram
actor AbstractWindowMonitor
participant SourceModel as QAbstractItemModel_major
participant RoleCombineModel
AbstractWindowMonitor->>SourceModel: clearTrackedWindows()
SourceModel->>SourceModel: beginResetModel()
SourceModel-->>RoleCombineModel: modelReset
RoleCombineModel->>RoleCombineModel: beginResetModel()
RoleCombineModel->>RoleCombineModel: m_indexMap.clear()
RoleCombineModel->>SourceModel: rowCount()
RoleCombineModel->>SourceModel: columnCount()
loop rebuild_indexMap
RoleCombineModel->>SourceModel: index(i, j)
RoleCombineModel->>RoleCombineModel: func(majorIndex.data(majorRoles), m_minor)
RoleCombineModel->>RoleCombineModel: m_indexMap[qMakePair(i, j)] = qMakePair(minorRow, minorColumn)
end
RoleCombineModel->>RoleCombineModel: endResetModel()
SourceModel->>SourceModel: endResetModel()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review★ 总体评分：95分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 针对测试用例中暴露的遗留问题：补充 minor 源模型 modelReset 信号的处理
// 文件：panels/dock/taskmanager/rolecombinemodel.cpp
// 在构造函数中，紧接现有的 major modelReset connect 之后添加：
// forward modelReset from minor source
if (m_minor) {
connect(m_minor, &QAbstractItemModel::modelReset, this, [this, majorRoles, func]() {
beginResetModel();
m_indexMap.clear();
int rowCount = sourceModel()->rowCount();
int columnCount = sourceModel()->columnCount();
for (int i = 0; i < rowCount; i++) {
for (int j = 0; j < columnCount; j++) {
QModelIndex majorIndex = sourceModel()->index(i, j);
QModelIndex minorIndex = func(majorIndex.data(majorRoles), m_minor);
if (majorIndex.isValid() && minorIndex.isValid())
m_indexMap[qMakePair(i, j)] = qMakePair(minorIndex.row(), minorIndex.column());
}
}
endResetModel();
});
} |
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.
Bug 1 (RoleCombineModel 缺少 modelReset 转发) — 可触发
AbstractWindowMonitor::clearTrackedWindows() 会调用 beginResetModel() / endResetModel()，在以下时机被调用：
treelandwindowmonitor.cpp:105 和 x11windowmonitor.cpp:94 — 当显示服务器重连或 compositor 重启时，窗口监视器会清空所有跟踪窗口并复位模型 这个 modelReset 会穿过 BoolFilterModel 到达 RoleCombineModel 的 major model。修复前，复位后 m_indexMap 残留旧映射且下游收不到 reset 信号，导致新旧数据冲突。这是一个生产环境可触发的 bug。
Bug 2 (RoleGroupModel rowsRemoved 跳过后续分组) — 当前不可触发，属于防御性修复
RoleGroupModel 的源模型是 DockGlobalElementModel，其所有删除路径（destroyWindow 单窗口关闭、DockGlobalElementModel 内部循环）都是逐个调用 beginRemoveRows(pos, pos) / endRemoveRows()，每次只有 first == last。因此 range 分支在当前代码中永远不会被执行，--i 缺失不会造成问题。但 handler 的接口签名支持 first..last 范围，LargeDataMemoryStabilityTest 测试也直接传入了范围，若未来有代码路径直接传入多行范围，就会触发。这是一个防御性修复。
Log:
Summary by Sourcery
Fix potential desync issues between RoleCombineModel/RoleGroupModel and their source models when models are reset or rows are removed in ranges.
Bug Fixes:
Enhancements:
Tests: