Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23 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 GuideIntroduces a serial QQmlComponent/QQmlIncubator pipeline for non-blocking plugin main-QML loading, with explicit ownership and cancellation handling, while sharing object-parenting logic with synchronous loading and preserving plugin-manager completion semantics on failures. Sequence diagram for serial asynchronous plugin QML loadingsequenceDiagram
participant Manager as DccPluginManager
participant Queue as DccAsyncModuleLoader
participant Component as QQmlComponent
participant Incubator as QQmlIncubator
participant Loader as DccPluginLoader
participant Timer as DccLoadTimer
Manager->>Queue: enqueue(loader, loadTimer)
Queue->>Queue: startNext()
Queue->>Component: loadUrl/loadFromModule(Asynchronous)
Component-->>Queue: onModuleComponentStatus(Ready)
Queue->>Incubator: create(Asynchronous)
Incubator-->>Queue: onModuleIncubated(Ready)
Queue->>Loader: setModule(module)
Queue->>Queue: scheduleNext()
Queue->>Component: loadUrl/loadFromModule(Asynchronous)
Component-->>Queue: onMainComponentStatus(Ready)
Queue->>Incubator: create(context)
Incubator-->>Queue: onMainIncubated(Ready)
Queue->>Loader: setMainObj(mainObj)
Queue->>Queue: scheduleNext()
Queue->>Timer: finishPlugin(name)
Flow diagram for asynchronous plugin error completionflowchart TD
A[Plugin queued] --> B[startNext]
B --> C[Load module or main QML asynchronously]
C --> D{Component or incubator status}
D -->|Ready| E[Create and attach plugin object]
E --> F{Next loading phase?}
F -->|Yes| B
F -->|No| G[Plugin loading complete]
D -->|Error or null object| H[transitionStatus with Err and End]
H --> I["finishPlugin(name)"]
I --> B
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 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/dde-control-center/dccasyncmodloader.cpp" line_range="341-345" />
<code_context>
+ return;
+ }
+ m_current = m_queue.dequeue();
+ DccPluginLoader::StatusFlags status = m_current.loader->status();
+ if ((status & (DccPluginLoader::MetaDataEnd | DccPluginLoader::ModuleLoad)) == DccPluginLoader::MetaDataEnd) {
+ doLoadModule();
+ } else if ((status & (DccPluginLoader::DataEnd | DccPluginLoader::MainObjLoad)) == DccPluginLoader::DataEnd) {
+ doLoadMain();
+ } else {
+ scheduleNext();
</code_context>
<issue_to_address>
**issue (bug_risk):** After every module completion, `startNext()` sees the persistent `MetaDataEnd` bit and invokes `doLoadModule()` again instead of starting `doLoadMain()`. The same condition also matches after main completion and error paths, so queued plugins repeatedly reload their module and never reach normal load completion.
**Triggers:** When `ASYNC_MAIN` is enabled, which is the current build configuration.
**Suggested fix:** Select the next phase from the completion bits (`ModuleEnd` should lead to `doLoadMain()` and `MainObjEnd`/error should finish the item), or clear/track phase-specific status before scheduling the next item.
</issue_to_address>
### Comment 2
<location path="src/dde-control-center/pluginmanager.cpp" line_range="120-121" />
<code_context>
}
loader->transitionStatus(DccPluginLoader::PluginEnd);
} else if ((loader->status() & (DccPluginLoader::DataEnd | DccPluginLoader::MainObjLoad)) == DccPluginLoader::DataEnd) {
+#ifdef ASYNC_MAIN
+ m_asyncLoader->enqueue(loader, &m_loadTimer);
+#else
loader->transitionStatus(DccPluginLoader::MainObjLoad);
</code_context>
<issue_to_address>
**issue (broader_impact):** The asynchronous main-QML branch enqueues the loader without calling `createDccObject()` or `updateParent()`, whereas the synchronous branch performs both operations before `loadMain()`. Consequently, plugins using their factory-created `m_soObj` lose that object and its required parenting/initialization on the async path.
**Triggers:** When a plugin provides a factory/library object used alongside or instead of its QML main object.
**Suggested fix:** Call `createDccObject()` and `updateParent()` before enqueueing the asynchronous main load, or move those operations into the shared async path.
</issue_to_address>
### Comment 3
<location path="src/dde-control-center/dccasyncmodloader.cpp" line_range="179-191" />
<code_context>
+ return;
+ }
+
+ DccObject *module = qobject_cast<DccObject *>(rawObj);
+ bool visible = module && module->isVisibleToApp();
+ DCC_BENCHMARK(name, "loading-module-QML");
+ qCDebug(dccAsyncLog) << name << "module ready, visible=" << visible;
+
+ m_current.loader->setModule(module);
+ if (!visible) {
+ // Module is hidden — skip main QML and finish plugin
+ m_current.loader->setLog("create module finished, module is hidden");
+ m_current.loader->transitionStatus(DccPluginLoader::ModuleEnd | DccPluginLoader::PluginEnd);
+ m_current.loadTimer->finishPlugin(name);
+ scheduleNext();
+ return;
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** A successfully created module that is not a `DccObject` is converted to `nullptr`, but the async path treats `!visible` as a hidden module and marks the entire plugin `PluginEnd`. The synchronous path does not apply that test when the cast fails and continues to main loading, so async and sync paths have different behavior for the same QML result.
**Triggers:** When module QML creates a QObject that is not a DccObject.
**Suggested fix:** Distinguish a failed `DccObject` cast from a valid hidden module and mirror `DccPluginLoader::loadModule()` semantics, including an explicit error policy for an invalid module object.
</issue_to_address>| DccPluginLoader::StatusFlags status = m_current.loader->status(); | ||
| if ((status & (DccPluginLoader::MetaDataEnd | DccPluginLoader::ModuleLoad)) == DccPluginLoader::MetaDataEnd) { | ||
| doLoadModule(); | ||
| } else if ((status & (DccPluginLoader::DataEnd | DccPluginLoader::MainObjLoad)) == DccPluginLoader::DataEnd) { | ||
| doLoadMain(); |
There was a problem hiding this comment.
issue (bug_risk): After every module completion, startNext() sees the persistent MetaDataEnd bit and invokes doLoadModule() again instead of starting doLoadMain(). The same condition also matches after main completion and error paths, so queued plugins repeatedly reload their module and never reach normal load completion.
Triggers: When ASYNC_MAIN is enabled, which is the current build configuration.
Suggested fix: Select the next phase from the completion bits (ModuleEnd should lead to doLoadMain() and MainObjEnd/error should finish the item), or clear/track phase-specific status before scheduling the next item.
| DccObject *module = qobject_cast<DccObject *>(rawObj); | ||
| bool visible = module && module->isVisibleToApp(); | ||
| DCC_BENCHMARK(name, "loading-module-QML"); | ||
| qCDebug(dccAsyncLog) << name << "module ready, visible=" << visible; | ||
|
|
||
| m_current.loader->setModule(module); | ||
| if (!visible) { | ||
| // Module is hidden — skip main QML and finish plugin | ||
| m_current.loader->setLog("create module finished, module is hidden"); | ||
| m_current.loader->transitionStatus(DccPluginLoader::ModuleEnd | DccPluginLoader::PluginEnd); | ||
| m_current.loadTimer->finishPlugin(name); | ||
| scheduleNext(); | ||
| return; |
There was a problem hiding this comment.
issue (bug_risk): A successfully created module that is not a DccObject is converted to nullptr, but the async path treats !visible as a hidden module and marks the entire plugin PluginEnd. The synchronous path does not apply that test when the cast fails and continues to main loading, so async and sync paths have different behavior for the same QML result.
Triggers: When module QML creates a QObject that is not a DccObject.
Suggested fix: Distinguish a failed DccObject cast from a valid hidden module and mirror DccPluginLoader::loadModule() semantics, including an explicit error policy for an invalid module object.
| #ifdef ASYNC_MAIN | ||
| m_asyncLoader->enqueue(loader, &m_loadTimer); |
There was a problem hiding this comment.
m_loadTimer 不需要当做参数传递吧,它跟这个函数的逻辑没关系,可以单独提一个函数去设置,
| } | ||
| loader->transitionStatus(DccPluginLoader::PluginEnd); | ||
| } else if ((loader->status() & (DccPluginLoader::DataEnd | DccPluginLoader::MainObjLoad)) == DccPluginLoader::DataEnd) { | ||
| #ifdef ASYNC_MAIN |
There was a problem hiding this comment.
用运行时去判断,而不是编译去判断吧,可以加个环境变量去控制,动态切换,
| m_rootModule = root; | ||
| m_engine = engine; | ||
| qCDebug(dccLog()) << "plugin dir:" << dirs; | ||
| if(!m_asyncLoader){ |
1. Add DccAsyncModuleLoader to load plugin main QML asynchronously (QQmlComponent::Asynchronous + QQmlIncubator) via a serial queue so plugin pages no longer block the main thread after navigation is ready. 2. Completion is driven by queued startNext() calls, safe against synchronous incubator callbacks; component statusChanged slots guard against stale callbacks after cancel(). 3. Ownership: components/incubators held by unique_ptr; QQmlContext uses the "non-null = owned, null = handed over" convention and is parented to the created object, fixing a per-plugin context leak. 4. DccPluginLoader: extract setModule()/setMainObj() shared by sync and async paths for parenting and visibleToAppChanged wiring. 5. DccPluginManager: gate async stages with ASYNC_MODULE/ASYNC_MAIN (module sync for fast navigation, main async); error paths mirror sync semantics (*Err|*End + finishPlugin) so load completion can never stall on a bad plugin QML. Log: Control center opens with the window visible earlier; plugin pages load in the background without blocking the UI. Influence: 1. Launch control center: window shows, navigation list appears, then pages become clickable as they finish loading. 2. Verify a plugin with a broken main QML still lets the control center finish loading all other plugins (no stuck "loading"). feat: 使用孵化器队列异步加载插件主 QML 1. 新增 DccAsyncModuleLoader,通过串行队列以异步方式 (QQmlComponent::Asynchronous + QQmlIncubator)加载插件 main QML, 使导航就绪后插件页面不再阻塞主线程。 2. 各阶段完成由队列化的 startNext() 驱动,可安全应对孵化器同步回调; component statusChanged 槽对 cancel() 之后的过期回调做了防护。 3. 所有权规则:component/incubator 由 unique_ptr 持有;QQmlContext 采用 "非空=持有、空=已转让"约定,并挂接到所创建对象上,修复了 每插件泄漏一个 context 的问题。 4. DccPluginLoader:提取 setModule()/setMainObj(),供同步/异步 两条路径共享挂父逻辑与 visibleToAppChanged 接线。 5. DccPluginManager:以 ASYNC_MODULE/ASYNC_MAIN 宏门控各阶段 (module 同步以保证导航尽快就绪,main 异步);错误路径与同步语义 一致(*Err|*End + finishPlugin),坏插件 QML 不会卡死加载完成流程。 Log: 控制中心窗口更早可交互,插件页面后台加载不再阻塞界面。 Influence: 1. 启动控制中心:窗口先显示、导航列表先就绪,各页面加载完成后可点击。 2. 验证某个插件 main QML 损坏时,其余插件仍能正常加载完成(不会卡在加载中)。 PMS: TASK-394433
1df2026 to
9ef94c2
Compare
deepin pr auto reviewAI 代码审查报告
总体评分
代码变更概述本 PR 实现了插件主 QML 的异步加载功能,通过
维度1:语法逻辑(14/25)✕
问题1(一般错误 -8分):
|
Log: Control center opens with the window visible earlier; plugin pages load in the background without blocking the UI.
Influence:
feat: 使用孵化器队列异步加载插件主 QML
Log: 控制中心窗口更早可交互,插件页面后台加载不再阻塞界面。
Influence:
PMS: TASK-394433
Summary by Sourcery
Keep the control center responsive by asynchronously loading plugin pages while preserving reliable completion and error handling.
New Features:
Bug Fixes:
Enhancements: