From 08566134e96971a8e8552a10ca7d6f31fca8a618 Mon Sep 17 00:00:00 2001 From: Aaron Kennedy Date: Fri, 10 Feb 2012 12:57:57 +0000 Subject: Don't store object and property in QDeclarativeAbstractBinding Change-Id: Ia164655f6329ec80dc466a1a3a5613a73e1c23ac Reviewed-by: Roberto Raggi --- src/declarative/qml/qdeclarativebinding.cpp | 84 +++++++++++++++-------- src/declarative/qml/qdeclarativebinding_p.h | 31 ++++++--- src/declarative/qml/qdeclarativebinding_p_p.h | 3 + src/declarative/qml/qdeclarativecompiler.cpp | 2 +- src/declarative/qml/qdeclarativeproperty.cpp | 52 ++++++++++---- src/declarative/qml/qdeclarativeproperty_p.h | 1 + src/declarative/qml/qdeclarativepropertycache_p.h | 9 +++ src/declarative/qml/qdeclarativevme.cpp | 19 ++++- src/declarative/qml/v4/qv4bindings.cpp | 10 +++ src/declarative/qml/v4/qv4bindings_p.h | 2 + src/declarative/qml/v8/qv8bindings.cpp | 19 +++-- src/declarative/qml/v8/qv8bindings_p.h | 4 +- 12 files changed, 175 insertions(+), 61 deletions(-) (limited to 'src/declarative') diff --git a/src/declarative/qml/qdeclarativebinding.cpp b/src/declarative/qml/qdeclarativebinding.cpp index 359f169fac..39032e0ca6 100644 --- a/src/declarative/qml/qdeclarativebinding.cpp +++ b/src/declarative/qml/qdeclarativebinding.cpp @@ -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(""); + 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(""); } 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 diff --git a/src/declarative/qml/qdeclarativebinding_p.h b/src/declarative/qml/qdeclarativebinding_p.h index e29465c4b3..61cf7dd550 100644 --- a/src/declarative/qml/qdeclarativebinding_p.h +++ b/src/declarative/qml/qdeclarativebinding_p.h @@ -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; - QObject *m_object; - int m_propertyIndex; - typedef QSharedPointer SharedPointer; // To save memory, we also store the rarely used weakPointer() instance in here QPointerValuePair 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); } diff --git a/src/declarative/qml/qdeclarativebinding_p_p.h b/src/declarative/qml/qdeclarativebinding_p_p.h index 9bce0c4edd..030bd32ef3 100644 --- a/src/declarative/qml/qdeclarativebinding_p_p.h +++ b/src/declarative/qml/qdeclarativebinding_p_p.h @@ -79,6 +79,9 @@ private: bool enabled:1; int columnNumber; QDeclarativeProperty property; + + QObject *target; + int targetProperty; }; QT_END_NAMESPACE diff --git a/src/declarative/qml/qdeclarativecompiler.cpp b/src/declarative/qml/qdeclarativecompiler.cpp index ef4f24d842..abaf5cbeb5 100644 --- a/src/declarative/qml/qdeclarativecompiler.cpp +++ b/src/declarative/qml/qdeclarativecompiler.cpp @@ -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()) { binding.dataType = BindingReference::V8; sharedBindings.append(b); diff --git a/src/declarative/qml/qdeclarativeproperty.cpp b/src/declarative/qml/qdeclarativeproperty.cpp index c50c2d7026..7ba801d72f 100644 --- a/src/declarative/qml/qdeclarativeproperty.cpp +++ b/src/declarative/qml/qdeclarativeproperty.cpp @@ -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; } diff --git a/src/declarative/qml/qdeclarativeproperty_p.h b/src/declarative/qml/qdeclarativeproperty_p.h index f7ada4e354..fc106cc463 100644 --- a/src/declarative/qml/qdeclarativeproperty_p.h +++ b/src/declarative/qml/qdeclarativeproperty_p.h @@ -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 *, diff --git a/src/declarative/qml/qdeclarativepropertycache_p.h b/src/declarative/qml/qdeclarativepropertycache_p.h index ef8cefd821..3f5de5ada5 100644 --- a/src/declarative/qml/qdeclarativepropertycache_p.h +++ b/src/declarative/qml/qdeclarativepropertycache_p.h @@ -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 { diff --git a/src/declarative/qml/qdeclarativevme.cpp b/src/declarative/qml/qdeclarativevme.cpp index a75eae1a63..6f323e4c57 100644 --- a/src/declarative/qml/qdeclarativevme.cpp +++ b/src/declarative/qml/qdeclarativevme.cpp @@ -769,7 +769,11 @@ QObject *QDeclarativeVME::run(QList *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 *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 *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) diff --git a/src/declarative/qml/v4/qv4bindings.cpp b/src/declarative/qml/v4/qv4bindings.cpp index f321628cb0..93f5bb4685 100644 --- a/src/declarative/qml/v4/qv4bindings.cpp +++ b/src/declarative/qml/v4/qv4bindings.cpp @@ -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(e); diff --git a/src/declarative/qml/v4/qv4bindings_p.h b/src/declarative/qml/v4/qv4bindings_p.h index a3e8ed8a3c..58dd4328af 100644 --- a/src/declarative/qml/v4/qv4bindings_p.h +++ b/src/declarative/qml/v4/qv4bindings_p.h @@ -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; diff --git a/src/declarative/qml/v8/qv8bindings.cpp b/src/declarative/qml/v8/qv8bindings.cpp index 648e8f67b7..439ba1e1a4 100644 --- a/src/declarative/qml/v8/qv8bindings.cpp +++ b/src/declarative/qml/v8/qv8bindings.cpp @@ -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); diff --git a/src/declarative/qml/v8/qv8bindings_p.h b/src/declarative/qml/v8/qv8bindings_p.h index b1bb169edb..14de2d1705 100644 --- a/src/declarative/qml/v8/qv8bindings_p.h +++ b/src/declarative/qml/v8/qv8bindings_p.h @@ -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. -- cgit v1.2.3