fix: fix control center initialization and navigation issues#3049
fix: fix control center initialization and navigation issues#3049deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts control center initialization to avoid invalid active object usage, adds defensive null checks in QML for navigation/back-button handling, and introduces a timer-based fallback to ensure the main UI shows even when plugins hang or fail to set an active object. Sequence diagram for control center startup and timer-based fallback UI displaysequenceDiagram
actor User
participant DccManager
participant PluginManager
participant QTimer
participant UI
User->>DccManager: startApplication()
activate DccManager
DccManager->>DccManager: construct DccManager
DccManager->>DccObject: new DccObject() as m_root
DccManager->>DccManager: m_activeObject = nullptr
DccManager->>PluginManager: connect addObject, loadAllFinished
DccManager->>QTimer: new QTimer(this) as m_showTimer
DccManager->>QTimer: start(5000)
deactivate DccManager
par Plugin_loading_completes
PluginManager-->>DccManager: loadAllFinished()
activate DccManager
DccManager->>DccManager: tryShow()
alt showUrl is empty and activeObject is null
DccManager->>DccManager: clearShowParam()
DccManager->>UI: showPage(m_root, empty)
else showUrl is empty and activeObject not null
DccManager->>DccManager: clearShowParam()
else showUrl is not empty
DccManager->>UI: showPage(activeObject, showUrl)
end
deactivate DccManager
and Timer_fallback_triggers
QTimer-->>DccManager: timeout()
activate DccManager
DccManager->>DccManager: tryShow()
alt showUrl is empty and activeObject is null
DccManager->>DccManager: clearShowParam()
DccManager->>UI: showPage(m_root, empty)
else other_conditions
DccManager->>DccManager: handleAccordingToState()
end
deactivate DccManager
end
Class diagram for updated DccManager initialization and fallback timerclassDiagram
class DccApp
class PluginManager
class DccObject
class QTimer
class DccManager {
- DccObject* m_root
- DccObject* m_activeObject
- DccObject* m_hideObjects
- DccObject* m_noAddObjects
- DccObject* m_noParentObjects
- PluginManager* m_plugins
- QTimer* m_showTimer
- QString m_showUrl
+ DccManager(QObject* parent)
+ ~DccManager()
+ void clearShowParam()
+ void tryShow()
+ void showPage(DccObject* object, QString params)
}
DccManager --|> DccApp
DccManager o--> DccObject : m_root
DccManager o--> DccObject : m_activeObject
DccManager o--> DccObject : m_hideObjects
DccManager o--> DccObject : m_noAddObjects
DccManager o--> DccObject : m_noParentObjects
DccManager o--> PluginManager : m_plugins
DccManager o--> QTimer : m_showTimer
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
m_showTimercurrently runs indefinitely and keeps callingtryShow()every 5 seconds; consider making it single-shot or explicitly stopping it once the UI has been shown or anm_activeObject/m_showUrlis set to avoid unnecessary repeated work. - In
tryShow(), when the fallback branch (m_showUrl.isEmpty() && !m_activeObject) callsshowPage(m_root, QString()), it might be safer to also stop or disable the timer there to prevent repeated fallback invocations after the root page has already been displayed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `m_showTimer` currently runs indefinitely and keeps calling `tryShow()` every 5 seconds; consider making it single-shot or explicitly stopping it once the UI has been shown or an `m_activeObject`/`m_showUrl` is set to avoid unnecessary repeated work.
- In `tryShow()`, when the fallback branch (`m_showUrl.isEmpty() && !m_activeObject`) calls `showPage(m_root, QString())`, it might be safer to also stop or disable the timer there to prevent repeated fallback invocations after the root page has already been displayed.
## Individual Comments
### Comment 1
<location path="src/dde-control-center/dccmanager.cpp" line_range="77-79" />
<code_context>
connect(m_plugins, &PluginManager::addObject, this, &DccManager::addObject, Qt::QueuedConnection);
connect(m_plugins, &PluginManager::loadAllFinished, this, &DccManager::tryShow, Qt::QueuedConnection);
- waitShowPage("system", QDBusMessage());
+ m_showTimer = new QTimer(this);
+ connect(m_showTimer, &QTimer::timeout, this, &DccManager::tryShow);
+ m_showTimer->start(5000); // 防止插件卡死不显示界面
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider stopping the timer once the initial page is successfully shown to avoid repeated wakeups.
With the new QTimer, `tryShow()` now runs every 5 seconds for as long as the window exists. Once the initial page is shown (e.g., `m_activeObject` is set and `m_showUrl` is stable), consider stopping `m_showTimer` in `tryShow()` to avoid unnecessary work and to prevent future calls from affecting state if `m_showUrl` is later cleared.
Suggested implementation:
```cpp
void DccManager::tryShow()
{
if(m_showUrl.isEmpty() && !m_activeObject){
clearShowParam();
showPage(m_root, QString());
if (m_showTimer) {
m_showTimer->stop();
}
return;
}
if (m_showUrl.isEmpty()) {
```
Depending on the rest of `tryShow()` (which is not fully visible here), you may also want to stop `m_showTimer` in other branches where the target page is successfully shown (e.g., after a successful `showPage(...)` based on a non-empty `m_showUrl`). The pattern would be the same: after confirming that the page has been shown as intended, call `if (m_showTimer) m_showTimer->stop();` to avoid further wakeups.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
TAG Bot New tag: 6.1.75 |
|
Code review points for PR #3049 (Translated to English due to tool limitations with Chinese input): 1. Default startup behavior change 2. m_showTimer lifecycle management 3. 5-second timeout is quite long 4. Code style issue in tryShow() if(m_showUrl.isEmpty() && !m_activeObject){Missing spaces after if (m_showUrl.isEmpty() && !m_activeObject) {5. Completeness of m_activeObject null pointer protection |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, robertkill 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. Initialize m_activeObject to nullptr instead of m_root to prevent invalid access 2. Add null pointer checks in QML components to avoid crashes when activeObject is null 3. Implement timer-based fallback mechanism to ensure UI displays even if plugins hang 4. Fix navigation logic to handle cases where no active object is set Log: Fixed control center startup issues and navigation stability Influence: 1. Test control center startup with various plugin loading scenarios 2. Verify navigation works correctly when switching between modules 3. Test back button functionality when no parent page exists 4. Verify UI stability when plugins fail to load 5. Test scrollbar functionality in second page view fix: 修复控制中心初始化和导航问题 1. 将 m_activeObject 初始化为 nullptr 而非 m_root,防止无效访问 2. 在 QML 组件中添加空指针检查,避免 activeObject 为空时崩溃 3. 实现基于定时器的回退机制,确保即使插件卡死也能显示界面 4. 修复导航逻辑,处理没有设置活动对象的情况 Log: 修复控制中心启动问题和导航稳定性 Influence: 1. 测试控制中心在各种插件加载场景下的启动情况 2. 验证在模块间切换时导航功能正常工作 3. 测试不存在父页面时的返回按钮功能 4. 验证插件加载失败时的 UI 稳定性 5. 测试第二页面视图中的滚动条功能 PMS: BUG-351937
|
/forcemerge |
|
This pr force merged! (status: blocked) |
deepin pr auto review这段代码主要对 DDE 控制中心的初始化流程、空指针安全性以及界面交互逻辑进行了修改。以下是详细的代码审查意见: 1. 代码逻辑与安全性分析修改点 1:
|
Log: Fixed control center startup issues and navigation stability
Influence:
fix: 修复控制中心初始化和导航问题
Log: 修复控制中心启动问题和导航稳定性
Influence:
PMS: BUG-351937
Summary by Sourcery
Improve control center initialization robustness and navigation behavior when no active object is available.
Bug Fixes:
Enhancements: