Fix crash bug related to QDeclarativeListModel

If QDeclarativeListModel is deleted, all references to this object in
QDeclarativeListModelWorkerAgent and WorkerScript objects should be removed
and additional checking is needed when process the pending sync() events.

Change-Id: I12b1f06699cc908e684af0886cf06d811c3fceb4
Reviewed-by: Michael Brasser <michael.brasser@nokia.com>
Reviewed-by: Glenn Watson <glenn.watson@nokia.com>

4 files changed, 60 insertions, 44 deletions

diff --git a/src/declarative/qml/qdeclarativelistmodel.cpp b/src/declarative/qml/qdeclarativelistmodel.cpp
index eff3c46c70..65b3de9d31 100644
--- a/src/declarative/qml/qdeclarativelistmodel.cpp
+++ b/src/declarative/qml/qdeclarativelistmodel.cpp
@@ -1501,8 +1501,10 @@ QDeclarativeListModel::~QDeclarativeListModel()
         m_listModel->destroy();
         delete m_listModel;
 
-        if (m_mainThread && m_agent)
+        if (m_mainThread && m_agent) {
+            m_agent->modelDestroyed();
             m_agent->release();
+        }
     }
 
     m_listModel = 0;
diff --git a/src/declarative/qml/qdeclarativelistmodelworkeragent.cpp b/src/declarative/qml/qdeclarativelistmodelworkeragent.cpp
index c7f671c025..3ae1b32646 100644
--- a/src/declarative/qml/qdeclarativelistmodelworkeragent.cpp
+++ b/src/declarative/qml/qdeclarativelistmodelworkeragent.cpp
@@ -117,6 +117,11 @@ void QDeclarativeListModelWorkerAgent::release()
         delete this;
 }
 
+void QDeclarativeListModelWorkerAgent::modelDestroyed()
+{
+    m_orig = 0;
+}
+
 int QDeclarativeListModelWorkerAgent::count() const
 {
     return m_copy->count();
@@ -178,49 +183,50 @@ void QDeclarativeListModelWorkerAgent::sync()
 bool QDeclarativeListModelWorkerAgent::event(QEvent *e)
 {
     if (e->type() == QEvent::User) {
-
+        bool cc = false;
         QMutexLocker locker(&mutex);
-        Sync *s = static_cast<Sync *>(e);
-
-        const QList<Change> &changes = s->data.changes;
-
-        bool cc = m_orig->count() != s->list->count();
-
-        QHash<int, QDeclarativeListModel *> targetModelDynamicHash;
-        QHash<int, ListModel *> targetModelStaticHash;
-
-        Q_ASSERT(m_orig->m_dynamicRoles == s->list->m_dynamicRoles);
-        if (m_orig->m_dynamicRoles)
-            QDeclarativeListModel::sync(s->list, m_orig, &targetModelDynamicHash);
-        else
-            ListModel::sync(s->list->m_listModel, m_orig->m_listModel, &targetModelStaticHash);
-
-        for (int ii = 0; ii < changes.count(); ++ii) {
-            const Change &change = changes.at(ii);
-
-            QDeclarativeListModel *model = 0;
-            if (m_orig->m_dynamicRoles) {
-                model = targetModelDynamicHash.value(change.modelUid);
-            } else {
-                ListModel *lm = targetModelStaticHash.value(change.modelUid);
-                if (lm)
-                    model = lm->m_modelCache;
-            }
+        if (m_orig) {
+            Sync *s = static_cast<Sync *>(e);
+            const QList<Change> &changes = s->data.changes;
+
+            cc = m_orig->count() != s->list->count();
+
+            QHash<int, QDeclarativeListModel *> targetModelDynamicHash;
+            QHash<int, ListModel *> targetModelStaticHash;
+
+            Q_ASSERT(m_orig->m_dynamicRoles == s->list->m_dynamicRoles);
+            if (m_orig->m_dynamicRoles)
+                QDeclarativeListModel::sync(s->list, m_orig, &targetModelDynamicHash);
+            else
+                ListModel::sync(s->list->m_listModel, m_orig->m_listModel, &targetModelStaticHash);
+
+            for (int ii = 0; ii < changes.count(); ++ii) {
+                const Change &change = changes.at(ii);
+
+                QDeclarativeListModel *model = 0;
+                if (m_orig->m_dynamicRoles) {
+                    model = targetModelDynamicHash.value(change.modelUid);
+                } else {
+                    ListModel *lm = targetModelStaticHash.value(change.modelUid);
+                    if (lm)
+                        model = lm->m_modelCache;
+                }
 
-            if (model) {
-                switch (change.type) {
-                case Change::Inserted:
-                    emit model->itemsInserted(change.index, change.count);
-                    break;
-                case Change::Removed:
-                    emit model->itemsRemoved(change.index, change.count);
-                    break;
-                case Change::Moved:
-                    emit model->itemsMoved(change.index, change.to, change.count);
-                    break;
-                case Change::Changed:
-                    emit model->itemsChanged(change.index, change.count, change.roles);
-                    break;
+                if (model) {
+                    switch (change.type) {
+                    case Change::Inserted:
+                        emit model->itemsInserted(change.index, change.count);
+                        break;
+                    case Change::Removed:
+                        emit model->itemsRemoved(change.index, change.count);
+                        break;
+                    case Change::Moved:
+                        emit model->itemsMoved(change.index, change.to, change.count);
+                        break;
+                    case Change::Changed:
+                        emit model->itemsChanged(change.index, change.count, change.roles);
+                        break;
+                    }
                 }
             }
         }
diff --git a/src/declarative/qml/qdeclarativelistmodelworkeragent_p.h b/src/declarative/qml/qdeclarativelistmodelworkeragent_p.h
index f84d598e2b..f2c971c1e1 100644
--- a/src/declarative/qml/qdeclarativelistmodelworkeragent_p.h
+++ b/src/declarative/qml/qdeclarativelistmodelworkeragent_p.h
@@ -108,6 +108,7 @@ public:
 
         QDeclarativeListModelWorkerAgent *a;
     };
+    void modelDestroyed();
 protected:
     virtual bool event(QEvent *);
 
diff --git a/src/declarative/qml/qdeclarativeworkerscript.cpp b/src/declarative/qml/qdeclarativeworkerscript.cpp
index b17c026e68..53283113b6 100644
--- a/src/declarative/qml/qdeclarativeworkerscript.cpp
+++ b/src/declarative/qml/qdeclarativeworkerscript.cpp
@@ -316,7 +316,6 @@ v8::Handle<v8::Object> QDeclarativeWorkerScriptEnginePrivate::getWorker(WorkerSc
 
 bool QDeclarativeWorkerScriptEnginePrivate::event(QEvent *event)
 {
-    // XXX must handle remove request
     if (event->type() == (QEvent::Type)WorkerDataEvent::WorkerData) {
         WorkerDataEvent *workerEvent = static_cast<WorkerDataEvent *>(event);
         processMessage(workerEvent->workerId(), workerEvent->data());
@@ -328,6 +327,10 @@ bool QDeclarativeWorkerScriptEnginePrivate::event(QEvent *event)
     } else if (event->type() == (QEvent::Type)WorkerDestroyEvent) {
         emit stopThread();
         return true;
+    } else if (event->type() == (QEvent::Type)WorkerRemoveEvent::WorkerRemove) {
+        WorkerRemoveEvent *workerEvent = static_cast<WorkerRemoveEvent *>(event);
+        workers.remove(workerEvent->workerId());
+        return true;
     } else {
         return QObject::event(event);
     }
@@ -513,7 +516,11 @@ int QDeclarativeWorkerScriptEngine::registerWorkerScript(QDeclarativeWorkerScrip
 
 void QDeclarativeWorkerScriptEngine::removeWorkerScript(int id)
 {
-    QCoreApplication::postEvent(d, new WorkerRemoveEvent(id));
+    QDeclarativeWorkerScriptEnginePrivate::WorkerScript* script = d->workers.value(id);
+    if (script) {
+        script->owner = 0;
+        QCoreApplication::postEvent(d, new WorkerRemoveEvent(id));
+    }
 }
 
 void QDeclarativeWorkerScriptEngine::executeUrl(int id, const QUrl &url)