diff options
author | Mitch Curtis <[email protected]> | 2025-03-07 14:34:32 +0800 |
---|---|---|
committer | Mitch Curtis <[email protected]> | 2025-07-04 13:02:04 +0800 |
commit | fa8bfd6ed5ec4150cd2b05cd17b199097193fb1b (patch) | |
tree | c7245941420d0ea7b3471658cc54e185ff982930 /tests | |
parent | 2e1337d87a60b8d1a30f5296bf3f614ec0bca383 (diff) |
Container's removeItem function removes and deletes items immediately.
This has caused issues in the past, like QTBUG-46798. QTBUG-133256 is
similar to that, except it also involves transitions.
This patch fixes the crash by listening for the destruction of items
created within an ObjectModel and removing them so that views don't
have dangling pointers to them.
Add removeAndDestroyObjectModelItem to tst_qquicklistview2, which is
similar to the test added for 1e3924d8f585dd9099eb74ffbc17950c693d14da
but adds more thorough checks and transitions.
Fixes: QTBUG-133256
Task-number: QTBUG-46798
Pick-to: 6.5 6.8 6.9 6.10
Change-Id: I9777713edfc6f82a4e9edefe2fd6c9831f03c261
Reviewed-by: Shawn Rutledge <[email protected]>
Reviewed-by: Richard Moe Gustavsen <[email protected]>
Diffstat (limited to 'tests')
4 files changed, 195 insertions, 0 deletions
diff --git a/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItem.qml b/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItem.qml new file mode 100644 index 0000000000..994bd8400b --- /dev/null +++ b/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItem.qml @@ -0,0 +1,30 @@ +// Copyright (C) 2025 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only + +import QtQml.Models +import QtQuick + +ListView { + width: 200 + height: 200 + + model: ObjectModel { + id: objectModel + + Rectangle { + width: 200 + height: 200 + color: "red" + } + Rectangle { + width: 200 + height: 200 + color: "green" + } + Rectangle { + width: 200 + height: 200 + color: "blue" + } + } +} diff --git a/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItemWithTransitions.qml b/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItemWithTransitions.qml new file mode 100644 index 0000000000..b979bfd118 --- /dev/null +++ b/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItemWithTransitions.qml @@ -0,0 +1,38 @@ +// Copyright (C) 2025 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only + +import QtQml.Models +import QtQuick + +ListView { + width: 200 + height: 200 + + displaced: Transition { + NumberAnimation { properties: "x,y"; duration: 400; easing.type: Easing.OutBounce } + } + + remove: Transition { + NumberAnimation { properties: "scale"; from: 1; to: 0; duration: 400 } + } + + model: ObjectModel { + id: objectModel + + Rectangle { + width: 200 + height: 200 + color: "red" + } + Rectangle { + width: 200 + height: 200 + color: "green" + } + Rectangle { + width: 200 + height: 200 + color: "blue" + } + } +} diff --git a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp index ed95f06f26..94feee92f3 100644 --- a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp +++ b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp @@ -4,6 +4,7 @@ #include "delegatemodelkinds.h" #include <QtTest/QTest> +#include <QtQmlModels/private/qqmlobjectmodel_p.h> #include <QtQuick/qquickview.h> #include <QtQuick/private/qquickitemview_p_p.h> #include <QtQuick/private/qquicklistview_p.h> @@ -80,6 +81,8 @@ private slots: void delegateModelAccess_data(); void delegateModelAccess(); + void removeAndDestroyObjectModelItem_data(); + void removeAndDestroyObjectModelItem(); private: void flickWithTouch(QQuickWindow *window, const QPoint &from, const QPoint &to); @@ -1480,6 +1483,56 @@ void tst_QQuickListView2::delegateModelAccess() QCOMPARE(delegate->property("modelX").toDouble(), expected); } +enum RemovalPolicy { + RemoveAndDestroy, + OnlyDestroy +}; + +void tst_QQuickListView2::removeAndDestroyObjectModelItem_data() +{ + QTest::addColumn<QString>("qmlFilePath"); + QTest::addColumn<RemovalPolicy>("removalPolicy"); + + QTest::addRow("remove and destroy, no transitions") + << "removeAndDestroyObjectModelItem.qml" << RemoveAndDestroy; + QTest::addRow("destroy, no transitions") + << "removeAndDestroyObjectModelItem.qml" << OnlyDestroy; + QTest::addRow("remove and destroy, transitions") + << "removeAndDestroyObjectModelItemWithTransitions.qml" << RemoveAndDestroy; + QTest::addRow("destroy, transitions") + << "removeAndDestroyObjectModelItemWithTransitions.qml" << OnlyDestroy; +} + +// QTBUG-46798, QTBUG-133256 +void tst_QQuickListView2::removeAndDestroyObjectModelItem() +{ + QFETCH(QString, qmlFilePath); + QFETCH(RemovalPolicy, removalPolicy); + + QQuickView window; + QVERIFY(QQuickTest::showView(window, testFileUrl(qmlFilePath))); + + QQuickListView *listView = qobject_cast<QQuickListView*>(window.rootObject()); + QVERIFY(listView); + + auto *objectModel = listView->model().value<QQmlObjectModel *>(); + QVERIFY(objectModel); + QPointer<QObject> firstItem = objectModel->get(0); + QVERIFY(firstItem); + // Shouldn't crash. + if (removalPolicy == RemoveAndDestroy) + objectModel->remove(0); + firstItem->deleteLater(); + QTRY_VERIFY(!firstItem); + + // Now try moving the view. It also shouldn't crash. + if (removalPolicy == RemoveAndDestroy) { + if (QQuickTest::qIsPolishScheduled(listView)) + QVERIFY(QQuickTest::qWaitForPolish(listView)); + } + listView->positionViewAtEnd(); +} + QTEST_MAIN(tst_QQuickListView2) #include "tst_qquicklistview2.moc" diff --git a/tests/auto/quickcontrols/controls/data/tst_container.qml b/tests/auto/quickcontrols/controls/data/tst_container.qml index a829404a21..f6c50699e2 100644 --- a/tests/auto/quickcontrols/controls/data/tst_container.qml +++ b/tests/auto/quickcontrols/controls/data/tst_container.qml @@ -293,4 +293,78 @@ TestCase { compare(control.implicitWidth, 100) compare(control.implicitHeight, 30) } + + Transition { + id: yTransition + + NumberAnimation { + properties: "y" + duration: 1000 + } + } + + Transition { + id: scaleTransition + + NumberAnimation { + properties: "scale" + from: 1 + to: 0 + duration: 400 + } + } + + Component { + id: listViewContainerComponent + + Container { + id: container + width: 200 + height: 200 + + property alias addDisplaced: view.addDisplaced + property alias moveDisplaced: view.moveDisplaced + property alias removeDisplaced: view.removeDisplaced + property alias displaced: view.displaced + property alias remove: view.remove + + contentItem: ListView { + id: view + objectName: "containerListView" + model: container.contentModel + spacing: 5 + } + } + } + + function test_listViewTransitions_data() { + return [ + { tag: "addDisplaced", addDisplaced: yTransition }, + { tag: "moveDisplaced", moveDisplaced: yTransition }, + { tag: "removeDisplaced", removeDisplaced: yTransition }, + { tag: "displaced", displaced: yTransition }, + { tag: "remove", remove: scaleTransition } + ] + } + + function test_listViewTransitions(data) { + let control = createTemporaryObject(listViewContainerComponent, testCase, { + addDisplaced: data.addDisplaced ?? null, + moveDisplaced: data.moveDisplaced ?? null, + removeDisplaced: data.removeDisplaced ?? null, + displaced: data.displaced ?? null, + remove: data.remove ?? null + }) + verify(control) + + let rect = rectangle.createObject(control, + { width: 200, height: 40, color: "tomato", objectName: "tomatoRect" }) + control.addItem(rect) + verify(isPolishScheduled(testCase.Window.window)) + verify(waitForPolish(testCase.Window.window)) + + control.removeItem(rect) + verify(isPolishScheduled(testCase.Window.window)) + verify(waitForPolish(testCase.Window.window)) + } } |