From 11045230c3b8db2b615e59e2d6622c1df4fad12d Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Mon, 6 Nov 2023 16:23:35 +0100 Subject: [PATCH] QML: Before processing deep aliases, remove pending bindings We may have additional bindings scheduled for the deep alias, but those are to be overridden by the alias. They should therefore be removed like bindings to target properties of shallow aliases. For QProperty bindings we have to apply a separate trick and set and clear the binding bit in the right places. We don't have access to the actual binding when writing the value, after all. Pick-to: 6.2 Fixes: QTBUG-115579 Change-Id: Ia915e59905d7e3185a17c5b6613926264ad9bc6b Reviewed-by: Qt CI Bot Reviewed-by: Fabian Kosmale (cherry picked from commit 0fdf9042ce36c1dcfed64d1600f1526ac176768d) (cherry picked from commit c07f63d06402ff76ae85508afe9576cad038b4f6) Reviewed-by: Qt Cherry-pick Bot --- src/qml/qml/qqmlobjectcreator.cpp | 22 +++++++++++++--- src/qml/qml/qqmlvmemetaobject.cpp | 26 +++++++++++++------ .../qml/qqmllanguage/data/DeepAliasOnIC.qml | 3 +++ .../qqmllanguage/data/deepAliasOnICUser.qml | 1 + .../qml/qqmllanguage/tst_qqmllanguage.cpp | 6 +---- 5 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp index 956d143dba..d8d5aee857 100644 --- a/src/qml/qml/qqmlobjectcreator.cpp +++ b/src/qml/qml/qqmlobjectcreator.cpp @@ -913,10 +913,13 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper && !(bindingFlags & QV4::CompiledData::Binding::IsPropertyObserver) && !_valueTypeProperty; - if (_ddata->hasBindingBit(bindingProperty->coreIndex()) && allowedToRemoveBinding) { - QQmlPropertyPrivate::removeBinding(_bindingTarget, QQmlPropertyIndex(bindingProperty->coreIndex())); - } else if (bindingProperty->isBindable() && allowedToRemoveBinding) { - removePendingBinding(_bindingTarget, bindingProperty->coreIndex()); + if (allowedToRemoveBinding) { + if (bindingProperty->isBindable()) { + removePendingBinding(_bindingTarget, bindingProperty->coreIndex()); + } else { + QQmlPropertyPrivate::removeBinding( + _bindingTarget, QQmlPropertyIndex(bindingProperty->coreIndex())); + } } if (bindingType == QV4::CompiledData::Binding::Type_Script || binding->isTranslationBinding()) { @@ -962,6 +965,9 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper qmlBinding = QQmlPropertyBinding::create(bindingProperty, runtimeFunction, _scopeObject, context, currentQmlContext(), _bindingTarget, index); } sharedState.data()->allQPropertyBindings.push_back(DeferredQPropertyBinding {_bindingTarget, bindingProperty->coreIndex(), qmlBinding }); + + QQmlData *data = QQmlData::get(_bindingTarget, true); + data->setBindingBit(_bindingTarget, bindingProperty->coreIndex()); } else { // When writing bindings to grouped properties implemented as value types, // such as point.x: { someExpression; }, then the binding is installed on @@ -1458,6 +1464,14 @@ bool QQmlObjectCreator::finalize(QQmlInstantiationInterrupt &interrupt) while (!sharedState->allQPropertyBindings.isEmpty()) { auto& [target, index, qmlBinding] = sharedState->allQPropertyBindings.first(); + + QQmlData *data = QQmlData::get(target); + if (!data || !data->hasBindingBit(index)) { + // The target property has been overwritten since we stashed the binding. + sharedState->allQPropertyBindings.pop_front(); + continue; + } + QUntypedBindable bindable; void *argv[] = { &bindable }; // allow interception diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp index a736cc7359..11d5a5ef41 100644 --- a/src/qml/qml/qqmlvmemetaobject.cpp +++ b/src/qml/qml/qqmlvmemetaobject.cpp @@ -1031,15 +1031,20 @@ int QQmlVMEMetaObject::metaCall(QObject *o, QMetaObject::Call c, int _id, void * int coreIndex = encodedIndex.coreIndex(); const int valueTypePropertyIndex = encodedIndex.valueTypeIndex(); - // Remove binding (if any) on write - if(c == QMetaObject::WriteProperty) { - int flags = *reinterpret_cast(a[3]); - if (flags & QQmlPropertyData::RemoveBindingOnAliasWrite) { - QQmlData *targetData = QQmlData::get(target); - if (targetData && targetData->hasBindingBit(coreIndex)) - QQmlPropertyPrivate::removeBinding(target, encodedIndex); + const auto removePendingBinding + = [c, a](QObject *target, int coreIndex, QQmlPropertyIndex encodedIndex) { + // Remove binding (if any) on write + if (c == QMetaObject::WriteProperty) { + int flags = *reinterpret_cast(a[3]); + if (flags & QQmlPropertyData::RemoveBindingOnAliasWrite) { + QQmlData *targetData = QQmlData::get(target); + if (targetData && targetData->hasBindingBit(coreIndex)) { + QQmlPropertyPrivate::removeBinding(target, encodedIndex); + targetData->clearBindingBit(coreIndex); + } + } } - } + }; if (valueTypePropertyIndex != -1) { if (!targetDData->propertyCache) @@ -1049,6 +1054,7 @@ int QQmlVMEMetaObject::metaCall(QObject *o, QMetaObject::Call c, int _id, void * QQmlGadgetPtrWrapper *valueType = QQmlGadgetPtrWrapper::instance( ctxt->engine(), pd->propType()); if (valueType) { + removePendingBinding(target, coreIndex, encodedIndex); valueType->read(target, coreIndex); int rv = QMetaObject::metacall(valueType, c, valueTypePropertyIndex, a); @@ -1061,10 +1067,14 @@ int QQmlVMEMetaObject::metaCall(QObject *o, QMetaObject::Call c, int _id, void * // deep alias void *argv[1] = { &target }; QMetaObject::metacall(target, QMetaObject::ReadProperty, coreIndex, argv); + removePendingBinding( + target, valueTypePropertyIndex, + QQmlPropertyIndex(valueTypePropertyIndex)); return QMetaObject::metacall(target, c, valueTypePropertyIndex, a); } } else { + removePendingBinding(target, coreIndex, encodedIndex); return QMetaObject::metacall(target, c, coreIndex, a); } diff --git a/tests/auto/qml/qqmllanguage/data/DeepAliasOnIC.qml b/tests/auto/qml/qqmllanguage/data/DeepAliasOnIC.qml index 36a8cc6078..37dd2695b0 100644 --- a/tests/auto/qml/qqmllanguage/data/DeepAliasOnIC.qml +++ b/tests/auto/qml/qqmllanguage/data/DeepAliasOnIC.qml @@ -2,6 +2,7 @@ import QtQml QtObject { id: root + objectName: "theRoot" component ObjectWithColor: QtObject { property string color @@ -10,12 +11,14 @@ QtObject { property ObjectWithColor border: ObjectWithColor { id: border + objectName: root.objectName color: root.trueBorderColor varvar: root.trueBorderVarvar } readonly property rect readonlyRect: ({x: 12, y: 13, width: 14, height: 15}) + property alias borderObjectName: border.objectName property alias borderColor: border.color property alias borderVarvar: border.varvar property alias readonlyRectX: root.readonlyRect.x diff --git a/tests/auto/qml/qqmllanguage/data/deepAliasOnICUser.qml b/tests/auto/qml/qqmllanguage/data/deepAliasOnICUser.qml index ef6001cd89..50eaa7c3e2 100644 --- a/tests/auto/qml/qqmllanguage/data/deepAliasOnICUser.qml +++ b/tests/auto/qml/qqmllanguage/data/deepAliasOnICUser.qml @@ -1,6 +1,7 @@ import QtQml DeepAliasOnIC { + borderObjectName: "theLeaf" borderColor: "black" borderVarvar: "mauve" } diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp index bc2baa6aef..7278d87c39 100644 --- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp +++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp @@ -8125,15 +8125,11 @@ void tst_qqmllanguage::deepAliasOnICOrReadonly() QScopedPointer o(c.create()); QVERIFY(!o.isNull()); - // We are mostly testing that it doesn't crash here. The actual bug is fixed separately. - - QEXPECT_FAIL("", "QTBUG-115579 is not fixed yet", Continue); QCOMPARE(o->property("borderColor").toString(), QLatin1String("black")); + QCOMPARE(o->property("borderObjectName").toString(), QLatin1String("theLeaf")); const QVariant var = o->property("borderVarvar"); - QEXPECT_FAIL("", "QTBUG-115579 is not fixed yet", Continue); QCOMPARE(var.metaType(), QMetaType::fromType()); - QEXPECT_FAIL("", "QTBUG-115579 is not fixed yet", Continue); QCOMPARE(var.toString(), QLatin1String("mauve")); QQmlComponent c2(&engine, testFileUrl("deepAliasOnReadonly.qml"));