ParentChange: handle bindable properties

QQuickParentChange::actions was unable to handle new style properties.
Fix this by using a QQmlAnyBinding create function instead of
QQmlBinding.

This required adding the necessary support for creation from a
QQmlScriptString to QQmlProperyBinding and QQmlAnyBinding.

Extra care had to be taken in reverseRewindHelper, which assumed that
setting x/y/width/height would keep binding intact. We therefore use
setValueBypassingBindings instead of the plain setter.

Fixes: QTBUG-97480
Change-Id: I0cc6d61c655d9d37846adf4b09fe103f507bb329
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
(cherry picked from commit 785addcd17c7988be90a2ca17599c12b07e4e170)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

7 files changed, 139 insertions, 16 deletions

diff --git a/src/qml/qml/qqmlanybinding_p.h b/src/qml/qml/qqmlanybinding_p.h
index c999b9dce7..a83d4f3752 100644
--- a/src/qml/qml/qqmlanybinding_p.h
+++ b/src/qml/qml/qqmlanybinding_p.h
@@ -149,6 +149,30 @@ public:
 
     /*!
         \internal
+        Creates a binding for property \a prop from \a script.
+        \a obj is the scope object which shall be used for the function and \a ctxt its QML scope.
+        The binding is not installed on the property (but if a QQmlBinding is created, it has its
+        target set to \a prop).
+     */
+    static QQmlAnyBinding createFromScriptString(const QQmlProperty &prop, const QQmlScriptString &script,
+                                                  QObject *obj,  QQmlContext *ctxt)
+    {
+        QQmlAnyBinding binding;
+        auto propPriv = QQmlPropertyPrivate::get(prop);
+        if (prop.isBindable()) {
+            auto index = QQmlPropertyIndex(propPriv->core.coreIndex(), -1);
+            binding = QQmlPropertyBinding::createFromScriptString(&propPriv->core, script, obj, ctxt, prop.object(), index);
+        } else {
+            auto qmlBinding = QQmlBinding::create(&propPriv->core, script, obj, ctxt);
+            qmlBinding->setTarget(prop);
+            binding = qmlBinding;
+        }
+        return binding;
+    }
+
+
+    /*!
+        \internal
         Removes the binding from \a prop if there is any.
      */
     static void removeBindingFrom(QQmlProperty &prop)
diff --git a/src/qml/qml/qqmlpropertybinding.cpp b/src/qml/qml/qqmlpropertybinding.cpp
index 82c873e687..57a0339e93 100644
--- a/src/qml/qml/qqmlpropertybinding.cpp
+++ b/src/qml/qml/qqmlpropertybinding.cpp
@@ -42,6 +42,9 @@
 #include <private/qv4jscall_p.h>
 #include <qqmlinfo.h>
 #include <QtCore/qloggingcategory.h>
+#include <private/qqmlscriptstring_p.h>
+#include <private/qqmlbinding_p.h>
+#include <private/qv4qmlcontext_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -91,6 +94,44 @@ QUntypedPropertyBinding QQmlPropertyBinding::createFromCodeString(const QQmlProp
     return QUntypedPropertyBinding(static_cast<QPropertyBindingPrivate *>(QPropertyBindingPrivatePtr(binding).data()));
 }
 
+QUntypedPropertyBinding QQmlPropertyBinding::createFromScriptString(const QQmlPropertyData *property, const QQmlScriptString &script, QObject *obj, QQmlContext *ctxt, QObject *target, QQmlPropertyIndex targetIndex)
+{
+    const QQmlScriptStringPrivate *scriptPrivate = script.d.data();
+    // without a valid context, we cannot create anything
+    if (!ctxt && (!scriptPrivate->context || !scriptPrivate->context->isValid())) {
+        return {};
+    }
+
+    auto scopeObject = obj ? obj : scriptPrivate->scope;
+
+    QV4::Function *runtimeFunction = nullptr;
+    QString url;
+    QQmlRefPointer<QQmlContextData> ctxtdata = QQmlContextData::get(scriptPrivate->context);
+    QQmlEnginePrivate *engine = QQmlEnginePrivate::get(scriptPrivate->context->engine());
+    if (engine && ctxtdata && !ctxtdata->urlString().isEmpty() && ctxtdata->typeCompilationUnit()) {
+        url = ctxtdata->urlString();
+        if (scriptPrivate->bindingId != QQmlBinding::Invalid)
+            runtimeFunction = ctxtdata->typeCompilationUnit()->runtimeFunctions.at(scriptPrivate->bindingId);
+    }
+    // Do we actually have a function in the script string? If not, this becomes createCodeFromString
+    if (!runtimeFunction)
+        return createFromCodeString(property, scriptPrivate->script, obj, ctxtdata, url, scriptPrivate->lineNumber, target, targetIndex);
+
+    auto buffer = new std::byte[QQmlPropertyBinding::getSizeEnsuringAlignment()
+            + sizeof(QQmlPropertyBindingJS)+jsExpressionOffsetLength()]; // QQmlPropertyBinding uses delete[]
+    auto binding = new(buffer) QQmlPropertyBinding(QMetaType(property->propType()), target, targetIndex, TargetData::WithoutBoundFunction);
+    auto js = new(buffer + QQmlPropertyBinding::getSizeEnsuringAlignment() + jsExpressionOffsetLength()) QQmlPropertyBindingJS();
+    Q_ASSERT(binding->jsExpression() == js);
+    Q_ASSERT(js->asBinding() == binding);
+    js->setContext(QQmlContextData::get(ctxt ? ctxt : scriptPrivate->context));
+
+    QV4::ExecutionEngine *v4 = engine->v4engine();
+    QV4::Scope scope(v4);
+    QV4::Scoped<QV4::QmlContext> qmlContext(scope, QV4::QmlContext::create(v4->rootContext(), ctxtdata, scopeObject));
+    js->setupFunction(qmlContext, runtimeFunction);
+    return QUntypedPropertyBinding(static_cast<QPropertyBindingPrivate *>(QPropertyBindingPrivatePtr(binding).data()));
+}
+
 QUntypedPropertyBinding QQmlPropertyBinding::createFromBoundFunction(const QQmlPropertyData *pd, QV4::BoundFunction *function, QObject *obj, const QQmlRefPointer<QQmlContextData> &ctxt, QV4::ExecutionContext *scope, QObject *target, QQmlPropertyIndex targetIndex)
 {
     auto buffer = new std::byte[QQmlPropertyBinding::getSizeEnsuringAlignment()
diff --git a/src/qml/qml/qqmlpropertybinding_p.h b/src/qml/qml/qqmlpropertybinding_p.h
index c2af93869a..17f74d5675 100644
--- a/src/qml/qml/qqmlpropertybinding_p.h
+++ b/src/qml/qml/qqmlpropertybinding_p.h
@@ -129,6 +129,10 @@ public:
                                                         const QQmlRefPointer<QQmlContextData> &ctxt,
                                                         const QString &url, quint16 lineNumber,
                                                         QObject *target, QQmlPropertyIndex targetIndex);
+    static QUntypedPropertyBinding createFromScriptString(const QQmlPropertyData *property,
+                                                          const QQmlScriptString& script, QObject *obj,
+                                                          QQmlContext *ctxt, QObject *target,
+                                                          QQmlPropertyIndex targetIndex);
 
     static QUntypedPropertyBinding createFromBoundFunction(const QQmlPropertyData *pd, QV4::BoundFunction *function,
                                           QObject *obj, const QQmlRefPointer<QQmlContextData> &ctxt,
diff --git a/src/qml/qml/qqmlscriptstring.h b/src/qml/qml/qqmlscriptstring.h
index a768dabf9f..d2ccfe5bc6 100644
--- a/src/qml/qml/qqmlscriptstring.h
+++ b/src/qml/qml/qqmlscriptstring.h
@@ -86,6 +86,7 @@ private:
     friend class QQmlScriptStringPrivate;
     friend class QQmlExpression;
     friend class QQmlBinding;
+    friend class QQmlPropertyBinding;
     friend struct QV4::QObjectWrapper;
 };
 
diff --git a/src/quick/items/qquickstateoperations.cpp b/src/quick/items/qquickstateoperations.cpp
index 00cfc714d7..a62d0e8bc2 100644
--- a/src/quick/items/qquickstateoperations.cpp
+++ b/src/quick/items/qquickstateoperations.cpp
@@ -359,8 +359,7 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
             actions << xa;
         } else {
             QQmlProperty property(d->target, QLatin1String("x"));
-            QQmlBinding *newBinding = QQmlBinding::create(&QQmlPropertyPrivate::get(property)->core, d->xString.value, d->target, qmlContext(this));
-            newBinding->setTarget(property);
+            auto newBinding = QQmlAnyBinding::createFromScriptString(property, d->xString.value, d->target, qmlContext(this));
             QQuickStateAction xa;
             xa.property = property;
             xa.toBinding = newBinding;
@@ -378,8 +377,7 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
             actions << ya;
         } else {
             QQmlProperty property(d->target, QLatin1String("y"));
-            QQmlBinding *newBinding = QQmlBinding::create(&QQmlPropertyPrivate::get(property)->core, d->yString.value, d->target, qmlContext(this));
-            newBinding->setTarget(property);
+            auto newBinding = QQmlAnyBinding::createFromScriptString(property, d->yString.value, d->target, qmlContext(this));
             QQuickStateAction ya;
             ya.property = property;
             ya.toBinding = newBinding;
@@ -397,8 +395,7 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
             actions << sa;
         } else {
             QQmlProperty property(d->target, QLatin1String("scale"));
-            QQmlBinding *newBinding = QQmlBinding::create(&QQmlPropertyPrivate::get(property)->core, d->scaleString.value, d->target, qmlContext(this));
-            newBinding->setTarget(property);
+            auto newBinding = QQmlAnyBinding::createFromScriptString(property, d->scaleString.value, d->target, qmlContext(this));
             QQuickStateAction sa;
             sa.property = property;
             sa.toBinding = newBinding;
@@ -416,8 +413,7 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
             actions << ra;
         } else {
             QQmlProperty property(d->target, QLatin1String("rotation"));
-            QQmlBinding *newBinding = QQmlBinding::create(&QQmlPropertyPrivate::get(property)->core, d->rotationString.value, d->target, qmlContext(this));
-            newBinding->setTarget(property);
+            auto newBinding = QQmlAnyBinding::createFromScriptString(property, d->rotationString.value, d->target, qmlContext(this));
             QQuickStateAction ra;
             ra.property = property;
             ra.toBinding = newBinding;
@@ -435,8 +431,7 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
             actions << wa;
         } else {
             QQmlProperty property(d->target, QLatin1String("width"));
-            QQmlBinding *newBinding = QQmlBinding::create(&QQmlPropertyPrivate::get(property)->core, d->widthString.value, d->target, qmlContext(this));
-            newBinding->setTarget(property);
+            auto newBinding = QQmlAnyBinding::createFromScriptString(property, d->widthString, d->target, qmlContext(this));
             QQuickStateAction wa;
             wa.property = property;
             wa.toBinding = newBinding;
@@ -454,8 +449,7 @@ QQuickStateOperation::ActionList QQuickParentChange::actions()
             actions << ha;
         } else {
             QQmlProperty property(d->target, QLatin1String("height"));
-            QQmlBinding *newBinding = QQmlBinding::create(&QQmlPropertyPrivate::get(property)->core, d->heightString.value, d->target, qmlContext(this));
-            newBinding->setTarget(property);
+            auto newBinding = QQmlAnyBinding::createFromScriptString(property, d->heightString, d->target, qmlContext(this));
             QQuickStateAction ha;
             ha.property = property;
             ha.toBinding = newBinding;
@@ -492,11 +486,13 @@ void QQuickParentChangePrivate::reverseRewindHelper(const std::unique_ptr<QQuick
 {
     if (!target || !snapshot)
         return;
-    target->setX(snapshot->x);
-    target->setY(snapshot->y);
+    auto targetPriv = QQuickItemPrivate::get(target);
+    // leave existing bindings alive; new bindings are applied in applyBindings
+    targetPriv->x.setValueBypassingBindings(snapshot->x);
+    targetPriv->y.setValueBypassingBindings(snapshot->y);
+    targetPriv->width.setValueBypassingBindings(snapshot->width);
+    targetPriv->height.setValueBypassingBindings(snapshot->height);
     target->setScale(snapshot->scale);
-    target->setWidth(snapshot->width);
-    target->setHeight(snapshot->height);
     target->setRotation(snapshot->rotation);
     target->setParentItem(snapshot->parent);
     if (snapshot->stackBefore)
diff --git a/tests/auto/quick/qquickstates/data/parentChangeInvolvingBindings.qml b/tests/auto/quick/qquickstates/data/parentChangeInvolvingBindings.qml
new file mode 100644
index 0000000000..9680e806b8
--- /dev/null
+++ b/tests/auto/quick/qquickstates/data/parentChangeInvolvingBindings.qml
@@ -0,0 +1,27 @@
+import QtQuick
+
+Item {
+    id: root
+    property alias childWidth: firstChild.width
+    property alias childRotation: firstChild.rotation
+    property double myrotation: 100
+    property double myrotation2: 200
+    height: 400
+
+    Item {
+        id: firstChild
+        height: parent.height
+        width: height
+        rotation: root.myrotation
+    }
+
+    states: State {
+        name: "reparented"
+        ParentChange { target: firstChild; parent: otherChild; width: 2*height; rotation:  root.myrotation2}
+    }
+
+    Item {
+        height: parent.height
+        id: otherChild
+    }
+}
diff --git a/tests/auto/quick/qquickstates/tst_qquickstates.cpp b/tests/auto/quick/qquickstates/tst_qquickstates.cpp
index 23a7691765..87d39228f4 100644
--- a/tests/auto/quick/qquickstates/tst_qquickstates.cpp
+++ b/tests/auto/quick/qquickstates/tst_qquickstates.cpp
@@ -202,6 +202,7 @@ private slots:
     void parentChangeCorrectReversal();
     void revertNullObjectBinding();
     void bindableProperties();
+    void parentChangeInvolvingBindings();
 };
 
 void tst_qquickstates::initTestCase()
@@ -1779,6 +1780,35 @@ void tst_qquickstates::bindableProperties()
     }
 }
 
+void tst_qquickstates::parentChangeInvolvingBindings()
+{
+   QQmlEngine engine;
+   QQmlComponent c(&engine, testFileUrl("parentChangeInvolvingBindings.qml"));
+   QScopedPointer<QQuickItem> root { qobject_cast<QQuickItem *>(c.create()) };
+   QVERIFY2(root, qPrintable(c.errorString()));
+   QCOMPARE(root->property("childWidth").toInt(), 400);
+   QCOMPARE(root->property("childRotation").toInt(), 100);
+   root->setState("reparented");
+
+   QCOMPARE(root->property("childWidth").toInt(), 800);
+   QCOMPARE(root->property("childRotation").toInt(), 200);
+
+   root->setProperty("myrotation2", 300);
+   root->setHeight(200);
+   QCOMPARE(root->property("childRotation").toInt(), 300);
+   QCOMPARE(root->property("childWidth").toInt(), 400);
+
+   root->setState("");
+   QCOMPARE(root->property("childRotation").toInt(), 100);
+   // QCOMPARE(root->property("childWidth").toInt(), 200);
+
+
+   root->setProperty("myrotation", 50);
+   root->setHeight(300);
+   QCOMPARE(root->property("childWidth").toInt(), 300);
+   QCOMPARE(root->property("childRotation").toInt(), 50);
+}
+
 QTEST_MAIN(tst_qquickstates)
 
 #include "tst_qquickstates.moc"