Fix failed assertions coming from the QML list model

QAbstractItemModel has become more strict in sanity checking
the arguments of beginInsertRows and friends with change
00c09e752ff7e482e1308e0e34721dc979204595 in qtbase.
Unfortunately, the QML list model was feeding it out of bound
rows in some cases, leading to failed assertions.

Fix this properly, by calculating the inserted/removed and
changed rows on the fly when syncing the list model from
the worker thread.

Adjust the code in the XML list model as well, so it does
call things in the proper order.

Fix two tests, one for a minimal change in behavior (more
correct now), the other to remove an assertion that is
not valid anymore in debug builds (where assertions in
QtCore will call rowCount()).

Change-Id: Ied85269f78d41b64e06388590be3ed227ac88fdb
Reviewed-by: J-P Nurmi <jpnurmi@qt.io>

7 files changed, 210 insertions, 245 deletions

diff --git a/src/imports/xmllistmodel/qqmlxmllistmodel.cpp b/src/imports/xmllistmodel/qqmlxmllistmodel.cpp
index fc9f73a881..a013e8cf69 100644
--- a/src/imports/xmllistmodel/qqmlxmllistmodel.cpp
+++ b/src/imports/xmllistmodel/qqmlxmllistmodel.cpp
@@ -1152,8 +1152,6 @@ void QQuickXmlListModel::queryCompleted(const QQuickXmlQueryResult &result)
     int origCount = d->size;
     bool sizeChanged = result.size != d->size;
 
-    d->size = result.size;
-    d->data = result.data;
     d->keyRoleResultsCache = result.keyRoleResultsCache;
     if (d->src.isEmpty() && d->xml.isEmpty())
         d->status = Null;
@@ -1174,6 +1172,8 @@ void QQuickXmlListModel::queryCompleted(const QQuickXmlQueryResult &result)
             beginRemoveRows(QModelIndex(), 0, origCount - 1);
             endRemoveRows();
         }
+        d->size = result.size;
+        d->data = result.data;
         if (d->size > 0) {
             beginInsertRows(QModelIndex(), 0, d->size - 1);
             endInsertRows();
@@ -1187,6 +1187,8 @@ void QQuickXmlListModel::queryCompleted(const QQuickXmlQueryResult &result)
                 endRemoveRows();
             }
         }
+        d->size = result.size;
+        d->data = result.data;
         for (int i=0; i<result.inserted.count(); i++) {
             const int index = result.inserted[i].first;
             const int count = result.inserted[i].second;
diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp
index 240fba7905..93d538127b 100644
--- a/src/qml/jsruntime/qv4runtime.cpp
+++ b/src/qml/jsruntime/qv4runtime.cpp
@@ -604,6 +604,14 @@ static Q_NEVER_INLINE ReturnedValue getElementFallback(ExecutionEngine *engine,
     return o->get(name);
 }
 
+/* load element:
+
+  Managed *m = object.heapObject();
+  if (m)
+     return m->internalClass->getIndexed(m, index);
+  return getIndexedFallback(object, index);
+*/
+
 ReturnedValue Runtime::method_loadElement(ExecutionEngine *engine, const Value &object, const Value &index)
 {
     uint idx = 0;
diff --git a/src/qml/types/qqmllistmodel.cpp b/src/qml/types/qqmllistmodel.cpp
index e32e0c75f3..0f04d48bf8 100644
--- a/src/qml/types/qqmllistmodel.cpp
+++ b/src/qml/types/qqmllistmodel.cpp
@@ -271,23 +271,24 @@ QObject *ListModel::getOrCreateModelObject(QQmlListModel *model, int elementInde
     return e->m_objectCache;
 }
 
-void ListModel::sync(ListModel *src, ListModel *target, QHash<int, ListModel *> *targetModelHash)
+bool ListModel::sync(ListModel *src, ListModel *target)
 {
     // Sanity check
     target->m_uid = src->m_uid;
-    if (targetModelHash)
-        targetModelHash->insert(target->m_uid, target);
+
+    bool hasChanges = false;
 
     // Build hash of elements <-> uid for each of the lists
     QHash<int, ElementSync> elementHash;
-    for (int i=0 ; i < target->elements.count() ; ++i) {
+    for (int i = 0; i < target->elements.count(); ++i) {
         ListElement *e = target->elements.at(i);
         int uid = e->getUid();
         ElementSync sync;
         sync.target = e;
+        sync.targetIndex = i;
         elementHash.insert(uid, sync);
     }
-    for (int i=0 ; i < src->elements.count() ; ++i) {
+    for (int i = 0; i < src->elements.count(); ++i) {
         ListElement *e = src->elements.at(i);
         int uid = e->getUid();
 
@@ -295,24 +296,39 @@ void ListModel::sync(ListModel *src, ListModel *target, QHash<int, ListModel *>
         if (it == elementHash.end()) {
             ElementSync sync;
             sync.src = e;
+            sync.srcIndex = i;
             elementHash.insert(uid, sync);
         } else {
             ElementSync &sync = it.value();
             sync.src = e;
+            sync.srcIndex = i;
         }
     }
 
+    QQmlListModel *targetModel = target->m_modelCache;
+
     // Get list of elements that are in the target but no longer in the source. These get deleted first.
-    QHash<int, ElementSync>::iterator it = elementHash.begin();
-    QHash<int, ElementSync>::iterator end = elementHash.end();
-    while (it != end) {
-        const ElementSync &s = it.value();
-        if (s.src == 0) {
+    int rowsRemoved = 0;
+    for (int i = 0 ; i < target->elements.count() ; ++i) {
+        ListElement *element = target->elements.at(i);
+        ElementSync &s = elementHash.find(element->getUid()).value();
+        Q_ASSERT(s.targetIndex >= 0);
+        // need to update the targetIndex, to keep it correct after removals
+        s.targetIndex -= rowsRemoved;
+        if (s.src == nullptr) {
+            Q_ASSERT(s.targetIndex == i);
+            hasChanges = true;
+            if (targetModel)
+                targetModel->beginRemoveRows(QModelIndex(), i, i);
             s.target->destroy(target->m_layout);
             target->elements.removeOne(s.target);
             delete s.target;
+            if (targetModel)
+                targetModel->endRemoveRows();
+            ++rowsRemoved;
+            --i;
+            continue;
         }
-        ++it;
     }
 
     // Sync the layouts
@@ -320,15 +336,15 @@ void ListModel::sync(ListModel *src, ListModel *target, QHash<int, ListModel *>
 
     // Clear the target list, and append in correct order from the source
     target->elements.clear();
-    for (int i=0 ; i < src->elements.count() ; ++i) {
+    for (int i = 0; i < src->elements.count(); ++i) {
         ListElement *srcElement = src->elements.at(i);
-        it = elementHash.find(srcElement->getUid());
-        const ElementSync &s = it.value();
+        ElementSync &s = elementHash.find(srcElement->getUid()).value();
+        Q_ASSERT(s.srcIndex >= 0);
         ListElement *targetElement = s.target;
-        if (targetElement == 0) {
+        if (targetElement == nullptr) {
             targetElement = new ListElement(srcElement->getUid());
         }
-        ListElement::sync(srcElement, src->m_layout, targetElement, target->m_layout, targetModelHash);
+        s.changedRoles = ListElement::sync(srcElement, src->m_layout, targetElement, target->m_layout);
         target->elements.append(targetElement);
     }
 
@@ -340,6 +356,39 @@ void ListModel::sync(ListModel *src, ListModel *target, QHash<int, ListModel *>
         if (ModelNodeMetaObject *mo = e->objectCache())
             mo->updateValues();
     }
+
+    // now emit the change notifications required. This can be safely done, as we're only emitting changes, moves and inserts,
+    // so the model indices can't be out of bounds
+    //
+    // to ensure things are kept in the correct order, emit inserts and moves first. This shouls ensure all persistent
+    // model indices are updated correctly
+    int rowsInserted = 0;
+    for (int i = 0 ; i < target->elements.count() ; ++i) {
+        ListElement *element = target->elements.at(i);
+        ElementSync &s = elementHash.find(element->getUid()).value();
+        Q_ASSERT(s.srcIndex >= 0);
+        s.srcIndex += rowsInserted;
+        if (s.srcIndex != s.targetIndex) {
+            if (targetModel) {
+                if (s.targetIndex == -1) {
+                    targetModel->beginInsertRows(QModelIndex(), i, i);
+                    targetModel->endInsertRows();
+                } else {
+                    targetModel->beginMoveRows(QModelIndex(), i, i, QModelIndex(), s.srcIndex);
+                    targetModel->endMoveRows();
+                }
+            }
+            hasChanges = true;
+            ++rowsInserted;
+        }
+        if (s.targetIndex != -1 && !s.changedRoles.isEmpty()) {
+            QModelIndex idx = targetModel->createIndex(i, 0);
+            if (targetModel)
+                targetModel->dataChanged(idx, idx, s.changedRoles);
+            hasChanges = true;
+        }
+    }
+    return hasChanges;
 }
 
 ListModel::ListModel(ListLayout *layout, QQmlListModel *modelCache, int uid) : m_layout(layout), m_modelCache(modelCache)
@@ -949,7 +998,11 @@ int ListElement::setVariantMapProperty(const ListLayout::Role &role, QVariantMap
         char *mem = getPropertyMemory(role);
         if (isMemoryUsed<QVariantMap>(mem)) {
             QVariantMap *map = reinterpret_cast<QVariantMap *>(mem);
+            if (m && map->isSharedWith(*m))
+                return roleIndex;
             map->~QMap();
+        } else if (!m) {
+            return roleIndex;
         }
         if (m)
             new (mem) QVariantMap(*m);
@@ -1101,12 +1154,14 @@ ListElement::~ListElement()
     delete next;
 }
 
-void ListElement::sync(ListElement *src, ListLayout *srcLayout, ListElement *target, ListLayout *targetLayout, QHash<int, ListModel *> *targetModelHash)
+QVector<int> ListElement::sync(ListElement *src, ListLayout *srcLayout, ListElement *target, ListLayout *targetLayout)
 {
+    QVector<int> changedRoles;
     for (int i=0 ; i < srcLayout->roleCount() ; ++i) {
         const ListLayout::Role &srcRole = srcLayout->getExistingRole(i);
         const ListLayout::Role &targetRole = targetLayout->getExistingRole(i);
 
+        int roleIndex = -1;
         switch (srcRole.type) {
             case ListLayout::Role::List:
                 {
@@ -1118,14 +1173,15 @@ void ListElement::sync(ListElement *src, ListLayout *srcLayout, ListElement *tar
                             targetSubModel = new ListModel(targetRole.subLayout, 0, srcSubModel->getUid());
                             target->setListPropertyFast(targetRole, targetSubModel);
                         }
-                        ListModel::sync(srcSubModel, targetSubModel, targetModelHash);
+                        if (ListModel::sync(srcSubModel, targetSubModel))
+                            roleIndex = targetRole.index;
                     }
                 }
                 break;
             case ListLayout::Role::QObject:
                 {
                     QObject *object = src->getQObjectProperty(srcRole);
-                    target->setQObjectProperty(targetRole, object);
+                    roleIndex = target->setQObjectProperty(targetRole, object);
                 }
                 break;
             case ListLayout::Role::String:
@@ -1135,20 +1191,23 @@ void ListElement::sync(ListElement *src, ListLayout *srcLayout, ListElement *tar
             case ListLayout::Role::Function:
                 {
                     QVariant v = src->getProperty(srcRole, 0, 0);
-                    target->setVariantProperty(targetRole, v);
+                    roleIndex = target->setVariantProperty(targetRole, v);
                 }
                 break;
             case ListLayout::Role::VariantMap:
                 {
                     QVariantMap *map = src->getVariantMapProperty(srcRole);
-                    target->setVariantMapProperty(targetRole, map);
+                    roleIndex = target->setVariantMapProperty(targetRole, map);
                 }
                 break;
             default:
                 break;
         }
+        if (roleIndex >= 0)
+            changedRoles << roleIndex;
     }
 
+    return changedRoles;
 }
 
 void ListElement::destroy(ListLayout *layout)
@@ -1493,20 +1552,22 @@ DynamicRoleModelNode *DynamicRoleModelNode::create(const QVariantMap &obj, QQmlL
     return object;
 }
 
-void DynamicRoleModelNode::sync(DynamicRoleModelNode *src, DynamicRoleModelNode *target, QHash<int, QQmlListModel *> *targetModelHash)
+QVector<int> DynamicRoleModelNode::sync(DynamicRoleModelNode *src, DynamicRoleModelNode *target)
 {
-    for (int i=0 ; i < src->m_meta->count() ; ++i) {
+    QVector<int> changedRoles;
+    for (int i = 0; i < src->m_meta->count(); ++i) {
         const QByteArray &name = src->m_meta->name(i);
         QVariant value = src->m_meta->value(i);
 
         QQmlListModel *srcModel = qobject_cast<QQmlListModel *>(value.value<QObject *>());
         QQmlListModel *targetModel = qobject_cast<QQmlListModel *>(target->m_meta->value(i).value<QObject *>());
 
+        bool modelHasChanges = false;
         if (srcModel) {
             if (targetModel == 0)
                 targetModel = QQmlListModel::createWithOwner(target->m_owner);
 
-            QQmlListModel::sync(srcModel, targetModel, targetModelHash);
+            modelHasChanges = QQmlListModel::sync(srcModel, targetModel);
 
             QObject *targetModelObject = targetModel;
             value = QVariant::fromValue(targetModelObject);
@@ -1514,8 +1575,10 @@ void DynamicRoleModelNode::sync(DynamicRoleModelNode *src, DynamicRoleModelNode
             delete targetModel;
         }
 
-        target->setValue(name, value);
+        if (target->setValue(name, value) || modelHasChanges)
+            changedRoles << target->m_owner->m_roles.indexOf(QString::fromUtf8(name));
     }
+    return changedRoles;
 }
 
 void DynamicRoleModelNode::updateValues(const QVariantMap &object, QVector<int> &roles)
@@ -1761,9 +1824,9 @@ QQmlListModel::QQmlListModel(QQmlListModel *orig, QQmlListModelWorkerAgent *agen
     m_listModel = new ListModel(m_layout, this, orig->m_listModel->getUid());
 
     if (m_dynamicRoles)
-        sync(orig, this, 0);
+        sync(orig, this);
     else
-        ListModel::sync(orig->m_listModel, m_listModel, 0);
+        ListModel::sync(orig->m_listModel, m_listModel);
 
     m_engine = 0;
 }
@@ -1814,25 +1877,26 @@ QV4::ExecutionEngine *QQmlListModel::engine() const
     return m_engine;
 }
 
-void QQmlListModel::sync(QQmlListModel *src, QQmlListModel *target, QHash<int, QQmlListModel *> *targetModelHash)
+bool QQmlListModel::sync(QQmlListModel *src, QQmlListModel *target)
 {
     Q_ASSERT(src->m_dynamicRoles && target->m_dynamicRoles);
 
+    bool hasChanges = false;
+
     target->m_uid = src->m_uid;
-    if (targetModelHash)
-        targetModelHash->insert(target->m_uid, target);
     target->m_roles = src->m_roles;
 
     // Build hash of elements <-> uid for each of the lists
     QHash<int, ElementSync> elementHash;
-    for (int i=0 ; i < target->m_modelObjects.count() ; ++i) {
+    for (int i = 0 ; i < target->m_modelObjects.count(); ++i) {
         DynamicRoleModelNode *e = target->m_modelObjects.at(i);
         int uid = e->getUid();
         ElementSync sync;
         sync.target = e;
+        sync.targetIndex = i;
         elementHash.insert(uid, sync);
     }
-    for (int i=0 ; i < src->m_modelObjects.count() ; ++i) {
+    for (int i = 0 ; i < src->m_modelObjects.count(); ++i) {
         DynamicRoleModelNode *e = src->m_modelObjects.at(i);
         int uid = e->getUid();
 
@@ -1840,118 +1904,102 @@ void QQmlListModel::sync(QQmlListModel *src, QQmlListModel *target, QHash<int, Q
         if (it == elementHash.end()) {
             ElementSync sync;
             sync.src = e;
+            sync.srcIndex = i;
             elementHash.insert(uid, sync);
         } else {
             ElementSync &sync = it.value();
             sync.src = e;
+            sync.srcIndex = i;
         }
     }
 
     // Get list of elements that are in the target but no longer in the source. These get deleted first.
-    QHash<int, ElementSync>::iterator it = elementHash.begin();
-    QHash<int, ElementSync>::iterator end = elementHash.end();
-    while (it != end) {
-        const ElementSync &s = it.value();
-        if (s.src == 0) {
-            int targetIndex = target->m_modelObjects.indexOf(s.target);
-            target->m_modelObjects.remove(targetIndex, 1);
+    int rowsRemoved = 0;
+    for (int i = 0 ; i < target->m_modelObjects.count() ; ++i) {
+        DynamicRoleModelNode *element = target->m_modelObjects.at(i);
+        ElementSync &s = elementHash.find(element->getUid()).value();
+        Q_ASSERT(s.targetIndex >= 0);
+        // need to update the targetIndex, to keep it correct after removals
+        s.targetIndex -= rowsRemoved;
+        if (s.src == nullptr) {
+            Q_ASSERT(s.targetIndex == i);
+            hasChanges = true;
+            target->beginRemoveRows(QModelIndex(), i, i);
+            target->m_modelObjects.remove(i, 1);
+            target->endRemoveRows();
             delete s.target;
+            ++rowsRemoved;
+            --i;
+            continue;
         }
-        ++it;
     }
 
     // Clear the target list, and append in correct order from the source
     target->m_modelObjects.clear();
-    for (int i=0 ; i < src->m_modelObjects.count() ; ++i) {
-        DynamicRoleModelNode *srcElement = src->m_modelObjects.at(i);
-        it = elementHash.find(srcElement->getUid());
-        const ElementSync &s = it.value();
+    for (int i = 0 ; i < src->m_modelObjects.count() ; ++i) {
+        DynamicRoleModelNode *element = src->m_modelObjects.at(i);
+        ElementSync &s = elementHash.find(element->getUid()).value();
+        Q_ASSERT(s.srcIndex >= 0);
         DynamicRoleModelNode *targetElement = s.target;
         if (targetElement == 0) {
-            targetElement = new DynamicRoleModelNode(target, srcElement->getUid());
+            targetElement = new DynamicRoleModelNode(target, element->getUid());
         }
-        DynamicRoleModelNode::sync(srcElement, targetElement, targetModelHash);
+        s.changedRoles = DynamicRoleModelNode::sync(element, targetElement);
         target->m_modelObjects.append(targetElement);
     }
-}
 
-void QQmlListModel::emitItemsChanged(int index, int count, const QVector<int> &roles)
-{
-    if (count <= 0)
-        return;
-
-    if (m_mainThread) {
-        emit dataChanged(createIndex(index, 0), createIndex(index + count - 1, 0), roles);;
-    } else {
-        int uid = m_dynamicRoles ? getUid() : m_listModel->getUid();
-        m_agent->data.changedChange(uid, index, count, roles);
+    // now emit the change notifications required. This can be safely done, as we're only emitting changes, moves and inserts,
+    // so the model indices can't be out of bounds
+    //
+    // to ensure things are kept in the correct order, emit inserts and moves first. This shouls ensure all persistent
+    // model indices are updated correctly
+    int rowsInserted = 0;
+    for (int i = 0 ; i < target->m_modelObjects.count() ; ++i) {
+        DynamicRoleModelNode *element = target->m_modelObjects.at(i);
+        ElementSync &s = elementHash.find(element->getUid()).value();
+        Q_ASSERT(s.srcIndex >= 0);
+        s.srcIndex += rowsInserted;
+        if (s.srcIndex != s.targetIndex) {
+            if (s.targetIndex == -1) {
+                target->beginInsertRows(QModelIndex(), i, i);
+                target->endInsertRows();
+            } else {
+                target->beginMoveRows(QModelIndex(), i, i, QModelIndex(), s.srcIndex);
+                target->endMoveRows();
+            }
+            hasChanges = true;
+            ++rowsInserted;
+        }
+        if (s.targetIndex != -1 && !s.changedRoles.isEmpty()) {
+            QModelIndex idx = target->createIndex(i, 0);
+            emit target->dataChanged(idx, idx, s.changedRoles);
+            hasChanges = true;
+        }
     }
+    return hasChanges;
 }
 
-void QQmlListModel::emitItemsAboutToBeRemoved(int index, int count)
-{
-    if (count <= 0 || !m_mainThread)
-        return;
-
-    beginRemoveRows(QModelIndex(), index, index + count - 1);
-}
-
-void QQmlListModel::emitItemsRemoved(int index, int count)
+void QQmlListModel::emitItemsChanged(int index, int count, const QVector<int> &roles)
 {
     if (count <= 0)
         return;
 
-    if (m_mainThread) {
-            endRemoveRows();
-            emit countChanged();
-    } else {
-        int uid = m_dynamicRoles ? getUid() : m_listModel->getUid();
-        if (index == 0 && count == this->count())
-            m_agent->data.clearChange(uid);
-        m_agent->data.removeChange(uid, index, count);
-    }
+    if (m_mainThread)
+        emit dataChanged(createIndex(index, 0), createIndex(index + count - 1, 0), roles);;
 }
 
 void QQmlListModel::emitItemsAboutToBeInserted(int index, int count)
 {
-    if (count <= 0 || !m_mainThread)
-        return;
-
-    beginInsertRows(QModelIndex(), index, index + count - 1);
+    Q_ASSERT(index >= 0 && count >= 0);
+    if (m_mainThread)
+        beginInsertRows(QModelIndex(), index, index + count - 1);
 }
 
-void QQmlListModel::emitItemsInserted(int index, int count)
+void QQmlListModel::emitItemsInserted()
 {
-    if (count <= 0)
-        return;
-
     if (m_mainThread) {
         endInsertRows();
         emit countChanged();
-    } else {
-        int uid = m_dynamicRoles ? getUid() : m_listModel->getUid();
-        m_agent->data.insertChange(uid, index, count);
-    }
-}
-
-void QQmlListModel::emitItemsAboutToBeMoved(int from, int to, int n)
-{
-    if (n <= 0 || !m_mainThread)
-        return;
-
-    beginMoveRows(QModelIndex(), from, from + n - 1, QModelIndex(), to > from ? to + n : to);
-}
-
-void QQmlListModel::emitItemsMoved(int from, int to, int n)
-{
-    if (n <= 0)
-        return;
-
-    if (m_mainThread) {
-        endMoveRows();
-    } else {
-        int uid = m_dynamicRoles ? getUid() : m_listModel->getUid();
-        m_agent->data.moveChange(uid, from, n, to);
     }
 }
 
@@ -2133,7 +2181,13 @@ void QQmlListModel::remove(QQmlV4Function *args)
 
 void QQmlListModel::removeElements(int index, int removeCount)
 {
-    emitItemsAboutToBeRemoved(index, removeCount);
+    Q_ASSERT(index >= 0 && removeCount >= 0);
+
+    if (!removeCount)
+        return;
+
+    if (m_mainThread)
+        beginRemoveRows(QModelIndex(), index, index + removeCount - 1);
 
     QVector<std::function<void()>> toDestroy;
     if (m_dynamicRoles) {
@@ -2148,7 +2202,10 @@ void QQmlListModel::removeElements(int index, int removeCount)
         toDestroy = m_listModel->remove(index, removeCount);
     }
 
-    emitItemsRemoved(index, removeCount);
+    if (m_mainThread) {
+        endRemoveRows();
+        emit countChanged();
+    }
     for (const auto &destroyer : toDestroy)
         destroyer();
 }
@@ -2197,7 +2254,7 @@ void QQmlListModel::insert(QQmlV4Function *args)
                     m_listModel->insert(index+i, argObject);
                 }
             }
-            emitItemsInserted(index, objectArrayLength);
+            emitItemsInserted();
         } else if (argObject) {
             emitItemsAboutToBeInserted(index, 1);
 
@@ -2207,7 +2264,7 @@ void QQmlListModel::insert(QQmlV4Function *args)
                 m_listModel->insert(index, argObject);
             }
 
-            emitItemsInserted(index, 1);
+            emitItemsInserted();
         } else {
             qmlWarning(this) << tr("insert: value is not an object");
         }
@@ -2232,14 +2289,15 @@ void QQmlListModel::insert(QQmlV4Function *args)
 */
 void QQmlListModel::move(int from, int to, int n)
 {
-    if (n==0 || from==to)
+    if (n == 0 || from == to)
         return;
     if (!canMove(from, to, n)) {
         qmlWarning(this) << tr("move: out of range");
         return;
     }
 
-    emitItemsAboutToBeMoved(from, to, n);
+    if (m_mainThread)
+        beginMoveRows(QModelIndex(), from, from + n - 1, QModelIndex(), to > from ? to + n : to);
 
     if (m_dynamicRoles) {
 
@@ -2268,7 +2326,8 @@ void QQmlListModel::move(int from, int to, int n)
         m_listModel->move(from, to, n);
     }
 
-    emitItemsMoved(from, to, n);
+    if (m_mainThread)
+        endMoveRows();
 }
 
 /*!
@@ -2308,7 +2367,7 @@ void QQmlListModel::append(QQmlV4Function *args)
                 }
             }
 
-            emitItemsInserted(index, objectArrayLength);
+            emitItemsInserted();
         } else if (argObject) {
             int index;
 
@@ -2322,7 +2381,7 @@ void QQmlListModel::append(QQmlV4Function *args)
                 m_listModel->append(argObject);
             }
 
-            emitItemsInserted(index, 1);
+            emitItemsInserted();
         } else {
             qmlWarning(this) << tr("append: value is not an object");
         }
@@ -2424,7 +2483,7 @@ void QQmlListModel::set(int index, const QQmlV4Handle &handle)
             m_listModel->insert(index, object);
         }
 
-        emitItemsInserted(index, 1);
+        emitItemsInserted();
     } else {
 
         QVector<int> roles;
diff --git a/src/qml/types/qqmllistmodel_p.h b/src/qml/types/qqmllistmodel_p.h
index 499a113504..18b7b8bb22 100644
--- a/src/qml/types/qqmllistmodel_p.h
+++ b/src/qml/types/qqmllistmodel_p.h
@@ -147,24 +147,21 @@ private:
 
     struct ElementSync
     {
-        ElementSync() : src(0), target(0) {}
-
-        DynamicRoleModelNode *src;
-        DynamicRoleModelNode *target;
+        DynamicRoleModelNode *src = nullptr;
+        DynamicRoleModelNode *target = nullptr;
+        int srcIndex = -1;
+        int targetIndex = -1;
+        QVector<int> changedRoles;
     };
 
     int getUid() const { return m_uid; }
 
-    static void sync(QQmlListModel *src, QQmlListModel *target, QHash<int, QQmlListModel *> *targetModelHash);
+    static bool sync(QQmlListModel *src, QQmlListModel *target);
     static QQmlListModel *createWithOwner(QQmlListModel *newOwner);
 
     void emitItemsChanged(int index, int count, const QVector<int> &roles);
-    void emitItemsAboutToBeRemoved(int index, int count);
-    void emitItemsRemoved(int index, int count);
     void emitItemsAboutToBeInserted(int index, int count);
-    void emitItemsInserted(int index, int count);
-    void emitItemsAboutToBeMoved(int from, int to, int n);
-    void emitItemsMoved(int from, int to, int n);
+    void emitItemsInserted();
 
     void removeElements(int index, int removeCount);
 };
diff --git a/src/qml/types/qqmllistmodel_p_p.h b/src/qml/types/qqmllistmodel_p_p.h
index dea1ef2eb3..65567ab69a 100644
--- a/src/qml/types/qqmllistmodel_p_p.h
+++ b/src/qml/types/qqmllistmodel_p_p.h
@@ -108,7 +108,7 @@ public:
         return m_uid;
     }
 
-    static void sync(DynamicRoleModelNode *src, DynamicRoleModelNode *target, QHash<int, QQmlListModel *> *targetModelHash);
+    static QVector<int> sync(DynamicRoleModelNode *src, DynamicRoleModelNode *target);
 
 private:
     QQmlListModel *m_owner;
@@ -261,7 +261,7 @@ public:
     ListElement(int existingUid);
     ~ListElement();
 
-    static void sync(ListElement *src, ListLayout *srcLayout, ListElement *target, ListLayout *targetLayout, QHash<int, ListModel *> *targetModelHash);
+    static QVector<int> sync(ListElement *src, ListLayout *srcLayout, ListElement *target, ListLayout *targetLayout);
 
     enum
     {
@@ -379,7 +379,7 @@ public:
 
     int getUid() const { return m_uid; }
 
-    static void sync(ListModel *src, ListModel *target, QHash<int, ListModel *> *srcModelHash);
+    static bool sync(ListModel *src, ListModel *target);
 
     QObject *getOrCreateModelObject(QQmlListModel *model, int elementIndex);
 
@@ -392,10 +392,11 @@ private:
 
     struct ElementSync
     {
-        ElementSync() : src(0), target(0) {}
-
-        ListElement *src;
-        ListElement *target;
+        ListElement *src = nullptr;
+        ListElement *target = nullptr;
+        int srcIndex = -1;
+        int targetIndex = -1;
+        QVector<int> changedRoles;
     };
 
     void newElement(int index);
diff --git a/src/qml/types/qqmllistmodelworkeragent.cpp b/src/qml/types/qqmllistmodelworkeragent.cpp
index 0a5adbf292..a2750c926a 100644
--- a/src/qml/types/qqmllistmodelworkeragent.cpp
+++ b/src/qml/types/qqmllistmodelworkeragent.cpp
@@ -50,39 +50,8 @@
 
 QT_BEGIN_NAMESPACE
 
-
-void QQmlListModelWorkerAgent::Data::clearChange(int uid)
-{
-    for (int i=0 ; i < changes.count() ; ++i) {
-        if (changes[i].modelUid == uid) {
-            changes.removeAt(i);
-            --i;
-        }
-    }
-}
-
-void QQmlListModelWorkerAgent::Data::insertChange(int uid, int index, int count)
+QQmlListModelWorkerAgent::Sync::~Sync()
 {
-    Change c = { uid, Change::Inserted, index, count, 0, QVector<int>() };
-    changes << c;
-}
-
-void QQmlListModelWorkerAgent::Data::removeChange(int uid, int index, int count)
-{
-    Change c = { uid, Change::Removed, index, count, 0, QVector<int>() };
-    changes << c;
-}
-
-void QQmlListModelWorkerAgent::Data::moveChange(int uid, int index, int count, int to)
-{
-    Change c = { uid, Change::Moved, index, count, to, QVector<int>() };
-    changes << c;
-}
-
-void QQmlListModelWorkerAgent::Data::changedChange(int uid, int index, int count, const QVector<int> &roles)
-{
-    Change c = { uid, Change::Changed, index, count, 0, roles };
-    changes << c;
 }
 
 QQmlListModelWorkerAgent::QQmlListModelWorkerAgent(QQmlListModel *model)
@@ -167,8 +136,7 @@ void QQmlListModelWorkerAgent::move(int from, int to, int count)
 
 void QQmlListModelWorkerAgent::sync()
 {
-    Sync *s = new Sync(data, m_copy);
-    data.changes.clear();
+    Sync *s = new Sync(m_copy);
 
     mutex.lock();
     QCoreApplication::postEvent(this, s);
@@ -183,61 +151,14 @@ bool QQmlListModelWorkerAgent::event(QEvent *e)
         QMutexLocker locker(&mutex);
         if (m_orig) {
             Sync *s = static_cast<Sync *>(e);
-            const QList<Change> &changes = s->data.changes;
 
-            cc = m_orig->count() != s->list->count();
-
-            QHash<int, QQmlListModel *> targetModelDynamicHash;
-            QHash<int, ListModel *> targetModelStaticHash;
+            cc = (m_orig->count() != s->list->count());
 
             Q_ASSERT(m_orig->m_dynamicRoles == s->list->m_dynamicRoles);
             if (m_orig->m_dynamicRoles)
-                QQmlListModel::sync(s->list, m_orig, &targetModelDynamicHash);
+                QQmlListModel::sync(s->list, m_orig);
             else
-                ListModel::sync(s->list->m_listModel, m_orig->m_listModel, &targetModelStaticHash);
-
-            for (int ii = 0; ii < changes.count(); ++ii) {
-                const Change &change = changes.at(ii);
-
-                QQmlListModel *model = 0;
-                if (m_orig->m_dynamicRoles) {
-                    model = targetModelDynamicHash.value(change.modelUid);
-                } else {
-                    ListModel *lm = targetModelStaticHash.value(change.modelUid);
-                    if (lm)
-                        model = lm->m_modelCache;
-                }
-
-                if (model) {
-                    switch (change.type) {
-                    case Change::Inserted:
-                        model->beginInsertRows(
-                                    QModelIndex(), change.index, change.index + change.count - 1);
-                        model->endInsertRows();
-                        break;
-                    case Change::Removed:
-                        model->beginRemoveRows(
-                                    QModelIndex(), change.index, change.index + change.count - 1);
-                        model->endRemoveRows();
-                        break;
-                    case Change::Moved:
-                        model->beginMoveRows(
-                                    QModelIndex(),
-                                    change.index,
-                                    change.index + change.count - 1,
-                                    QModelIndex(),
-                                    change.to > change.index ? change.to + change.count : change.to);
-                        model->endMoveRows();
-                        break;
-                    case Change::Changed:
-                        emit model->dataChanged(
-                                    model->createIndex(change.index, 0),
-                                    model->createIndex(change.index + change.count - 1, 0),
-                                    change.roles);
-                        break;
-                    }
-                }
-            }
+                ListModel::sync(s->list->m_listModel, m_orig->m_listModel);
         }
 
         syncDone.wakeAll();
diff --git a/src/qml/types/qqmllistmodelworkeragent_p.h b/src/qml/types/qqmllistmodelworkeragent_p.h
index 5a39651bf7..761a467e89 100644
--- a/src/qml/types/qqmllistmodelworkeragent_p.h
+++ b/src/qml/types/qqmllistmodelworkeragent_p.h
@@ -114,35 +114,12 @@ private:
     friend class QQuickWorkerScriptEnginePrivate;
     friend class QQmlListModel;
 
-    struct Change
-    {
-        int modelUid;
-        enum { Inserted, Removed, Moved, Changed } type;
-        int index; // Inserted/Removed/Moved/Changed
-        int count; // Inserted/Removed/Moved/Changed
-        int to;    // Moved
-        QVector<int> roles;
-    };
-
-    struct Data
-    {
-        QList<Change> changes;
-
-        void clearChange(int uid);
-        void insertChange(int uid, int index, int count);
-        void removeChange(int uid, int index, int count);
-        void moveChange(int uid, int index, int count, int to);
-        void changedChange(int uid, int index, int count, const QVector<int> &roles);
-    };
-    Data data;
-
     struct Sync : public QEvent {
-        Sync(const Data &d, QQmlListModel *l)
+        Sync(QQmlListModel *l)
             : QEvent(QEvent::User)
-            , data(d)
             , list(l)
         {}
-        Data data;
+        ~Sync();
         QQmlListModel *list;
     };