refactor: optimize plugin loading and QML registration#3136
Open
caixr23 wants to merge 2 commits intolinuxdeepin:masterfrom
Open
refactor: optimize plugin loading and QML registration#3136caixr23 wants to merge 2 commits intolinuxdeepin:masterfrom
caixr23 wants to merge 2 commits intolinuxdeepin:masterfrom
Conversation
This refactor improves plugin management by separating QML registration from plugin object creation. Key changes include: 1. Added QML_ENGINE_PROPERTY constant for QML engine property name 2. Modified DCC_FACTORY_CLASS macro to accept variadic arguments for QML registration 3. Added dccObject() method in factory to handle QML type registration on main thread 4. Created containsByName() method for more efficient module filtering 5. Improved plugin loading sequence with proper thread management 6. Moved all QML registration calls from constructors to factory's dccObject() method 7. Removed unnecessary QML integration headers from model classes The changes ensure QML type registration happens on the main thread while plugin objects are created in worker threads, preventing threading issues and improving performance. Log: Optimized plugin loading process and QML registration Influence: 1. Test plugin loading and module display in control center 2. Verify all QML components are properly registered and accessible 3. Check module hiding functionality with wildcard patterns 4. Test thread safety during plugin initialization 5. Verify no regression in existing functionality refactor: 优化插件加载和 QML 注册流程 本次重构改进了插件管理机制，将 QML 注册与插件对象创建分离。主要变更 包括： 1. 添加 QML_ENGINE_PROPERTY 常量用于 QML 引擎属性名 2. 修改 DCC_FACTORY_CLASS 宏支持可变参数用于 QML 注册 3. 在工厂类中添加 dccObject() 方法处理主线程中的 QML 类型注册 4. 创建 containsByName() 方法实现更高效的模块过滤 5. 改进插件加载序列，优化线程管理 6. 将所有 QML 注册调用从构造函数移至工厂的 dccObject() 方法 7. 从模型类中移除不必要的 QML 集成头文件 这些变更确保 QML 类型注册在主线程执行，而插件对象在工作线程创建，避免线 程问题并提升性能。 Log: 优化插件加载过程和 QML 注册机制 Influence: 1. 测试控制中心插件加载和模块显示 2. 验证所有 QML 组件正确注册并可访问 3. 检查通配符模式下的模块隐藏功能 4. 测试插件初始化期间的线程安全性 5. 验证现有功能无回归 PMS: BUG-309197
|
[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 GuideRefactors the plugin loading pipeline so that plugin instances are created in worker threads while all QML type registration is centralized in DccFactory::dccObject() and executed on the main thread, and introduces reusable helpers for module filtering by name. Sequence diagram for the new plugin loading and QML registration processsequenceDiagram
actor User
participant PluginManager
participant ManagerThread
participant LoadPluginTask
participant WorkerThread
participant DccFactory
participant MainThread
participant QQmlEngine
participant PluginData
participant PluginData.data
participant PluginData.factory
participant obj
User->>PluginManager: requestLoadPlugin()
PluginManager->>ManagerThread: schedule LoadPluginTask
ManagerThread->>LoadPluginTask: start doLoadSo()
activate LoadPluginTask
LoadPluginTask->>WorkerThread: load plugin .so
activate WorkerThread
WorkerThread->>DccFactory: create() // plugin instance in worker thread
DccFactory-->>WorkerThread: QObject* data
deactivate WorkerThread
LoadPluginTask->>LoadPluginTask: store data in PluginData.data
LoadPluginTask->>LoadPluginTask: store DccFactory in PluginData.factory
LoadPluginTask-->>PluginManager: emit updatePluginStatus(DataEnd)
deactivate LoadPluginTask
PluginManager->>PluginManager: loadPlugin(plugin) // reacts to DataEnd
PluginManager->>PluginData: move data to ManagerThread
PluginManager->>PluginData.data: setParent(PluginManager)
PluginManager->>QQmlEngine: engine()
QQmlEngine-->>PluginManager: QQmlEngine*
PluginManager->>PluginData.factory: setProperty(QML_ENGINE_PROPERTY, engine)
PluginManager->>PluginData.factory: moveToThread(ManagerThread)
PluginManager->>DccFactory: dccObject()
activate DccFactory
DccFactory->>DccFactory: executeQmlRegisters() // __VA_ARGS__ qmlRegisterType<...>()
DccFactory-->>PluginManager: QObject* obj (optional DccObject)
deactivate DccFactory
PluginManager->>PluginManager: dynamic_cast to DccObject
alt obj is DccObject
PluginManager->>PluginData: store as soObj
else obj not DccObject
PluginManager->>obj: deleteLater()
end
PluginManager->>PluginManager: loadMain(plugin)
PluginManager-->>User: plugin ready
Updated class diagram for DccFactory, PluginManager, and DccManager helpersclassDiagram
class QObject
class QThread
class QQmlEngine
namespace dccV25 {
class DccObject
class DccFactory
}
class DccFactory {
<<interface>>
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
}
class BiometricAuthControllerDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class soundInteractionDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class SystemInfoInteractionDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class BluetoothInteractionDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class CommonInfoInteractionDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class PersonalizationInterfaceDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class PrivacySecurityExportDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class AccountsControllerDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class DefAppModelDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class TouchScreenModelDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class DatetimeModelDccFactory {
+QObject* create(QObject* parent)
+QObject* dccObject(QObject* parent)
-executeQmlRegisters()
}
class soundInteraction {
+SoundWorker* m_soundWork
+SoundModel* m_soundModel
}
class PluginData {
+QString name
+dccV25::DccObject* mainObj
+dccV25::DccObject* soObj
+QObject* data
+dccV25::DccFactory* factory
+QThread* thread
+uint status
}
class PluginManager {
+void loadPlugin(PluginData* plugin)
+void loadMetaData(PluginData* plugin)
+void createMain(QQmlComponent* component)
+QThread* thread()
+QObject* parent()
-void addMainObject(PluginData* plugin)
-void loadMain(PluginData* plugin)
}
class DccManager {
+const QSet~QString~& hideModule()
+static bool isMatchByName(QString url, QString name)
+static bool isMatch(QString url, const dccV25::DccObject* obj)
+static bool isEqualByName(QString url, QString name)
+static bool isEqual(QString url, const dccV25::DccObject* obj)
+static bool containsByName(QSet~QString~ urls, QString name)
+static bool contains(QSet~QString~ urls, const dccV25::DccObject* obj)
-QStringList splitUrl(QString url, QString targetName)
-dccV25::DccObject* findObject(QString url)
-QVector~dccV25::DccObject*~ findObjects(QString url, bool one)
-const dccV25::DccObject* findParent(const dccV25::DccObject* obj)
}
QObject <|-- DccFactory
QObject <|-- dccV25.DccObject
DccFactory <|-- BiometricAuthControllerDccFactory
DccFactory <|-- soundInteractionDccFactory
DccFactory <|-- SystemInfoInteractionDccFactory
DccFactory <|-- BluetoothInteractionDccFactory
DccFactory <|-- CommonInfoInteractionDccFactory
DccFactory <|-- PersonalizationInterfaceDccFactory
DccFactory <|-- PrivacySecurityExportDccFactory
DccFactory <|-- AccountsControllerDccFactory
DccFactory <|-- DefAppModelDccFactory
DccFactory <|-- TouchScreenModelDccFactory
DccFactory <|-- DatetimeModelDccFactory
PluginManager "1" o-- "many" PluginData
PluginData "1" --> "0..1" dccV25.DccObject : mainObj
PluginData "1" --> "0..1" dccV25.DccObject : soObj
PluginData "1" --> "0..1" QObject : data
PluginData "1" --> "0..1" DccFactory : factory
PluginData "1" --> "0..1" QThread : thread
PluginManager ..> DccManager : uses hideModule()
PluginManager ..> QQmlEngine : engine()
PluginManager ..> DccFactory : dccObject()
soundInteraction --> SoundWorker
soundInteraction --> SoundModel
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 found 3 issues, and left some high level feedback:
- In
dccfactory.h, the newQML_ENGINE_PROPERTYvalue is spelled"QmlEngin"; if this is meant to be a stable key (and especially if read elsewhere), consider correcting it to"QmlEngine"now to avoid hard‑to‑trace bugs later. - The new
DCC_FACTORY_CLASSmacro droppedQ_DECL_EXPORTfrom the factory class declaration; please confirm this isn’t required for plugin symbol visibility on your platforms, and reintroduce it if necessary to avoid runtime plugin loading issues. - In the
DCC_FACTORY_CLASSimplementation,dccObject(QObject * = nullptr)ignores the parent argument and always returnsnullptr; if future factory implementations are expected to allocate and return aQObject, consider wiring the parent through and/or documenting that the macro-generated version is intended only for QML registration side effects.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `dccfactory.h`, the new `QML_ENGINE_PROPERTY` value is spelled `"QmlEngin"`; if this is meant to be a stable key (and especially if read elsewhere), consider correcting it to `"QmlEngine"` now to avoid hard‑to‑trace bugs later.
- The new `DCC_FACTORY_CLASS` macro dropped `Q_DECL_EXPORT` from the factory class declaration; please confirm this isn’t required for plugin symbol visibility on your platforms, and reintroduce it if necessary to avoid runtime plugin loading issues.
- In the `DCC_FACTORY_CLASS` implementation, `dccObject(QObject * = nullptr)` ignores the parent argument and always returns `nullptr`; if future factory implementations are expected to allocate and return a `QObject`, consider wiring the parent through and/or documenting that the macro-generated version is intended only for QML registration side effects.
## Individual Comments
### Comment 1
<location path="include/dccfactory.h" line_range="11" />
<code_context>
namespace dccV25 {
#define DccFactory_iid "org.deepin.dde.dcc-factory/v1.0"
+#define QML_ENGINE_PROPERTY "QmlEngin"
class DccObject;
</code_context>
<issue_to_address>
**issue (bug_risk):** The QML_ENGINE_PROPERTY string looks misspelled, which risks mismatches wherever it is read.
The macro value is "QmlEngin" (missing the final 'e'). Any code expecting "QmlEngine" will fail to read this property. Please either correct the spelling or verify that all existing usages intentionally match this exact string.
</issue_to_address>
### Comment 2
<location path="include/dccfactory.h" line_range="29-38" />
<code_context>
+#define DCC_FACTORY_CLASS(classname, ...) \
</code_context>
<issue_to_address>
**suggestion:** The variadic macro relies on call sites passing `__VA_ARGS__` as statements; some current usages instead use comma expressions.
Inside `dccObject`, `__VA_ARGS__` is injected directly into a lambda body, but some call sites use semicolon-terminated statements while others (e.g. `soundInteraction`, `SystemInfoInteraction`, `CommonInfoInteraction`) use comma-separated expressions. This works but is surprising and easy to misuse. Please clarify the intended usage (statements terminated with `;`) and either wrap `__VA_ARGS__` in a `do { __VA_ARGS__; } while (0)` or standardize call sites to avoid comma expressions.
Suggested implementation:
```c
/*
* DCC_FACTORY_CLASS
*
* Variadic arguments (`__VA_ARGS__`) are intended to be *statements* terminated
* with semicolons. They are injected into an implementation lambda as a
* statement block. Do not rely on comma-expressions here; always write
* normal statements, e.g.:
*
* DCC_FACTORY_CLASS(Foo,
* auto w = new FooWidget(parent);
* configureFoo(w);
* return w;
* )
*/
#define DCC_FACTORY_CLASS(classname, ...) \
namespace { \
class classname##DccFactory : public dccV25::DccFactory \
{ \
Q_OBJECT \
Q_PLUGIN_METADATA(IID DccFactory_iid) \
Q_INTERFACES(dccV25::DccFactory) \
public: \
using dccV25::DccFactory::DccFactory; \
QObject *create(QObject *parent = nullptr) override \
{ \
```
```c
QObject *create(QObject *parent = nullptr) override \
{ \
/* Ensure __VA_ARGS__ are always treated as a statement block */ \
do { __VA_ARGS__; } while (0); \
```
I only see the beginning of the macro and not the full body. The intent is that wherever `__VA_ARGS__` is currently injected directly into the lambda body (e.g. something like `auto dccObject = [parent] { __VA_ARGS__; };`), that occurrence should be replaced with `do { __VA_ARGS__; } while (0);` inside the lambda instead. If the macro currently *returns* the result of `__VA_ARGS__` (e.g. `return __VA_ARGS__;`), you may need to restructure it to:
```c++
auto impl = [&]() {
do { __VA_ARGS__; } while (0);
};
return impl();
```
so that the block form remains compatible. Please adjust the placement of the `do { __VA_ARGS__; } while (0);` line to match your actual lambda/body structure in `DCC_FACTORY_CLASS`.
</issue_to_address>
### Comment 3
<location path="src/plugin-sound/operation/soundInteraction.cpp" line_range="51-54" />
<code_context>
}
-DCC_FACTORY_CLASS(soundInteraction)
+DCC_FACTORY_CLASS(soundInteraction,\
+ qmlRegisterType<SoundWorker>("dcc", 1, 0, "SoundWorker"),\
+ qmlRegisterType<SoundModel>("dcc", 1, 0, "SoundModel"),\
+ qmlRegisterType<SoundDeviceModel>("SoundDeviceModel", 1, 0, "SoundDeviceModel"))
#include "soundInteraction.moc"
</code_context>
<issue_to_address>
**suggestion:** Using commas between `qmlRegisterType` calls here relies on the comma operator, which is non-obvious and inconsistent with other call sites.
Here, the macro’s lambda ends up chaining these calls via the comma operator. While it evaluates all calls, it’s unconventional and differs from nearby uses (e.g., `BiometricAuthController`, `BluetoothInteraction`) that use `;`. Using semicolons instead would align with those sites and reduce the risk of subtle errors if more logic is added later.
```suggestion
DCC_FACTORY_CLASS(soundInteraction,\
qmlRegisterType<SoundWorker>("dcc", 1, 0, "SoundWorker");\
qmlRegisterType<SoundModel>("dcc", 1, 0, "SoundModel");\
qmlRegisterType<SoundDeviceModel>("SoundDeviceModel", 1, 0, "SoundDeviceModel"))
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| namespace dccV25 { | ||
| #define DccFactory_iid "org.deepin.dde.dcc-factory/v1.0" | ||
| #define QML_ENGINE_PROPERTY "QmlEngin" |
There was a problem hiding this comment.
issue (bug_risk): The QML_ENGINE_PROPERTY string looks misspelled, which risks mismatches wherever it is read.
The macro value is "QmlEngin" (missing the final 'e'). Any code expecting "QmlEngine" will fail to read this property. Please either correct the spelling or verify that all existing usages intentionally match this exact string.
Comment on lines
+29
to
+38
| #define DCC_FACTORY_CLASS(classname, ...) \ | ||
| namespace { \ | ||
| class classname##DccFactory : public dccV25::DccFactory \ | ||
| { \ | ||
| Q_OBJECT \ | ||
| Q_PLUGIN_METADATA(IID DccFactory_iid) \ | ||
| Q_INTERFACES(dccV25::DccFactory) \ | ||
| public: \ | ||
| using dccV25::DccFactory::DccFactory; \ | ||
| QObject *create(QObject *parent = nullptr) override \ |
There was a problem hiding this comment.
suggestion: The variadic macro relies on call sites passing __VA_ARGS__ as statements; some current usages instead use comma expressions.
Inside dccObject, __VA_ARGS__ is injected directly into a lambda body, but some call sites use semicolon-terminated statements while others (e.g. soundInteraction, SystemInfoInteraction, CommonInfoInteraction) use comma-separated expressions. This works but is surprising and easy to misuse. Please clarify the intended usage (statements terminated with ;) and either wrap __VA_ARGS__ in a do { __VA_ARGS__; } while (0) or standardize call sites to avoid comma expressions.
Suggested implementation:
/*
* DCC_FACTORY_CLASS
*
* Variadic arguments (`__VA_ARGS__`) are intended to be *statements* terminated
* with semicolons. They are injected into an implementation lambda as a
* statement block. Do not rely on comma-expressions here; always write
* normal statements, e.g.:
*
* DCC_FACTORY_CLASS(Foo,
* auto w = new FooWidget(parent);
* configureFoo(w);
* return w;
* )
*/
#define DCC_FACTORY_CLASS(classname, ...) \
namespace { \
class classname##DccFactory : public dccV25::DccFactory \
{ \
Q_OBJECT \
Q_PLUGIN_METADATA(IID DccFactory_iid) \
Q_INTERFACES(dccV25::DccFactory) \
public: \
using dccV25::DccFactory::DccFactory; \
QObject *create(QObject *parent = nullptr) override \
{ \ QObject *create(QObject *parent = nullptr) override \
{ \
/* Ensure __VA_ARGS__ are always treated as a statement block */ \
do { __VA_ARGS__; } while (0); \I only see the beginning of the macro and not the full body. The intent is that wherever __VA_ARGS__ is currently injected directly into the lambda body (e.g. something like auto dccObject = [parent] { __VA_ARGS__; };), that occurrence should be replaced with do { __VA_ARGS__; } while (0); inside the lambda instead. If the macro currently returns the result of __VA_ARGS__ (e.g. return __VA_ARGS__;), you may need to restructure it to:
auto impl = [&]() {
do { __VA_ARGS__; } while (0);
};
return impl();so that the block form remains compatible. Please adjust the placement of the do { __VA_ARGS__; } while (0); line to match your actual lambda/body structure in DCC_FACTORY_CLASS.
Comment on lines
+51
to
+54
| DCC_FACTORY_CLASS(soundInteraction,\ | ||
| qmlRegisterType<SoundWorker>("dcc", 1, 0, "SoundWorker"),\ | ||
| qmlRegisterType<SoundModel>("dcc", 1, 0, "SoundModel"),\ | ||
| qmlRegisterType<SoundDeviceModel>("SoundDeviceModel", 1, 0, "SoundDeviceModel")) |
There was a problem hiding this comment.
suggestion: Using commas between qmlRegisterType calls here relies on the comma operator, which is non-obvious and inconsistent with other call sites.
Here, the macro’s lambda ends up chaining these calls via the comma operator. While it evaluates all calls, it’s unconventional and differs from nearby uses (e.g., BiometricAuthController, BluetoothInteraction) that use ;. Using semicolons instead would align with those sites and reduce the risk of subtle errors if more logic is added later.
Suggested change
| DCC_FACTORY_CLASS(soundInteraction,\ | |
| qmlRegisterType<SoundWorker>("dcc", 1, 0, "SoundWorker"),\ | |
| qmlRegisterType<SoundModel>("dcc", 1, 0, "SoundModel"),\ | |
| qmlRegisterType<SoundDeviceModel>("SoundDeviceModel", 1, 0, "SoundDeviceModel")) | |
| DCC_FACTORY_CLASS(soundInteraction,\ | |
| qmlRegisterType<SoundWorker>("dcc", 1, 0, "SoundWorker");\ | |
| qmlRegisterType<SoundModel>("dcc", 1, 0, "SoundModel");\ | |
| qmlRegisterType<SoundDeviceModel>("SoundDeviceModel", 1, 0, "SoundDeviceModel")) |
18202781743
reviewed
Mar 31, 2026
| } | ||
| if (plugin->factory) { | ||
| plugin->factory->setProperty(QML_ENGINE_PROPERTY, QVariant::fromValue(m_manager->engine())); | ||
| QObject *obj = plugin->factory->dccObject(); |
Contributor
There was a problem hiding this comment.
这个应该在module加载之前吧,不然有些类型没注册,在c++里是不是也存在问题呀,
18202781743
reviewed
Mar 31, 2026
| // 未提供qml的，可在此自己加载qml返回DccObject对象 | ||
| virtual DccObject *dccObject(QObject * = nullptr) { return nullptr; } | ||
| // 未提供qml的，可在此自己加载qml返回DccObject对象,qml相关操作，如注册qml类型(在主线程中执行) | ||
| virtual QObject *dccObject(QObject * = nullptr) { return nullptr; } |
Contributor
There was a problem hiding this comment.
加个虚函数吧,在创建dccData之前,保证在主线程中被调用,这样代码也容易维护,
也可以保证兼容性, 在高版本的dde-control-center编译也不会在低版本的上面安装运行,
|
TAG Bot New tag: 6.1.80 |
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.
This refactor improves plugin management by separating QML registration from plugin object creation. Key changes include:
The changes ensure QML type registration happens on the main thread while plugin objects are created in worker threads, preventing threading issues and improving performance.
Log: Optimized plugin loading process and QML registration
Influence:
refactor: 优化插件加载和 QML 注册流程
本次重构改进了插件管理机制，将 QML 注册与插件对象创建分离。主要变更
包括：
这些变更确保 QML 类型注册在主线程执行，而插件对象在工作线程创建，避免线
程问题并提升性能。
Log: 优化插件加载过程和 QML 注册机制
Influence:
PMS: BUG-309197
Summary by Sourcery
Refactor plugin loading and QML registration so that QML types are registered on the main thread via plugin factories while plugin data objects are created in worker threads, and improve module filtering and ownership handling during plugin initialization.
Bug Fixes:
Enhancements: