diff options
author | Kai Uwe Broulik <kde@privat.broulik.de> | 2023-08-18 17:24:15 +0200 |
---|---|---|
committer | Kai Uwe Broulik <kde@privat.broulik.de> | 2023-08-21 20:52:01 +0200 |
commit | 8c852c70c9cd415f852239395101c8b9884d5688 (patch) | |
tree | fabba6420f0a07e1e21f3c728821527172eab79c | |
parent | 9f4aeeabb982cfc4306c9d350dbb68f64914fb32 (diff) |
QQmlDMAbstractItemModelData: Guard against deletion during model write
When a `setData` call on a model results in our row being removed
(e.g. when a proxy model filters out the row as a result of this
change), an Instantiator would delete us and we'd try to emit a
property change on garbage `this`.
Amends commit 5db77fb406bb36a27de1ad2e3390720411c833f9which made
Instantiator clean up its objects on a model change.
Pick-to: 6.6 6.5
Change-Id: I5570453de588e32dc5e4235287a83565e5185aa2
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
3 files changed, 103 insertions, 0 deletions
diff --git a/src/qmlmodels/qqmldmabstractitemmodeldata.cpp b/src/qmlmodels/qqmldmabstractitemmodeldata.cpp index 257a6bfc31..901d8bbe53 100644 --- a/src/qmlmodels/qqmldmabstractitemmodeldata.cpp +++ b/src/qmlmodels/qqmldmabstractitemmodeldata.cpp @@ -42,7 +42,11 @@ int QQmlDMAbstractItemModelData::metaCall(QMetaObject::Call call, int id, void * QMetaObject::activate(this, meta, 0, nullptr); } } else if (*m_type->model) { + QQmlGuard<QQmlDMAbstractItemModelData> guard(this); setValue(m_type->propertyRoles.at(propertyIndex), *static_cast<QVariant *>(arguments[0])); + if (guard.isNull()) + return -1; + QMetaObject::activate(this, meta, propertyIndex, nullptr); } emit modelDataChanged(); diff --git a/tests/auto/qml/qqmlinstantiator/data/removeDuringModelChange.qml b/tests/auto/qml/qqmlinstantiator/data/removeDuringModelChange.qml new file mode 100644 index 0000000000..079e376549 --- /dev/null +++ b/tests/auto/qml/qqmlinstantiator/data/removeDuringModelChange.qml @@ -0,0 +1,10 @@ +import QtQuick 2.15 +import QtQml.Models 2.15 + +Instantiator { + delegate: QtObject { + function deactivate() { + model.active = false; + } + } +} diff --git a/tests/auto/qml/qqmlinstantiator/tst_qqmlinstantiator.cpp b/tests/auto/qml/qqmlinstantiator/tst_qqmlinstantiator.cpp index 1097c65f02..f6d2752889 100644 --- a/tests/auto/qml/qqmlinstantiator/tst_qqmlinstantiator.cpp +++ b/tests/auto/qml/qqmlinstantiator/tst_qqmlinstantiator.cpp @@ -2,6 +2,7 @@ // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 #include <qtest.h> #include <QSignalSpy> +#include <QSortFilterProxyModel> #include <QDebug> #include <QtQml/qqmlengine.h> @@ -28,6 +29,7 @@ private slots: void activeModelChangeInteraction(); void intModelChange(); void createAndRemove(); + void removeDuringModelChange(); void asynchronous_data(); void asynchronous(); @@ -306,6 +308,93 @@ void tst_qqmlinstantiator::boundDelegateComponent() QCOMPARE(b->objectAt(2)->objectName(), QStringLiteral("root3")); } +class SingleBoolItemModel : public QAbstractListModel +{ + Q_OBJECT + +public: + SingleBoolItemModel(QObject *parent = nullptr) : QAbstractListModel(parent) {} + + int rowCount(const QModelIndex &parent = QModelIndex()) const override + { + if (parent.isValid()) + return 0; + return 1; + } + + QVariant data(const QModelIndex &index, int role) const override + { + if (index.parent().isValid() || index.row() != 0 || index.column() != 0 + || role != Qt::UserRole) + return QVariant(); + + return m_active; + } + + bool setData(const QModelIndex &index, const QVariant &value, + int role) override { + if (index.parent().isValid() || index.row() != 0 || index.column() != 0 + || role != Qt::UserRole || m_active == value.toBool()) + return false; + + m_active = value.toBool(); + Q_EMIT dataChanged(index, index, QList<int>{Qt::UserRole}); + return true; + } + + QHash<int, QByteArray> roleNames() const override + { + return { {Qt::UserRole, "active"} }; + } + +private: + bool m_active = true; +}; + +class FilterBoolRoleProxyModel : public QSortFilterProxyModel +{ + Q_OBJECT + +public: + FilterBoolRoleProxyModel(QObject *parent = nullptr) + : QSortFilterProxyModel(parent) {} + + bool filterAcceptsRow(int source_row, + const QModelIndex &source_parent) const override + { + return sourceModel()->index(source_row, 0, source_parent).data(Qt::UserRole).toBool(); + } +}; + +void tst_qqmlinstantiator::removeDuringModelChange() +{ + SingleBoolItemModel model; + + FilterBoolRoleProxyModel proxyModel; + proxyModel.setSourceModel(&model); + proxyModel.setFilterRole(Qt::UserRole); + QCOMPARE(proxyModel.rowCount(), 1); + + QQmlEngine engine; + const QUrl url(testFileUrl("removeDuringModelChange.qml")); + QQmlComponent component(&engine, url); + QVERIFY2(component.isReady(), qPrintable(component.errorString())); + + QScopedPointer<QObject> o(component.create()); + QVERIFY2(!o.isNull(), qPrintable(component.errorString())); + + QQmlInstantiator *instantiator = qobject_cast<QQmlInstantiator *>(o.data()); + + instantiator->setModel(QVariant::fromValue(&proxyModel)); + + QSignalSpy removedSpy(instantiator, &QQmlInstantiator::objectRemoved); + QMetaObject::invokeMethod(instantiator->object(), "deactivate"); + + // We should still be alive at this point. + QCOMPARE(removedSpy.size(), 1); + QCOMPARE(proxyModel.rowCount(), 0); +} + QTEST_MAIN(tst_qqmlinstantiator) #include "tst_qqmlinstantiator.moc" |