Improve value type binding behavior

Changing value type bindings in state changes, and implicitly removing
them on property assignment was not reliable.  Internally the system
considered a binding on "font" and one on "font.x" as a binding on two
separate properties, even though the "font" binding completely
overrides the "font.x" property.

Following this change a binding to "font.x" creates a proxy binding object
on the "font" property in addition to the "font.x" binding itself.  This
allows behavior to be consistent across all operations.

QT-2920

6 files changed, 212 insertions, 35 deletions

diff --git a/src/declarative/qml/qdeclarativebinding.cpp b/src/declarative/qml/qdeclarativebinding.cpp
index 88ca5cd..bc78b5b 100644
--- a/src/declarative/qml/qdeclarativebinding.cpp
+++ b/src/declarative/qml/qdeclarativebinding.cpp
@@ -223,7 +223,7 @@ void QDeclarativeBinding::setEnabled(bool e, QDeclarativePropertyPrivate::WriteF
 int QDeclarativeBinding::propertyIndex()
 {
     Q_D(QDeclarativeBinding);
-    return d->bindingData()->property.index();
+    return QDeclarativePropertyPrivate::bindingIndex(d->bindingData()->property);
 }
 
 bool QDeclarativeBinding::enabled() const
@@ -259,23 +259,57 @@ void QDeclarativeAbstractBinding::addToObject(QObject *object)
 {
     Q_ASSERT(object);
 
+    if (m_object == object)
+        return;
+
+    int index = propertyIndex();
+
     removeFromObject();
 
     Q_ASSERT(!m_prevBinding);
 
-    QDeclarativeDeclarativeData *data = QDeclarativeDeclarativeData::get(object, true);
-    m_nextBinding = data->bindings;
-    if (m_nextBinding) m_nextBinding->m_prevBinding = &m_nextBinding;
-    m_prevBinding = &data->bindings;
-    data->bindings = this;
     m_object = object;
+    QDeclarativeDeclarativeData *data = QDeclarativeDeclarativeData::get(object, true);
+
+    if (index & 0xFF000000) {
+        // Value type
 
-    data->setBindingBit(m_object, propertyIndex());
+        int coreIndex = index & 0xFFFFFF;
+
+        // Find the value type proxy (if there is one)
+        QDeclarativeValueTypeProxyBinding *proxy = 0;
+        if (data->hasBindingBit(coreIndex)) {
+            QDeclarativeAbstractBinding *b = data->bindings;
+            while (b && b->propertyIndex() != coreIndex)
+                b = b->m_nextBinding;
+            Q_ASSERT(b && b->bindingType() == QDeclarativeAbstractBinding::ValueTypeProxy);
+            proxy = static_cast<QDeclarativeValueTypeProxyBinding *>(b);
+        }
+
+        if (!proxy) 
+            proxy = new QDeclarativeValueTypeProxyBinding(object, coreIndex);
+        proxy->addToObject(object);
+
+        m_nextBinding = proxy->m_bindings;
+        if (m_nextBinding) m_nextBinding->m_prevBinding = &m_nextBinding;
+        m_prevBinding = &proxy->m_bindings;
+        proxy->m_bindings = this;
+
+    } else {
+        m_nextBinding = data->bindings;
+        if (m_nextBinding) m_nextBinding->m_prevBinding = &m_nextBinding;
+        m_prevBinding = &data->bindings;
+        data->bindings = this;
+
+        data->setBindingBit(m_object, index);
+    }
 }
 
 void QDeclarativeAbstractBinding::removeFromObject()
 {
     if (m_prevBinding) {
+        int index = propertyIndex();
+
         Q_ASSERT(m_object);
 
         *m_prevBinding = m_nextBinding;
@@ -283,8 +317,14 @@ void QDeclarativeAbstractBinding::removeFromObject()
         m_prevBinding = 0;
         m_nextBinding = 0;
 
-        QDeclarativeDeclarativeData *data = QDeclarativeDeclarativeData::get(m_object, false);
-        if (data) data->clearBindingBit(propertyIndex());
+        if (index & 0xFF000000) {
+            // Value type - we don't remove the proxy from the object.  It will sit their happily
+            // doing nothing for ever more.
+        } else {
+            QDeclarativeDeclarativeData *data = QDeclarativeDeclarativeData::get(m_object, false);
+            if (data) data->clearBindingBit(index);
+        }
+
         m_object = 0;
     }
 }
@@ -305,4 +345,88 @@ void QDeclarativeAbstractBinding::setEnabled(bool e, QDeclarativePropertyPrivate
     if (e) m_mePtr = 0;
 }
 
+QDeclarativeValueTypeProxyBinding::QDeclarativeValueTypeProxyBinding(QObject *o, int index)
+: m_object(o), m_index(index), m_bindings(0)
+{
+}
+
+QDeclarativeValueTypeProxyBinding::~QDeclarativeValueTypeProxyBinding()
+{
+    while (m_bindings) {
+        QDeclarativeAbstractBinding *binding = m_bindings;
+        binding->setEnabled(false, 0);
+        binding->destroy();
+    }
+}
+
+void QDeclarativeValueTypeProxyBinding::setEnabled(bool e, QDeclarativePropertyPrivate::WriteFlags flags)
+{
+    if (e) {
+        addToObject(m_object);
+
+        QDeclarativeAbstractBinding *bindings = m_bindings;
+        m_bindings = 0;
+        recursiveEnable(bindings, flags);
+    } else {
+        removeFromObject();
+
+        QDeclarativeAbstractBinding *bindings = m_bindings;
+        m_bindings = 0;
+        recursiveDisable(bindings);
+    }
+}
+
+void QDeclarativeValueTypeProxyBinding::recursiveEnable(QDeclarativeAbstractBinding *b, QDeclarativePropertyPrivate::WriteFlags flags)
+{
+    if (!b)
+        return;
+
+    QDeclarativeAbstractBinding *next = b->m_nextBinding;
+    b->m_prevBinding = 0;
+    b->m_nextBinding = 0;
+    Q_ASSERT(b->m_mePtr == 0);
+    b->m_mePtr = &b;
+
+    recursiveEnable(next, flags);
+
+    if (b)
+        b->setEnabled(true, flags);
+}
+
+void QDeclarativeValueTypeProxyBinding::recursiveDisable(QDeclarativeAbstractBinding *b)
+{
+    if (!b)
+        return;
+
+    recursiveDisable(b->m_nextBinding);
+
+    b->setEnabled(false, 0);
+
+    Q_ASSERT(b->m_prevBinding == 0);
+    Q_ASSERT(b->m_nextBinding == 0);
+    b->m_nextBinding = m_bindings;
+    if (b->m_nextBinding) b->m_nextBinding->m_prevBinding = &b->m_nextBinding;
+    b->m_prevBinding = &m_bindings;
+    m_bindings = b;
+}
+
+int QDeclarativeValueTypeProxyBinding::propertyIndex()
+{
+    return m_index;
+}
+
+void QDeclarativeValueTypeProxyBinding::update(QDeclarativePropertyPrivate::WriteFlags)
+{
+}
+
+QDeclarativeAbstractBinding *QDeclarativeValueTypeProxyBinding::binding(int propertyIndex)
+{
+    QDeclarativeAbstractBinding *binding = m_bindings;
+    
+    while (binding && binding->propertyIndex() != propertyIndex) 
+        binding = binding->m_nextBinding;
+
+    return binding;
+}
+
 QT_END_NAMESPACE
diff --git a/src/declarative/qml/qdeclarativebinding_p.h b/src/declarative/qml/qdeclarativebinding_p.h
index 1a714f0..21e3248 100644
--- a/src/declarative/qml/qdeclarativebinding_p.h
+++ b/src/declarative/qml/qdeclarativebinding_p.h
@@ -74,6 +74,9 @@ public:
 
     virtual QString expression() const;
 
+    enum Type { PropertyBinding, ValueTypeProxy };
+    virtual Type bindingType() const { return PropertyBinding; }
+
     void setEnabled(bool e) { setEnabled(e, QDeclarativePropertyPrivate::DontRemoveBinding); }
     virtual void setEnabled(bool, QDeclarativePropertyPrivate::WriteFlags) = 0;
     virtual int propertyIndex() = 0;
@@ -92,6 +95,7 @@ private:
     friend class QDeclarativeProperty;
     friend class QDeclarativePropertyPrivate;
     friend class QDeclarativeVME;
+    friend class QDeclarativeValueTypeProxyBinding;
 
     QObject *m_object;
     QDeclarativeAbstractBinding **m_mePtr;
@@ -99,6 +103,30 @@ private:
     QDeclarativeAbstractBinding  *m_nextBinding;
 };
 
+class QDeclarativeValueTypeProxyBinding : public QDeclarativeAbstractBinding
+{
+public:
+    QDeclarativeValueTypeProxyBinding(QObject *o, int coreIndex);
+    virtual ~QDeclarativeValueTypeProxyBinding();
+
+    virtual Type bindingType() const { return ValueTypeProxy; }
+
+    virtual void setEnabled(bool, QDeclarativePropertyPrivate::WriteFlags);
+    virtual int propertyIndex();
+    virtual void update(QDeclarativePropertyPrivate::WriteFlags);
+
+    QDeclarativeAbstractBinding *binding(int propertyIndex);
+
+private:
+    void recursiveEnable(QDeclarativeAbstractBinding *, QDeclarativePropertyPrivate::WriteFlags);
+    void recursiveDisable(QDeclarativeAbstractBinding *);
+
+    friend class QDeclarativeAbstractBinding;
+    QObject *m_object;
+    int m_index;
+    QDeclarativeAbstractBinding *m_bindings;
+};
+
 class QDeclarativeContext;
 class QDeclarativeBindingPrivate;
 class Q_DECLARATIVE_EXPORT QDeclarativeBinding : public QDeclarativeExpression, public QDeclarativeAbstractBinding
diff --git a/src/declarative/qml/qdeclarativeobjectscriptclass.cpp b/src/declarative/qml/qdeclarativeobjectscriptclass.cpp
index dc4a676..68780b6 100644
--- a/src/declarative/qml/qdeclarativeobjectscriptclass.cpp
+++ b/src/declarative/qml/qdeclarativeobjectscriptclass.cpp
@@ -351,7 +351,8 @@ void QDeclarativeObjectScriptClass::setProperty(QObject *obj,
         }
     }
 
-    QDeclarativeAbstractBinding *delBinding = QDeclarativePropertyPrivate::setBinding(obj, *lastData, 0);
+    QDeclarativeAbstractBinding *delBinding = 
+        QDeclarativePropertyPrivate::setBinding(obj, lastData->coreIndex, -1, 0);
     if (delBinding)
         delBinding->destroy();
 
diff --git a/src/declarative/qml/qdeclarativeproperty.cpp b/src/declarative/qml/qdeclarativeproperty.cpp
index 945d098..8ca5406 100644
--- a/src/declarative/qml/qdeclarativeproperty.cpp
+++ b/src/declarative/qml/qdeclarativeproperty.cpp
@@ -598,7 +598,6 @@ QMetaMethod QDeclarativeProperty::method() const
         return QMetaMethod();
 }
 
-
 /*!
     Returns the binding associated with this property, or 0 if no binding 
     exists.
@@ -617,13 +616,18 @@ QDeclarativePropertyPrivate::binding(const QDeclarativeProperty &that)
         return 0;
 
     QDeclarativeAbstractBinding *binding = data->bindings;
-    while (binding) {
-        // ### This wont work for value types
-        if (binding->propertyIndex() == that.d->core.coreIndex)
-            return binding; 
+    while (binding && binding->propertyIndex() != that.d->core.coreIndex) 
         binding = binding->m_nextBinding;
+
+    if (binding && that.d->valueType.valueTypeCoreIdx != -1) {
+        if (binding->bindingType() == QDeclarativeAbstractBinding::ValueTypeProxy) {
+            QDeclarativeValueTypeProxyBinding *proxy = static_cast<QDeclarativeValueTypeProxyBinding *>(binding);
+
+            binding = proxy->binding(bindingIndex(that));
+        }
     }
-    return 0;
+
+    return binding;
 }
 
 /*!
@@ -650,36 +654,36 @@ QDeclarativePropertyPrivate::setBinding(const QDeclarativeProperty &that,
         return 0;
     }
 
-    return that.d->setBinding(that.d->object, that.d->core, newBinding, flags);
+    return that.d->setBinding(that.d->object, that.d->core.coreIndex, 
+                              that.d->valueType.valueTypeCoreIdx, newBinding, flags);
 }
 
 QDeclarativeAbstractBinding *
-QDeclarativePropertyPrivate::setBinding(QObject *object, const QDeclarativePropertyCache::Data &core, 
-                                   QDeclarativeAbstractBinding *newBinding, WriteFlags flags)
+QDeclarativePropertyPrivate::setBinding(QObject *object, int coreIndex, int valueTypeIndex,
+                                        QDeclarativeAbstractBinding *newBinding, WriteFlags flags)
 {
     QDeclarativeDeclarativeData *data = QDeclarativeDeclarativeData::get(object, 0 != newBinding);
+    QDeclarativeAbstractBinding *binding = 0;
 
-    if (data && data->hasBindingBit(core.coreIndex)) {
-        QDeclarativeAbstractBinding *binding = data->bindings;
-        while (binding) {
-            // ### This wont work for value types
-            if (binding->propertyIndex() == core.coreIndex) {
-                binding->setEnabled(false);
+    if (data && data->hasBindingBit(coreIndex)) {
+        binding = data->bindings;
 
-                if (newBinding) 
-                    newBinding->setEnabled(true, flags);
+        while (binding && binding->propertyIndex() != coreIndex) 
+            binding = binding->m_nextBinding;
+    }
 
-                return binding; // ### QDeclarativeAbstractBinding;
-            }
+    if (binding && valueTypeIndex != -1 && binding->bindingType() == QDeclarativeAbstractBinding::ValueTypeProxy) {
+        int index = coreIndex | (valueTypeIndex << 24);
+        binding = static_cast<QDeclarativeValueTypeProxyBinding *>(binding)->binding(index);
+    }
 
-            binding = binding->m_nextBinding;
-        }
-    } 
+    if (binding) 
+        binding->setEnabled(false);
 
-    if (newBinding)
+    if (newBinding) 
         newBinding->setEnabled(true, flags);
 
-    return 0;
+    return binding;
 }
 
 /*!
@@ -1253,6 +1257,18 @@ int QDeclarativePropertyPrivate::valueTypeCoreIndex(const QDeclarativeProperty &
     return that.d->valueType.valueTypeCoreIdx;
 }
 
+/*!
+    Returns the "property index" for use in bindings.  The top 8 bits are the value type
+    offset, and 0 otherwise.  The bottom 24-bits are the regular property index.
+*/
+int QDeclarativePropertyPrivate::bindingIndex(const QDeclarativeProperty &that)
+{
+    int rv = that.d->core.coreIndex;
+    if (rv != -1 && that.d->valueType.valueTypeCoreIdx != -1)
+        rv = rv | (that.d->valueType.valueTypeCoreIdx << 24);
+    return rv;
+}
+
 struct SerializedData {
     bool isValueType;
     QDeclarativePropertyCache::Data core;
diff --git a/src/declarative/qml/qdeclarativeproperty_p.h b/src/declarative/qml/qdeclarativeproperty_p.h
index c31e2d3..26b85b8 100644
--- a/src/declarative/qml/qdeclarativeproperty_p.h
+++ b/src/declarative/qml/qdeclarativeproperty_p.h
@@ -110,7 +110,7 @@ public:
                                   const QVariant &value, int flags);
     static bool write(QObject *, const QDeclarativePropertyCache::Data &, const QVariant &, 
                       QDeclarativeContext *, WriteFlags flags = 0);
-    static QDeclarativeAbstractBinding *setBinding(QObject *, const QDeclarativePropertyCache::Data &, 
+    static QDeclarativeAbstractBinding *setBinding(QObject *, int coreIndex, int valueTypeIndex /* -1 */,
                                                    QDeclarativeAbstractBinding *,
                                                    WriteFlags flags = DontRemoveBinding);
 
@@ -133,6 +133,7 @@ public:
                                                        QDeclarativeExpression *) ;
     static bool write(const QDeclarativeProperty &that, const QVariant &, WriteFlags);
     static int valueTypeCoreIndex(const QDeclarativeProperty &that);
+    static int bindingIndex(const QDeclarativeProperty &that);
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QDeclarativePropertyPrivate::WriteFlags)
diff --git a/src/declarative/qml/qdeclarativevaluetypescriptclass.cpp b/src/declarative/qml/qdeclarativevaluetypescriptclass.cpp
index 9cb65f8..a567c38 100644
--- a/src/declarative/qml/qdeclarativevaluetypescriptclass.cpp
+++ b/src/declarative/qml/qdeclarativevaluetypescriptclass.cpp
@@ -41,6 +41,8 @@
 
 #include "qdeclarativevaluetypescriptclass_p.h"
 
+#include "qdeclarativebinding_p.h"
+#include "qdeclarativeproperty_p.h"
 #include "qdeclarativeengine_p.h"
 #include "qdeclarativeguard_p.h"
 
@@ -115,6 +117,11 @@ void QDeclarativeValueTypeScriptClass::setProperty(Object *obj, const Identifier
 {
     QDeclarativeValueTypeReference *ref = static_cast<QDeclarativeValueTypeReference *>(obj);
 
+    QDeclarativeAbstractBinding *delBinding = 
+        QDeclarativePropertyPrivate::setBinding(ref->object, ref->property, m_lastIndex, 0);
+    if (delBinding) 
+        delBinding->destroy();
+
     QVariant v = QDeclarativeScriptClass::toVariant(engine, value);
 
     ref->type->read(ref->object, ref->property);