Change repeater item to handle model being deleted.

The repeater item previously stored a raw QObject pointer in
a variant. When this pointer was a dynamic list model element
that was deleted, the variant would continue to hold a stale
pointer. Change repeater to use a guard object to hold the model
when it is a QObject. Continue to use a variant to hold models
that are not based on QObject to maintain same semantics.

Change-Id: Ie100947132923803263c725e86efa68206382f12
Reviewed-by: Martin Jones <martin.jones@nokia.com>

diff --git a/src/quick/items/qquickrepeater.cpp b/src/quick/items/qquickrepeater.cpp
index d26ebed..1f7578c 100644 (file)
--- a/src/quick/items/qquickrepeater.cpp
+++ b/src/quick/items/qquickrepeater.cpp
@@ -51,7 +51,7 @@
 QT_BEGIN_NAMESPACE
 
 QQuickRepeaterPrivate::QQuickRepeaterPrivate()
-    : model(0), ownModel(false), inRequest(false), itemCount(0), createFrom(-1)
+    : model(0), ownModel(false), inRequest(false), dataSourceIsObject(false), itemCount(0), createFrom(-1)
 {
 }
 
@@ -175,6 +175,12 @@ QQuickRepeater::~QQuickRepeater()
 QVariant QQuickRepeater::model() const
 {
     Q_D(const QQuickRepeater);
+
+    if (d->dataSourceIsObject) {
+        QObject *o = d->dataSourceAsObject;
+        return QVariant::fromValue(o);
+    }
+
     return d->dataSource;
 }
 
@@ -194,6 +200,8 @@ void QQuickRepeater::setModel(const QVariant &model)
     }
     d->dataSource = model;
     QObject *object = qvariant_cast<QObject*>(model);
+    d->dataSourceAsObject = object;
+    d->dataSourceIsObject = object != 0;
     QQuickVisualModel *vim = 0;
     if (object && (vim = qobject_cast<QQuickVisualModel *>(object))) {
         if (d->ownModel) {

diff --git a/src/quick/items/qquickrepeater_p_p.h b/src/quick/items/qquickrepeater_p_p.h
index 88e0c94..41d77ad 100644 (file)
--- a/src/quick/items/qquickrepeater_p_p.h
+++ b/src/quick/items/qquickrepeater_p_p.h
@@ -75,8 +75,10 @@ private:
 
     QQuickVisualModel *model;
     QVariant dataSource;
+    QQmlGuard<QObject> dataSourceAsObject;
     bool ownModel : 1;
     bool inRequest : 1;
+    bool dataSourceIsObject : 1;
     int itemCount;
     int createFrom;
 

diff --git a/tests/auto/quick/qquickrepeater/data/dynamicmodelcrash.qml b/tests/auto/quick/qquickrepeater/data/dynamicmodelcrash.qml
new file mode 100644 (file)
index 0000000..0280df0
--- /dev/null
+++ b/tests/auto/quick/qquickrepeater/data/dynamicmodelcrash.qml
@@ -0,0 +1,20 @@
+import QtQuick 2.0
+
+Item {
+    ListModel {
+        id: lm;
+    }
+
+    Component.onCompleted: {
+        lm.append({ subModel: [ {d:0} ] });
+        rep.model = lm.get(0).subModel;
+        rep.model;
+        lm.remove(0);
+        rep.model;
+    }
+
+    Repeater {
+        objectName: "rep"
+        id: rep
+    }
+}

diff --git a/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp b/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp
index 1b07a6e..d9cbed0 100644 (file)
--- a/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp
+++ b/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp
@@ -77,6 +77,7 @@ private slots:
     void properties();
     void asynchronous();
     void initParent();
+    void dynamicModelCrash();
 };
 
 class TestObject : public QObject
@@ -639,6 +640,20 @@ void tst_QQuickRepeater::initParent()
     QCOMPARE(qvariant_cast<QQuickItem*>(rootObject->property("parentItem")), rootObject);
 }
 
+void tst_QQuickRepeater::dynamicModelCrash()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine, testFileUrl("dynamicmodelcrash.qml"));
+
+    // Don't crash
+    QQuickItem *rootObject = qobject_cast<QQuickItem*>(component.create());
+    QVERIFY(rootObject);
+
+    QQuickRepeater *repeater = findItem<QQuickRepeater>(rootObject, "rep");
+    QVERIFY(repeater);
+    QVERIFY(qvariant_cast<QObject *>(repeater->model()) == 0);
+}
+
 QTEST_MAIN(tst_QQuickRepeater)
 
 #include "tst_qquickrepeater.moc"