Merge remote-tracking branch 'origin/5.9' into 5.10

 Conflicts:
	src/qml/compiler/qv4compileddata.cpp
	src/qml/compiler/qv4compileddata_p.h
	src/qml/jsruntime/qv4engine.cpp
	src/qml/jsruntime/qv4qmlcontext.cpp
	src/qml/jsruntime/qv4qmlcontext_p.h
	src/qml/jsruntime/qv4regexpobject.cpp
	src/qml/jsruntime/qv4regexpobject_p.h
	src/qml/types/qqmllistmodel.cpp
	src/quick/items/qquickanimatedimage_p.h
	src/quick/scenegraph/qsgrenderloop.cpp
	tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp

Change-Id: If20ef62b2c98bdf656cb2f5d27b1897b754d3dc0

4 files changed, 47 insertions, 17 deletions

diff --git a/src/qml/types/qqmldelegatemodel.cpp b/src/qml/types/qqmldelegatemodel.cpp
index d54bdeede6..4d2a9746c3 100644
--- a/src/qml/types/qqmldelegatemodel.cpp
+++ b/src/qml/types/qqmldelegatemodel.cpp
@@ -269,7 +269,8 @@ QQmlDelegateModel::~QQmlDelegateModel()
             delete cacheItem->object;
 
             cacheItem->object = 0;
-            cacheItem->contextData->destroy();
+            cacheItem->contextData->invalidate();
+            Q_ASSERT(cacheItem->contextData->refCount == 1);
             cacheItem->contextData = 0;
             cacheItem->scriptRef -= 1;
         }
@@ -840,7 +841,8 @@ void QQDMIncubationTask::statusChanged(Status status)
         delete incubating->object;
         incubating->object = 0;
         if (incubating->contextData) {
-            incubating->contextData->destroy();
+            incubating->contextData->invalidate();
+            Q_ASSERT(incubating->contextData->refCount == 1);
             incubating->contextData = 0;
         }
         incubating->scriptRef = 0;
@@ -900,8 +902,10 @@ void QQmlDelegateModelPrivate::incubatorStatusChanged(QQDMIncubationTask *incuba
         delete cacheItem->object;
         cacheItem->object = 0;
         cacheItem->scriptRef -= 1;
-        if (cacheItem->contextData)
-            cacheItem->contextData->destroy();
+        if (cacheItem->contextData) {
+            cacheItem->contextData->invalidate();
+            Q_ASSERT(cacheItem->contextData->refCount == 1);
+        }
         cacheItem->contextData = 0;
 
         if (!cacheItem->isReferenced()) {
@@ -983,7 +987,7 @@ QObject *QQmlDelegateModelPrivate::object(Compositor::Group group, int index, bo
             if (QQmlAdaptorModelProxyInterface *proxy
                     = qobject_cast<QQmlAdaptorModelProxyInterface *>(cacheItem)) {
                 ctxt = new QQmlContextData;
-                ctxt->setParent(cacheItem->contextData, true);
+                ctxt->setParent(cacheItem->contextData);
                 ctxt->contextObject = proxy->proxiedObject();
             }
         }
@@ -1264,6 +1268,13 @@ void QQmlDelegateModel::_q_itemsInserted(int index, int count)
     d->emitChanges();
 }
 
+//### This method should be split in two. It will remove delegates, and it will re-render the list.
+// When e.g. QQmlListModel::remove is called, the removal of the delegates should be done on
+// QAbstractItemModel::rowsAboutToBeRemoved, and the re-rendering on
+// QAbstractItemModel::rowsRemoved. Currently both are done on the latter signal. The problem is
+// that the destruction of an item will emit a changed signal that ends up at the delegate, which
+// in turn will try to load the data from the model (which should have already freed it), resulting
+// in a use-after-free. See QTBUG-59256.
 void QQmlDelegateModelPrivate::itemsRemoved(
         const QVector<Compositor::Remove> &removes,
         QVarLengthArray<QVector<QQmlChangeSet::Change>, Compositor::MaximumGroupCount> *translatedRemoves,
@@ -1944,8 +1955,11 @@ void QQmlDelegateModelItem::destroyObject()
 
     QQmlData *data = QQmlData::get(object);
     Q_ASSERT(data);
-    if (data->ownContext && data->context)
-        data->context->clearContext();
+    if (data->ownContext) {
+        data->ownContext->clearContext();
+        data->ownContext = 0;
+        data->context = 0;
+    }
     object->deleteLater();
 
     if (attached) {
@@ -1953,7 +1967,7 @@ void QQmlDelegateModelItem::destroyObject()
         attached = 0;
     }
 
-    contextData->destroy();
+    contextData->invalidate();
     contextData = 0;
     object = 0;
 }
diff --git a/src/qml/types/qqmldelegatemodel_p_p.h b/src/qml/types/qqmldelegatemodel_p_p.h
index 4ebfd9b938..3759fe8667 100644
--- a/src/qml/types/qqmldelegatemodel_p_p.h
+++ b/src/qml/types/qqmldelegatemodel_p_p.h
@@ -135,7 +135,7 @@ public:
 
     QV4::ExecutionEngine *v4;
     QQmlDelegateModelItemMetaType * const metaType;
-    QQmlContextData *contextData;
+    QQmlContextDataRef contextData;
     QPointer<QObject> object;
     QPointer<QQmlDelegateModelAttached> attached;
     QQDMIncubationTask *incubationTask;
diff --git a/src/qml/types/qqmllistmodel.cpp b/src/qml/types/qqmllistmodel.cpp
index 20000557ee..d088a5da42 100644
--- a/src/qml/types/qqmllistmodel.cpp
+++ b/src/qml/types/qqmllistmodel.cpp
@@ -572,14 +572,20 @@ void ListModel::clear()
     elements.clear();
 }
 
-void ListModel::remove(int index, int count)
+QVector<std::function<void()>> ListModel::remove(int index, int count)
 {
+    QVector<std::function<void()>> toDestroy;
+    auto layout = m_layout;
     for (int i=0 ; i < count ; ++i) {
-        elements[index+i]->destroy(m_layout);
-        delete elements[index+i];
+        auto element = elements[index+i];
+        toDestroy.append([element, layout](){
+            element->destroy(layout);
+            delete element;
+        });
     }
     elements.remove(index, count);
     updateCacheIndices(index);
+    return toDestroy;
 }
 
 void ListModel::insert(int elementIndex, QV4::Object *object)
@@ -1394,7 +1400,10 @@ void ModelObject::advanceIterator(Managed *m, ObjectIterator *it, Value *name, u
         p->value = v4->fromVariant(value);
         return;
     }
-    QV4::QObjectWrapper::advanceIterator(m, it, name, index, p, attributes);
+    // Fall back to QV4::Object as opposed to QV4::QObjectWrapper otherwise it will add
+    // unnecessary entries that relate to the roles used. These just create extra work
+    // later on as they will just be ignored.
+    QV4::Object::advanceIterator(m, it, name, index, p, attributes);
 }
 
 DEFINE_OBJECT_VTABLE(ModelObject);
@@ -2059,15 +2068,22 @@ void QQmlListModel::remove(QQmlV4Function *args)
 
         emitItemsAboutToBeRemoved(index, removeCount);
 
+        QVector<std::function<void()>> toDestroy;
         if (m_dynamicRoles) {
-            for (int i=0 ; i < removeCount ; ++i)
-                delete m_modelObjects[index+i];
+            for (int i=0 ; i < removeCount ; ++i) {
+                auto modelObject = m_modelObjects[index+i];
+                toDestroy.append([modelObject](){
+                    delete modelObject;
+                });
+            }
             m_modelObjects.remove(index, removeCount);
         } else {
-            m_listModel->remove(index, removeCount);
+            toDestroy = m_listModel->remove(index, removeCount);
         }
 
         emitItemsRemoved(index, removeCount);
+        for (const auto &destroyer : toDestroy)
+            destroyer();
     } else {
         qmlWarning(this) << tr("remove: incorrect number of arguments");
     }
diff --git a/src/qml/types/qqmllistmodel_p_p.h b/src/qml/types/qqmllistmodel_p_p.h
index e2653c220d..77d2002afa 100644
--- a/src/qml/types/qqmllistmodel_p_p.h
+++ b/src/qml/types/qqmllistmodel_p_p.h
@@ -367,7 +367,7 @@ public:
     void insert(int elementIndex, QV4::Object *object);
 
     void clear();
-    void remove(int index, int count);
+    Q_REQUIRED_RESULT QVector<std::function<void()>> remove(int index, int count);
 
     int appendElement();
     void insertElement(int index);