From 50683f5882e7c983ac23aa4634f21a5869d69c9c Mon Sep 17 00:00:00 2001
From: Christian Stenger <christian.stenger@qt.io>
Date: Mon, 6 Mar 2017 15:02:35 +0100
Subject: [PATCH] AutoTest: Fix potential crash

It is a bad idea to remove child items while iterating over them.
Introduced a while ago, but forgotten to fix in f00a113e.

Change-Id: I8d335cec34c2e6a9e7dff99d10c68066ffa8933d
Reviewed-by: David Schulz <david.schulz@qt.io>
---
 src/plugins/autotest/testtreemodel.cpp | 25 +++++++++++++------------
 src/plugins/autotest/testtreemodel.h   |  1 +
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/plugins/autotest/testtreemodel.cpp b/src/plugins/autotest/testtreemodel.cpp
index 454cc6a37ca..d3cba6c76d1 100644
--- a/src/plugins/autotest/testtreemodel.cpp
+++ b/src/plugins/autotest/testtreemodel.cpp
@@ -67,14 +67,7 @@ TestTreeModel *TestTreeModel::instance()
 
 TestTreeModel::~TestTreeModel()
 {
-    const Utils::TreeItem *invisibleRoot = rootItem();
-    const int frameworkRootCount = invisibleRoot ? invisibleRoot->childCount() : 0;
-    for (int row = frameworkRootCount - 1; row >= 0; --row) {
-        Utils::TreeItem *item = invisibleRoot->childAt(row);
-        item->removeChildren();
-        takeItem(item); // do NOT delete the item as it's still a ptr hold by TestFrameworkManager
-    }
-
+    removeTestRootNodes();
     s_instance = nullptr;
 }
 
@@ -170,10 +163,7 @@ QList<TestConfiguration *> TestTreeModel::getSelectedTests() const
 void TestTreeModel::syncTestFrameworks()
 {
     // remove all currently registered
-    for (Utils::TreeItem *item : *rootItem()) {
-        item->removeChildren();
-        takeItem(item); // do NOT delete the item as it's still a ptr hold by TestFrameworkManager
-    }
+    removeTestRootNodes();
 
     TestFrameworkManager *frameworkManager = TestFrameworkManager::instance();
     QVector<Core::Id> sortedIds = frameworkManager->sortedActiveFrameworkIds();
@@ -285,6 +275,17 @@ void TestTreeModel::removeAllTestItems()
     emit testTreeModelChanged();
 }
 
+void TestTreeModel::removeTestRootNodes()
+{
+    const Utils::TreeItem *invisibleRoot = rootItem();
+    const int frameworkRootCount = invisibleRoot ? invisibleRoot->childCount() : 0;
+    for (int row = frameworkRootCount - 1; row >= 0; --row) {
+        Utils::TreeItem *item = invisibleRoot->childAt(row);
+        item->removeChildren();
+        takeItem(item); // do NOT delete the item as it's still a ptr held by TestFrameworkManager
+    }
+}
+
 #ifdef WITH_TESTS
 // we're inside tests - so use some internal knowledge to make testing easier
 TestTreeItem *qtRootNode()
diff --git a/src/plugins/autotest/testtreemodel.h b/src/plugins/autotest/testtreemodel.h
index 7f5080f3fef..57099204e37 100644
--- a/src/plugins/autotest/testtreemodel.h
+++ b/src/plugins/autotest/testtreemodel.h
@@ -83,6 +83,7 @@ private:
     void onParseResultReady(const TestParseResultPtr result);
     void handleParseResult(const TestParseResult *result, TestTreeItem *rootNode);
     void removeAllTestItems();
+    void removeTestRootNodes();
     void removeFiles(const QStringList &files);
     bool sweepChildren(TestTreeItem *item);