modelChanged() should not be emitted until view is repopulated

Otherwise, itemAt() returns invalid values at the time that
modelChanged() is emitted.

Task-number: QTBUG-17156
Reviewed-by: Martin Jones

3 files changed, 60 insertions, 1 deletions

diff --git a/src/declarative/graphicsitems/qdeclarativerepeater.cpp b/src/declarative/graphicsitems/qdeclarativerepeater.cpp
index 8455513..4d0f34c 100644
--- a/src/declarative/graphicsitems/qdeclarativerepeater.cpp
+++ b/src/declarative/graphicsitems/qdeclarativerepeater.cpp
@@ -198,7 +198,6 @@ void QDeclarativeRepeater::setModel(const QVariant &model)
     */
     }
     d->dataSource = model;
-    emit modelChanged();
     QObject *object = qvariant_cast<QObject*>(model);
     QDeclarativeVisualModel *vim = 0;
     if (object && (vim = qobject_cast<QDeclarativeVisualModel *>(object))) {
@@ -226,6 +225,7 @@ void QDeclarativeRepeater::setModel(const QVariant &model)
         */
         regenerate();
     }
+    emit modelChanged();
     emit countChanged();
 }
 
diff --git a/tests/auto/declarative/qdeclarativerepeater/data/modelChanged.qml b/tests/auto/declarative/qdeclarativerepeater/data/modelChanged.qml
new file mode 100644
index 0000000..0b57d50
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativerepeater/data/modelChanged.qml
@@ -0,0 +1,26 @@
+import QtQuick 1.1
+
+Column {
+    Repeater {
+        id: repeater
+        objectName: "repeater"
+
+        property int itemsCount
+        property variant itemsFound: []
+
+        delegate: Rectangle {
+            color: "red"
+            width: (index+1)*50
+            height: 50
+        }
+
+        onModelChanged: {
+            repeater.itemsCount = repeater.count
+            var items = []
+            for (var i=0; i<repeater.count; i++)
+                items.push(repeater.itemAt(i))
+            repeater.itemsFound = items
+        }
+    }
+}
+
diff --git a/tests/auto/declarative/qdeclarativerepeater/tst_qdeclarativerepeater.cpp b/tests/auto/declarative/qdeclarativerepeater/tst_qdeclarativerepeater.cpp
index 6b840a3..820a282 100644
--- a/tests/auto/declarative/qdeclarativerepeater/tst_qdeclarativerepeater.cpp
+++ b/tests/auto/declarative/qdeclarativerepeater/tst_qdeclarativerepeater.cpp
@@ -74,6 +74,7 @@ private slots:
     void dataModel_changes();
     void itemModel();
     void resetModel();
+    void modelChanged();
     void properties();
     void testQtQuick11Attributes();
     void testQtQuick11Attributes_data();
@@ -534,7 +535,10 @@ void tst_QDeclarativeRepeater::resetModel()
     QVERIFY(container != 0);
 
     QCOMPARE(repeater->count(), dataA.count());
+    for (int i=0; i<repeater->count(); i++)
+        QCOMPARE(repeater->itemAt(i), container->childItems().at(i+1)); // +1 to skip first Text object
 
+    QSignalSpy modelChangedSpy(repeater, SIGNAL(modelChanged()));
     QSignalSpy countSpy(repeater, SIGNAL(countChanged()));
     QSignalSpy addedSpy(repeater, SIGNAL(itemAdded(int,QDeclarativeItem*)));
     QSignalSpy removedSpy(repeater, SIGNAL(itemRemoved(int,QDeclarativeItem*)));
@@ -547,6 +551,7 @@ void tst_QDeclarativeRepeater::resetModel()
     ctxt->setContextProperty("testData", dataB);
     QCOMPARE(repeater->count(), dataB.count());
 
+    QCOMPARE(modelChangedSpy.count(), 1);
     QCOMPARE(countSpy.count(), 1);
     QCOMPARE(removedSpy.count(), dataA.count());
     QCOMPARE(addedSpy.count(), dataB.count());
@@ -554,6 +559,7 @@ void tst_QDeclarativeRepeater::resetModel()
         QCOMPARE(addedSpy.at(i).at(0).toInt(), i);
         QCOMPARE(addedSpy.at(i).at(1).value<QDeclarativeItem*>(), repeater->itemAt(i));
     }
+    modelChangedSpy.clear();
     countSpy.clear();
     removedSpy.clear();
     addedSpy.clear();
@@ -562,6 +568,7 @@ void tst_QDeclarativeRepeater::resetModel()
     repeater->setModel(dataA);
     QCOMPARE(repeater->count(), dataA.count());
 
+    QCOMPARE(modelChangedSpy.count(), 1);
     QCOMPARE(countSpy.count(), 1);
     QCOMPARE(removedSpy.count(), dataB.count());
     QCOMPARE(addedSpy.count(), dataA.count());
@@ -569,6 +576,32 @@ void tst_QDeclarativeRepeater::resetModel()
         QCOMPARE(addedSpy.at(i).at(0).toInt(), i);
         QCOMPARE(addedSpy.at(i).at(1).value<QDeclarativeItem*>(), repeater->itemAt(i));
     }
+
+    delete canvas;
+}
+
+// QTBUG-17156
+void tst_QDeclarativeRepeater::modelChanged()
+{
+    QDeclarativeEngine engine;
+    QDeclarativeComponent component(&engine, TEST_FILE("/modelChanged.qml"));
+
+    QDeclarativeItem *rootObject = qobject_cast<QDeclarativeItem*>(component.create());
+    QVERIFY(rootObject);
+    QDeclarativeRepeater *repeater = findItem<QDeclarativeRepeater>(rootObject, "repeater");
+    QVERIFY(repeater);
+
+    repeater->setModel(4);
+    QCOMPARE(repeater->count(), 4);
+    QCOMPARE(repeater->property("itemsCount"), 4);
+    QCOMPARE(repeater->property("itemsFound").toList().count(), 4);
+
+    repeater->setModel(10);
+    QCOMPARE(repeater->count(), 10);
+    QCOMPARE(repeater->property("itemsCount"), 10);
+    QCOMPARE(repeater->property("itemsFound").toList().count(), 10);
+
+    delete rootObject;
 }
 
 void tst_QDeclarativeRepeater::properties()