Don't construct VisualDataModel attached properties unless requested.

This saves allocating a QObject per item model in the common case.

Change-Id: I0e77e6c6c0c64ac6c5e482ef55e194c68e778b32
Reviewed-by: Bea Lam <bea.lam@nokia.com>

diff --git a/src/quick/items/qquickvisualdatamodel.cpp b/src/quick/items/qquickvisualdatamodel.cpp
index 7f80fa0..a1da179 100644 (file)
--- a/src/quick/items/qquickvisualdatamodel.cpp
+++ b/src/quick/items/qquickvisualdatamodel.cpp
@@ -74,8 +74,6 @@ QQuickVisualDataModelParts::QQuickVisualDataModelParts(QQuickVisualDataModel *pa
 
 //---------------------------------------------------------------------------
 
-QHash<QObject*, QQuickVisualDataModelAttached*> QQuickVisualDataModelAttached::attachedProperties;
-
 /*!
     \qmlclass VisualDataModel QQuickVisualDataModel
     \inqmlmodule QtQuick 2
@@ -418,29 +416,17 @@ int QQuickVisualDataModel::count() const
     return d->m_compositor.count(d->m_compositorGroup);
 }
 
-void QQuickVisualDataModelPrivate::destroy(QObject *object)
-{
-    QObjectPrivate *p = QObjectPrivate::get(object);
-    Q_ASSERT(p->declarativeData);
-    QQmlData *data = static_cast<QQmlData*>(p->declarativeData);
-    if (data->ownContext && data->context)
-        data->context->clearContext();
-    object->deleteLater();
-}
-
 QQuickVisualDataModel::ReleaseFlags QQuickVisualDataModelPrivate::release(QObject *object)
 {
     QQuickVisualDataModel::ReleaseFlags stat = 0;
     if (!object)
         return stat;
 
-    if (QQuickVisualDataModelAttached *attached = QQuickVisualDataModelAttached::properties(object)) {
-        QQuickVisualDataModelItem *cacheItem = attached->m_cacheItem;
+    if (QQuickVisualDataModelItem *cacheItem = QQuickVisualDataModelItem::dataForObject(object)) {
         if (cacheItem->releaseObject()) {
-            destroy(object);
+            cacheItem->destroyObject();
             if (QQuickItem *item = qobject_cast<QQuickItem *>(object))
                 emitDestroyingItem(item);
-            cacheItem->setObject(0);
             if (cacheItem->incubationTask) {
                 releaseIncubator(cacheItem->incubationTask);
                 cacheItem->incubationTask = 0;
@@ -486,13 +472,13 @@ void QQuickVisualDataModel::cancel(int index)
             cacheItem->incubationTask = 0;
         }
         if (cacheItem->object() && !cacheItem->isObjectReferenced()) {
-            d->destroy(cacheItem->object());
-            if (QQuickPackage *package = qobject_cast<QQuickPackage *>(cacheItem->object()))
+            QObject *object = cacheItem->object();
+            cacheItem->destroyObject();
+            if (QQuickPackage *package = qobject_cast<QQuickPackage *>(object))
                 d->emitDestroyingPackage(package);
-            else if (QQuickItem *item = qobject_cast<QQuickItem *>(cacheItem->object()))
+            else if (QQuickItem *item = qobject_cast<QQuickItem *>(object))
                 d->emitDestroyingItem(item);
-            cacheItem->setObject(0);
-            cacheItem->Dispose();
+            cacheItem->scriptRef -= 1;
         }
         if (!cacheItem->isReferenced()) {
             d->m_compositor.clearFlags(Compositor::Cache, it.cacheIndex, 1, Compositor::CacheFlag);
@@ -786,10 +772,6 @@ void QQuickVisualDataModelPrivate::setInitialState(QVDMIncubationTask *incubatio
     QQml_setParent_noEvent(incubationTask->incubatingContext, cacheItem->object());
     incubationTask->incubatingContext = 0;
 
-    cacheItem->attached = QQuickVisualDataModelAttached::properties(cacheItem->object());
-    cacheItem->attached->setCacheItem(cacheItem);
-    new QQuickVisualDataModelAttachedMetaObject(cacheItem->attached, m_cacheMetaType);
-    cacheItem->attached->emitChanges();
 
     if (QQuickPackage *package = qobject_cast<QQuickPackage *>(cacheItem->object()))
         emitInitPackage(cacheItem, package);
@@ -908,8 +890,8 @@ QString QQuickVisualDataModel::stringValue(int index, const QString &name)
 int QQuickVisualDataModel::indexOf(QQuickItem *item, QObject *) const
 {
     Q_D(const QQuickVisualDataModel);
-    if (QQuickVisualDataModelAttached *attached = QQuickVisualDataModelAttached::properties(item))
-        return attached->m_cacheItem->index[d->m_compositorGroup];
+    if (QQuickVisualDataModelItem *cacheItem = QQuickVisualDataModelItem::dataForObject(item))
+        return cacheItem->index[d->m_compositorGroup];
     return -1;
 }
 
@@ -1125,10 +1107,11 @@ void QQuickVisualDataModelPrivate::itemsRemoved(
             for (; cacheIndex < remove.cacheIndex + remove.count - removedCache; ++cacheIndex) {
                 QQuickVisualDataModelItem *cacheItem = m_cache.at(cacheIndex);
                 if (remove.inGroup(Compositor::Persisted) && cacheItem->objectRef == 0 && cacheItem->object()) {
-                    destroy(cacheItem->object());
-                    if (QQuickPackage *package = qobject_cast<QQuickPackage *>(cacheItem->object()))
+                    QObject *object = cacheItem->object();
+                    cacheItem->destroyObject();
+                    if (QQuickPackage *package = qobject_cast<QQuickPackage *>(object))
                         emitDestroyingPackage(package);
-                    else if (QQuickItem *item = qobject_cast<QQuickItem *>(cacheItem->object()))
+                    else if (QQuickItem *item = qobject_cast<QQuickItem *>(object))
                         emitDestroyingItem(item);
                     cacheItem->setObject(0);
                     cacheItem->scriptRef -= 1;
@@ -1371,7 +1354,13 @@ void QQuickVisualDataModel::_q_layoutChanged()
 
 QQuickVisualDataModelAttached *QQuickVisualDataModel::qmlAttachedProperties(QObject *obj)
 {
-    return QQuickVisualDataModelAttached::properties(obj);
+    if (QQuickVisualDataModelItem *cacheItem = QQuickVisualDataModelItem::dataForObject(obj)) {
+        if (cacheItem->object() == obj) { // Don't create attached item for child objects.
+            cacheItem->attached = new QQuickVisualDataModelAttached(cacheItem, obj);
+            return cacheItem->attached;
+        }
+    }
+    return new QQuickVisualDataModelAttached(obj);
 }
 
 bool QQuickVisualDataModelPrivate::insert(
@@ -1628,6 +1617,8 @@ v8::Handle<v8::Value> QQuickVisualDataModelItemMetaType::get_index(
 
 //---------------------------------------------------------------------------
 
+QHash<QObject*, QQuickVisualDataModelItem *> QQuickVisualDataModelItem::contextData;
+
 QQuickVisualDataModelItem::QQuickVisualDataModelItem(
         QQuickVisualDataModelItemMetaType *metaType, int modelIndex)
     : QV8ObjectResource(metaType->v8Engine)
@@ -1675,8 +1666,40 @@ void QQuickVisualDataModelItem::Dispose()
     delete this;
 }
 
-void QQuickVisualDataModelItem::objectDestroyed(QObject *)
+void QQuickVisualDataModelItem::setObject(QObject *g)
+{
+    if (QObject *previous = object())
+        contextData.remove(previous);
+    if (g)
+        contextData.insert(g, this);
+
+    QQmlGuard<QObject>::setObject(g);
+}
+
+void QQuickVisualDataModelItem::destroyObject()
+{
+    QObject * const obj = object();
+    setObject(0);
+
+    Q_ASSERT(obj);
+
+    QObjectPrivate *p = QObjectPrivate::get(obj);
+    Q_ASSERT(p->declarativeData);
+    QQmlData *data = static_cast<QQmlData*>(p->declarativeData);
+    if (data->ownContext && data->context)
+        data->context->clearContext();
+    obj->deleteLater();
+
+    if (attached) {
+        attached->m_cacheItem = 0;
+        attached = 0;
+    }
+}
+
+void QQuickVisualDataModelItem::objectDestroyed(QObject *object)
 {
+    contextData.remove(object);
+
     attached = 0;
     Dispose();
 }
@@ -1688,6 +1711,9 @@ QQuickVisualDataModelAttachedMetaObject::QQuickVisualDataModelAttachedMetaObject
     : attached(attached)
     , metaType(metaType)
 {
+    if (!metaType->metaObject)
+        metaType->initializeMetaObject();
+
     metaType->addref();
     *static_cast<QMetaObject *>(this) = *metaType->metaObject;
     QObjectPrivate::get(attached)->metaObject = this;
@@ -1738,11 +1764,25 @@ int QQuickVisualDataModelAttachedMetaObject::metaCall(QMetaObject::Call call, in
     return attached->qt_metacall(call, _id, arguments);
 }
 
-void QQuickVisualDataModelAttached::setCacheItem(QQuickVisualDataModelItem *item)
+QQuickVisualDataModelAttached::QQuickVisualDataModelAttached(QObject *parent)
+    : m_cacheItem(0)
+    , m_previousGroups(0)
+    , m_modelChanged(false)
 {
-    m_cacheItem = item;
+    QQml_setParent_noEvent(this, parent);
+}
+
+QQuickVisualDataModelAttached::QQuickVisualDataModelAttached(
+        QQuickVisualDataModelItem *cacheItem, QObject *parent)
+    : m_cacheItem(cacheItem)
+    , m_previousGroups(cacheItem->groups)
+    , m_modelChanged(false)
+{
+    QQml_setParent_noEvent(this, parent);
     for (int i = 1; i < m_cacheItem->metaType->groupCount; ++i)
         m_previousIndex[i] = m_cacheItem->index[i];
+
+    new QQuickVisualDataModelAttachedMetaObject(this, cacheItem->metaType);
 }
 
 /*!
@@ -2299,7 +2339,7 @@ void QQuickVisualDataGroup::resolve(QQmlV8Function *args)
         Q_ASSERT(model->m_cache.count() == model->m_compositor.count(Compositor::Cache));
     } else {
         cacheItem->resolveIndex(model->m_adaptorModel, resolvedIndex);
-        if (cacheItem->object())
+        if (cacheItem->attached)
             cacheItem->attached->emitUnresolvedChanged();
     }
 
@@ -2729,8 +2769,8 @@ int QQuickVisualPartsModel::indexOf(QQuickItem *item, QObject *) const
 {
     QHash<QObject *, QQuickPackage *>::const_iterator it = m_packaged.find(item);
     if (it != m_packaged.end()) {
-        if (QQuickVisualDataModelAttached *attached = QQuickVisualDataModelAttached::properties(*it))
-            return attached->m_cacheItem->index[m_compositorGroup];
+        if (QQuickVisualDataModelItem *cacheItem = QQuickVisualDataModelItem::dataForObject(*it))
+            return cacheItem->index[m_compositorGroup];
     }
     return -1;
 }

diff --git a/src/quick/items/qquickvisualdatamodel_p.h b/src/quick/items/qquickvisualdatamodel_p.h
index 1752e40..114d394 100644 (file)
--- a/src/quick/items/qquickvisualdatamodel_p.h
+++ b/src/quick/items/qquickvisualdatamodel_p.h
@@ -196,14 +196,9 @@ class QQuickVisualDataModelAttached : public QObject
     Q_PROPERTY(QStringList groups READ groups WRITE setGroups NOTIFY groupsChanged)
     Q_PROPERTY(bool isUnresolved READ isUnresolved NOTIFY unresolvedChanged)
 public:
-    QQuickVisualDataModelAttached(QObject *parent)
-        : m_cacheItem(0)
-        , m_previousGroups(0)
-        , m_modelChanged(false)
-    {
-        QQml_setParent_noEvent(this, parent);
-    }
-    ~QQuickVisualDataModelAttached() { attachedProperties.remove(parent()); }
+    QQuickVisualDataModelAttached(QObject *parent);
+    QQuickVisualDataModelAttached(QQuickVisualDataModelItem *cacheItem, QObject *parent);
+    ~QQuickVisualDataModelAttached() {}
 
     void setCacheItem(QQuickVisualDataModelItem *item);
 
@@ -218,16 +213,6 @@ public:
 
     void emitUnresolvedChanged() { emit unresolvedChanged(); }
 
-    static QQuickVisualDataModelAttached *properties(QObject *obj)
-    {
-        QQuickVisualDataModelAttached *rv = attachedProperties.value(obj);
-        if (!rv) {
-            rv = new QQuickVisualDataModelAttached(obj);
-            attachedProperties.insert(obj, rv);
-        }
-        return rv;
-    }
-
 Q_SIGNALS:
     void modelChanged();
     void groupsChanged();
@@ -239,8 +224,6 @@ public:
     int m_previousIndex[QQuickListCompositor::MaximumGroupCount];
     bool m_modelChanged;
 
-    static QHash<QObject*, QQuickVisualDataModelAttached*> attachedProperties;
-
     friend class QQuickVisualDataModelAttachedMetaObject;
 };
 

diff --git a/src/quick/items/qquickvisualdatamodel_p_p.h b/src/quick/items/qquickvisualdatamodel_p_p.h
index 5fd99d0..7dd6603 100644 (file)
--- a/src/quick/items/qquickvisualdatamodel_p_p.h
+++ b/src/quick/items/qquickvisualdatamodel_p_p.h
@@ -103,7 +103,7 @@ public:
 class QQuickVisualAdaptorModel;
 class QVDMIncubationTask;
 
-class QQuickVisualDataModelItem : public QObject, public QV8ObjectResource, public QQmlGuard<QObject>
+class QQuickVisualDataModelItem : public QObject, public QV8ObjectResource, private QQmlGuard<QObject>
 {
     Q_OBJECT
     Q_PROPERTY(int index READ modelIndex NOTIFY modelIndexChanged)
@@ -127,6 +127,12 @@ public:
 
     QObject *modelObject() { return this; }
 
+    inline QObject *object() const { return QQmlGuard<QObject>::object(); }
+    void setObject(QObject *g);
+    void destroyObject();
+
+    static QQuickVisualDataModelItem *dataForObject(QObject *object) { return contextData.value(object, 0); }
+
     int modelIndex() const { return index[0]; }
     void setModelIndex(int idx) { index[0] = idx; emit modelIndexChanged(); }
 
@@ -151,6 +157,12 @@ Q_SIGNALS:
 
 protected:
     void objectDestroyed(QObject *);
+
+private:
+    // A static hash of context data for all delegate object isn't ideal, but it provides a way
+    // to get the context data when constructing attached objects rather than constructing the
+    // attached objects on suspicion.
+    static QHash<QObject*, QQuickVisualDataModelItem *> contextData;
 };
 
 
@@ -236,7 +248,6 @@ public:
     void connectModel(QQuickVisualAdaptorModel *model);
 
     QObject *object(Compositor::Group group, int index, bool asynchronous, bool reference);
-    void destroy(QObject *object);
     QQuickVisualDataModel::ReleaseFlags release(QObject *object);
     QString stringValue(Compositor::Group group, int index, const QString &name);
     void emitCreatedPackage(QQuickVisualDataModelItem *cacheItem, QQuickPackage *package);
@@ -295,7 +306,6 @@ public:
 
     QString m_filterGroup;
 
-
     int m_count;
     int m_groupCount;
 

diff --git a/tests/auto/quick/qquickvisualdatamodel/data/invalidAttachment.qml b/tests/auto/quick/qquickvisualdatamodel/data/invalidAttachment.qml
new file mode 100644 (file)
index 0000000..2758f56
--- /dev/null
+++ b/tests/auto/quick/qquickvisualdatamodel/data/invalidAttachment.qml
@@ -0,0 +1,19 @@
+import QtQuick 2.0
+
+Item {
+    property VisualDataModel invalidVdm: VisualDataModel.model
+    Repeater {
+        model: 1
+        delegate: Item {
+            id: outer
+            objectName: "delegate"
+
+            property VisualDataModel validVdm: outer.VisualDataModel.model
+            property VisualDataModel invalidVdm: inner.VisualDataModel.model
+
+            Item {
+                id: inner
+            }
+        }
+    }
+}

diff --git a/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp b/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp
index b8d46ac..9047dcc 100644 (file)
--- a/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp
+++ b/tests/auto/quick/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp
@@ -229,6 +229,7 @@ private slots:
     void resolve();
     void warnings_data();
     void warnings();
+    void invalidAttachment();
 
 private:
     template <int N> void groups_verify(
@@ -3439,6 +3440,31 @@ void tst_qquickvisualdatamodel::warnings()
     QCOMPARE(evaluate<int>(listView, "count"), count);
 }
 
+void tst_qquickvisualdatamodel::invalidAttachment()
+{
+    QQmlComponent component(&engine);
+    component.loadUrl(testFileUrl("invalidAttachment.qml"));
+
+    QScopedPointer<QObject> object(component.create());
+    QVERIFY(object);
+    QCOMPARE(component.errors().count(), 0);
+
+    QVariant property = object->property("invalidVdm");
+    QCOMPARE(property.userType(), qMetaTypeId<QQuickVisualDataModel *>());
+    QVERIFY(!property.value<QQuickVisualDataModel *>());
+
+    QQuickItem *item = object->findChild<QQuickItem *>("delegate");
+    QVERIFY(item);
+
+    property = item->property("validVdm");
+    QCOMPARE(property.userType(), qMetaTypeId<QQuickVisualDataModel *>());
+    QVERIFY(property.value<QQuickVisualDataModel *>());
+
+    property = item->property("invalidVdm");
+    QCOMPARE(property.userType(), qMetaTypeId<QQuickVisualDataModel *>());
+    QVERIFY(!property.value<QQuickVisualDataModel *>());
+}
+
 
 QTEST_MAIN(tst_qquickvisualdatamodel)