Fix ChangeHandler notification for eager properties

ChangeHandler's evaluated the binding to detect if the value actually
changed. This is a valid strategy for lazy bindings, but eager bindings
were already evaluated at that point, and thus the change would not be
detected.
Change the binding loop test, so that there isn't a fixpoint in the
binding loop, and we can still detect it. Changing the binding loop
detection code to deal with this case is left as an exercise for the
future.

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

2 files changed, 18 insertions, 9 deletions

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index 3a2c8dcc0c..14ee881455 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -239,8 +239,11 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
         if (observer)
             newBinding->prependObserver(observer);
         newBinding->setStaticObserver(staticObserverCallback, guardCallback);
-        if (newBinding->requiresEagerEvaluation())
-            newBinding->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr);
+        if (newBinding->requiresEagerEvaluation()) {
+            auto changed = newBinding->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr);
+            if (changed)
+                observer.notify(newBinding.data(), propertyDataPtr, /*alreadyKnownToHaveChanged=*/true);
+        }
     } else if (observer) {
         d.setObservers(observer.ptr);
     } else {
@@ -435,9 +438,18 @@ void QPropertyObserverPointer::setBindingToMarkDirty(QPropertyBindingPrivate *bi
     ptr->next.setTag(QPropertyObserver::ObserverNotifiesBinding);
 }
 
-void QPropertyObserverPointer::notify(QPropertyBindingPrivate *triggeringBinding, QUntypedPropertyData *propertyDataPtr)
+/*! \internal
+  \a propertyDataPtr is a pointer to the observed property's property data
+  In case that property has a binding, \a triggeringBinding points to the binding's QPropertyBindingPrivate
+  \a alreadyKnownToHaveChanged is an optional parameter, which is needed in the case
+  of eager evaluation:
+  There, we have already evaluated the binding, and thus the change detection for the
+  ObserverNotifiesChangeHandler case would not work. Thus we instead pass the knowledge of
+  whether the value has changed we obtained when evaluating the binding eagerly along
+ */
+void QPropertyObserverPointer::notify(QPropertyBindingPrivate *triggeringBinding, QUntypedPropertyData *propertyDataPtr,bool alreadyKnownToHaveChanged)
 {
-    bool knownIfPropertyChanged = false;
+    bool knownIfPropertyChanged = alreadyKnownToHaveChanged;
     bool propertyChanged = true;
 
     auto observer = const_cast<QPropertyObserver*>(ptr);
diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h
index 1bf308e8ea..aa81aa9203 100644
--- a/src/corelib/kernel/qproperty_p.h
+++ b/src/corelib/kernel/qproperty_p.h
@@ -106,7 +106,7 @@ struct QPropertyObserverPointer
     void setChangeHandler(QPropertyObserver::ChangeHandler changeHandler);
     void setAliasedProperty(QUntypedPropertyData *propertyPtr);
 
-    void notify(QPropertyBindingPrivate *triggeringBinding, QUntypedPropertyData *propertyDataPtr);
+    void notify(QPropertyBindingPrivate *triggeringBinding, QUntypedPropertyData *propertyDataPtr, const bool alreadyKnownToHaveChanged = false);
     void observeProperty(QPropertyBindingDataPointer property);
 
     explicit operator bool() const { return ptr != nullptr; }
@@ -409,10 +409,7 @@ public:
     {
         QtPrivate::QPropertyBindingData *bd = qGetBindingStorage(owner())->bindingData(this, true);
         QUntypedPropertyBinding oldBinding(bd->setBinding(newBinding, this, nullptr, bindingWrapper));
-        // refetch the binding data, as the eager evaluation in setBinding() above could cause a reallocation
-        // in the binding storage
-        bd = qGetBindingStorage(owner())->bindingData(this);
-        notify(bd);
+        // notification is already handled in QPropertyBindingData::setBinding
         return static_cast<QPropertyBinding<T> &>(oldBinding);
     }