Cleanup QBindingPrivate

Simplify the data structure. We only need one pointer for either
the static callback or a bindingWrapper, so don't share it
with the dependency observer array.

Also ensure we reset the propertyDataPtr and clear the observers
when the binding gets removed from a property.

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

3 files changed, 34 insertions, 37 deletions

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index 7cacd3f873..3c1d682b7c 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -68,8 +68,6 @@ QPropertyBindingPrivate::~QPropertyBindingPrivate()
 {
     if (firstObserver)
         firstObserver.unlink();
-    if (!hasStaticObserver)
-        inlineDependencyObservers.~ObserverArray(); // Explicit because of union.
 }
 
 void QPropertyBindingPrivate::unlinkAndDeref()
@@ -115,8 +113,8 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
 
     bool changed = false;
 
-    if (hasStaticObserver && staticGuardCallback) {
-        changed = staticGuardCallback(metaType, propertyDataPtr, evaluationFunction);
+    if (hasBindingWrapper) {
+        changed = staticBindingWrapper(metaType, propertyDataPtr, evaluationFunction);
     } else {
         changed = evaluationFunction(metaType, propertyDataPtr);
     }
@@ -231,7 +229,7 @@ QPropertyBindingData::~QPropertyBindingData()
 QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyBinding &binding,
                                                   QUntypedPropertyData *propertyDataPtr,
                                                   QPropertyObserverCallback staticObserverCallback,
-                                                  QtPrivate::QPropertyGuardFunction guardCallback)
+                                                  QtPrivate::QPropertyBindingWrapper guardCallback)
 {
     QPropertyBindingPrivatePtr oldBinding;
     QPropertyBindingPrivatePtr newBinding = binding.d;
@@ -265,6 +263,9 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
         d_ptr &= ~QPropertyBindingData::BindingBit;
     }
 
+    if (oldBinding)
+        oldBinding->detachFromProperty();
+
     return QUntypedPropertyBinding(oldBinding.data());
 }
 
diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h
index 1085c037e6..fbc9a072c3 100644
--- a/src/corelib/kernel/qproperty_p.h
+++ b/src/corelib/kernel/qproperty_p.h
@@ -140,17 +140,16 @@ private:
     bool dirty = false;
     bool updating = false;
     bool hasStaticObserver = false;
+    bool hasBindingWrapper = false;
 
     QUntypedPropertyBinding::BindingEvaluationFunction evaluationFunction;
 
     QPropertyObserverPointer firstObserver;
     union {
-        ObserverArray inlineDependencyObservers;
-        struct {
-            QtPrivate::QPropertyObserverCallback staticObserverCallback;
-            QtPrivate::QPropertyGuardFunction staticGuardCallback;
-        };
+        QtPrivate::QPropertyObserverCallback staticObserverCallback = nullptr;
+        QtPrivate::QPropertyBindingWrapper staticBindingWrapper;
     };
+    ObserverArray inlineDependencyObservers;
     QScopedPointer<std::vector<QPropertyObserver>> heapObservers;
 
     QUntypedPropertyData *propertyDataPtr = nullptr;
@@ -175,29 +174,21 @@ public:
 
     void setDirty(bool d) { dirty = d; }
     void setProperty(QUntypedPropertyData *propertyPtr) { propertyDataPtr = propertyPtr; }
-    void setStaticObserver(QtPrivate::QPropertyObserverCallback callback, QtPrivate::QPropertyGuardFunction guardCallback)
+    void setStaticObserver(QtPrivate::QPropertyObserverCallback callback, QtPrivate::QPropertyBindingWrapper guardCallback)
     {
+        Q_ASSERT(!(callback && guardCallback));
         if (callback) {
-            if (!hasStaticObserver) {
-                if (dependencyObserverCount > 0) {
-                    if (!heapObservers)
-                        heapObservers.reset(new std::vector<QPropertyObserver>());
-                    for (size_t i = 0, end = qMin(dependencyObserverCount, inlineDependencyObservers.size()); i < end; ++i)
-                        heapObservers->push_back(std::move(inlineDependencyObservers[i]));
-                }
-                inlineDependencyObservers.~ObserverArray();
-            }
-
             hasStaticObserver = true;
+            hasBindingWrapper = false;
             staticObserverCallback = callback;
-            staticGuardCallback = guardCallback;
-        } else if (hasStaticObserver) {
+        } else if (guardCallback) {
+            hasStaticObserver = false;
+            hasBindingWrapper = true;
+            staticBindingWrapper = guardCallback;
+        } else {
             hasStaticObserver = false;
-            new (&inlineDependencyObservers) ObserverArray();
-            for (size_t i = 0, end = qMin(dependencyObserverCount, inlineDependencyObservers.size()); i < end; ++i) {
-                inlineDependencyObservers[i] = std::move(heapObservers->back());
-                heapObservers->pop_back();
-            }
+            hasBindingWrapper = false;
+            staticObserverCallback = nullptr;
         }
     }
     void prependObserver(QPropertyObserverPointer observer) {
@@ -213,18 +204,16 @@ public:
     }
 
     void clearDependencyObservers() {
-        if (!hasStaticObserver) {
-            for (size_t i = 0; i < qMin(dependencyObserverCount, inlineDependencyObservers.size()); ++i) {
-                QPropertyObserverPointer p{&inlineDependencyObservers[i]};
-                p.unlink();
-            }
+        for (size_t i = 0; i < qMin(dependencyObserverCount, inlineDependencyObservers.size()); ++i) {
+            QPropertyObserverPointer p{&inlineDependencyObservers[i]};
+            p.unlink();
         }
         if (heapObservers)
             heapObservers->clear();
         dependencyObserverCount = 0;
     }
     QPropertyObserverPointer allocateDependencyObserver() {
-        if (!hasStaticObserver && dependencyObserverCount < inlineDependencyObservers.size()) {
+        if (dependencyObserverCount < inlineDependencyObservers.size()) {
             ++dependencyObserverCount;
             return {&inlineDependencyObservers[dependencyObserverCount - 1]};
         }
@@ -249,6 +238,13 @@ public:
     void setError(QPropertyBindingError &&e)
     { error = std::move(e); }
 
+    void detachFromProperty() {
+        hasStaticObserver = false;
+        hasBindingWrapper = false;
+        propertyDataPtr = nullptr;
+        clearDependencyObservers();
+    }
+
     static QPropertyBindingPrivate *currentlyEvaluatingBinding();
 };
 
diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h
index 33ecb405f2..adc347db84 100644
--- a/src/corelib/kernel/qpropertyprivate.h
+++ b/src/corelib/kernel/qpropertyprivate.h
@@ -77,7 +77,7 @@ namespace QtPrivate {
 // writes binding result into dataPtr
 using QPropertyBindingFunction = std::function<bool(QMetaType metaType, QUntypedPropertyData *dataPtr)>;
 
-using QPropertyGuardFunction = bool(*)(QMetaType, QUntypedPropertyData *dataPtr,
+using QPropertyBindingWrapper = bool(*)(QMetaType, QUntypedPropertyData *dataPtr,
                                        QPropertyBindingFunction);
 using QPropertyObserverCallback = void (*)(QUntypedPropertyData *);
 
@@ -102,7 +102,7 @@ public:
     QUntypedPropertyBinding setBinding(const QUntypedPropertyBinding &newBinding,
                                        QUntypedPropertyData *propertyDataPtr,
                                        QPropertyObserverCallback staticObserverCallback = nullptr,
-                                       QPropertyGuardFunction guardCallback = nullptr);
+                                       QPropertyBindingWrapper guardCallback = nullptr);
     QPropertyBindingPrivate *binding() const;
 
     void evaluateIfDirty() const;
@@ -184,7 +184,7 @@ namespace detail {
 template<typename T, typename Class, auto Guard, bool = std::is_same_v<decltype(Guard), std::nullptr_t>>
 struct QPropertyGuardFunctionHelper
 {
-    static constexpr QPropertyGuardFunction guard = nullptr;
+    static constexpr QPropertyBindingWrapper guard = nullptr;
 };
 template<typename T, typename Class, auto Guard>
 struct QPropertyGuardFunctionHelper<T, Class, Guard, false>