QProperty: Avoid spurious dependencies by suspending binding state

Avoid spurious bindings by resetting the binding state before calling
the setter of eager properties.

Fixes: QTBUG-88999
Pick-to: 6.0
Change-Id: I1e3b5662307d906598335a21d306be9c606529d4
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

src/corelib/kernel/qproperty.cpp

@@ -303,7 +303,7 @@ BindingEvaluationState::BindingEvaluationState(QPropertyBindingPrivate *binding,
     binding->clearDependencyObservers();
 }
 
-CurrentCompatProperty::CurrentCompatProperty(QBindingStatus *status, QUntypedPropertyData *property)
+CompatPropertySafePoint::CompatPropertySafePoint(QBindingStatus *status, QUntypedPropertyData *property)
     : property(property)
 {
     // store a pointer to the currentBindingEvaluationState to avoid a TLS lookup in
@@ -311,6 +311,10 @@ CurrentCompatProperty::CurrentCompatProperty(QBindingStatus *status, QUntypedPro
     currentState = &status->currentCompatProperty;
     previousState = *currentState;
     *currentState = this;
+
+    currentlyEvaluatingBindingList = &bindingStatus.currentlyEvaluatingBinding;
+    bindingState = *currentlyEvaluatingBindingList;
+    *currentlyEvaluatingBindingList = nullptr;
 }
 
 QPropertyBindingPrivate *QPropertyBindingPrivate::currentlyEvaluatingBinding()
@@ -1486,4 +1490,17 @@ QPropertyBindingData *QBindingStorage::bindingData_helper(QUntypedPropertyData *
     return QBindingStoragePrivate(d).get(data, create);
 }
 
+
+BindingEvaluationState *suspendCurrentBindingStatus()
+{
+    auto ret = bindingStatus.currentlyEvaluatingBinding;
+    bindingStatus.currentlyEvaluatingBinding = nullptr;
+    return ret;
+}
+
+void restoreBindingStatus(BindingEvaluationState *status)
+{
+    bindingStatus.currentlyEvaluatingBinding = status;
+}
+
 QT_END_NAMESPACE

src/corelib/kernel/qproperty.h

@@ -783,13 +783,13 @@ public:
 namespace QtPrivate {
 
 struct BindingEvaluationState;
-struct CurrentCompatProperty;
+struct CompatPropertySafePoint;
 }
 
 struct QBindingStatus
 {
     QtPrivate::BindingEvaluationState *currentlyEvaluatingBinding = nullptr;
-    QtPrivate::CurrentCompatProperty *currentCompatProperty = nullptr;
+    QtPrivate::CompatPropertySafePoint *currentCompatProperty = nullptr;
 };
 
 struct QBindingStorageData;

src/corelib/kernel/qproperty_p.h

@@ -134,16 +134,26 @@ struct BindingEvaluationState
     BindingEvaluationState **currentState = nullptr;
 };
 
-struct CurrentCompatProperty
+/*!
+ * \internal
+ * CompatPropertySafePoint needs to be constructed before the setter of
+ * a QObjectCompatProperty runs. It prevents spurious binding dependencies
+ * caused by reads of properties inside the compat property setter.
+ * Moreover, it ensures that we don't destroy bindings when using operator=
+ */
+struct CompatPropertySafePoint
 {
-    Q_CORE_EXPORT CurrentCompatProperty(QBindingStatus *status, QUntypedPropertyData *property);
-    ~CurrentCompatProperty()
+    Q_CORE_EXPORT CompatPropertySafePoint(QBindingStatus *status, QUntypedPropertyData *property);
+    ~CompatPropertySafePoint()
     {
         *currentState = previousState;
+        *currentlyEvaluatingBindingList = bindingState;
     }
     QUntypedPropertyData *property;
-    CurrentCompatProperty *previousState = nullptr;
-    CurrentCompatProperty **currentState = nullptr;
+    CompatPropertySafePoint *previousState = nullptr;
+    CompatPropertySafePoint **currentState = nullptr;
+    QtPrivate::BindingEvaluationState **currentlyEvaluatingBindingList = nullptr;
+    QtPrivate::BindingEvaluationState *bindingState = nullptr;
 };
 
 }
@@ -360,7 +370,7 @@ class QObjectCompatProperty : public QPropertyData<T>
                 return false;
         // ensure value and setValue know we're currently evaluating our binding
         QBindingStorage *storage = qGetBindingStorage(thisData->owner());
-        QtPrivate::CurrentCompatProperty guardThis(storage->bindingStatus, thisData);
+        QtPrivate::CompatPropertySafePoint guardThis(storage->bindingStatus, thisData);
         (thisData->owner()->*Setter)(copy.valueBypassingBindings());
         return true;
     }

tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp

@@ -1401,7 +1401,6 @@ void tst_QProperty::noFakeDependencies()
     QCOMPARE(slotCounter, 1); // sanity check
     int old = bindingFunctionCalled;
     fdc.setProp3(100);
-    QEXPECT_FAIL("", "Known to create a spurious dependency", Continue);
     QCOMPARE(old, bindingFunctionCalled);
 }