Fix issue with bindings to aliases that cannot yet be resolved

When an alias points to a child object which has not yet been
initialized, it's id won't have been registered yet, so setting up a
binding to it will result in a crash.

The fix is: when setting a binding target fails, and its target property
is an alias, queue them until all bindings have been set up, and try
again.

Task-number: QTBUG-57041
Change-Id: I4dc5a6d25c0a32fed9fd952c955e2006c76be45a
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

5 files changed, 97 insertions, 57 deletions

diff --git a/src/qml/qml/qqmlbinding.cpp b/src/qml/qml/qqmlbinding.cpp
index bc7fccb2c0..c314833304 100644
--- a/src/qml/qml/qqmlbinding.cpp
+++ b/src/qml/qml/qqmlbinding.cpp
@@ -515,13 +515,13 @@ void QQmlBinding::setTarget(const QQmlProperty &prop)
     setTarget(prop.object(), pd->core, &pd->valueTypeData);
 }
 
-void QQmlBinding::setTarget(QObject *object, const QQmlPropertyData &core, const QQmlPropertyData *valueType)
+bool QQmlBinding::setTarget(QObject *object, const QQmlPropertyData &core, const QQmlPropertyData *valueType)
 {
     m_target = object;
 
     if (!object) {
         m_targetIndex = QQmlPropertyIndex();
-        return;
+        return false;
     }
 
     int coreIndex = core.coreIndex();
@@ -531,9 +531,10 @@ void QQmlBinding::setTarget(QObject *object, const QQmlPropertyData &core, const
 
         int aValueTypeIndex;
         if (!vme->aliasTarget(coreIndex, &object, &coreIndex, &aValueTypeIndex)) {
+            // can't resolve id (yet)
             m_target = nullptr;
             m_targetIndex = QQmlPropertyIndex();
-            return;
+            return false;
         }
         if (valueTypeIndex == -1)
             valueTypeIndex = aValueTypeIndex;
@@ -542,7 +543,7 @@ void QQmlBinding::setTarget(QObject *object, const QQmlPropertyData &core, const
         if (!data || !data->propertyCache) {
             m_target = nullptr;
             m_targetIndex = QQmlPropertyIndex();
-            return;
+            return false;
         }
         QQmlPropertyData *propertyData = data->propertyCache->property(coreIndex);
         Q_ASSERT(propertyData);
@@ -558,6 +559,8 @@ void QQmlBinding::setTarget(QObject *object, const QQmlPropertyData &core, const
         data->propertyCache = QQmlEnginePrivate::get(context()->engine)->cache(m_target->metaObject());
         data->propertyCache->addref();
     }
+
+    return true;
 }
 
 void QQmlBinding::getPropertyData(QQmlPropertyData **propertyData, QQmlPropertyData *valueTypeData) const
diff --git a/src/qml/qml/qqmlbinding_p.h b/src/qml/qml/qqmlbinding_p.h
index 19ec3f5d4f..b67b02975f 100644
--- a/src/qml/qml/qqmlbinding_p.h
+++ b/src/qml/qml/qqmlbinding_p.h
@@ -72,6 +72,8 @@ class Q_QML_PRIVATE_EXPORT QQmlBinding : public QQmlJavaScriptExpression,
 {
     friend class QQmlAbstractBinding;
 public:
+    typedef QExplicitlySharedDataPointer<QQmlBinding> Ptr;
+
     static QQmlBinding *create(const QQmlPropertyData *, const QQmlScriptString &, QObject *, QQmlContext *);
     static QQmlBinding *create(const QQmlPropertyData *, const QString &, QObject *, QQmlContextData *,
                                const QString &url = QString(), quint16 lineNumber = 0);
@@ -82,7 +84,7 @@ public:
     ~QQmlBinding() override;
 
     void setTarget(const QQmlProperty &);
-    void setTarget(QObject *, const QQmlPropertyData &, const QQmlPropertyData *valueType);
+    bool setTarget(QObject *, const QQmlPropertyData &, const QQmlPropertyData *valueType);
 
     void setNotifyOnValueChanged(bool);
 
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index 36e56a01f8..90f3beb40b 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -780,7 +780,7 @@ void QQmlObjectCreator::setupBindings(bool applyDeferredBindings)
     qSwap(_currentList, savedList);
 }
 
-bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, const QV4::CompiledData::Binding *binding)
+bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProperty, const QV4::CompiledData::Binding *binding)
 {
     if (binding->type == QV4::CompiledData::Binding::Type_AttachedProperty) {
         Q_ASSERT(stringAt(qmlUnit->objectAt(binding->value.objectIndex)->inheritedTypeNameIndex).isEmpty());
@@ -802,7 +802,7 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
     }
 
     // ### resolve this at compile time
-    if (property && property->propType() == qMetaTypeId<QQmlScriptString>()) {
+    if (bindingProperty && bindingProperty->propType() == qMetaTypeId<QQmlScriptString>()) {
         QQmlScriptString ss(binding->valueAsScriptString(qmlUnit), context->asQQmlContext(), _scopeObject);
         ss.d.data()->bindingId = binding->type == QV4::CompiledData::Binding::Type_Script ? binding->value.compiledScriptIndex : (quint32)QQmlBinding::Invalid;
         ss.d.data()->lineNumber = binding->location.line;
@@ -815,7 +815,7 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
                                                             QQmlPropertyData::RemoveBindingOnAliasWrite;
         int propertyWriteStatus = -1;
         void *argv[] = { &ss, nullptr, &propertyWriteStatus, &propertyWriteFlags };
-        QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, property->coreIndex(), argv);
+        QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, bindingProperty->coreIndex(), argv);
         return true;
     }
 
@@ -826,7 +826,7 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
             return false;
     }
 
-    if (!property) // ### error
+    if (!bindingProperty) // ### error
         return true;
 
     if (binding->type == QV4::CompiledData::Binding::Type_GroupProperty) {
@@ -838,20 +838,20 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
             const QQmlPropertyData *valueTypeProperty = nullptr;
             QObject *bindingTarget = _bindingTarget;
 
-            if (QQmlValueTypeFactory::isValueType(property->propType())) {
-                valueType = QQmlValueTypeFactory::valueType(property->propType());
+            if (QQmlValueTypeFactory::isValueType(bindingProperty->propType())) {
+                valueType = QQmlValueTypeFactory::valueType(bindingProperty->propType());
                 if (!valueType) {
                     recordError(binding->location, tr("Cannot set properties on %1 as it is null").arg(stringAt(binding->propertyNameIndex)));
                     return false;
                 }
 
-                valueType->read(_qobject, property->coreIndex());
+                valueType->read(_qobject, bindingProperty->coreIndex());
 
                 groupObject = valueType;
-                valueTypeProperty = property;
+                valueTypeProperty = bindingProperty;
             } else {
                 void *argv[1] = { &groupObject };
-                QMetaObject::metacall(_qobject, QMetaObject::ReadProperty, property->coreIndex(), argv);
+                QMetaObject::metacall(_qobject, QMetaObject::ReadProperty, bindingProperty->coreIndex(), argv);
                 if (!groupObject) {
                     recordError(binding->location, tr("Cannot set properties on %1 as it is null").arg(stringAt(binding->propertyNameIndex)));
                     return false;
@@ -864,21 +864,21 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
                 return false;
 
             if (valueType)
-                valueType->write(_qobject, property->coreIndex(), QQmlPropertyData::BypassInterceptor);
+                valueType->write(_qobject, bindingProperty->coreIndex(), QQmlPropertyData::BypassInterceptor);
 
             return true;
         }
     }
 
-    if (_ddata->hasBindingBit(property->coreIndex()) && !(binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression)
+    if (_ddata->hasBindingBit(bindingProperty->coreIndex()) && !(binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression)
         && !(binding->flags & QV4::CompiledData::Binding::IsOnAssignment)
         && !_valueTypeProperty)
-        QQmlPropertyPrivate::removeBinding(_bindingTarget, QQmlPropertyIndex(property->coreIndex()));
+        QQmlPropertyPrivate::removeBinding(_bindingTarget, QQmlPropertyIndex(bindingProperty->coreIndex()));
 
     if (binding->type == QV4::CompiledData::Binding::Type_Script || binding->containsTranslations()) {
         if (binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression) {
             QV4::Function *runtimeFunction = compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex];
-            int signalIndex = _propertyCache->methodIndexToSignalIndex(property->coreIndex());
+            int signalIndex = _propertyCache->methodIndexToSignalIndex(bindingProperty->coreIndex());
             QQmlBoundSignal *bs = new QQmlBoundSignal(_bindingTarget, signalIndex, _scopeObject, engine);
             QQmlBoundSignalExpression *expr = new QQmlBoundSignalExpression(_bindingTarget, signalIndex,
                                                                             context, _scopeObject, runtimeFunction, currentQmlContext());
@@ -890,34 +890,44 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
             // the point property (_qobjectForBindings) and after evaluating the expression,
             // the result is written to a value type virtual property, that contains the sub-index
             // of the "x" property.
-            QQmlBinding *qmlBinding;
-            const QQmlPropertyData *prop = property;
+            QQmlBinding::Ptr qmlBinding;
+            const QQmlPropertyData *targetProperty = bindingProperty;
             const QQmlPropertyData *subprop = nullptr;
             if (_valueTypeProperty) {
-                prop = _valueTypeProperty;
-                subprop = property;
+                targetProperty = _valueTypeProperty;
+                subprop = bindingProperty;
             }
             if (binding->containsTranslations()) {
                 qmlBinding = QQmlBinding::createTranslationBinding(compilationUnit, binding, _scopeObject, context);
             } else {
                 QV4::Function *runtimeFunction = compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex];
-                qmlBinding = QQmlBinding::create(prop, runtimeFunction, _scopeObject, context, currentQmlContext());
+                qmlBinding = QQmlBinding::create(targetProperty, runtimeFunction, _scopeObject, context, currentQmlContext());
             }
-            qmlBinding->setTarget(_bindingTarget, *prop, subprop);
 
-            sharedState->allCreatedBindings.push(QQmlAbstractBinding::Ptr(qmlBinding));
+            auto bindingTarget = _bindingTarget;
+            auto valueTypeProperty = _valueTypeProperty;
+            auto assignBinding = [qmlBinding, bindingTarget, targetProperty, subprop, bindingProperty, valueTypeProperty](QQmlObjectCreatorSharedState *sharedState) -> bool {
+                if (!qmlBinding->setTarget(bindingTarget, *targetProperty, subprop) && targetProperty->isAlias())
+                    return false;
+
+                sharedState->allCreatedBindings.push(qmlBinding);
 
-            if (property->isAlias()) {
-                QQmlPropertyPrivate::setBinding(qmlBinding, QQmlPropertyPrivate::DontEnable);
-            } else {
-                qmlBinding->addToObject();
+                if (bindingProperty->isAlias()) {
+                    QQmlPropertyPrivate::setBinding(qmlBinding.data(), QQmlPropertyPrivate::DontEnable);
+                } else {
+                    qmlBinding->addToObject();
 
-                if (!_valueTypeProperty) {
-                    QQmlData *targetDeclarativeData = QQmlData::get(_bindingTarget);
-                    Q_ASSERT(targetDeclarativeData);
-                    targetDeclarativeData->setPendingBindingBit(_bindingTarget, property->coreIndex());
+                    if (!valueTypeProperty) {
+                        QQmlData *targetDeclarativeData = QQmlData::get(bindingTarget);
+                        Q_ASSERT(targetDeclarativeData);
+                        targetDeclarativeData->setPendingBindingBit(bindingTarget, bindingProperty->coreIndex());
+                    }
                 }
-            }
+
+                return true;
+            };
+            if (!assignBinding(sharedState.data()))
+                pendingAliasBindings.push_back(assignBinding);
         }
         return true;
     }
@@ -934,9 +944,9 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
                 QObject *target = createdSubObject->parent();
                 QQmlProperty prop;
                 if (_valueTypeProperty)
-                    prop = QQmlPropertyPrivate::restore(target, *_valueTypeProperty, property, context);
+                    prop = QQmlPropertyPrivate::restore(target, *_valueTypeProperty, bindingProperty, context);
                 else
-                    prop = QQmlPropertyPrivate::restore(target, *property, nullptr, context);
+                    prop = QQmlPropertyPrivate::restore(target, *bindingProperty, nullptr, context);
                 vs->setTarget(prop);
                 return true;
             }
@@ -946,8 +956,8 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
                 QObject *target = createdSubObject->parent();
 
                 QQmlPropertyIndex propertyIndex;
-                if (property->isAlias()) {
-                    QQmlPropertyIndex originalIndex(property->coreIndex(), _valueTypeProperty ? _valueTypeProperty->coreIndex() : -1);
+                if (bindingProperty->isAlias()) {
+                    QQmlPropertyIndex originalIndex(bindingProperty->coreIndex(), _valueTypeProperty ? _valueTypeProperty->coreIndex() : -1);
                     QQmlPropertyIndex propIndex;
                     QQmlPropertyPrivate::findAliasTarget(target, originalIndex, &target, &propIndex);
                     QQmlData *data = QQmlData::get(target);
@@ -964,9 +974,9 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
                 } else {
                     QQmlProperty prop;
                     if (_valueTypeProperty)
-                        prop = QQmlPropertyPrivate::restore(target, *_valueTypeProperty, property, context);
+                        prop = QQmlPropertyPrivate::restore(target, *_valueTypeProperty, bindingProperty, context);
                     else
-                        prop = QQmlPropertyPrivate::restore(target, *property, nullptr, context);
+                        prop = QQmlPropertyPrivate::restore(target, *bindingProperty, nullptr, context);
                     vi->setTarget(prop);
                     propertyIndex = QQmlPropertyPrivate::propertyIndex(prop);
                 }
@@ -982,8 +992,8 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
 
         // Assigning object to signal property?
         if (binding->flags & QV4::CompiledData::Binding::IsSignalHandlerObject) {
-            if (!property->isFunction()) {
-                recordError(binding->valueLocation, tr("Cannot assign an object to signal property %1").arg(property->name(_qobject)));
+            if (!bindingProperty->isFunction()) {
+                recordError(binding->valueLocation, tr("Cannot assign an object to signal property %1").arg(bindingProperty->name(_qobject)));
                 return false;
             }
             QMetaMethod method = QQmlMetaType::defaultMethod(createdSubObject);
@@ -992,7 +1002,7 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
                 return false;
             }
 
-            QMetaMethod signalMethod = _qobject->metaObject()->method(property->coreIndex());
+            QMetaMethod signalMethod = _qobject->metaObject()->method(bindingProperty->coreIndex());
             if (!QMetaObject::checkConnectArgs(signalMethod, method)) {
                 recordError(binding->valueLocation,
                             tr("Cannot connect mismatched signal/slot %1 %vs. %2")
@@ -1001,7 +1011,7 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
                 return false;
             }
 
-            QQmlPropertyPrivate::connect(_qobject, property->coreIndex(), createdSubObject, method.methodIndex());
+            QQmlPropertyPrivate::connect(_qobject, bindingProperty->coreIndex(), createdSubObject, method.methodIndex());
             return true;
         }
 
@@ -1010,32 +1020,32 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
         int propertyWriteStatus = -1;
         void *argv[] = { nullptr, nullptr, &propertyWriteStatus, &propertyWriteFlags };
 
-        if (const char *iid = QQmlMetaType::interfaceIId(property->propType())) {
+        if (const char *iid = QQmlMetaType::interfaceIId(bindingProperty->propType())) {
             void *ptr = createdSubObject->qt_metacast(iid);
             if (ptr) {
                 argv[0] = &ptr;
-                QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, property->coreIndex(), argv);
+                QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, bindingProperty->coreIndex(), argv);
             } else {
                 recordError(binding->location, tr("Cannot assign object to interface property"));
                 return false;
             }
-        } else if (property->propType() == QMetaType::QVariant) {
-            if (property->isVarProperty()) {
+        } else if (bindingProperty->propType() == QMetaType::QVariant) {
+            if (bindingProperty->isVarProperty()) {
                 QV4::Scope scope(v4);
                 QV4::ScopedValue wrappedObject(scope, QV4::QObjectWrapper::wrap(engine->handle(), createdSubObject));
-                _vmeMetaObject->setVMEProperty(property->coreIndex(), wrappedObject);
+                _vmeMetaObject->setVMEProperty(bindingProperty->coreIndex(), wrappedObject);
             } else {
                 QVariant value = QVariant::fromValue(createdSubObject);
                 argv[0] = &value;
-                QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, property->coreIndex(), argv);
+                QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, bindingProperty->coreIndex(), argv);
             }
-        } else if (property->isQList()) {
+        } else if (bindingProperty->isQList()) {
             Q_ASSERT(_currentList.object);
 
             void *itemToAdd = createdSubObject;
 
             const char *iid = nullptr;
-            int listItemType = QQmlEnginePrivate::get(engine)->listType(property->propType());
+            int listItemType = QQmlEnginePrivate::get(engine)->listType(bindingProperty->propType());
             if (listItemType != -1)
                 iid = QQmlMetaType::interfaceIId(listItemType);
             if (iid)
@@ -1051,17 +1061,17 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
         } else {
             // pointer compatibility was tested in QQmlPropertyValidator at type compile time
             argv[0] = &createdSubObject;
-            QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, property->coreIndex(), argv);
+            QMetaObject::metacall(_qobject, QMetaObject::WriteProperty, bindingProperty->coreIndex(), argv);
         }
         return true;
     }
 
-    if (property->isQList()) {
+    if (bindingProperty->isQList()) {
         recordError(binding->location, tr("Cannot assign primitives to lists"));
         return false;
     }
 
-    setPropertyValue(property, binding);
+    setPropertyValue(bindingProperty, binding);
     return true;
 }
 
@@ -1274,12 +1284,33 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isCo
 
     qSwap(_qmlContext, qmlContext);
 
-    bool result = populateInstance(index, instance, /*binding target*/instance, /*value type property*/nullptr);
+    bool ok = populateInstance(index, instance, /*binding target*/instance, /*value type property*/nullptr);
+    if (ok) {
+        if (isContextObject && !pendingAliasBindings.empty()) {
+            bool processedAtLeastOneBinding = false;
+            do {
+                processedAtLeastOneBinding = false;
+                for (std::vector<PendingAliasBinding>::iterator it = pendingAliasBindings.begin();
+                        it != pendingAliasBindings.end(); ) {
+                    if ((*it)(sharedState.data())) {
+                        it = pendingAliasBindings.erase(it);
+                        processedAtLeastOneBinding = true;
+                    } else {
+                        ++it;
+                    }
+                }
+            } while (processedAtLeastOneBinding && pendingAliasBindings.empty());
+            Q_ASSERT(pendingAliasBindings.empty());
+        }
+    } else {
+        // an error occurred, so we can't setup the pending alias bindings
+        pendingAliasBindings.clear();
+    }
 
     qSwap(_qmlContext, qmlContext);
     qSwap(_scopeObject, scopeObject);
 
-    return result ? instance : nullptr;
+    return ok ? instance : nullptr;
 }
 
 QQmlContextData *QQmlObjectCreator::finalize(QQmlInstantiationInterrupt &interrupt)
diff --git a/src/qml/qml/qqmlobjectcreator_p.h b/src/qml/qml/qqmlobjectcreator_p.h
index 399a5f6d4a..67a5bdd827 100644
--- a/src/qml/qml/qqmlobjectcreator_p.h
+++ b/src/qml/qml/qqmlobjectcreator_p.h
@@ -161,6 +161,9 @@ private:
     QV4::QmlContext *_qmlContext;
 
     friend struct QQmlObjectCreatorRecursionWatcher;
+
+    typedef std::function<bool(QQmlObjectCreatorSharedState *sharedState)> PendingAliasBinding;
+    std::vector<PendingAliasBinding> pendingAliasBindings;
 };
 
 struct QQmlObjectCreatorRecursionWatcher
diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp
index ebf296b29d..c4487f91a3 100644
--- a/src/qml/qml/qqmlproperty.cpp
+++ b/src/qml/qml/qqmlproperty.cpp
@@ -877,6 +877,7 @@ void QQmlPropertyPrivate::findAliasTarget(QObject *object, QQmlPropertyIndex bin
 void QQmlPropertyPrivate::setBinding(QQmlAbstractBinding *binding, BindingFlags flags, QQmlPropertyData::WriteFlags writeFlags)
 {
     Q_ASSERT(binding);
+    Q_ASSERT(binding->targetObject());
 
     QObject *object = binding->targetObject();
     const QQmlPropertyIndex index = binding->targetPropertyIndex();