refactor: replace beginCreate with createWithInitialProperties#734
Draft
wineee wants to merge 1 commit intolinuxdeepin:masterfrom
Draft
refactor: replace beginCreate with createWithInitialProperties#734wineee wants to merge 1 commit intolinuxdeepin:masterfrom
wineee wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wineee 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 QML component and menu creation to use Qt's create/createWithInitialProperties APIs instead of the manual beginCreate/completeCreate sequence, adding context/null checks, softening fatal errors to critical logs, and ensuring proper cleanup on failures. Sequence diagram for updated QML component creation flowsequenceDiagram
actor Caller
participant QmlEngine
participant QQmlComponent
participant QQmlContext
participant QObject
participant QQuickItem
participant QtQQmlEngine
Caller->>QmlEngine: createComponent(component, parent, properties)
QmlEngine->>QQmlContext: qmlContext(parent)
QQmlContext-->>QmlEngine: context
alt context is null
QmlEngine->>QmlEngine: log qCCritical Cant_get_QML_context_from_parent
QmlEngine-->>Caller: nullptr
else context is valid
alt properties not empty
QmlEngine->>QQmlComponent: createWithInitialProperties(properties, context)
else properties empty
QmlEngine->>QQmlComponent: create(context)
end
QQmlComponent-->>QmlEngine: obj
alt obj is null
QmlEngine->>QmlEngine: log qCCritical Component_creation_failed
QmlEngine-->>Caller: nullptr
else obj is not null
QmlEngine->>QQuickItem: qobject_cast_QQuickItem(obj)
QQuickItem-->>QmlEngine: item
alt item is null
QmlEngine->>QmlEngine: log qCCritical Cant_cast_to_QQuickItem
QmlEngine->>QObject: delete obj
QmlEngine-->>Caller: nullptr
else item is valid
QmlEngine->>QtQQmlEngine: setObjectOwnership(item, objectOwnership(parent))
QtQQmlEngine-->>QmlEngine: void
QmlEngine->>QQuickItem: setParent(parent)
QmlEngine->>QQuickItem: setParentItem(parent)
QmlEngine-->>Caller: item
end
end
end
Class diagram for updated QmlEngine QML creation methodsclassDiagram
class QmlEngine {
+QQuickItem* createComponent(QQmlComponent &component, QQuickItem *parent, const QVariantMap &properties)
+QObject* createWindowMenu(QObject *parent)
}
class QQmlComponent {
+QObject* create(QQmlContext *context)
+QObject* createWithInitialProperties(QVariantMap properties, QmlContext *context)
+QString errorString()
}
class QQmlContext {
}
class QQuickItem {
+void setParent(QObject *parent)
+void setParentItem(QQuickItem *parent)
}
class QObject {
+void setParent(QObject *parent)
}
class QtQQmlEngine {
+static void setObjectOwnership(QObject *object, int ownership)
+static int objectOwnership(QObject *object)
}
QmlEngine --> QQmlComponent : uses
QmlEngine --> QQuickItem : creates
QmlEngine --> QObject : creates
QmlEngine --> QQmlContext : uses
QmlEngine --> QtQQmlEngine : calls_static
QQuickItem --|> QObject
QtQQmlEngine ..> QObject : manages_ownership
QQmlComponent ..> QQmlContext : created_with_context
File-Level Changes
Possibly linked issues
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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/core/qmlengine.cpp:74-57` </location>
<code_context>
auto item = qobject_cast<QQuickItem *>(obj);
if (!item) {
- qCFatal(qLcQmlEngine) << "Can't create component:" << component.errorString();
+ qCCritical(qLcQmlEngine) << "Can't cast to QQuickItem:" << component.errorString();
+ delete obj;
+ return nullptr;
}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Downgrading from `qCFatal` to `qCCritical` changes failure mode from aborting to continuing after a serious construction error.
Previously, a failed cast here would abort the process; now we just log, delete `obj`, and return `nullptr`. If callers still assume a non-null `QQuickItem*`, this can turn into a later null dereference or subtle misbehavior. Either keep this fatal, or audit/update all call sites to handle a `nullptr` return explicitly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| auto obj = component.beginCreate(context); | ||
| if (!context) { | ||
| qCCritical(qLcQmlEngine) << "Can't get QML context from parent"; | ||
| return nullptr; |
There was a problem hiding this comment.
issue (bug_risk): Downgrading from qCFatal to qCCritical changes failure mode from aborting to continuing after a serious construction error.
Previously, a failed cast here would abort the process; now we just log, delete obj, and return nullptr. If callers still assume a non-null QQuickItem*, this can turn into a later null dereference or subtle misbehavior. Either keep this fatal, or audit/update all call sites to handle a nullptr return explicitly.
Open
Member
Author
|
可能修复：#358 的 64 至少逻辑上是优化 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors QML component creation in src/core/qmlengine.cpp to use Qt's recommended createWithInitialProperties/create APIs instead of the manual beginCreate/setInitialProperties/completeCreate flow. The changes improve error handling by adding null checks for contexts and created objects, and replace fatal errors with critical errors plus appropriate cleanup to prevent crashes.
Changes:
- Replaced
beginCreate/completeCreatepattern withcreateWithInitialProperties/createAPIs increateComponentandcreateWindowMenufunctions - Added null checks for QML contexts before component creation
- Changed
qCFataltoqCCriticalwith proper object cleanup (delete) to prevent memory leaks when errors occur
Refactored QML component creation to use createWithInitialProperties API instead of manual beginCreate/completeCreate flow. This simplifies the code and follows Qt's recommended pattern for component creation with initial properties. Added proper error handling and null checks for context and created objects. Changed fatal errors to critical errors with appropriate cleanup to prevent crashes. 重构 QML 组件创建逻辑，使用 createWithInitialProperties API 替代手动 beginCreate/completeCreate 流程。简化代码并遵循 Qt 推荐的带初始属性组件 创建模式。添加了适当的错误处理和对上下文及创建对象的空值检查。将致命错误 改为关键错误并添加适当的清理操作以防止崩溃。
zccrs
requested changes
Feb 5, 2026
| QQmlEngine::setObjectOwnership(item, QQmlEngine::objectOwnership(parent)); | ||
| item->setParent(parent); | ||
| item->setParentItem(parent); | ||
| component.completeCreate(); |
Member
There was a problem hiding this comment.
之前之所以用 being/complete 的分体式方式实现，就是为了在初始化好它的parent对象后再标记为创建完成。
Member
There was a problem hiding this comment.
要么改为使用 QQmlComponentPrivate::createWithProperties
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.
Refactored QML component creation to use createWithInitialProperties API instead of manual beginCreate/completeCreate flow. This simplifies the code and follows Qt's recommended pattern for component creation with initial properties. Added proper error handling and null checks for context and created objects. Changed fatal errors to critical errors with appropriate cleanup to prevent crashes.
重构 QML 组件创建逻辑，使用 createWithInitialProperties API 替代手动 beginCreate/completeCreate 流程。简化代码并遵循 Qt 推荐的带初始属性组件 创建模式。添加了适当的错误处理和对上下文及创建对象的空值检查。将致命错误
改为关键错误并添加适当的清理操作以防止崩溃。
Summary by Sourcery
Refactor QML component and window menu creation to use Qt's create/createWithInitialProperties APIs with safer error handling and ownership management.
Bug Fixes:
Enhancements: