QQmlPropertyBinding: handle reset

Bindings are allowed to toggle between a defined state, and undefined
which calls the property's reset function. Calls to the reset function
must not remove the binding, even when they write to the property.

To support this, we put the binding in a special undefined state, in
which it is still active (and installed on the property), but does not
actually provide its evaluated value to the property. Then, when the
binding later becomes defined again, the binding leaves its undefined
state and works normally again.

Notes:
- Calling the reset function during binding evaluation could have all
  kinds of unwelcome side-effects. We therefore have to suspend binding
  evaluation before the reset call (and restore that state afterwards).
- QObjectCompatProperty expects that we write the current value into the
  propertyDataPtr. If we do not do this, it will overwrite the current
  value with the default constructed value of its property. Arguably, we
  should change the API so that we communicate that nothing has changed;
  but for now, we have to live with that limitation and read the
  current value and write it back again.
- We currently do not handle the case correctly where a non-resettable
  property implemented via QObjectCompatProperty gets assigned undefined
  in a binding. Such a binding is likely unintentional (as the undefined
  assignment only creates a warning), and thus less of a priority.
  Nevertheless, a test marked with QEXPECT_FAIL is added for it.

Fixes: QTBUG-91001
Change-Id: I7ecaa6c8dc1a1f1b33e67b1af65f552c4ca6ffb1
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

1 files changed, 80 insertions, 0 deletions

diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index ce0eb5f5a1..43fb3fc1eb 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -382,6 +382,10 @@ private slots:
     void saveAccumulatorBeforeToInt32();
     void intMinDividedByMinusOne();
     void undefinedPropertiesInObjectWrapper();
+    void qpropertyBindingHandlesUndefinedCorrectly_data();
+    void qpropertyBindingHandlesUndefinedCorrectly();
+    void qpropertyBindingHandlesUndefinedWithoutResetCorrectly_data();
+    void qpropertyBindingHandlesUndefinedWithoutResetCorrectly();
     void hugeRegexpQuantifiers();
     void singletonTypeWrapperLookup();
     void getThisObject();
@@ -9193,6 +9197,82 @@ void tst_qqmlecmascript::undefinedPropertiesInObjectWrapper()
     QVERIFY(!object.isNull());
 }
 
+void tst_qqmlecmascript::qpropertyBindingHandlesUndefinedCorrectly_data()
+{
+    QTest::addColumn<QString>("fileName");
+    QTest::addRow("QProperty") << "qpropertyBindingUndefined.qml";
+    QTest::addRow("QObjectProperty") << "qpropertyBindingUndefined2.qml";
+}
+
+void tst_qqmlecmascript::qpropertyBindingHandlesUndefinedCorrectly()
+{
+    QFETCH(QString, fileName);
+    QQmlEngine engine;
+    QQmlComponent component(&engine, testFileUrl(fileName));
+    QScopedPointer<QObject> root(component.create());
+    QVERIFY2(root, qPrintable(component.errorString()));
+    // sanity check
+    QCOMPARE(root->property("value").toInt(), 1);
+    // If the binding evaluates to undefined,
+    root->setProperty("toggle", true);
+    // then the property gets reset.
+    QCOMPARE(root->property("value").toInt(), 2);
+    // If the binding reevaluates, the value should not change
+    root->setProperty("anotherValue", 3);
+    QCOMPARE(root->property("value").toInt(), 2);
+    // When the binding becomes defined again
+    root->setProperty("toggle", false);
+    // we obtain the correct new value.
+    QCOMPARE(root->property("value").toInt(), 3);
+}
+
+struct ClassWithQCompatProperty : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(int value READ value WRITE setValue BINDABLE bindableValue RESET resetValue)
+    Q_PROPERTY(int value2 READ value2 WRITE setValue2 BINDABLE bindableValue2)
+public:
+    QBindable<int> bindableValue() {return &m_value;}
+    void resetValue() { m_value.setValue(2); }
+    int value() { return m_value; }
+    void setValue(int i) { m_value.setValue(i); }
+    Q_OBJECT_COMPAT_PROPERTY(ClassWithQCompatProperty, int, m_value, &ClassWithQCompatProperty::setValue);
+
+    QBindable<int> bindableValue2() {return &m_value2;}
+    int value2() { return m_value2; }
+    void setValue2(int i) { m_value2.setValue(i); }
+    Q_OBJECT_COMPAT_PROPERTY(ClassWithQCompatProperty, int, m_value2, &ClassWithQCompatProperty::setValue2);
+};
+
+void tst_qqmlecmascript::qpropertyBindingHandlesUndefinedWithoutResetCorrectly_data()
+{
+        qmlRegisterType<ClassWithQCompatProperty>("Qt.test", 1, 0, "ClassWithQCompatProperty");
+    QTest::addColumn<QString>("fileName");
+    QTest::addRow("QProperty") << "qpropertyBindingUndefinedWithoutReset1.qml";
+    QTest::addRow("QObjectProperty") << "qpropertyBindingUndefinedWithoutReset2.qml";
+    QTest::addRow("QCompatProperty") << "qpropertyBindingUndefinedWithoutReset3.qml";
+}
+
+void tst_qqmlecmascript::qpropertyBindingHandlesUndefinedWithoutResetCorrectly()
+{
+    QQmlEngine engine;
+    QFETCH(QString, fileName);
+    QQmlComponent component(&engine, testFileUrl(fileName));
+    QScopedPointer<QObject> root(component.create());
+    QVERIFY2(root, qPrintable(component.errorString()));
+    // sanity check
+    QCOMPARE(root->property("value2").toInt(), 1);
+    // If the binding evaluates to undefined,
+    root->setProperty("toggle", true);
+    // then the value still stays the same.
+    QEXPECT_FAIL("QCompatProperty", "Not implemented for QObjectCompatProperty", Continue);
+    QCOMPARE(root->property("value2").toInt(), 1);
+    // and the binding is still active
+    root->setProperty("anotherValue", 2);
+    root->setProperty("toggle", false);
+    QCOMPARE(root->property("value2").toInt(), 2);
+}
+
 void tst_qqmlecmascript::hugeRegexpQuantifiers()
 {
     QJSEngine engine;