QProperty: Steal currentCompatProperty while evaluating a different one

currentCompatProperty should point to the compat property that's
currently being evaluated. As soon as we start evaluating a new compat
property, it's invalid by definition. Temporarily disable it then.

Pick-to: 6.6 6.5 6.2
Fixes: QTBUG-109465
Change-Id: I7baba9350ebf488370a63a71f0f8dbd7516bf578
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>

2 files changed, 140 insertions, 5 deletions

diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h
index a263fe3730..a3f846364a 100644
--- a/src/corelib/kernel/qproperty_p.h
+++ b/src/corelib/kernel/qproperty_p.h
@@ -225,6 +225,33 @@ struct CompatPropertySafePoint
     QtPrivate::BindingEvaluationState *bindingState = nullptr;
 };
 
+/*!
+ * \internal
+ * While the regular QProperty notification for a compat property runs we
+ * don't want to have any currentCompatProperty set. This would be a _different_
+ * one than the one we are current evaluating. Therefore it's misleading and
+ * prevents the registering of actual dependencies.
+ */
+struct CurrentCompatPropertyThief
+{
+    Q_DISABLE_COPY_MOVE(CurrentCompatPropertyThief)
+public:
+    CurrentCompatPropertyThief(QBindingStatus *status)
+        : status(&status->currentCompatProperty)
+        , stolen(std::exchange(status->currentCompatProperty, nullptr))
+    {
+    }
+
+    ~CurrentCompatPropertyThief()
+    {
+        *status = stolen;
+    }
+
+private:
+    CompatPropertySafePoint **status = nullptr;
+    CompatPropertySafePoint *stolen = nullptr;
+};
+
 }
 
 class Q_CORE_EXPORT QPropertyBindingPrivate : public QtPrivate::RefCounted
@@ -493,13 +520,16 @@ class QObjectCompatProperty : public QPropertyData<T>
     static bool bindingWrapper(QMetaType type, QUntypedPropertyData *dataPtr, QtPrivate::QPropertyBindingFunction binding)
     {
         auto *thisData = static_cast<ThisType *>(dataPtr);
+        QBindingStorage *storage = qGetBindingStorage(thisData->owner());
         QPropertyData<T> copy;
-        binding.vtable->call(type, &copy, binding.functor);
-        if constexpr (QTypeTraits::has_operator_equal_v<T>)
-            if (copy.valueBypassingBindings() == thisData->valueBypassingBindings())
-                return false;
+        {
+            QtPrivate::CurrentCompatPropertyThief thief(storage->bindingStatus);
+            binding.vtable->call(type, &copy, binding.functor);
+            if constexpr (QTypeTraits::has_operator_equal_v<T>)
+                if (copy.valueBypassingBindings() == thisData->valueBypassingBindings())
+                    return false;
+        }
         // ensure value and setValue know we're currently evaluating our binding
-        QBindingStorage *storage = qGetBindingStorage(thisData->owner());
         QtPrivate::CompatPropertySafePoint guardThis(storage->bindingStatus, thisData);
         (thisData->owner()->*Setter)(copy.valueBypassingBindings());
         return true;
diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
index acbeea4730..28d9a6eea4 100644
--- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
+++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
@@ -106,6 +106,7 @@ private slots:
     void notifyAfterAllDepsGone();
 
     void propertyAdaptorBinding();
+    void propertyUpdateViaSignaledProperty();
 };
 
 void tst_QProperty::functorBinding()
@@ -2372,6 +2373,110 @@ void tst_QProperty::notifyAfterAllDepsGone()
     QCOMPARE(changeCounter, 2);
 }
 
+class TestObject : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(int signaled READ signaled WRITE setSignaled NOTIFY signaledChanged FINAL)
+    Q_PROPERTY(int bindable1 READ bindable1 WRITE setBindable1 BINDABLE bindable1Bindable NOTIFY bindable1Changed FINAL)
+    Q_PROPERTY(int bindable2 READ bindable2 WRITE setBindable2 BINDABLE bindable2Bindable NOTIFY bindable2Changed FINAL)
+
+public:
+    int signaled() const
+    {
+        return m_signaled;
+    }
+
+    void setSignaled(int newSignaled)
+    {
+        if (m_signaled == newSignaled)
+            return;
+        m_signaled = newSignaled;
+        emit signaledChanged();
+    }
+
+    int bindable1() const
+    {
+        return m_bindable1;
+    }
+
+    void setBindable1(int newBindable1)
+    {
+        if (m_bindable1 == newBindable1)
+            return;
+        m_bindable1 = newBindable1;
+        emit bindable1Changed();
+    }
+
+    QBindable<int> bindable1Bindable()
+    {
+        return QBindable<int>(&m_bindable1);
+    }
+
+    int bindable2() const
+    {
+        return m_bindable2;
+    }
+
+    void setBindable2(int newBindable2)
+    {
+        if (m_bindable2 == newBindable2)
+            return;
+        m_bindable2 = newBindable2;
+        emit bindable2Changed();
+    }
+
+    QBindable<int> bindable2Bindable()
+    {
+        return QBindable<int>(&m_bindable2);
+    }
+
+signals:
+    void signaledChanged();
+    void bindable1Changed();
+    void bindable2Changed();
+
+private:
+    int m_signaled = 0;
+    Q_OBJECT_COMPAT_PROPERTY(TestObject, int, m_bindable1, &TestObject::setBindable1, &TestObject::bindable1Changed);
+    Q_OBJECT_COMPAT_PROPERTY(TestObject, int, m_bindable2, &TestObject::setBindable2, &TestObject::bindable2Changed);
+};
+
+void tst_QProperty::propertyUpdateViaSignaledProperty()
+{
+    TestObject o;
+    QProperty<int> rootTrigger;
+    QProperty<int> signalTrigger;
+
+    o.bindable1Bindable().setBinding([&]() {
+        return rootTrigger.value();
+    });
+
+    QObject::connect(&o, &TestObject::bindable1Changed, &o, [&]() {
+        // Signaled changes only once, doesn't actually depend on bindable1.
+        // In reality, there could be some complicated calculation behind this that changes
+        // on certain checkpoints, but not on every iteration.
+        o.setSignaled(40);
+    });
+
+    o.bindable2Bindable().setBinding([&]() {
+        return signalTrigger.value() - o.bindable1();
+    });
+
+    QObject::connect(&o, &TestObject::signaledChanged, &o, [&]() {
+        signalTrigger.setValue(o.signaled());
+    });
+
+    rootTrigger.setValue(2);
+    QCOMPARE(o.bindable1(), 2);
+    QCOMPARE(o.bindable2(), 38);
+    rootTrigger.setValue(3);
+    QCOMPARE(o.bindable1(), 3);
+    QCOMPARE(o.bindable2(), 37);
+    rootTrigger.setValue(4);
+    QCOMPARE(o.bindable1(), 4);
+    QCOMPARE(o.bindable2(), 36);
+}
+
 QTEST_MAIN(tst_QProperty);
 
 #undef QT_SOURCE_LOCATION_NAMESPACE