QQmlListModel: Invalidate ModelObject when necessary

Both the object and the listmodel may be deleted during the life time of
ModelObject. Don't crash when that happens.

Also, fix QV4QPointer to actually name the type of the pointer it
stores. Apparently this is the first time we add a QV4QPointer of
something that's not a plain QObject.

Pick-to: 5.15
Task-number: QTBUG-118024
Change-Id: I208d8749bcd67970f7bfbe569eed7a472f909508
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit 90c55e859e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit aa253878fe)
(cherry picked from commit f7182e0124)
Reviewed-by: Sami Shalayel <sami.shalayel@qt.io>
(cherry picked from commit ca5bd206d49fafb813c52570a89006ed1967b17c)
This commit is contained in:
Ulf Hermann 2024-08-08 12:56:12 +02:00 committed by Qt Cherry-pick Bot
parent b0a0eb700a
commit 0351ad8c60
6 changed files with 144 additions and 17 deletions

View File

@ -251,7 +251,7 @@ struct QV4QPointer {
private:
QtSharedPointer::ExternalRefCountData *d;
QObject *qObject;
T *qObject;
};
Q_STATIC_ASSERT(std::is_trivial< QV4QPointer<QObject> >::value);
#endif

View File

@ -1699,9 +1699,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())
@ -1717,7 +1720,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)
@ -1730,7 +1737,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);
}
@ -1753,16 +1760,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)};

View File

@ -115,6 +115,8 @@ public:
bool dynamicRoles() const { return m_dynamicRoles; }
void setDynamicRoles(bool enableDynamicRoles);
ListModel *listModel() const { return m_listModel; }
Q_SIGNALS:
void countChanged();

View File

@ -161,13 +161,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;
};
}
@ -177,8 +187,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);

View File

@ -0,0 +1,45 @@
import QtQml
QtObject {
function swapCorpses() {
const lhsData = getModelData(lhsButtonListModel);
const rhsData = getModelData(rhsButtonListModel);
lhsButtonListModel.clear();
rhsButtonListModel.clear();
addToModel(lhsButtonListModel, rhsData);
addToModel(rhsButtonListModel, lhsData);
}
property ListModel l1: ListModel {
id: lhsButtonListModel
}
property ListModel l2: ListModel {
id: rhsButtonListModel
}
Component.onCompleted: {
lhsButtonListModel.append({ "ident": 1, "buttonText": "B 1"});
lhsButtonListModel.append({ "ident": 2, "buttonText": "B 2"});
lhsButtonListModel.append({ "ident": 3, "buttonText": "B 3"});
rhsButtonListModel.append({ "ident": 4, "buttonText": "B 4"});
rhsButtonListModel.append({ "ident": 5, "buttonText": "B 5"});
rhsButtonListModel.append({ "ident": 6, "buttonText": "B 6"});
}
function getModelData(model) {
var dataList = []
for (var i = 0; i < model.count; ++i)
dataList.push(model.get(i));
return dataList;
}
function addToModel(model, buttonData) {
for (var i = 0; i < buttonData.length; ++i)
model.append(buttonData[i]);
}
}

View File

@ -140,6 +140,7 @@ private slots:
void destroyComponentObject();
void objectOwnershipFlip();
void protectQObjectFromGC();
void deadModelData();
};
bool tst_qqmllistmodel::compareVariantList(const QVariantList &testList, QVariant object)
@ -1940,6 +1941,67 @@ void tst_qqmllistmodel::protectQObjectFromGC()
}
}
void tst_qqmllistmodel::deadModelData()
{
QQmlEngine engine;
QQmlComponent component(&engine, testFileUrl("deadModelData.qml"));
QVERIFY2(component.isReady(), qPrintable(component.errorString()));
QScopedPointer<QObject> o(component.create());
QVERIFY(!o.isNull());
QQmlListModel *l1 = o->property("l1").value<QQmlListModel *>();
QVERIFY(l1);
QQmlListModel *l2 = o->property("l2").value<QQmlListModel *>();
QVERIFY(l2);
QCOMPARE(l1->count(), 3);
QCOMPARE(l2->count(), 3);
for (int i = 0; i < 3; ++i) {
QObject *i1 = qjsvalue_cast<QObject *>(l1->get(i));
QVERIFY(i1);
QCOMPARE(i1->property("ident").value<double>(), i + 1);
QCOMPARE(i1->property("buttonText").value<QString>(),
QLatin1String("B %1").arg(QLatin1Char('0' + i + 1)));
QObject *i2 = qjsvalue_cast<QObject *>(l2->get(i));
QVERIFY(i2);
QCOMPARE(i2->property("ident").value<double>(), i + 4);
QCOMPARE(i2->property("buttonText").value<QString>(),
QLatin1String("B %1").arg(QLatin1Char('0' + i + 4)));
}
for (int i = 0; i < 6; ++i) {
QTest::ignoreMessage(
QtWarningMsg,
QRegularExpression(".*: ident is undefined. Adding an object with a undefined "
"member does not create a role for it."));
QTest::ignoreMessage(
QtWarningMsg,
QRegularExpression(".*: buttonText is undefined. Adding an object with a undefined "
"member does not create a role for it."));
}
QMetaObject::invokeMethod(o.data(), "swapCorpses");
// We get default-created values for all the roles now.
QCOMPARE(l1->count(), 3);
QCOMPARE(l2->count(), 3);
for (int i = 0; i < 3; ++i) {
QObject *i1 = qjsvalue_cast<QObject *>(l1->get(i));
QVERIFY(i1);
QCOMPARE(i1->property("ident").value<double>(), double());
QCOMPARE(i1->property("buttonText").value<QString>(), QString());
QObject *i2 = qjsvalue_cast<QObject *>(l2->get(i));
QVERIFY(i2);
QCOMPARE(i2->property("ident").value<double>(), double());
QCOMPARE(i2->property("buttonText").value<QString>(), QString());
}
}
QTEST_MAIN(tst_qqmllistmodel)
#include "tst_qqmllistmodel.moc"