Fix property caches out of sync with grouped properties that have a VME meta object

Because QQmlObjectCreator::populateInstance would take the property cache to
install from the outside and also pass it as the cache to use for the VME meta
object to install, it could happen that the wrong cache was installed - the one
supplied by engine->cache(propType) instead of the cache created together with
the VME meta-object at type compilation time.

This patch ensures that they're always in sync and correct by removing the
responsibility of the caller to supply the cache to use and install. Instead
the function will always use the cache calculated at type compile time (and
also use that when installing the VME meta object).

Installation of the property cache on the declarative data of the instance is
now done only at createInstance() time, which fortunately also simplifies the
code.

Change-Id: Ia722cd57bc48007aaf725f1f59daa2f21569e324
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

1 files changed, 17 insertions, 30 deletions

diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index a5f7988bf8..50fe5861a6 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -737,9 +737,7 @@ bool QQmlObjectCreator::setPropertyBinding(QQmlPropertyData *property, const QV4
         QQmlType *attachedType = tr->type;
         const int id = attachedType->attachedPropertiesId();
         QObject *qmlObject = qmlAttachedPropertiesObjectById(id, _qobject);
-        QQmlRefPointer<QQmlPropertyCache> cache = QQmlEnginePrivate::get(engine)->cache(attachedType->attachedPropertiesType());
-        Q_ASSERT(!cache.isNull());
-        if (!populateInstance(binding->value.objectIndex, qmlObject, cache, qmlObject, /*value type property*/0))
+        if (!populateInstance(binding->value.objectIndex, qmlObject, qmlObject, /*value type property*/0))
             return false;
         return true;
     }
@@ -776,8 +774,6 @@ bool QQmlObjectCreator::setPropertyBinding(QQmlPropertyData *property, const QV4
         const QV4::CompiledData::Object *obj = qmlUnit->objectAt(binding->value.objectIndex);
         if (stringAt(obj->inheritedTypeNameIndex).isEmpty()) {
 
-            QQmlEnginePrivate *enginePrivate = QQmlEnginePrivate::get(engine);
-            QQmlRefPointer<QQmlPropertyCache> groupObjectPropertyCache;
             QObject *groupObject = 0;
             QQmlValueType *valueType = 0;
             QQmlPropertyData *valueTypeProperty = 0;
@@ -792,7 +788,6 @@ bool QQmlObjectCreator::setPropertyBinding(QQmlPropertyData *property, const QV4
 
                 valueType->read(_qobject, property->coreIndex);
 
-                groupObjectPropertyCache = enginePrivate->cache(valueType);
                 groupObject = valueType;
                 valueTypeProperty = property;
             } else {
@@ -803,20 +798,10 @@ bool QQmlObjectCreator::setPropertyBinding(QQmlPropertyData *property, const QV4
                     return false;
                 }
 
-                if (QQmlData *groupDeclarativeData = QQmlData::get(groupObject))
-                    groupObjectPropertyCache = groupDeclarativeData->propertyCache;
-                if (!groupObjectPropertyCache)
-                    groupObjectPropertyCache = enginePrivate->propertyCacheForType(property->propType);
-                if (!groupObjectPropertyCache) {
-                    recordError(binding->location, tr("Invalid grouped property access"));
-                    return false;
-                }
-
-
                 bindingTarget = groupObject;
             }
 
-            if (!populateInstance(binding->value.objectIndex, groupObject, groupObjectPropertyCache, bindingTarget, valueTypeProperty))
+            if (!populateInstance(binding->value.objectIndex, groupObject, bindingTarget, valueTypeProperty))
                 return false;
 
             if (valueType)
@@ -1139,6 +1124,12 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent)
 
     QQmlRefPointer<QQmlPropertyCache> cache = propertyCaches.value(index);
     Q_ASSERT(!cache.isNull());
+    if (installPropertyCache) {
+        if (ddata->propertyCache)
+            ddata->propertyCache->release();;
+        ddata->propertyCache = cache;
+        ddata->propertyCache->addref();
+    }
 
     QObject *scopeObject = instance;
     qSwap(_scopeObject, scopeObject);
@@ -1153,7 +1144,7 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent)
 
     qSwap(_qmlContext, qmlContext);
 
-    bool result = populateInstance(index, instance, cache, /*binding target*/instance, /*value type property*/0, installPropertyCache, bindingsToSkip);
+    bool result = populateInstance(index, instance, /*binding target*/instance, /*value type property*/0, bindingsToSkip);
 
     qSwap(_qmlContext, qmlContext);
     qSwap(_scopeObject, scopeObject);
@@ -1262,14 +1253,13 @@ void QQmlObjectCreator::clear()
     phase = Done;
 }
 
-bool QQmlObjectCreator::populateInstance(int index, QObject *instance, QQmlRefPointer<QQmlPropertyCache> cache, QObject *bindingTarget, QQmlPropertyData *valueTypeProperty, bool installPropertyCache, const QBitArray &bindingsToSkip)
+bool QQmlObjectCreator::populateInstance(int index, QObject *instance, QObject *bindingTarget, QQmlPropertyData *valueTypeProperty, const QBitArray &bindingsToSkip)
 {
     const QV4::CompiledData::Object *obj = qmlUnit->objectAt(index);
     Q_QML_VME_PROFILE(sharedState->profiler, push());
 
     QQmlData *declarativeData = QQmlData::get(instance, /*create*/true);
 
-    qSwap(_propertyCache, cache);
     qSwap(_qobject, instance);
     qSwap(_valueTypeProperty, valueTypeProperty);
     qSwap(_compiledObject, obj);
@@ -1280,28 +1270,25 @@ bool QQmlObjectCreator::populateInstance(int index, QObject *instance, QQmlRefPo
     QV4::Scope valueScope(v4);
     QV4::ScopedValue scopeObjectProtector(valueScope);
 
+    QQmlRefPointer<QQmlPropertyCache> cache = propertyCaches.value(index);
+
     QQmlVMEMetaObject *vmeMetaObject = 0;
     const QByteArray data = vmeMetaObjectData.value(index);
     if (!data.isEmpty()) {
+        Q_ASSERT(!cache.isNull());
         // install on _object
-        vmeMetaObject = new QQmlVMEMetaObject(_qobject, _propertyCache, reinterpret_cast<const QQmlVMEMetaData*>(data.constData()));
+        vmeMetaObject = new QQmlVMEMetaObject(_qobject, cache, reinterpret_cast<const QQmlVMEMetaData*>(data.constData()));
         if (_ddata->propertyCache)
             _ddata->propertyCache->release();
-        Q_ASSERT(installPropertyCache);
+        _ddata->propertyCache = cache;
+        _ddata->propertyCache->addref();
         scopeObjectProtector = _ddata->jsWrapper.value();
     } else {
         vmeMetaObject = QQmlVMEMetaObject::get(_qobject);
     }
-    if (installPropertyCache) {
-        Q_ASSERT(_propertyCache);
-        _ddata->propertyCache = _propertyCache;
-        _ddata->propertyCache->addref();
-    }
-
+    qSwap(_propertyCache, cache);
     qSwap(_vmeMetaObject, vmeMetaObject);
 
-    QVector<QQmlAbstractBinding*> createdBindings(_compiledObject->nBindings, 0);
-
     QBitArray bindingSkipList = bindingsToSkip;
     {
         QHash<int, QBitArray>::ConstIterator deferredBindings = compiledData->deferredBindingsPerObject.find(index);