6 files changed, 83 insertions, 36 deletions
diff --git a/src/qmlmodels/qqmldelegatemodel.cpp b/src/qmlmodels/qqmldelegatemodel.cpp
index ec75a5775c..13965d316b 100644
--- a/src/qmlmodels/qqmldelegatemodel.cpp
+++ b/src/qmlmodels/qqmldelegatemodel.cpp
@@ -167,6 +167,7 @@ QQmlDelegateModelPrivate::QQmlDelegateModelPrivate(QQmlContext *ctxt)
     , m_transaction(false)
     , m_incubatorCleanupScheduled(false)
     , m_waitingToFetchMore(false)
+    , m_maybeResetRoleNames(false)
     , m_cacheItems(nullptr)
     , m_items(nullptr)
     , m_persistedItems(nullptr)
@@ -371,6 +372,8 @@ void QQmlDelegateModelPrivate::connectToAbstractItemModel()
     QObject::connect(aim, &QAbstractItemModel::modelAboutToBeReset, q, &QQmlDelegateModel::_q_modelAboutToBeReset);
     qmlobject_connect(aim, QAbstractItemModel, SIGNAL(layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)),
                       q, QQmlDelegateModel, SLOT(_q_layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)));
+    QObject::connect(aim, &QAbstractItemModel::modelReset, q, &QQmlDelegateModel::handleModelReset);
+    QObject::connect(aim, &QAbstractItemModel::layoutChanged, q, &QQmlDelegateModel::_q_layoutChanged);
 }
 
 void QQmlDelegateModelPrivate::disconnectFromAbstractItemModel()
@@ -400,6 +403,8 @@ void QQmlDelegateModelPrivate::disconnectFromAbstractItemModel()
     QObject::disconnect(aim, &QAbstractItemModel::modelAboutToBeReset, q, &QQmlDelegateModel::_q_modelAboutToBeReset);
     QObject::disconnect(aim, SIGNAL(layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)),
                         q, SLOT(_q_layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)));
+    QObject::disconnect(aim, &QAbstractItemModel::modelReset, q, &QQmlDelegateModel::handleModelReset);
+    QObject::disconnect(aim, &QAbstractItemModel::layoutChanged, q, &QQmlDelegateModel::_q_layoutChanged);
 }
 
 void QQmlDelegateModel::setModel(const QVariant &model)
@@ -1851,24 +1856,28 @@ void QQmlDelegateModelPrivate::emitChanges()
 
 void QQmlDelegateModel::_q_modelAboutToBeReset()
 {
-    auto aim = static_cast<QAbstractItemModel *>(sender());
-    auto oldRoleNames = aim->roleNames();
-    // this relies on the fact that modelAboutToBeReset must be followed
-    // by a modelReset signal before any further modelAboutToBeReset can occur
-    QObject::connect(aim, &QAbstractItemModel::modelReset, this, [&, oldRoleNames](){
-        auto aim = static_cast<QAbstractItemModel *>(sender());
-        if (oldRoleNames == aim->roleNames()) {
-            // if the rolenames stayed the same (most common case), then we don't have
-            // to throw away all the setup that we did
-            handleModelReset();
-        } else {
-            // If they did change, we give up and just start from scratch via setMode
-            setModel(QVariant::fromValue(model()));
-            // but we still have to call handleModelReset, otherwise views will
-            // not refresh
-            handleModelReset();
-        }
-    }, Qt::SingleShotConnection);
+    /*
+        roleNames are generally guaranteed to be stable (given that QAIM has no
+        change signal for them), except that resetting the model is allowed to
+        invalidate them (QTBUG-32132). DelegateModel must take this into account by
+        snapshotting the current roleNames before the model is reset.
+        Afterwards, if we detect that roleNames has changed, we throw the
+        current model set up away and rebuild everything from scratch – it is
+        unlikely that a more efficient implementation would be worth it.
+
+        If we detect no changes, we simply use the existing logic to handle the
+        model reset.
+
+        This (role name resetting) logic relies on the fact that
+        modelAboutToBeReset must be followed by a modelReset signal before any
+        further modelAboutToBeReset can occur. However, it's possible for user
+        code to begin the reset before connectToAbstractItemModel is called
+        (QTBUG-125053), in which case we don't attempt to reset the role names.
+    */
+    Q_D(QQmlDelegateModel);
+    Q_ASSERT(!d->m_maybeResetRoleNames);
+    d->m_maybeResetRoleNames = true;
+    d->m_roleNamesBeforeReset = d->m_adaptorModel.aim()->roleNames();
 }
 
 void QQmlDelegateModel::handleModelReset()
@@ -1878,6 +1887,23 @@ void QQmlDelegateModel::handleModelReset()
         return;
 
     int oldCount = d->m_count;
+
+    if (d->m_maybeResetRoleNames) {
+        auto aim = d->m_adaptorModel.aim();
+        if (!d->m_adaptorModel.adaptsAim() || d->m_adaptorModel.aim() != aim)
+            return;
+
+        // If the role names stayed the same (most common case), then we don't have
+        // to throw away all the setup that we did.
+        // If they did change, we give up and just start from scratch via setModel.
+        // We do this before handling the reset to ensure that views refresh.
+        if (aim->roleNames() != d->m_roleNamesBeforeReset)
+            setModel(QVariant::fromValue(model()));
+
+        d->m_maybeResetRoleNames = false;
+        d->m_roleNamesBeforeReset.clear();
+    }
+
     d->m_adaptorModel.rootIndex = QModelIndex();
 
     if (d->m_complete) {
diff --git a/src/qmlmodels/qqmldelegatemodel_p_p.h b/src/qmlmodels/qqmldelegatemodel_p_p.h
index a7d22dfaeb..2fab7b35eb 100644
--- a/src/qmlmodels/qqmldelegatemodel_p_p.h
+++ b/src/qmlmodels/qqmldelegatemodel_p_p.h
@@ -332,6 +332,7 @@ public:
     QQmlReusableDelegateModelItemsPool m_reusableItemsPool;
     QList<QQDMIncubationTask *> m_finishedIncubating;
     QList<QByteArray> m_watchedRoles;
+    QHash<int, QByteArray> m_roleNamesBeforeReset;
 
     QString m_filterGroup;
 
@@ -345,6 +346,7 @@ public:
     bool m_transaction : 1;
     bool m_incubatorCleanupScheduled : 1;
     bool m_waitingToFetchMore : 1;
+    bool m_maybeResetRoleNames : 1;
 
     union {
         struct {
diff --git a/src/qmlmodels/qqmllistmodel.cpp b/src/qmlmodels/qqmllistmodel.cpp
index 28a2523561..94b4a023c6 100644
--- a/src/qmlmodels/qqmllistmodel.cpp
+++ b/src/qmlmodels/qqmllistmodel.cpp
@@ -1669,9 +1669,12 @@ bool ModelObject::virtualPut(Managed *m, PropertyKey id, const Value &value, Val
 
     ExecutionEngine *eng = that->engine();
     const int elementIndex = that->d()->elementIndex();
-    int roleIndex = that->d()->m_model->m_listModel->setExistingProperty(elementIndex, propName, value, eng);
-    if (roleIndex != -1)
-        that->d()->m_model->emitItemsChanged(elementIndex, 1, QVector<int>(1, roleIndex));
+    if (QQmlListModel *model = that->d()->m_model) {
+        const int roleIndex
+                = model->listModel()->setExistingProperty(elementIndex, propName, value, eng);
+        if (roleIndex != -1)
+            model->emitItemsChanged(elementIndex, 1, QVector<int>(1, roleIndex));
+    }
 
     ModelNodeMetaObject *mo = ModelNodeMetaObject::get(that->object());
     if (mo->initialized())
@@ -1687,7 +1690,11 @@ ReturnedValue ModelObject::virtualGet(const Managed *m, PropertyKey id, const Va
     const ModelObject *that = static_cast<const ModelObject*>(m);
     Scope scope(that);
     ScopedString name(scope, id.asStringOrSymbol());
-    const ListLayout::Role *role = that->d()->m_model->m_listModel->getExistingRole(name);
+    QQmlListModel *model = that->d()->m_model;
+    if (!model)
+        return QObjectWrapper::virtualGet(m, id, receiver, hasProperty);
+
+    const ListLayout::Role *role = model->listModel()->getExistingRole(name);
     if (!role)
         return QObjectWrapper::virtualGet(m, id, receiver, hasProperty);
     if (hasProperty)
@@ -1700,7 +1707,7 @@ ReturnedValue ModelObject::virtualGet(const Managed *m, PropertyKey id, const Va
     }
 
     const int elementIndex = that->d()->elementIndex();
-    QVariant value = that->d()->m_model->data(elementIndex, role->index);
+    QVariant value = model->data(elementIndex, role->index);
     return that->engine()->fromVariant(value);
 }
 
@@ -1723,16 +1730,19 @@ PropertyKey ModelObjectOwnPropertyKeyIterator::next(const Object *o, Property *p
     const ModelObject *that = static_cast<const ModelObject *>(o);
 
     ExecutionEngine *v4 = that->engine();
-    if (roleNameIndex < that->listModel()->roleCount()) {
+
+    QQmlListModel *model = that->d()->m_model;
+    ListModel *listModel = model ? model->listModel() : nullptr;
+    if (listModel && roleNameIndex < listModel->roleCount()) {
         Scope scope(that->engine());
-        const ListLayout::Role &role = that->listModel()->getExistingRole(roleNameIndex);
+        const ListLayout::Role &role = listModel->getExistingRole(roleNameIndex);
         ++roleNameIndex;
         ScopedString roleName(scope, v4->newString(role.name));
         if (attrs)
             *attrs = QV4::Attr_Data;
         if (pd) {
 
-            QVariant value = that->d()->m_model->data(that->d()->elementIndex(), role.index);
+            QVariant value = model->data(that->d()->elementIndex(), role.index);
             if (auto recursiveListModel = qvariant_cast<QQmlListModel*>(value)) {
                 auto size = recursiveListModel->count();
                 auto array = ScopedArrayObject{scope, v4->newArrayObject(size)};
diff --git a/src/qmlmodels/qqmllistmodel_p.h b/src/qmlmodels/qqmllistmodel_p.h
index 5c44405626..64f6a07421 100644
--- a/src/qmlmodels/qqmllistmodel_p.h
+++ b/src/qmlmodels/qqmllistmodel_p.h
@@ -79,6 +79,8 @@ public:
     bool dynamicRoles() const { return m_dynamicRoles; }
     void setDynamicRoles(bool enableDynamicRoles);
 
+    ListModel *listModel() const { return m_listModel; }
+
 Q_SIGNALS:
     void countChanged();
 
diff --git a/src/qmlmodels/qqmllistmodel_p_p.h b/src/qmlmodels/qqmllistmodel_p_p.h
index 662a12f9e7..6a28a511f1 100644
--- a/src/qmlmodels/qqmllistmodel_p_p.h
+++ b/src/qmlmodels/qqmllistmodel_p_p.h
@@ -125,13 +125,23 @@ struct ModelObject : public QObjectWrapper {
     {
         QObjectWrapper::init(object);
         m_model = model;
-        QObjectPrivate *op = QObjectPrivate::get(object);
-        m_nodeModelMetaObject = static_cast<ModelNodeMetaObject *>(op->metaObject);
     }
-    void destroy() { QObjectWrapper::destroy(); }
-    int elementIndex() const { return m_nodeModelMetaObject->m_elementIndex; }
-    QQmlListModel *m_model;
-    ModelNodeMetaObject *m_nodeModelMetaObject;
+
+    void destroy()
+    {
+        m_model.destroy();
+        QObjectWrapper::destroy();
+    }
+
+    int elementIndex() const {
+        if (const QObject *o = object()) {
+            const QObjectPrivate *op = QObjectPrivate::get(o);
+            return static_cast<ModelNodeMetaObject *>(op->metaObject)->m_elementIndex;
+        }
+        return -1;
+    }
+
+    QV4QPointer<QQmlListModel> m_model;
 };
 
 }
@@ -141,8 +151,6 @@ struct ModelObject : public QObjectWrapper
     V4_OBJECT2(ModelObject, QObjectWrapper)
     V4_NEEDS_DESTROY
 
-    ListModel *listModel() const { return d()->m_model->m_listModel; }
-
 protected:
     static bool virtualPut(Managed *m, PropertyKey id, const Value& value, Value *receiver);
     static ReturnedValue virtualGet(const Managed *m, PropertyKey id, const Value *receiver, bool *hasProperty);
diff --git a/src/qmlmodels/qqmltableinstancemodel.cpp b/src/qmlmodels/qqmltableinstancemodel.cpp
index dcc15f90a5..6668c28ce2 100644
--- a/src/qmlmodels/qqmltableinstancemodel.cpp
+++ b/src/qmlmodels/qqmltableinstancemodel.cpp
@@ -122,7 +122,6 @@ QObject *QQmlTableInstanceModel::object(int index, QQmlIncubator::IncubationMode
 {
     Q_ASSERT(m_delegate);
     Q_ASSERT(index >= 0 && index < m_adaptorModel.count());
-    Q_ASSERT(m_qmlContext && m_qmlContext->isValid());
 
     QQmlDelegateModelItem *modelItem = resolveModelItem(index);
     if (!modelItem)
@@ -290,7 +289,7 @@ void QQmlTableInstanceModel::incubateModelItem(QQmlDelegateModelItem *modelItem,
         const bool sync = (incubationMode == QQmlIncubator::Synchronous || incubationMode == QQmlIncubator::AsynchronousIfNested);
         if (sync && modelItem->incubationTask->incubationMode() == QQmlIncubator::Asynchronous)
             modelItem->incubationTask->forceCompletion();
-    } else {
+    } else if (m_qmlContext && m_qmlContext->isValid()) {
         modelItem->incubationTask = new QQmlTableInstanceModelIncubationTask(this, modelItem, incubationMode);
 
         QQmlContext *creationContext = modelItem->delegate->creationContext();