From be8675ab01132ffb03b81cc81842775d6a8fa3f9 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Mon, 12 Mar 2012 13:20:22 +1000 Subject: [PATCH] Change repeater item to handle model being deleted. The repeater item previously stored a raw QObject pointer in a variant. When this pointer was a dynamic list model element that was deleted, the variant would continue to hold a stale pointer. Change repeater to use a guard object to hold the model when it is a QObject. Continue to use a variant to hold models that are not based on QObject to maintain same semantics. Change-Id: Ie100947132923803263c725e86efa68206382f12 Reviewed-by: Martin Jones --- src/quick/items/qquickrepeater.cpp | 10 +++++++++- src/quick/items/qquickrepeater_p_p.h | 2 ++ .../quick/qquickrepeater/data/dynamicmodelcrash.qml | 20 ++++++++++++++++++++ .../auto/quick/qquickrepeater/tst_qquickrepeater.cpp | 15 +++++++++++++++ 4 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 tests/auto/quick/qquickrepeater/data/dynamicmodelcrash.qml diff --git a/src/quick/items/qquickrepeater.cpp b/src/quick/items/qquickrepeater.cpp index d26ebed..1f7578c 100644 --- a/src/quick/items/qquickrepeater.cpp +++ b/src/quick/items/qquickrepeater.cpp @@ -51,7 +51,7 @@ QT_BEGIN_NAMESPACE QQuickRepeaterPrivate::QQuickRepeaterPrivate() - : model(0), ownModel(false), inRequest(false), itemCount(0), createFrom(-1) + : model(0), ownModel(false), inRequest(false), dataSourceIsObject(false), itemCount(0), createFrom(-1) { } @@ -175,6 +175,12 @@ QQuickRepeater::~QQuickRepeater() QVariant QQuickRepeater::model() const { Q_D(const QQuickRepeater); + + if (d->dataSourceIsObject) { + QObject *o = d->dataSourceAsObject; + return QVariant::fromValue(o); + } + return d->dataSource; } @@ -194,6 +200,8 @@ void QQuickRepeater::setModel(const QVariant &model) } d->dataSource = model; QObject *object = qvariant_cast(model); + d->dataSourceAsObject = object; + d->dataSourceIsObject = object != 0; QQuickVisualModel *vim = 0; if (object && (vim = qobject_cast(object))) { if (d->ownModel) { diff --git a/src/quick/items/qquickrepeater_p_p.h b/src/quick/items/qquickrepeater_p_p.h index 88e0c94..41d77ad 100644 --- a/src/quick/items/qquickrepeater_p_p.h +++ b/src/quick/items/qquickrepeater_p_p.h @@ -75,8 +75,10 @@ private: QQuickVisualModel *model; QVariant dataSource; + QQmlGuard dataSourceAsObject; bool ownModel : 1; bool inRequest : 1; + bool dataSourceIsObject : 1; int itemCount; int createFrom; diff --git a/tests/auto/quick/qquickrepeater/data/dynamicmodelcrash.qml b/tests/auto/quick/qquickrepeater/data/dynamicmodelcrash.qml new file mode 100644 index 0000000..0280df0 --- /dev/null +++ b/tests/auto/quick/qquickrepeater/data/dynamicmodelcrash.qml @@ -0,0 +1,20 @@ +import QtQuick 2.0 + +Item { + ListModel { + id: lm; + } + + Component.onCompleted: { + lm.append({ subModel: [ {d:0} ] }); + rep.model = lm.get(0).subModel; + rep.model; + lm.remove(0); + rep.model; + } + + Repeater { + objectName: "rep" + id: rep + } +} diff --git a/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp b/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp index 1b07a6e..d9cbed0 100644 --- a/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp +++ b/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp @@ -77,6 +77,7 @@ private slots: void properties(); void asynchronous(); void initParent(); + void dynamicModelCrash(); }; class TestObject : public QObject @@ -639,6 +640,20 @@ void tst_QQuickRepeater::initParent() QCOMPARE(qvariant_cast(rootObject->property("parentItem")), rootObject); } +void tst_QQuickRepeater::dynamicModelCrash() +{ + QQmlEngine engine; + QQmlComponent component(&engine, testFileUrl("dynamicmodelcrash.qml")); + + // Don't crash + QQuickItem *rootObject = qobject_cast(component.create()); + QVERIFY(rootObject); + + QQuickRepeater *repeater = findItem(rootObject, "rep"); + QVERIFY(repeater); + QVERIFY(qvariant_cast(repeater->model()) == 0); +} + QTEST_MAIN(tst_QQuickRepeater) #include "tst_qquickrepeater.moc" -- 2.7.4