Call onAdd() for first items added to ListView and GridView

The bug was caused by the fact that the itemsInserted()
implementation in ListView and GridView returned early for cases
where (model->count() <= 1) and so did not call onAdd() unlike for
other cases. This change also fixes some incorrect values in the
header() and footer() unit tests for ListView and GridView.

Change-Id: I24f5c86afcd62c26e17b0932f257f4767a287b8e
Task-number: QTBUG-15642
Reviewed-by: Martin Jones

6 files changed, 340 insertions, 57 deletions

diff --git a/src/declarative/graphicsitems/qdeclarativegridview.cpp b/src/declarative/graphicsitems/qdeclarativegridview.cpp
index 9aade98..f4f7d29 100644
--- a/src/declarative/graphicsitems/qdeclarativegridview.cpp
+++ b/src/declarative/graphicsitems/qdeclarativegridview.cpp
@@ -2388,24 +2388,9 @@ void QDeclarativeGridView::itemsInserted(int modelIndex, int count)
     Q_D(QDeclarativeGridView);
     if (!isComponentComplete())
         return;
-    if (!d->visibleItems.count() || d->model->count() <= 1) {
-        d->scheduleLayout();
-        if (d->itemCount && d->currentIndex >= modelIndex) {
-            // adjust current item index
-            d->currentIndex += count;
-            if (d->currentItem)
-                d->currentItem->index = d->currentIndex;
-            emit currentIndexChanged();
-        } else if (!d->currentIndex || (d->currentIndex < 0 && !d->currentIndexCleared)) {
-            d->updateCurrent(0);
-        }
-        d->itemCount += count;
-        emit countChanged();
-        return;
-    }
 
-    int index = d->mapFromModel(modelIndex);
-    if (index == -1) {
+    int index = d->visibleItems.count() ? d->mapFromModel(modelIndex) : 0;
+    if (index < 0) {
         int i = d->visibleItems.count() - 1;
         while (i > 0 && d->visibleItems.at(i)->index == -1)
             --i;
@@ -2436,28 +2421,35 @@ void QDeclarativeGridView::itemsInserted(int modelIndex, int count)
         }
     }
 
-    // At least some of the added items will be visible
     int insertCount = count;
-    if (index < d->visibleIndex) {
+    if (index < d->visibleIndex && d->visibleItems.count()) {
         insertCount -= d->visibleIndex - index;
         index = d->visibleIndex;
         modelIndex = d->visibleIndex;
     }
 
-    index -= d->visibleIndex;
     int to = d->buffer+d->position()+d->size()-1;
-    int colPos, rowPos;
-    if (index < d->visibleItems.count()) {
-        colPos = d->visibleItems.at(index)->colPos();
-        rowPos = d->visibleItems.at(index)->rowPos();
-    } else {
-        // appending items to visible list
-        colPos = d->visibleItems.at(index-1)->colPos() + d->colSize();
-        rowPos = d->visibleItems.at(index-1)->rowPos();
-        if (colPos > d->colSize() * (d->columns-1)) {
-            colPos = 0;
-            rowPos += d->rowSize();
+    int colPos = 0;
+    int rowPos = 0;
+    if (d->visibleItems.count()) {
+        index -= d->visibleIndex;
+        if (index < d->visibleItems.count()) {
+            colPos = d->visibleItems.at(index)->colPos();
+            rowPos = d->visibleItems.at(index)->rowPos();
+        } else {
+            // appending items to visible list
+            colPos = d->visibleItems.at(index-1)->colPos() + d->colSize();
+            rowPos = d->visibleItems.at(index-1)->rowPos();
+            if (colPos > d->colSize() * (d->columns-1)) {
+                colPos = 0;
+                rowPos += d->rowSize();
+            }
         }
+    } else if (d->itemCount == 0 && d->header) {
+        if (d->flow == QDeclarativeGridView::LeftToRight)
+            rowPos = d->headerSize();
+        else
+            colPos = d->headerSize();
     }
 
     // Update the indexes of the following visible items.
@@ -2510,6 +2502,8 @@ void QDeclarativeGridView::itemsInserted(int modelIndex, int count)
         if (d->currentItem) {
             d->currentItem->index = d->currentIndex;
             d->currentItem->setPosition(d->colPosAt(d->currentIndex), d->rowPosAt(d->currentIndex));
+        } else if (!d->currentIndex || (d->currentIndex < 0 && !d->currentIndexCleared)) {
+            d->updateCurrent(0);
         }
         emit currentIndexChanged();
     }
diff --git a/src/declarative/graphicsitems/qdeclarativelistview.cpp b/src/declarative/graphicsitems/qdeclarativelistview.cpp
index 075c3af..74e9826 100644
--- a/src/declarative/graphicsitems/qdeclarativelistview.cpp
+++ b/src/declarative/graphicsitems/qdeclarativelistview.cpp
@@ -2848,23 +2848,8 @@ void QDeclarativeListView::itemsInserted(int modelIndex, int count)
         return;
     d->updateUnrequestedIndexes();
     d->moveReason = QDeclarativeListViewPrivate::Other;
-    if (!d->visibleItems.count() || d->model->count() <= 1) {
-        d->scheduleLayout();
-        if (d->itemCount && d->currentIndex >= modelIndex) {
-            // adjust current item index
-            d->currentIndex += count;
-            if (d->currentItem)
-                d->currentItem->index = d->currentIndex;
-            emit currentIndexChanged();
-        } else if (!d->currentIndex || (d->currentIndex < 0 && !d->currentIndexCleared)) {
-            d->updateCurrent(0);
-        }
-        d->itemCount += count;
-        emit countChanged();
-        return;
-    }
 
-    int index = d->mapFromModel(modelIndex);
+    int index = d->visibleItems.count() ? d->mapFromModel(modelIndex) : 0;
     if (index < 0) {
         int i = d->visibleItems.count() - 1;
         while (i > 0 && d->visibleItems.at(i)->index == -1)
@@ -2900,11 +2885,15 @@ void QDeclarativeListView::itemsInserted(int modelIndex, int count)
         }
     }
 
-    // At least some of the added items will be visible
-
     // index can be the next item past the end of the visible items list (i.e. appended)
-    int pos = index < d->visibleItems.count() ? d->visibleItems.at(index)->position()
+    int pos = 0;
+    if (d->visibleItems.count()) {
+        pos = index < d->visibleItems.count() ? d->visibleItems.at(index)->position()
                                                 : d->visibleItems.last()->endPosition()+d->spacing+1;
+    } else if (d->itemCount == 0 && d->header) {
+        pos = d->header->size();
+    }
+
     int initialPos = pos;
     int diff = 0;
     QList<FxListItem*> added;
@@ -2971,6 +2960,8 @@ void QDeclarativeListView::itemsInserted(int modelIndex, int count)
         if (d->currentItem) {
             d->currentItem->index = d->currentIndex;
             d->currentItem->setPosition(d->currentItem->position() + diff);
+        } else if (!d->currentIndex || (d->currentIndex < 0 && !d->currentIndexCleared)) {
+            d->updateCurrent(0);
         }
         emit currentIndexChanged();
     }
diff --git a/tests/auto/declarative/qdeclarativegridview/data/attachedSignals.qml b/tests/auto/declarative/qdeclarativegridview/data/attachedSignals.qml
new file mode 100644
index 0000000..d527e9d
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativegridview/data/attachedSignals.qml
@@ -0,0 +1,27 @@
+import QtQuick 1.0
+
+GridView {
+    id: view
+    width: 240; height: 320
+
+    property variant addedDelegates: []
+    property int removedDelegateCount
+
+    model: testModel
+
+    cellWidth: delegateWidth; cellHeight: delegateHeight
+
+    delegate: Rectangle {
+        width: delegateWidth; height: delegateHeight
+        border.width: 1
+        GridView.onAdd: {
+            var obj = GridView.view.addedDelegates
+            obj.push(model.name)
+            GridView.view.addedDelegates = obj
+        }
+        GridView.onRemove: {
+            view.removedDelegateCount += 1
+        }
+    }
+}
+
diff --git a/tests/auto/declarative/qdeclarativegridview/tst_qdeclarativegridview.cpp b/tests/auto/declarative/qdeclarativegridview/tst_qdeclarativegridview.cpp
index 82a1a4a..79189a7 100644
--- a/tests/auto/declarative/qdeclarativegridview/tst_qdeclarativegridview.cpp
+++ b/tests/auto/declarative/qdeclarativegridview/tst_qdeclarativegridview.cpp
@@ -86,6 +86,10 @@ private slots:
     void footer();
     void header();
     void indexAt();
+    void onAdd();
+    void onAdd_data();
+    void onRemove();
+    void onRemove_data();
     void testQtQuick11Attributes();
     void testQtQuick11Attributes_data();
 
@@ -131,6 +135,13 @@ public:
         emit endInsertRows();
     }
 
+    void addItems(const QList<QPair<QString, QString> > &items) {
+        emit beginInsertRows(QModelIndex(), list.count(), list.count()+items.count()-1);
+        for (int i=0; i<items.count(); i++)
+            list.append(QPair<QString,QString>(items[i].first, items[i].second));
+        emit endInsertRows();
+    }
+
     void insertItem(int index, const QString &name, const QString &number) {
         emit beginInsertRows(QModelIndex(), index, index);
         list.insert(index, QPair<QString,QString>(name, number));
@@ -143,6 +154,13 @@ public:
         emit endRemoveRows();
     }
 
+    void removeItems(int index, int count) {
+        emit beginRemoveRows(QModelIndex(), index, index+count-1);
+        while (count--)
+            list.removeAt(index);
+        emit endRemoveRows();
+    }
+
     void moveItem(int from, int to) {
         emit beginMoveRows(QModelIndex(), from, from, QModelIndex(), to);
         list.move(from, to);
@@ -1388,7 +1406,7 @@ void tst_QDeclarativeGridView::footer()
     footer = findItem<QDeclarativeText>(contentItem, "footer2");
     QVERIFY(footer);
 
-    QCOMPARE(footer->y(), 0.0);
+    QCOMPARE(footer->y(), 600.0);
     QCOMPARE(footer->height(), 20.0);
     QCOMPARE(gridview->contentY(), 0.0);
 }
@@ -1437,9 +1455,9 @@ void tst_QDeclarativeGridView::header()
     header = findItem<QDeclarativeText>(contentItem, "header2");
     QVERIFY(header);
 
-    QCOMPARE(header->y(), 0.0);
+    QCOMPARE(header->y(), 10.0);
     QCOMPARE(header->height(), 20.0);
-    QCOMPARE(gridview->contentY(), 0.0);
+    QCOMPARE(gridview->contentY(), 10.0);
 }
 
 void tst_QDeclarativeGridView::indexAt()
@@ -1479,6 +1497,117 @@ void tst_QDeclarativeGridView::indexAt()
     delete canvas;
 }
 
+void tst_QDeclarativeGridView::onAdd()
+{
+    QFETCH(int, initialItemCount);
+    QFETCH(int, itemsToAdd);
+
+    const int delegateWidth = 50;
+    const int delegateHeight = 100;
+    TestModel model;
+    QDeclarativeView *canvas = createView();
+    canvas->setFixedSize(5 * delegateWidth, 5 * delegateHeight); // just ensure all items fit
+
+    // these initial items should not trigger GridView.onAdd
+    for (int i=0; i<initialItemCount; i++)
+        model.addItem("dummy value", "dummy value");
+
+    QDeclarativeContext *ctxt = canvas->rootContext();
+    ctxt->setContextProperty("testModel", &model);
+    ctxt->setContextProperty("delegateWidth", delegateWidth);
+    ctxt->setContextProperty("delegateHeight", delegateHeight);
+    canvas->setSource(QUrl::fromLocalFile(SRCDIR "/data/attachedSignals.qml"));
+
+    QObject *object = canvas->rootObject();
+    object->setProperty("width", canvas->width());
+    object->setProperty("height", canvas->height());
+    qApp->processEvents();
+
+    QList<QPair<QString, QString> > items;
+    for (int i=0; i<itemsToAdd; i++)
+        items << qMakePair(QString("value %1").arg(i), QString::number(i));
+    model.addItems(items);
+
+    qApp->processEvents();
+
+    QVariantList result = object->property("addedDelegates").toList();
+    QCOMPARE(result.count(), items.count());
+    for (int i=0; i<items.count(); i++)
+        QCOMPARE(result[i].toString(), items[i].first);
+
+    delete canvas;
+}
+
+void tst_QDeclarativeGridView::onAdd_data()
+{
+    QTest::addColumn<int>("initialItemCount");
+    QTest::addColumn<int>("itemsToAdd");
+
+    QTest::newRow("0, add 1") << 0 << 1;
+    QTest::newRow("0, add 2") << 0 << 2;
+    QTest::newRow("0, add 10") << 0 << 10;
+
+    QTest::newRow("1, add 1") << 1 << 1;
+    QTest::newRow("1, add 2") << 1 << 2;
+    QTest::newRow("1, add 10") << 1 << 10;
+
+    QTest::newRow("5, add 1") << 5 << 1;
+    QTest::newRow("5, add 2") << 5 << 2;
+    QTest::newRow("5, add 10") << 5 << 10;
+}
+
+void tst_QDeclarativeGridView::onRemove()
+{
+    QFETCH(int, initialItemCount);
+    QFETCH(int, indexToRemove);
+    QFETCH(int, removeCount);
+
+    const int delegateWidth = 50;
+    const int delegateHeight = 100;
+    TestModel model;
+    for (int i=0; i<initialItemCount; i++)
+        model.addItem(QString("value %1").arg(i), "dummy value");
+
+    QDeclarativeView *canvas = createView();
+    QDeclarativeContext *ctxt = canvas->rootContext();
+    ctxt->setContextProperty("testModel", &model);
+    ctxt->setContextProperty("delegateWidth", delegateWidth);
+    ctxt->setContextProperty("delegateHeight", delegateHeight);
+    canvas->setSource(QUrl::fromLocalFile(SRCDIR "/data/attachedSignals.qml"));
+    QObject *object = canvas->rootObject();
+
+    qApp->processEvents();
+
+    model.removeItems(indexToRemove, removeCount);
+    qApp->processEvents();
+    QCOMPARE(object->property("removedDelegateCount"), QVariant(removeCount));
+
+    delete canvas;
+}
+
+void tst_QDeclarativeGridView::onRemove_data()
+{
+    QTest::addColumn<int>("initialItemCount");
+    QTest::addColumn<int>("indexToRemove");
+    QTest::addColumn<int>("removeCount");
+
+    QTest::newRow("remove first") << 1 << 0 << 1;
+    QTest::newRow("two items, remove first") << 2 << 0 << 1;
+    QTest::newRow("two items, remove last") << 2 << 1 << 1;
+    QTest::newRow("two items, remove all") << 2 << 0 << 2;
+
+    QTest::newRow("four items, remove first") << 4 << 0 << 1;
+    QTest::newRow("four items, remove 0-2") << 4 << 0 << 2;
+    QTest::newRow("four items, remove 1-3") << 4 << 1 << 2;
+    QTest::newRow("four items, remove 2-4") << 4 << 2 << 2;
+    QTest::newRow("four items, remove last") << 4 << 3 << 1;
+    QTest::newRow("four items, remove all") << 4 << 0 << 4;
+
+    QTest::newRow("ten items, remove 1-8") << 10 << 0 << 8;
+    QTest::newRow("ten items, remove 2-7") << 10 << 2 << 5;
+    QTest::newRow("ten items, remove 4-10") << 10 << 4 << 6;
+}
+
 void tst_QDeclarativeGridView::testQtQuick11Attributes()
 {
     QFETCH(QString, code);
diff --git a/tests/auto/declarative/qdeclarativelistview/data/attachedSignals.qml b/tests/auto/declarative/qdeclarativelistview/data/attachedSignals.qml
new file mode 100644
index 0000000..5ca1a45
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativelistview/data/attachedSignals.qml
@@ -0,0 +1,24 @@
+import QtQuick 1.0
+
+ListView {
+    id: view
+    width: 240; height: 320
+
+    property variant addedDelegates: []
+    property int removedDelegateCount
+
+    model: testModel
+
+    delegate: Rectangle {
+        width: 200; height: delegateHeight
+        border.width: 1
+        ListView.onAdd: {
+            var obj = ListView.view.addedDelegates
+            obj.push(model.name)
+            ListView.view.addedDelegates = obj
+        }
+        ListView.onRemove: {
+            view.removedDelegateCount += 1
+        }
+    }
+}
diff --git a/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp b/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp
index 86b68ca..cfb04f9 100644
--- a/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp
+++ b/tests/auto/declarative/qdeclarativelistview/tst_qdeclarativelistview.cpp
@@ -108,6 +108,10 @@ private slots:
     void QTBUG_16037();
     void indexAt();
     void incrementalModel();
+    void onAdd();
+    void onAdd_data();
+    void onRemove();
+    void onRemove_data();
     void testQtQuick11Attributes();
     void testQtQuick11Attributes_data();
 
@@ -296,6 +300,13 @@ public:
         emit endInsertRows();
     }
 
+    void addItems(const QList<QPair<QString, QString> > &items) {
+        emit beginInsertRows(QModelIndex(), list.count(), list.count()+items.count()-1);
+        for (int i=0; i<items.count(); i++)
+            list.append(QPair<QString,QString>(items[i].first, items[i].second));
+        emit endInsertRows();
+    }
+
     void insertItem(int index, const QString &name, const QString &number) {
         emit beginInsertRows(QModelIndex(), index, index);
         list.insert(index, QPair<QString,QString>(name, number));
@@ -1612,7 +1623,7 @@ void tst_QDeclarativeListView::QTBUG_9791()
 
     // Confirm items positioned correctly
     int itemCount = findItems<QDeclarativeItem>(contentItem, "wrapper").count();
-    QVERIFY(itemCount == 3);
+    QCOMPARE(itemCount, 3);
 
     for (int i = 0; i < itemCount; ++i) {
         QDeclarativeItem *item = findItem<QDeclarativeItem>(contentItem, "wrapper", i);
@@ -1753,9 +1764,9 @@ void tst_QDeclarativeListView::header()
         header = findItem<QDeclarativeText>(contentItem, "header2");
         QVERIFY(header);
 
-        QCOMPARE(header->y(), 0.0);
+        QCOMPARE(header->y(), 10.0);
         QCOMPARE(header->height(), 10.0);
-        QCOMPARE(listview->contentY(), 0.0);
+        QCOMPARE(listview->contentY(), 10.0);
 
         delete canvas;
     }
@@ -1829,7 +1840,7 @@ void tst_QDeclarativeListView::footer()
     footer = findItem<QDeclarativeText>(contentItem, "footer2");
     QVERIFY(footer);
 
-    QCOMPARE(footer->y(), 0.0);
+    QCOMPARE(footer->y(), 600.0);
     QCOMPARE(footer->height(), 20.0);
     QCOMPARE(listview->contentY(), 0.0);
 
@@ -2089,6 +2100,113 @@ void tst_QDeclarativeListView::incrementalModel()
     delete canvas;
 }
 
+void tst_QDeclarativeListView::onAdd()
+{
+    QFETCH(int, initialItemCount);
+    QFETCH(int, itemsToAdd);
+
+    const int delegateHeight = 10;
+    TestModel2 model;
+
+    // these initial items should not trigger ListView.onAdd
+    for (int i=0; i<initialItemCount; i++)
+        model.addItem("dummy value", "dummy value");
+
+    QDeclarativeView *canvas = createView();
+    canvas->setFixedSize(200, delegateHeight * (initialItemCount + itemsToAdd));
+    QDeclarativeContext *ctxt = canvas->rootContext();
+    ctxt->setContextProperty("testModel", &model);
+    ctxt->setContextProperty("delegateHeight", delegateHeight);
+    canvas->setSource(QUrl::fromLocalFile(SRCDIR "/data/attachedSignals.qml"));
+
+    QObject *object = canvas->rootObject();
+    object->setProperty("width", canvas->width());
+    object->setProperty("height", canvas->height());
+    qApp->processEvents();
+
+    QList<QPair<QString, QString> > items;
+    for (int i=0; i<itemsToAdd; i++)
+        items << qMakePair(QString("value %1").arg(i), QString::number(i));
+    model.addItems(items);
+
+    qApp->processEvents();
+
+    QVariantList result = object->property("addedDelegates").toList();
+    QCOMPARE(result.count(), items.count());
+    for (int i=0; i<items.count(); i++)
+        QCOMPARE(result[i].toString(), items[i].first);
+
+    delete canvas;
+}
+
+void tst_QDeclarativeListView::onAdd_data()
+{
+    QTest::addColumn<int>("initialItemCount");
+    QTest::addColumn<int>("itemsToAdd");
+
+    QTest::newRow("0, add 1") << 0 << 1;
+    QTest::newRow("0, add 2") << 0 << 2;
+    QTest::newRow("0, add 10") << 0 << 10;
+
+    QTest::newRow("1, add 1") << 1 << 1;
+    QTest::newRow("1, add 2") << 1 << 2;
+    QTest::newRow("1, add 10") << 1 << 10;
+
+    QTest::newRow("5, add 1") << 5 << 1;
+    QTest::newRow("5, add 2") << 5 << 2;
+    QTest::newRow("5, add 10") << 5 << 10;
+}
+
+void tst_QDeclarativeListView::onRemove()
+{
+    QFETCH(int, initialItemCount);
+    QFETCH(int, indexToRemove);
+    QFETCH(int, removeCount);
+
+    const int delegateHeight = 10;
+    TestModel2 model;
+    for (int i=0; i<initialItemCount; i++)
+        model.addItem(QString("value %1").arg(i), "dummy value");
+
+    QDeclarativeView *canvas = createView();
+    QDeclarativeContext *ctxt = canvas->rootContext();
+    ctxt->setContextProperty("testModel", &model);
+    ctxt->setContextProperty("delegateHeight", delegateHeight);
+    canvas->setSource(QUrl::fromLocalFile(SRCDIR "/data/attachedSignals.qml"));
+    QObject *object = canvas->rootObject();
+
+    qApp->processEvents();
+
+    model.removeItems(indexToRemove, removeCount);
+    qApp->processEvents();
+    QCOMPARE(object->property("removedDelegateCount"), QVariant(removeCount));
+
+    delete canvas;
+}
+
+void tst_QDeclarativeListView::onRemove_data()
+{
+    QTest::addColumn<int>("initialItemCount");
+    QTest::addColumn<int>("indexToRemove");
+    QTest::addColumn<int>("removeCount");
+
+    QTest::newRow("remove first") << 1 << 0 << 1;
+    QTest::newRow("two items, remove first") << 2 << 0 << 1;
+    QTest::newRow("two items, remove last") << 2 << 1 << 1;
+    QTest::newRow("two items, remove all") << 2 << 0 << 2;
+
+    QTest::newRow("four items, remove first") << 4 << 0 << 1;
+    QTest::newRow("four items, remove 0-2") << 4 << 0 << 2;
+    QTest::newRow("four items, remove 1-3") << 4 << 1 << 2;
+    QTest::newRow("four items, remove 2-4") << 4 << 2 << 2;
+    QTest::newRow("four items, remove last") << 4 << 3 << 1;
+    QTest::newRow("four items, remove all") << 4 << 0 << 4;
+
+    QTest::newRow("ten items, remove 1-8") << 10 << 0 << 8;
+    QTest::newRow("ten items, remove 2-7") << 10 << 2 << 5;
+    QTest::newRow("ten items, remove 4-10") << 10 << 4 << 6;
+}
+
 void tst_QDeclarativeListView::testQtQuick11Attributes()
 {
     QFETCH(QString, code);