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>

1 files changed, 25 insertions, 29 deletions

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();
 };