QProperty: Fix notification logic for eager properties

This ensurse that we do not do dobule notifications in setValue.
Moerover we avoid needless notifications in markDirtyAndNotifyObservers
when the value did not change. Lastly, if the value did actually change,
we pass that information along to notify, so that we do not evaluate the
eager property twice.
Fixes a test-case which errorneously relied on the old behavior, and
adds a new test which verifies that the fix works.

Change-Id: I8ec6fa2fe8611565dfc603ceab3ba5f92999b26c
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

src/corelib/kernel/qproperty.cpp

@@ -110,12 +110,15 @@ void QPropertyBindingPrivate::markDirtyAndNotifyObservers()
 
     eagerlyUpdating = true;
     QScopeGuard guard([&](){eagerlyUpdating = false;});
+    bool knownIfChanged = false;
     if (requiresEagerEvaluation()) {
         // these are compat properties that we will need to evaluate eagerly
-        evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr);
+        if (!evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr))
+            return;
+        knownIfChanged = true;
     }
     if (firstObserver)
-        firstObserver.notify(this, propertyDataPtr);
+        firstObserver.notify(this, propertyDataPtr, knownIfChanged);
     if (hasStaticObserver)
         staticObserverCallback(propertyDataPtr);
 }

src/corelib/kernel/qproperty_p.h

@@ -422,13 +422,15 @@ public:
         QBindingStorage *storage = qGetBindingStorage(owner());
         auto *bd = storage->bindingData(this);
         // make sure we don't remove the binding if called from the bindingWrapper
-        if (bd && !inBindingWrapper(storage))
+        const bool inWrapper = inBindingWrapper(storage);
+        if (bd && !inWrapper)
             bd->removeBinding();
         if constexpr (QTypeTraits::has_operator_equal_v<T>)
             if (this->val == t)
                 return;
         this->val = t;
-        notify(bd);
+        if (!inWrapper)
+            notify(bd);
     }
 
     QObjectCompatProperty &operator=(parameter_type newValue)

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

@@ -86,6 +86,7 @@ private slots:
     void aliasOnMetaProperty();
 
     void modifyObserverListWhileIterating();
+    void compatPropertyNoDobuleNotification();
 };
 
 void tst_QProperty::functorBinding()
@@ -552,11 +553,12 @@ void tst_QProperty::realloc()
         QCOMPARE(tester.prop1(), 12);
 
         tester.bindableProp1().setBinding([&](){return tester.prop5();});
+        QCOMPARE(modificationCount, 2);
         tester.bindableProp2().setBinding([&](){return tester.prop5();});
         tester.bindableProp3().setBinding([&](){return tester.prop5();});
         tester.bindableProp4().setBinding([&](){return tester.prop5();});
         tester.bindableProp5().setBinding([&]() -> int{return 42;});
-        QCOMPARE(modificationCount, 2);
+        QCOMPARE(modificationCount, 3);
     }
 };
 
@@ -1326,6 +1328,31 @@ void tst_QProperty::modifyObserverListWhileIterating()
     }
 }
 
+class CompatPropertyTester : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(int prop1 READ prop1 WRITE setProp1 BINDABLE bindableProp1)
+    public:
+    CompatPropertyTester(QObject *parent = nullptr) : QObject(parent) { }
+
+    int prop1() {return prop1Data.value();}
+    void setProp1(int i) { prop1Data = i; }
+    QBindable<int> bindableProp1() {return QBindable<int>(&prop1Data);}
+    Q_OBJECT_COMPAT_PROPERTY(CompatPropertyTester, int, prop1Data, &CompatPropertyTester::setProp1)
+
+};
+
+void tst_QProperty::compatPropertyNoDobuleNotification()
+{
+    CompatPropertyTester tester;
+    int counter = 0;
+    QProperty<int> iprop {1};
+    tester.bindableProp1().setBinding([&]() -> int {return iprop;});
+    auto observer = tester.bindableProp1().onValueChanged([&](){++counter;});
+    iprop.setValue(2);
+    QCOMPARE(counter, 1);
+}
+
 QTEST_MAIN(tst_QProperty);
 
 #include "tst_qproperty.moc"