DelegateModel: Do not redundantly re-scan model data all the time

Once we've scanned a given QVariant for its metaobject-relevant
properties, we don't have to do it again until it changes.

This also avoids rescanning the model data when tearing down a list
view, which might otherwise access dangling pointers of deleted model
objects.

Fixes: QTBUG-120113
Change-Id: I373a69141668e3ec0e7ee3f3a86248da88882b41
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
(cherry picked from commit 0a7f89007a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

src/qmlmodels/qqmldmlistaccessordata.cpp

@@ -21,6 +21,7 @@ void QQmlDMListAccessorData::setModelData(const QVariant &data) {
         return;
 
     cachedData = data;
+    cachedDataClean = false;
     static_cast<const VDMListDelegateDataType *>(QObjectPrivate::get(this)->metaObject)
             ->emitAllSignals(this);
 }
@@ -28,6 +29,8 @@ void QQmlDMListAccessorData::setModelData(const QVariant &data) {
 void QQmlDMListAccessorData::setValue(const QString &role, const QVariant &value)
 {
     // Used only for initialization of the cached data. Does not have to emit change signals.
+    Q_ASSERT(!cachedDataClean);
+
     if (role == QLatin1String("modelData") || role.isEmpty())
         cachedData = value;
     else
@@ -84,10 +87,12 @@ int VDMListDelegateDataType::metaCall(
         if (argument == value(&data, name))
             return -1;
         setValue(&data, name, argument);
-        if (accessor->index == -1)
+        if (accessor->index == -1) {
             accessor->cachedData = data;
-        else
+            accessor->cachedDataClean = false;
+        } else {
             model->list.set(accessor->index, data);
+        }
         QMetaObject::activate(accessor, this, id - propertyOffset, nullptr);
         emit accessor->modelDataChanged();
         return -1;
@@ -134,7 +139,11 @@ QMetaObject *VDMListDelegateDataType::toDynamicMetaObject(QObject *object)
 
     // If the context object is not the model object, we are using required properties.
     // In that case, create any extra properties.
-    createMissingProperties(&static_cast<QQmlDMListAccessorData *>(object)->cachedData);
+    QQmlDMListAccessorData *data = static_cast<QQmlDMListAccessorData *>(object);
+    if (!data->cachedDataClean) {
+        createMissingProperties(&data->cachedData);
+        data->cachedDataClean = true;
+    }
     return this;
 }
 

src/qmlmodels/qqmldmlistaccessordata_p.h

@@ -83,6 +83,9 @@ Q_SIGNALS:
 private:
     friend class VDMListDelegateDataType;
     QVariant cachedData;
+
+    // Gets cleaned when the metaobject has processed it.
+    bool cachedDataClean = false;
 };
 
 

tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp

@@ -63,6 +63,8 @@ private slots:
     void changingOrientationResetsPreviousAxisValues_data();
     void changingOrientationResetsPreviousAxisValues();
 
+    void clearObjectListModel();
+
 private:
     void flickWithTouch(QQuickWindow *window, const QPoint &from, const QPoint &to);
     QScopedPointer<QPointingDevice> touchDevice = QScopedPointer<QPointingDevice>(QTest::createTouchDevice());
@@ -1153,6 +1155,39 @@ void tst_QQuickListView2::changingOrientationResetsPreviousAxisValues() // QTBUG
     QVERIFY(!listView->property("isYReset").toBool());
 }
 
+void tst_QQuickListView2::clearObjectListModel()
+{
+    QQmlEngine engine;
+    QQmlComponent delegate(&engine);
+
+    // Need one required property to trigger the incremental rebuilding of metaobjects.
+    delegate.setData("import QtQuick\nItem { required property int index }", QUrl());
+
+    QQuickListView list;
+    engine.setContextForObject(&list, engine.rootContext());
+    list.setDelegate(&delegate);
+    list.setWidth(640);
+    list.setHeight(480);
+
+    QScopedPointer modelObject(new QObject);
+
+    // Use a list that might also carry something non-QObject
+
+    list.setModel(QVariantList {
+        QVariant::fromValue(modelObject.data()),
+        QVariant::fromValue(modelObject.data())
+    });
+
+    QVERIFY(list.itemAtIndex(0));
+
+    modelObject.reset();
+
+    // list should not access dangling pointer from old model data anymore.
+    list.setModel(QVariantList());
+
+    QVERIFY(!list.itemAtIndex(0));
+}
+
 QTEST_MAIN(tst_QQuickListView2)
 
 #include "tst_qquicklistview2.moc"