Restore value bindings when disabling a Binding element

We previously only restored script bindings that were replaced by
a Binding. Now we handle both.

[ChangeLog] QML Binding elements now support restoring previous values
of the bound property when the binding is disabled. This will be the
default behavior in Qt 5.15. Reliance on the old behavior of only
restoring binding, not literal values results in a warning now.

Fixes: QTBUG-33444
Change-Id: I833403b0645c08eee486fbd4acf5d3c7de2ef73a
Reviewed-by: Michael Brasser <michael.brasser@live.com>

7 files changed, 401 insertions, 5 deletions

diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index 8faace521a..f977934b3e 100644
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -204,6 +204,7 @@ void QQmlEnginePrivate::defineModule()
     qmlRegisterType<QObject>(uri, 2, 0, "QtObject");
     qmlRegisterType<QQmlBind>(uri, 2, 0, "Binding");
     qmlRegisterType<QQmlBind, 8>(uri, 2, 8, "Binding"); // Only available in >= 2.8
+    qmlRegisterType<QQmlBind, 14>(uri, 2, 14, "Binding");
     qmlRegisterCustomType<QQmlConnections>(uri, 2, 0, "Connections", new QQmlConnectionsParser);
     qmlRegisterCustomType<QQmlConnections, 1>(uri, 2, 3, "Connections", new QQmlConnectionsParser); // Only available in QtQml >= 2.3
 #if QT_CONFIG(qml_animation)
diff --git a/src/qml/types/qqmlbind.cpp b/src/qml/types/qqmlbind.cpp
index 513f7f2997..1ba015f796 100644
--- a/src/qml/types/qqmlbind.cpp
+++ b/src/qml/types/qqmlbind.cpp
@@ -43,6 +43,8 @@
 #include <private/qqmlproperty_p.h>
 #include <private/qqmlbinding_p.h>
 #include <private/qqmlmetatype_p.h>
+#include <private/qqmlvmemetaobject_p.h>
+#include <private/qv4persistent_p.h>
 
 #include <qqmlengine.h>
 #include <qqmlcontext.h>
@@ -60,7 +62,21 @@ QT_BEGIN_NAMESPACE
 class QQmlBindPrivate : public QObjectPrivate
 {
 public:
-    QQmlBindPrivate() : obj(nullptr), componentComplete(true), delayed(false), pendingEval(false) {}
+    QQmlBindPrivate()
+        : obj(nullptr)
+        , prevBind(QQmlAbstractBinding::Ptr())
+        , prevIsVariant(false)
+        , componentComplete(true)
+        , delayed(false)
+        , pendingEval(false)
+        , restoreBinding(true)
+#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
+        , restoreValue(false)
+        , restoreModeExplicit(false)
+#else
+        , restoreValue(true)
+#endif
+    {}
     ~QQmlBindPrivate() { }
 
     QQmlNullableValue<bool> when;
@@ -69,11 +85,20 @@ public:
     QQmlNullableValue<QVariant> value;
     QQmlProperty prop;
     QQmlAbstractBinding::Ptr prevBind;
+    QV4::PersistentValue v4Value;
+    QVariant prevValue;
+    bool prevIsVariant:1;
     bool componentComplete:1;
     bool delayed:1;
     bool pendingEval:1;
+    bool restoreBinding:1;
+    bool restoreValue:1;
+#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
+    bool restoreModeExplicit:1;
+#endif
 
     void validate(QObject *binding) const;
+    void clearPrev();
 };
 
 void QQmlBindPrivate::validate(QObject *binding) const
@@ -323,6 +348,57 @@ void QQmlBind::setDelayed(bool delayed)
         eval();
 }
 
+/*!
+    \qmlproperty enumeration QtQml::Binding::restoreMode
+    \since 5.14
+
+    This property can be used to describe if and how the original value should
+    be restored when the binding is disabled.
+
+    The possible values are:
+    \list
+    \li Binding.RestoreNone The original value is not restored at all
+    \li Binding.RestoreBinding The original value is restored if it was another
+        binding. In that case the old binding is in effect again.
+    \li Binding.RestoreValue The original value is restored if it was a plain
+        value rather than a binding.
+    \li Binding.RestoreBindingOrValue The original value is always restored.
+    \list
+
+    \warning The default value is Binding.RestoreBinding. This will change in
+    Qt 5.15 to Binding.RestoreBindingOrValue.
+
+    If you rely on any specific behavior regarding the restoration of plain
+    values when bindings get disabled you should migrate to explicitly set the
+    restoreMode.
+
+    Reliance on a restoreMode that doesn't restore the previous binding or value
+    for a specific property results in a run-time warning.
+*/
+QQmlBind::RestorationMode QQmlBind::restoreMode() const
+{
+    Q_D(const QQmlBind);
+    unsigned result = RestoreNone;
+    if (d->restoreValue)
+        result |= RestoreValue;
+    if (d->restoreBinding)
+        result |= RestoreBinding;
+    return RestorationMode(result);
+}
+
+void QQmlBind::setRestoreMode(RestorationMode newMode)
+{
+    Q_D(QQmlBind);
+    if (newMode != restoreMode()) {
+        d->restoreValue = (newMode & RestoreValue);
+        d->restoreBinding = (newMode & RestoreBinding);
+#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
+        d->restoreModeExplicit = true;
+#endif
+        emit restoreModeChanged();
+    }
+}
+
 void QQmlBind::setTarget(const QQmlProperty &p)
 {
     Q_D(QQmlBind);
@@ -358,6 +434,14 @@ void QQmlBind::prepareEval()
     }
 }
 
+void QQmlBindPrivate::clearPrev()
+{
+    prevBind = nullptr;
+    v4Value.clear();
+    prevValue.clear();
+    prevIsVariant = false;
+}
+
 void QQmlBind::eval()
 {
     Q_D(QQmlBind);
@@ -369,16 +453,64 @@ void QQmlBind::eval()
         if (!d->when) {
             //restore any previous binding
             if (d->prevBind) {
-                QQmlAbstractBinding::Ptr p = d->prevBind;
-                d->prevBind = nullptr;
-                QQmlPropertyPrivate::setBinding(p.data());
+                if (d->restoreBinding) {
+                    QQmlAbstractBinding::Ptr p = d->prevBind;
+                    d->clearPrev(); // Do that before setBinding(), as setBinding() may recurse.
+                    QQmlPropertyPrivate::setBinding(p.data());
+                }
+            } else if (!d->v4Value.isEmpty()) {
+                if (d->restoreValue) {
+                    auto propPriv = QQmlPropertyPrivate::get(d->prop);
+                    QQmlVMEMetaObject *vmemo = QQmlVMEMetaObject::get(propPriv->object);
+                    Q_ASSERT(vmemo);
+                    vmemo->setVMEProperty(propPriv->core.coreIndex(), *d->v4Value.valueRef());
+                    d->clearPrev();
+#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
+                } else if (!d->restoreModeExplicit) {
+                    qmlWarning(this)
+                            << "Not restoring previous value because restoreMode has not been set."
+                            << "This behavior is deprecated."
+                            << "In Qt < 5.15 the default is Binding.RestoreBinding."
+                            << "In Qt >= 5.15 the default is Binding.RestoreBindingOrValue.";
+#endif
+                }
+            } else if (d->prevIsVariant) {
+                if (d->restoreValue) {
+                    d->prop.write(d->prevValue);
+                    d->clearPrev();
+#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
+                } else if (!d->restoreModeExplicit) {
+                    qmlWarning(this)
+                            << "Not restoring previous value because restoreMode has not been set."
+                            << "This behavior is deprecated."
+                            << "In Qt < 5.15 the default is Binding.RestoreBinding."
+                            << "In Qt >= 5.15 the default is Binding.RestoreBindingOrValue.";
+#endif
+                }
             }
             return;
         }
 
         //save any set binding for restoration
-        if (!d->prevBind)
+        if (!d->prevBind && d->v4Value.isEmpty() && !d->prevIsVariant) {
+            // try binding first
             d->prevBind = QQmlPropertyPrivate::binding(d->prop);
+
+            if (!d->prevBind) { // nope, try a V4 value next
+                auto propPriv = QQmlPropertyPrivate::get(d->prop);
+                auto propData = propPriv->core;
+                if (!propPriv->valueTypeData.isValid() && propData.isVarProperty()) {
+                    QQmlVMEMetaObject *vmemo = QQmlVMEMetaObject::get(propPriv->object);
+                    Q_ASSERT(vmemo);
+                    auto retVal = vmemo->vmeProperty(propData.coreIndex());
+                    d->v4Value = QV4::PersistentValue(vmemo->engine, retVal);
+                } else { // nope, use the meta object to get a QVariant
+                    d->prevValue = d->prop.read();
+                    d->prevIsVariant = true;
+                }
+            }
+        }
+
         QQmlPropertyPrivate::removeBinding(d->prop);
     }
 
diff --git a/src/qml/types/qqmlbind_p.h b/src/qml/types/qqmlbind_p.h
index 5bf9ef85c6..22007a3d25 100644
--- a/src/qml/types/qqmlbind_p.h
+++ b/src/qml/types/qqmlbind_p.h
@@ -60,6 +60,15 @@ QT_BEGIN_NAMESPACE
 class QQmlBindPrivate;
 class Q_AUTOTEST_EXPORT QQmlBind : public QObject, public QQmlPropertyValueSource, public QQmlParserStatus
 {
+public:
+    enum RestorationMode {
+        RestoreNone    = 0x0,
+        RestoreBinding = 0x1,
+        RestoreValue   = 0x2,
+        RestoreBindingOrValue = RestoreBinding | RestoreValue
+    };
+
+private:
     Q_OBJECT
     Q_DECLARE_PRIVATE(QQmlBind)
     Q_INTERFACES(QQmlParserStatus)
@@ -69,6 +78,9 @@ class Q_AUTOTEST_EXPORT QQmlBind : public QObject, public QQmlPropertyValueSourc
     Q_PROPERTY(QVariant value READ value WRITE setValue)
     Q_PROPERTY(bool when READ when WRITE setWhen)
     Q_PROPERTY(bool delayed READ delayed WRITE setDelayed REVISION 8)
+    Q_PROPERTY(RestorationMode restoreMode READ restoreMode WRITE setRestoreMode
+               NOTIFY restoreModeChanged REVISION 14)
+    Q_ENUM(RestorationMode)
 
 public:
     QQmlBind(QObject *parent=nullptr);
@@ -89,6 +101,12 @@ public:
     bool delayed() const;
     void setDelayed(bool);
 
+    RestorationMode restoreMode() const;
+    void setRestoreMode(RestorationMode);
+
+Q_SIGNALS:
+    void restoreModeChanged();
+
 protected:
     void setTarget(const QQmlProperty &) override;
     void classBegin() override;
diff --git a/tests/auto/qml/qqmlbinding/data/restoreBinding2.qml b/tests/auto/qml/qqmlbinding/data/restoreBinding2.qml
new file mode 100644
index 0000000000..b42f975fe0
--- /dev/null
+++ b/tests/auto/qml/qqmlbinding/data/restoreBinding2.qml
@@ -0,0 +1,55 @@
+/****************************************************************************
+**
+** Copyright (C) 2019 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+import QtQuick 2.0
+import QtQml 2.14
+
+Rectangle {
+    height: 400
+    width: 400
+
+    Rectangle {
+        property bool when: true
+
+        id: myItem
+        objectName:  "myItem"
+        height: 300
+        width: 200
+    }
+
+    property var boundValue: 100
+
+    Binding {
+        restoreMode: Binding.RestoreValue
+        objectName: "theBinding"
+        target: myItem
+        property: "height"
+        when: myItem.when
+        value: boundValue
+    }
+}
diff --git a/tests/auto/qml/qqmlbinding/data/restoreBinding3.qml b/tests/auto/qml/qqmlbinding/data/restoreBinding3.qml
new file mode 100644
index 0000000000..1f63457ab6
--- /dev/null
+++ b/tests/auto/qml/qqmlbinding/data/restoreBinding3.qml
@@ -0,0 +1,56 @@
+/****************************************************************************
+**
+** Copyright (C) 2019 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+import QtQuick 2.0
+import QtQml 2.14
+
+Rectangle {
+    height: 400
+    width: 400
+
+    Rectangle {
+        property bool when: true
+
+        id: myItem
+        objectName:  "myItem"
+        height: 300
+        width: 200
+        property var foo: 42
+    }
+
+    property var boundValue: 13
+
+    Binding {
+        restoreMode: Binding.RestoreBindingOrValue
+        objectName: "theBinding"
+        target: myItem
+        property: "foo"
+        when: myItem.when
+        value: boundValue
+    }
+}
diff --git a/tests/auto/qml/qqmlbinding/data/restoreBinding4.qml b/tests/auto/qml/qqmlbinding/data/restoreBinding4.qml
new file mode 100644
index 0000000000..f34c3b40cf
--- /dev/null
+++ b/tests/auto/qml/qqmlbinding/data/restoreBinding4.qml
@@ -0,0 +1,59 @@
+/****************************************************************************
+**
+** Copyright (C) 2019 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+import QtQuick 2.0
+import QtQml 2.14
+
+Rectangle {
+    height: 400
+    width: 400
+
+    Rectangle {
+        property bool when: true
+
+        id: myItem
+        objectName:  "myItem"
+        height: 300
+        width: 200
+        property var foo: original
+        property bool fooCheck: foo === original && foo.bar() === 42
+    }
+
+    property var original: ({ bar: function() { return 42 } })
+
+    property var boundValue: 13
+
+    Binding {
+        restoreMode: Binding.RestoreBinding
+        objectName: "theBinding"
+        target: myItem
+        property: "foo"
+        when: myItem.when
+        value: boundValue
+    }
+}
diff --git a/tests/auto/qml/qqmlbinding/tst_qqmlbinding.cpp b/tests/auto/qml/qqmlbinding/tst_qqmlbinding.cpp
index 34cf21024d..717fd5dbd1 100644
--- a/tests/auto/qml/qqmlbinding/tst_qqmlbinding.cpp
+++ b/tests/auto/qml/qqmlbinding/tst_qqmlbinding.cpp
@@ -42,6 +42,9 @@ private slots:
     void binding();
     void whenAfterValue();
     void restoreBinding();
+    void restoreBindingValue();
+    void restoreBindingVarValue();
+    void restoreBindingJSValue();
     void restoreBindingWithLoop();
     void restoreBindingWithoutCrash();
     void deletedObject();
@@ -134,6 +137,78 @@ void tst_qqmlbinding::restoreBinding()
     delete rect;
 }
 
+void tst_qqmlbinding::restoreBindingValue()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, testFileUrl("restoreBinding2.qml"));
+    QScopedPointer<QQuickRectangle> rect(qobject_cast<QQuickRectangle*>(c.create()));
+    QVERIFY(!rect.isNull());
+
+    auto myItem = qobject_cast<QQuickRectangle*>(rect->findChild<QQuickRectangle*>("myItem"));
+    QVERIFY(myItem != nullptr);
+
+    QCOMPARE(myItem->height(), 100);
+    myItem->setProperty("when", QVariant(false));
+    QCOMPARE(myItem->height(), 300); // make sure the original value was restored
+
+    myItem->setProperty("when", QVariant(true));
+    QCOMPARE(myItem->height(), 100); // make sure the value specified in Binding is set
+    rect->setProperty("boundValue", 200);
+    QCOMPARE(myItem->height(), 200); // make sure the changed binding value is set
+    myItem->setProperty("when", QVariant(false));
+    // make sure that the original value is back, not e.g. the value from before the
+    // change (i.e. 100)
+    QCOMPARE(myItem->height(), 300);
+}
+
+void tst_qqmlbinding::restoreBindingVarValue()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, testFileUrl("restoreBinding3.qml"));
+    QScopedPointer<QQuickRectangle> rect(qobject_cast<QQuickRectangle*>(c.create()));
+    QVERIFY(!rect.isNull());
+
+    auto myItem = qobject_cast<QQuickRectangle*>(rect->findChild<QQuickRectangle*>("myItem"));
+    QVERIFY(myItem != nullptr);
+
+    QCOMPARE(myItem->property("foo"), 13);
+    myItem->setProperty("when", QVariant(false));
+    QCOMPARE(myItem->property("foo"), 42); // make sure the original value was restored
+
+    myItem->setProperty("when", QVariant(true));
+    QCOMPARE(myItem->property("foo"), 13); // make sure the value specified in Binding is set
+    rect->setProperty("boundValue", 31337);
+    QCOMPARE(myItem->property("foo"), 31337); // make sure the changed binding value is set
+    myItem->setProperty("when", QVariant(false));
+    // make sure that the original value is back, not e.g. the value from before the
+    // change (i.e. 100)
+    QCOMPARE(myItem->property("foo"), 42);
+}
+
+void tst_qqmlbinding::restoreBindingJSValue()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, testFileUrl("restoreBinding4.qml"));
+    QScopedPointer<QQuickRectangle> rect(qobject_cast<QQuickRectangle*>(c.create()));
+    QVERIFY(!rect.isNull());
+
+    auto myItem = qobject_cast<QQuickRectangle*>(rect->findChild<QQuickRectangle*>("myItem"));
+    QVERIFY(myItem != nullptr);
+
+    QCOMPARE(myItem->property("fooCheck"), false);
+    myItem->setProperty("when", QVariant(false));
+    QCOMPARE(myItem->property("fooCheck"), true); // make sure the original value was restored
+
+    myItem->setProperty("when", QVariant(true));
+    QCOMPARE(myItem->property("fooCheck"), false); // make sure the value specified in Binding is set
+    rect->setProperty("boundValue", 31337);
+    QCOMPARE(myItem->property("fooCheck"), false); // make sure the changed binding value is set
+    myItem->setProperty("when", QVariant(false));
+    // make sure that the original value is back, not e.g. the value from before the change
+    QCOMPARE(myItem->property("fooCheck"), true);
+
+}
+
 void tst_qqmlbinding::restoreBindingWithLoop()
 {
     QQmlEngine engine;