Don't store object and property in QDeclarativeAbstractBinding
authorAaron Kennedy <aaron.kennedy@nokia.com>
Fri, 10 Feb 2012 12:57:57 +0000 (12:57 +0000)
committerQt by Nokia <qt-info@nokia.com>
Mon, 20 Feb 2012 15:25:32 +0000 (16:25 +0100)
Change-Id: Ia164655f6329ec80dc466a1a3a5613a73e1c23ac
Reviewed-by: Roberto Raggi <roberto.raggi@nokia.com>
12 files changed:
src/declarative/qml/qdeclarativebinding.cpp
src/declarative/qml/qdeclarativebinding_p.h
src/declarative/qml/qdeclarativebinding_p_p.h
src/declarative/qml/qdeclarativecompiler.cpp
src/declarative/qml/qdeclarativeproperty.cpp
src/declarative/qml/qdeclarativeproperty_p.h
src/declarative/qml/qdeclarativepropertycache_p.h
src/declarative/qml/qdeclarativevme.cpp
src/declarative/qml/v4/qv4bindings.cpp
src/declarative/qml/v4/qv4bindings_p.h
src/declarative/qml/v8/qv8bindings.cpp
src/declarative/qml/v8/qv8bindings_p.h

index 359f169..39032e0 100644 (file)
@@ -56,7 +56,7 @@
 QT_BEGIN_NAMESPACE
 
 QDeclarativeAbstractBinding::QDeclarativeAbstractBinding()
-: m_object(0), m_propertyIndex(-1), m_prevBinding(0), m_nextBinding(0)
+: m_prevBinding(0), m_nextBinding(0)
 {
 }
 
@@ -89,21 +89,16 @@ being bound.
 
 However, it does not enable the binding itself or call update() on it.
 */
-void QDeclarativeAbstractBinding::addToObject(QObject *object, int index)
+void QDeclarativeAbstractBinding::addToObject()
 {
-    Q_ASSERT(object);
-
-    if (m_object == object && m_propertyIndex == index)
-        return;
-
-    removeFromObject();
-
     Q_ASSERT(!m_prevBinding);
 
-    m_object = object;
-    m_propertyIndex = index;
+    QObject *obj = object();
+    Q_ASSERT(obj);
 
-    QDeclarativeData *data = QDeclarativeData::get(object, true);
+    int index = propertyIndex();
+
+    QDeclarativeData *data = QDeclarativeData::get(obj, true);
 
     if (index & 0xFF000000) {
         // Value type
@@ -121,8 +116,12 @@ void QDeclarativeAbstractBinding::addToObject(QObject *object, int index)
         }
 
         if (!proxy) {
-            proxy = new QDeclarativeValueTypeProxyBinding(object, coreIndex);
-            proxy->addToObject(object, coreIndex);
+            proxy = new QDeclarativeValueTypeProxyBinding(obj, coreIndex);
+
+            Q_ASSERT(proxy->propertyIndex() == coreIndex);
+            Q_ASSERT(proxy->object() == obj);
+
+            proxy->addToObject();
         }
 
         m_nextBinding = proxy->m_bindings;
@@ -136,7 +135,7 @@ void QDeclarativeAbstractBinding::addToObject(QObject *object, int index)
         m_prevBinding = &data->bindings;
         data->bindings = this;
 
-        data->setBindingBit(m_object, index);
+        data->setBindingBit(obj, index);
     }
 }
 
@@ -157,13 +156,10 @@ void QDeclarativeAbstractBinding::removeFromObject()
             // Value type - we don't remove the proxy from the object.  It will sit their happily
             // doing nothing until it is removed by a write, a binding change or it is reused
             // to hold more sub-bindings.
-        } else if (m_object) {
-            QDeclarativeData *data = QDeclarativeData::get(m_object, false);
+        } else if (QObject *obj = object()) {
+            QDeclarativeData *data = QDeclarativeData::get(obj, false);
             if (data) data->clearBindingBit(index);
         }
-
-        m_object = 0;
-        m_propertyIndex = -1;
     }
 }
 
@@ -187,19 +183,14 @@ void QDeclarativeAbstractBinding::clear()
     }
 }
 
-QString QDeclarativeAbstractBinding::expression() const
+void QDeclarativeAbstractBinding::retargetBinding(QObject *, int)
 {
-    return QLatin1String("<Unknown>");
+    qFatal("QDeclarativeAbstractBinding::retargetBinding() called on illegal binding.");
 }
 
-QObject *QDeclarativeAbstractBinding::object() const
-{
-    return m_object;
-}
-
-int QDeclarativeAbstractBinding::propertyIndex() const
+QString QDeclarativeAbstractBinding::expression() const
 {
-    return m_propertyIndex;
+    return QLatin1String("<Unknown>");
 }
 
 void QDeclarativeAbstractBinding::setEnabled(bool enabled, QDeclarativePropertyPrivate::WriteFlags flags)
@@ -216,7 +207,7 @@ void QDeclarativeBindingPrivate::refresh()
 }
 
 QDeclarativeBindingPrivate::QDeclarativeBindingPrivate()
-: updating(false), enabled(false)
+: updating(false), enabled(false), target(), targetProperty(0)
 {
 }
 
@@ -298,6 +289,8 @@ void QDeclarativeBinding::setTarget(const QDeclarativeProperty &prop)
 {
     Q_D(QDeclarativeBinding);
     d->property = prop;
+    d->target = d->property.object();
+    d->targetProperty = QDeclarativePropertyPrivate::get(d->property)->core.encodedIndex();
 
     update();
 }
@@ -308,6 +301,8 @@ void QDeclarativeBinding::setTarget(QObject *object,
 {
     Q_D(QDeclarativeBinding);
     d->property = QDeclarativePropertyPrivate::restore(object, core, ctxt);
+    d->target = d->property.object();
+    d->targetProperty = QDeclarativePropertyPrivate::get(d->property)->core.encodedIndex();
 
     update();
 }
@@ -442,6 +437,25 @@ QString QDeclarativeBinding::expression() const
     return QDeclarativeExpression::expression();
 }
 
+int QDeclarativeBinding::propertyIndex() const
+{
+    Q_D(const QDeclarativeBinding);
+    return d->targetProperty;
+}
+
+QObject *QDeclarativeBinding::object() const
+{
+    Q_D(const QDeclarativeBinding);
+    return d->target;
+}
+
+void QDeclarativeBinding::retargetBinding(QObject *t, int i)
+{
+    Q_D(QDeclarativeBinding);
+    d->target = t;
+    d->targetProperty = i;
+}
+
 QDeclarativeValueTypeProxyBinding::QDeclarativeValueTypeProxyBinding(QObject *o, int index)
 : m_object(o), m_index(index), m_bindings(0)
 {
@@ -524,4 +538,14 @@ void QDeclarativeValueTypeProxyBinding::removeBindings(quint32 mask)
     }
 }
 
+int QDeclarativeValueTypeProxyBinding::propertyIndex() const
+{
+    return m_index;
+}
+
+QObject *QDeclarativeValueTypeProxyBinding::object() const
+{
+    return m_object;
+}
+
 QT_END_NAMESPACE
index e29465c..61cf7dd 100644 (file)
@@ -80,8 +80,13 @@ public:
     enum Type { PropertyBinding, ValueTypeProxy };
     virtual Type bindingType() const { return PropertyBinding; }
 
-    QObject *object() const;
-    int propertyIndex() const;
+    // Should return the encoded property index for the binding.  Should return this value
+    // even if the binding is not enabled or added to an object.
+    // Encoding is:  coreIndex | (valueTypeIndex << 24)
+    virtual int propertyIndex() const = 0;
+    // Should return the object for the binding.  Should return this object even if the
+    // binding is not enabled or added to the object.
+    virtual QObject *object() const = 0;
 
     void setEnabled(bool e) { setEnabled(e, QDeclarativePropertyPrivate::DontRemoveBinding); }
     virtual void setEnabled(bool, QDeclarativePropertyPrivate::WriteFlags) = 0;
@@ -89,7 +94,7 @@ public:
     void update() { update(QDeclarativePropertyPrivate::DontRemoveBinding); }
     virtual void update(QDeclarativePropertyPrivate::WriteFlags) = 0;
 
-    void addToObject(QObject *, int);
+    void addToObject();
     void removeFromObject();
 
     static inline Pointer getPointer(QDeclarativeAbstractBinding *p);
@@ -98,6 +103,11 @@ protected:
     virtual ~QDeclarativeAbstractBinding();
     void clear();
 
+    // Called by QDeclarativePropertyPrivate to "move" a binding to a different property.
+    // This is only used for alias properties, and only used by QDeclarativeBinding not
+    // V8 or V4 bindings.  The default implementation qFatal()'s to ensure that the
+    // method is never called for V4 or V8 bindings.
+    virtual void retargetBinding(QObject *, int);
 private:
     Pointer weakPointer();
 
@@ -108,9 +118,6 @@ private:
     friend class QDeclarativeVME;
     friend class QtSharedPointer::ExternalRefCount<QDeclarativeAbstractBinding>;
 
-    QObject *m_object;
-    int m_propertyIndex;
-
     typedef QSharedPointer<QDeclarativeAbstractBinding> SharedPointer;
     // To save memory, we also store the rarely used weakPointer() instance in here
     QPointerValuePair<QDeclarativeAbstractBinding*, SharedPointer> m_mePtr;
@@ -128,6 +135,8 @@ public:
 
     virtual void setEnabled(bool, QDeclarativePropertyPrivate::WriteFlags);
     virtual void update(QDeclarativePropertyPrivate::WriteFlags);
+    virtual int propertyIndex() const;
+    virtual QObject *object() const;
 
     QDeclarativeAbstractBinding *binding(int propertyIndex);
 
@@ -148,7 +157,8 @@ private:
 
 class QDeclarativeContext;
 class QDeclarativeBindingPrivate;
-class Q_DECLARATIVE_PRIVATE_EXPORT QDeclarativeBinding : public QDeclarativeExpression, public QDeclarativeAbstractBinding
+class Q_DECLARATIVE_PRIVATE_EXPORT QDeclarativeBinding : public QDeclarativeExpression,
+                                                         public QDeclarativeAbstractBinding
 {
 Q_OBJECT
 public:
@@ -174,10 +184,15 @@ public:
     virtual void setEnabled(bool, QDeclarativePropertyPrivate::WriteFlags flags);
     virtual void update(QDeclarativePropertyPrivate::WriteFlags flags);
     virtual QString expression() const;
+    virtual int propertyIndex() const;
+    virtual QObject *object() const;
+    virtual void retargetBinding(QObject *, int);
 
     typedef int Identifier;
     static Identifier Invalid;
-    static QDeclarativeBinding *createBinding(Identifier, QObject *, QDeclarativeContext *, const QString &, int, QObject *parent=0);
+    static QDeclarativeBinding *createBinding(Identifier, QObject *, QDeclarativeContext *,
+                                              const QString &, int, QObject *parent=0);
+
 
 public Q_SLOTS:
     void update() { update(QDeclarativePropertyPrivate::DontRemoveBinding); }
index 9bce0c4..030bd32 100644 (file)
@@ -79,6 +79,9 @@ private:
     bool enabled:1;
     int columnNumber;
     QDeclarativeProperty property; 
+
+    QObject *target;
+    int targetProperty;
 };
 
 QT_END_NAMESPACE
index ef4f24d..abaf5cb 100644 (file)
@@ -3609,7 +3609,7 @@ bool QDeclarativeCompiler::completeComponentBuild()
         bool isSharable = false;
         binding.rewrittenExpression = rewriteBinding(binding.expression.asAST(), expression, &isSharable);
 
-        if (isSharable && !binding.property->isAlias /* See above re alias */ &&
+        if (isSharable && !binding.property->isValueTypeSubProperty && !binding.property->isAlias /* See above re alias */ &&
             binding.property->type != qMetaTypeId<QDeclarativeBinding*>()) {
             binding.dataType = BindingReference::V8;
             sharedBindings.append(b);
index c50c2d7..7ba801d 100644 (file)
@@ -694,8 +694,8 @@ QDeclarativePropertyPrivate::binding(const QDeclarativeProperty &that)
 */
 QDeclarativeAbstractBinding *
 QDeclarativePropertyPrivate::setBinding(const QDeclarativeProperty &that,
-                                            QDeclarativeAbstractBinding *newBinding, 
-                                            WriteFlags flags) 
+                                        QDeclarativeAbstractBinding *newBinding,
+                                        WriteFlags flags)
 {
     if (!that.d || !that.isProperty() || !that.d->object) {
         if (newBinding)
@@ -703,9 +703,21 @@ QDeclarativePropertyPrivate::setBinding(const QDeclarativeProperty &that,
         return 0;
     }
 
-    return that.d->setBinding(that.d->object, that.d->core.coreIndex, 
-                              that.d->core.getValueTypeCoreIndex(),
-                              newBinding, flags);
+    if (newBinding) {
+        // In the case that the new binding is provided, we must target the property it
+        // is associated with.  If we don't do this, retargetBinding() can fail.
+        QObject *object = newBinding->object();
+        int pi = newBinding->propertyIndex();
+
+        int core = pi & 0xFFFFFF;
+        int vt = (pi & 0xFF000000)?(pi >> 24):-1;
+
+        return setBinding(object, core, vt, newBinding, flags);
+    } else {
+        return setBinding(that.d->object, that.d->core.coreIndex,
+                          that.d->core.getValueTypeCoreIndex(),
+                          newBinding, flags);
+    }
 }
 
 QDeclarativeAbstractBinding *
@@ -727,7 +739,8 @@ QDeclarativePropertyPrivate::binding(QObject *object, int coreIndex, int valueTy
 
         // This will either be a value type sub-reference or an alias to a value-type sub-reference not both
         Q_ASSERT(valueTypeIndex == -1 || aValueTypeIndex == -1);
-        return binding(aObject, aCoreIndex, (valueTypeIndex == -1)?aValueTypeIndex:valueTypeIndex);
+        aValueTypeIndex = (valueTypeIndex == -1)?aValueTypeIndex:valueTypeIndex;
+        return binding(aObject, aCoreIndex, aValueTypeIndex);
     }
 
     if (!data->hasBindingBit(coreIndex))
@@ -804,8 +817,8 @@ QDeclarativePropertyPrivate::setBinding(QObject *object, int coreIndex, int valu
 
             // This will either be a value type sub-reference or an alias to a value-type sub-reference not both
             Q_ASSERT(valueTypeIndex == -1 || aValueTypeIndex == -1);
-            return setBinding(aObject, aCoreIndex, (valueTypeIndex == -1)?aValueTypeIndex:valueTypeIndex,
-                              newBinding, flags);
+            aValueTypeIndex = (valueTypeIndex == -1)?aValueTypeIndex:valueTypeIndex;
+            return setBinding(aObject, aCoreIndex, aValueTypeIndex, newBinding, flags);
         }
     }
 
@@ -829,7 +842,13 @@ QDeclarativePropertyPrivate::setBinding(QObject *object, int coreIndex, int valu
     }
 
     if (newBinding) {
-        newBinding->addToObject(object, index);
+        if (newBinding->propertyIndex() != index || newBinding->object() != object)
+            newBinding->retargetBinding(object, index);
+
+        Q_ASSERT(newBinding->propertyIndex() == index);
+        Q_ASSERT(newBinding->object() == object);
+
+        newBinding->addToObject();
         newBinding->setEnabled(true, flags);
     }
 
@@ -858,8 +877,8 @@ QDeclarativePropertyPrivate::setBindingNoEnable(QObject *object, int coreIndex,
 
             // This will either be a value type sub-reference or an alias to a value-type sub-reference not both
             Q_ASSERT(valueTypeIndex == -1 || aValueTypeIndex == -1);
-            return setBindingNoEnable(aObject, aCoreIndex, (valueTypeIndex == -1)?aValueTypeIndex:valueTypeIndex,
-                                      newBinding);
+            aValueTypeIndex = (valueTypeIndex == -1)?aValueTypeIndex:valueTypeIndex;
+            return setBindingNoEnable(aObject, aCoreIndex, aValueTypeIndex, newBinding);
         }
     }
 
@@ -880,8 +899,15 @@ QDeclarativePropertyPrivate::setBindingNoEnable(QObject *object, int coreIndex,
     if (binding) 
         binding->removeFromObject();
 
-    if (newBinding) 
-        newBinding->addToObject(object, index);
+    if (newBinding) {
+        if (newBinding->propertyIndex() != index || newBinding->object() != object)
+            newBinding->retargetBinding(object, index);
+
+        Q_ASSERT(newBinding->propertyIndex() == index);
+        Q_ASSERT(newBinding->object() == object);
+
+        newBinding->addToObject();
+    }
 
     return binding;
 }
index f7ada4e..fc106cc 100644 (file)
@@ -110,6 +110,7 @@ public:
     static bool write(QObject *, const QDeclarativePropertyData &, const QVariant &,
                       QDeclarativeContextData *, WriteFlags flags = 0);
     static void findAliasTarget(QObject *, int, QObject **, int *);
+
     static QDeclarativeAbstractBinding *setBinding(QObject *, int coreIndex,
                                                    int valueTypeIndex /* -1 */,
                                                    QDeclarativeAbstractBinding *,
index ef8cefd..3f5de5a 100644 (file)
@@ -153,6 +153,10 @@ public:
     // Returns -1 if not a value type virtual property
     inline int getValueTypeCoreIndex() const;
 
+    // Returns the "encoded" index for use with bindings.  Encoding is:
+    //     coreIndex | (valueTypeCoreIndex << 24)
+    inline int encodedIndex() const;
+
     union {
         int propType;             // When !NotFullyResolved
         const char *propTypeName; // When NotFullyResolved
@@ -339,6 +343,11 @@ int QDeclarativePropertyRawData::getValueTypeCoreIndex() const
     return isValueTypeVirtual()?valueTypeCoreIndex:-1;
 }
 
+int QDeclarativePropertyRawData::encodedIndex() const
+{
+    return isValueTypeVirtual()?(coreIndex | (valueTypeCoreIndex << 24)):coreIndex;
+}
+
 QDeclarativePropertyData *
 QDeclarativePropertyCache::overrideData(QDeclarativePropertyData *data) const
 {
index a75eae1..6f323e4 100644 (file)
@@ -769,7 +769,11 @@ QObject *QDeclarativeVME::run(QList<QDeclarativeError> *errors,
             bind->m_mePtr = &bindValues.top();
             bind->setTarget(target, instr.property, CTXT);
 
-            bind->addToObject(target, QDeclarativePropertyPrivate::bindingIndex(instr.property));
+            typedef QDeclarativePropertyPrivate QDPP;
+            Q_ASSERT(bind->propertyIndex() == QDPP::bindingIndex(instr.property));
+            Q_ASSERT(bind->object() == target);
+
+            bind->addToObject();
         QML_END_INSTR(StoreBinding)
 
         QML_BEGIN_INSTR(StoreBindingOnAlias)
@@ -810,7 +814,11 @@ QObject *QDeclarativeVME::run(QList<QDeclarativeError> *errors,
                                                 instr.line, instr.column);
             bindValues.push(binding);
             binding->m_mePtr = &bindValues.top();
-            binding->addToObject(target, property);
+
+            Q_ASSERT(binding->propertyIndex() == property);
+            Q_ASSERT(binding->object() == target);
+
+            binding->addToObject();
         QML_END_INSTR(StoreV4Binding)
 
         QML_BEGIN_INSTR(StoreV8Binding)
@@ -827,7 +835,12 @@ QObject *QDeclarativeVME::run(QList<QDeclarativeError> *errors,
             if (binding) {
                 bindValues.push(binding);
                 binding->m_mePtr = &bindValues.top();
-                binding->addToObject(target, QDeclarativePropertyPrivate::bindingIndex(instr.property));
+
+                typedef QDeclarativePropertyPrivate QDPP;
+                Q_ASSERT(binding->propertyIndex() == QDPP::bindingIndex(instr.property));
+                Q_ASSERT(binding->object() == target);
+
+                binding->addToObject();
             }
         QML_END_INSTR(StoreV8Binding)
 
index f321628..93f5bb4 100644 (file)
@@ -254,6 +254,16 @@ void QV4Bindings::Binding::destroy()
     parent->release();
 }
 
+int QV4Bindings::Binding::propertyIndex() const
+{
+    return property;
+}
+
+QObject *QV4Bindings::Binding::object() const
+{
+    return target;
+}
+
 void QV4Bindings::Subscription::subscriptionCallback(QDeclarativeNotifierEndpoint *e) 
 {
     Subscription *s = static_cast<Subscription *>(e);
index a3e8ed8..58dd432 100644 (file)
@@ -90,6 +90,8 @@ private:
         virtual void setEnabled(bool, QDeclarativePropertyPrivate::WriteFlags flags);
         virtual void update(QDeclarativePropertyPrivate::WriteFlags flags);
         virtual void destroy();
+        virtual int propertyIndex() const;
+        virtual QObject *object() const;
 
         int index:30;
         bool enabled:1;
index 648e8f6..439ba1e 100644 (file)
@@ -59,7 +59,7 @@ static QDeclarativeJavaScriptExpression::VTable QV8Bindings_Binding_jsvtable = {
 };
 
 QV8Bindings::Binding::Binding()
-: QDeclarativeJavaScriptExpression(&QV8Bindings_Binding_jsvtable), object(0), parent(0)
+: QDeclarativeJavaScriptExpression(&QV8Bindings_Binding_jsvtable), target(0), parent(0)
 {
 }
 
@@ -84,6 +84,16 @@ void QV8Bindings::Binding::refresh()
     update();
 }
 
+int QV8Bindings::Binding::propertyIndex() const
+{
+    return instruction->property.encodedIndex();
+}
+
+QObject *QV8Bindings::Binding::object() const
+{
+    return target;
+}
+
 void QV8Bindings::Binding::update(QDeclarativePropertyPrivate::WriteFlags flags)
 {
     if (!enabledFlag())
@@ -120,7 +130,7 @@ void QV8Bindings::Binding::update(QDeclarativePropertyPrivate::WriteFlags flags)
         bool needsErrorData = false;
         if (!watcher.wasDeleted() && !hasError()) {
             typedef QDeclarativePropertyPrivate PP;
-            needsErrorData = !PP::writeBinding(object, instruction->property, context, this, result,
+            needsErrorData = !PP::writeBinding(target, instruction->property, context, this, result,
                                                isUndefined, flags);
         }
 
@@ -147,7 +157,7 @@ void QV8Bindings::Binding::update(QDeclarativePropertyPrivate::WriteFlags flags)
         ep->dereferenceScarceResources(); 
 
     } else {
-        QDeclarativeProperty p = QDeclarativePropertyPrivate::restore(object, instruction->property,
+        QDeclarativeProperty p = QDeclarativePropertyPrivate::restore(target, instruction->property,
                                                                       context);
         QDeclarativeBindingPrivate::printBindingLoopError(p);
     }
@@ -245,8 +255,7 @@ QV8Bindings::configBinding(QObject *target, QObject *scope,
     QV8Bindings::Binding *rv = bindings + i->value;
 
     rv->instruction = i;
-
-    rv->object = target;
+    rv->target = target;
     rv->setScopeObject(scope);
     rv->setUseSharedContext(true);
     rv->setNotifyOnValueChanged(true);
index b1bb169..14de2d1 100644 (file)
@@ -96,8 +96,10 @@ public:
         virtual void setEnabled(bool, QDeclarativePropertyPrivate::WriteFlags flags);
         virtual void update(QDeclarativePropertyPrivate::WriteFlags flags);
         virtual void destroy();
+        virtual int propertyIndex() const;
+        virtual QObject *object() const;
 
-        QObject *object;
+        QObject *target;
         QV8Bindings *parent;
 
         // To save memory, we store flags inside the instruction pointer.