ObjectModel: provide unique move IDs

QQuickItemViewPrivate::removeItem() uses a QHash to store items that
were removed due to a move. If the move IDs are not unique, multiple
buffered moves end up overriding each other. This results to leaked
items that are never released.

Task-number: QTBUG-62607
Change-Id: I7e7e7fcd6b1b0aa50ed55643ba5674e98536f89f
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
Reviewed-by: Michael Brasser <michael.brasser@live.com>

2 files changed, 8 insertions, 8 deletions

diff --git a/src/qml/types/qqmlobjectmodel.cpp b/src/qml/types/qqmlobjectmodel.cpp
index 64d0169f6b..dcd0360199 100644
--- a/src/qml/types/qqmlobjectmodel.cpp
+++ b/src/qml/types/qqmlobjectmodel.cpp
@@ -72,7 +72,7 @@ public:
         int ref;
     };
 
-    QQmlObjectModelPrivate() : QObjectPrivate() {}
+    QQmlObjectModelPrivate() : QObjectPrivate(), moveId(0) {}
 
     static void children_append(QQmlListProperty<QObject> *prop, QObject *item) {
         int index = static_cast<QQmlObjectModelPrivate *>(prop->data)->children.count();
@@ -129,7 +129,7 @@ public:
         }
 
         QQmlChangeSet changeSet;
-        changeSet.move(from, to, n, 0);
+        changeSet.move(from, to, n, ++moveId);
         emit q->modelUpdated(changeSet, false);
         emit q->childrenChanged();
     }
@@ -166,7 +166,7 @@ public:
         return -1;
     }
 
-
+    uint moveId;
     QList<Item> children;
 };
 
diff --git a/tests/auto/qml/qqmlobjectmodel/tst_qqmlobjectmodel.cpp b/tests/auto/qml/qqmlobjectmodel/tst_qqmlobjectmodel.cpp
index 6ac0412ae5..fb63d811a8 100644
--- a/tests/auto/qml/qqmlobjectmodel/tst_qqmlobjectmodel.cpp
+++ b/tests/auto/qml/qqmlobjectmodel/tst_qqmlobjectmodel.cpp
@@ -47,18 +47,18 @@ static bool compareItems(QQmlObjectModel *model, const QObjectList &items)
     return true;
 }
 
-static bool verifyChangeSet(const QQmlChangeSet &changeSet, int expectedInserts, int expectedRemoves, bool isMove)
+static bool verifyChangeSet(const QQmlChangeSet &changeSet, int expectedInserts, int expectedRemoves, bool isMove, int moveId = -1)
 {
     int actualRemoves = 0;
     for (const QQmlChangeSet::Change &r : changeSet.removes()) {
-        if (r.isMove() != isMove)
+        if (r.isMove() != isMove && (!isMove || moveId == r.moveId))
             return false;
         actualRemoves += r.count;
     }
 
     int actualInserts = 0;
     for (const QQmlChangeSet::Change &i : changeSet.inserts()) {
-        if (i.isMove() != isMove)
+        if (i.isMove() != isMove && (!isMove || moveId == i.moveId))
             return false;
         actualInserts += i.count;
     }
@@ -129,7 +129,7 @@ void tst_QQmlObjectModel::changes()
     QCOMPARE(countSpy.count(), countSignals);
     QCOMPARE(childrenSpy.count(), ++childrenSignals);
     QCOMPARE(modelUpdateSpy.count(), ++modelUpdateSignals);
-    QVERIFY(verifyChangeSet(modelUpdateSpy.last().first().value<QQmlChangeSet>(), 1, 1, true));
+    QVERIFY(verifyChangeSet(modelUpdateSpy.last().first().value<QQmlChangeSet>(), 1, 1, true, 1));
 
     // move(3, 2) -> [item0, item1, item2, item3]
     model.move(3, 2); items.move(3, 2);
@@ -138,7 +138,7 @@ void tst_QQmlObjectModel::changes()
     QCOMPARE(countSpy.count(), countSignals);
     QCOMPARE(childrenSpy.count(), ++childrenSignals);
     QCOMPARE(modelUpdateSpy.count(), ++modelUpdateSignals);
-    QVERIFY(verifyChangeSet(modelUpdateSpy.last().first().value<QQmlChangeSet>(), 1, 1, true));
+    QVERIFY(verifyChangeSet(modelUpdateSpy.last().first().value<QQmlChangeSet>(), 1, 1, true, 2));
 
     // remove(0) -> [item1, item2, item3]
     model.remove(0); items.removeAt(0);