QProperty: Support multiple observers

Previously, only the first observer would get notified. Also, make sure
that the notifiers are always retained when switching between bindings
and values.

Change-Id: I9c25c0f2e288dac3a335b68e618f7ddeb44be25a
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

src/corelib/kernel/qproperty.cpp

@@ -83,8 +83,11 @@ void QPropertyBase::moveAssign(QPropertyBase &&other, void *propertyDataPtr)
 QPropertyBase::~QPropertyBase()
 {
     QPropertyBasePointer d{this};
-    if (auto observer = d.firstObserver())
+    for (auto observer = d.firstObserver(); observer;) {
+        auto next = observer.nextObserver();
         observer.unlink();
+        observer = next;
+    }
     if (auto binding = d.bindingPtr())
         binding->unlinkAndDeref();
 }
@@ -95,18 +98,19 @@ QUntypedPropertyBinding QPropertyBase::setBinding(const QUntypedPropertyBinding
     QPropertyBindingPrivatePtr newBinding = binding.d;
 
     QPropertyBasePointer d{this};
-
-    auto observer = d.firstObserver();
-    if (observer)
-        observer.unlink();
+    QPropertyObserverPointer observer;
 
     if (auto *existingBinding = d.bindingPtr()) {
         if (existingBinding == newBinding.data())
             return QUntypedPropertyBinding(oldBinding.data());
         oldBinding = QPropertyBindingPrivatePtr(existingBinding);
+        observer = oldBinding->takeObservers();
         oldBinding->unlinkAndDeref();
         d_ptr &= FlagMask;
+    } else {
+        observer = d.firstObserver();
     }
+
     if (newBinding) {
         newBinding.data()->ref.ref();
         d_ptr = (d_ptr & FlagMask) | reinterpret_cast<quintptr>(newBinding.data());
@@ -115,8 +119,10 @@ QUntypedPropertyBinding QPropertyBase::setBinding(const QUntypedPropertyBinding
         newBinding->setProperty(propertyDataPtr);
         if (observer)
             newBinding->prependObserver(observer);
+    } else if (observer) {
+        d.setObservers(observer.ptr);
     } else {
-        d_ptr &= ~BindingBit;
+        d_ptr &= ~QPropertyBase::BindingBit;
     }
 
     return QUntypedPropertyBinding(oldBinding.data());
@@ -137,6 +143,12 @@ QPropertyBindingPrivate *QPropertyBasePointer::bindingPtr() const
     return nullptr;
 }
 
+void QPropertyBasePointer::setObservers(QPropertyObserver *observer)
+{
+    observer->prev = reinterpret_cast<QPropertyObserver**>(&(ptr->d_ptr));
+    ptr->d_ptr = (reinterpret_cast<quintptr>(observer) & ~QPropertyBase::FlagMask);
+}
+
 void QPropertyBasePointer::addObserver(QPropertyObserver *observer)
 {
     if (auto *binding = bindingPtr()) {
@@ -199,18 +211,13 @@ void QPropertyBase::removeBinding()
 {
     QPropertyBasePointer d{this};
 
-    auto observer = d.firstObserver();
-    if (observer)
-        observer.unlink();
-
     if (auto *existingBinding = d.bindingPtr()) {
+        auto observer = existingBinding->takeObservers();
         existingBinding->unlinkAndDeref();
-        d_ptr &= FlagMask;
+        d_ptr &= ExtraBit;
+        if (observer)
+            d.setObservers(observer.ptr);
     }
-    d_ptr &= ~BindingBit;
-
-    if (observer)
-        observer.observeProperty(d);
 }
 
 void QPropertyBase::registerWithCurrentlyEvaluatingBinding() const

src/corelib/kernel/qproperty.h

@@ -254,16 +254,16 @@ public:
 
     void setValue(T &&newValue)
     {
+        d.priv.removeBinding();
         if (d.setValueAndReturnTrueIfChanged(std::move(newValue)))
             notify();
-        d.priv.removeBinding();
     }
 
     void setValue(const T &newValue)
     {
+        d.priv.removeBinding();
         if (d.setValueAndReturnTrueIfChanged(newValue))
             notify();
-        d.priv.removeBinding();
     }
 
     QProperty<T> &operator=(T &&newValue)

src/corelib/kernel/qproperty_p.h

@@ -65,6 +65,7 @@ struct Q_AUTOTEST_EXPORT QPropertyBasePointer
 
     QPropertyBindingPrivate *bindingPtr() const;
 
+    void setObservers(QPropertyObserver *observer);
     void addObserver(QPropertyObserver *observer);
     void setFirstObserver(QPropertyObserver *observer);
     QPropertyObserverPointer firstObserver() const;

src/corelib/kernel/qpropertybinding_p.h

@@ -101,6 +101,13 @@ public:
         firstObserver = observer;
     }
 
+    QPropertyObserverPointer takeObservers()
+    {
+        auto observers = firstObserver;
+        firstObserver.ptr = nullptr;
+        return observers;
+    }
+
     void clearDependencyObservers() {
         for (size_t i = 0; i < inlineDependencyObservers.size(); ++i) {
             QPropertyObserver empty;

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

@@ -69,6 +69,7 @@ private slots:
     void genericPropertyBindingBool();
     void staticChangeHandler();
     void setBindingFunctor();
+    void multipleObservers();
 };
 
 void tst_QProperty::functorBinding()
@@ -686,6 +687,41 @@ void tst_QProperty::setBindingFunctor()
     QCOMPARE(property.value(), 200);
 }
 
+void tst_QProperty::multipleObservers()
+{
+    QProperty<int> property;
+    property.setValue(5);
+    QCOMPARE(property.value(), 5);
+
+    int value1 = 1;
+    auto changeHandler = property.onValueChanged([&]() { value1 = property.value(); });
+    QCOMPARE(value1, 1);
+
+    int value2 = 2;
+    auto subscribeHandler = property.subscribe([&]() { value2 = property.value(); });
+    QCOMPARE(value2, 5);
+
+    property.setValue(6);
+    QCOMPARE(property.value(), 6);
+    QCOMPARE(value1, 6);
+    QCOMPARE(value2, 6);
+
+    property.setBinding([]() { return 12; });
+    QCOMPARE(value1, 12);
+    QCOMPARE(value2, 12);
+    QCOMPARE(property.value(), 12);
+
+    property.setBinding(QPropertyBinding<int>());
+    QCOMPARE(value1, 12);
+    QCOMPARE(value2, 12);
+    QCOMPARE(property.value(), 12);
+
+    property.setValue(22);
+    QCOMPARE(value1, 22);
+    QCOMPARE(value2, 22);
+    QCOMPARE(property.value(), 22);
+}
+
 QTEST_MAIN(tst_QProperty);
 
 #include "tst_qproperty.moc"