Wizards: Do not crash when adding files to existing projectv4.4.0

Do not crash when adding a file to an existing project that finishes
parsing while the wizard is still open.

Make sure the Node that is passed into the wizard is still valid after
parsing. Provide more information to the wizard so that this can be
checked -- and to find the similar node in the new project tree.

Also avoid a crash when adding existing files.

Do not crash when project parsing finishes while the wizard
summary page is open.

Do not crash when the project gets closed while the summary page
is open.

Do not have the ProjectTree send signals about subTreeChanges if
the node that changed is not part of the ProjectTree. This avoids
an infinite loop when updating the combobox on the summary page.

Add a treeChanged signal to ProjectTree.

Task-number: QTCREATORBUG-18651
Change-Id: Iaed3d0d1f079c09e54389821a11bda596139f35c
Reviewed-by: Eike Ziller <eike.ziller@qt.io>

8 files changed, 99 insertions, 13 deletions

diff --git a/src/plugins/projectexplorer/jsonwizard/jsonsummarypage.cpp b/src/plugins/projectexplorer/jsonwizard/jsonsummarypage.cpp
index 5f02bc7779..27b4b682ab 100644
--- a/src/plugins/projectexplorer/jsonwizard/jsonsummarypage.cpp
+++ b/src/plugins/projectexplorer/jsonwizard/jsonsummarypage.cpp
@@ -31,6 +31,8 @@
 #include "../projectnodes.h"
 #include "../session.h"
 
+#include "../projecttree.h"
+
 #include <coreplugin/coreconstants.h>
 #include <coreplugin/iversioncontrol.h>
 
@@ -125,10 +127,19 @@ void JsonSummaryPage::initializePage()
                                  });
     }
 
-    Node *contextNode = m_wizard->value(QLatin1String(Constants::PREFERRED_PROJECT_NODE))
-            .value<Node *>();
-    initializeProjectTree(contextNode, files, kind,
-                          isProject ? AddSubProject : AddNewFile);
+    // Use static cast from void * to avoid qobject_cast (which needs a valid object) in value()
+    // in the following code:
+    auto contextNode = findWizardContextNode(static_cast<Node *>(m_wizard->value(Constants::PREFERRED_PROJECT_NODE).value<void *>()));
+    const ProjectAction currentAction = isProject ? AddSubProject : AddNewFile;
+
+    initializeProjectTree(contextNode, files, kind, currentAction);
+
+    // Refresh combobox on project tree changes:
+    connect(ProjectTree::instance(), &ProjectTree::treeChanged,
+            this, [this, files, kind, currentAction]() {
+        initializeProjectTree(findWizardContextNode(currentNode()), files, kind, currentAction);
+    });
+
 
     bool hideProjectUi = JsonWizard::boolFromVariant(m_hideProjectUiValue, m_wizard->expander());
     setProjectUiVisible(!hideProjectUi);
@@ -207,6 +218,23 @@ void JsonSummaryPage::summarySettingsHaveChanged()
     updateProjectData(currentNode());
 }
 
+Node *JsonSummaryPage::findWizardContextNode(Node *contextNode) const
+{
+    if (contextNode && !ProjectTree::hasNode(contextNode)) {
+        contextNode = nullptr;
+
+        // Static cast from void * to avoid qobject_cast (which needs a valid object) in value().
+        auto project = static_cast<Project *>(m_wizard->value(Constants::PROJECT_POINTER).value<void *>());
+        if (SessionManager::projects().contains(project) && project->rootProjectNode()) {
+            const QString path = m_wizard->value(Constants::PREFERRED_PROJECT_NODE_PATH).toString();
+            contextNode = project->rootProjectNode()->findNode([path](const Node *n) {
+                return path == n->filePath().toString();
+            });
+        }
+    }
+    return contextNode;
+}
+
 void JsonSummaryPage::updateFileList()
 {
     m_fileList = m_wizard->generateFileList();
diff --git a/src/plugins/projectexplorer/jsonwizard/jsonsummarypage.h b/src/plugins/projectexplorer/jsonwizard/jsonsummarypage.h
index 1e0bd1800f..2fa134a38f 100644
--- a/src/plugins/projectexplorer/jsonwizard/jsonsummarypage.h
+++ b/src/plugins/projectexplorer/jsonwizard/jsonsummarypage.h
@@ -33,6 +33,7 @@
 namespace ProjectExplorer {
 
 class FolderNode;
+class Node;
 
 // Documentation inside.
 class JsonSummaryPage : public Internal::ProjectWizardPage
@@ -52,6 +53,7 @@ public:
     void summarySettingsHaveChanged();
 
 private:
+    Node *findWizardContextNode(Node *contextNode) const;
     void updateFileList();
     void updateProjectData(FolderNode *node);
 
diff --git a/src/plugins/projectexplorer/projectexplorer.cpp b/src/plugins/projectexplorer/projectexplorer.cpp
index f47f2e0510..acd546e09b 100644
--- a/src/plugins/projectexplorer/projectexplorer.cpp
+++ b/src/plugins/projectexplorer/projectexplorer.cpp
@@ -3046,13 +3046,18 @@ void ProjectExplorerPluginPrivate::updateContextMenuActions()
 void ProjectExplorerPluginPrivate::addNewFile()
 {
     QTC_ASSERT(ProjectTree::currentNode(), return);
-    QString location = directoryFor(ProjectTree::currentNode());
+    Node *currentNode = ProjectTree::currentNode();
+    QString location = directoryFor(currentNode);
 
     QVariantMap map;
-    map.insert(QLatin1String(Constants::PREFERRED_PROJECT_NODE), QVariant::fromValue(ProjectTree::currentNode()));
-    if (ProjectTree::currentProject()) {
-        QList<Id> profileIds = Utils::transform(ProjectTree::currentProject()->targets(), &Target::id);
+    // store void pointer to avoid QVariant to use qobject_cast, which might core-dump when trying
+    // to access meta data on an object that get deleted in the meantime:
+    map.insert(QLatin1String(Constants::PREFERRED_PROJECT_NODE), QVariant::fromValue(static_cast<void *>(currentNode)));
+    map.insert(Constants::PREFERRED_PROJECT_NODE_PATH, currentNode->filePath().toString());
+    if (Project *p = ProjectTree::currentProject()) {
+        QList<Id> profileIds = Utils::transform(p->targets(), &Target::id);
         map.insert(QLatin1String(Constants::PROJECT_KIT_IDS), QVariant::fromValue(profileIds));
+        map.insert(Constants::PROJECT_POINTER, QVariant::fromValue(static_cast<void *>(p)));
     }
     ICore::showNewItemDialog(tr("New File", "Title of dialog"),
                              Utils::filtered(IWizardFactory::allWizardFactories(),
@@ -3121,7 +3126,8 @@ void ProjectExplorerPluginPrivate::addExistingDirectory()
 
 void ProjectExplorerPlugin::addExistingFiles(FolderNode *folderNode, const QStringList &filePaths)
 {
-    if (!folderNode) // can happen when project is not yet parsed
+    // can happen when project is not yet parsed or finished parsing while the dialog was open:
+    if (!folderNode || !ProjectTree::hasNode(folderNode))
         return;
 
     const QString dir = directoryFor(folderNode);
diff --git a/src/plugins/projectexplorer/projectexplorerconstants.h b/src/plugins/projectexplorer/projectexplorerconstants.h
index 559b11fb6a..13dfe2431a 100644
--- a/src/plugins/projectexplorer/projectexplorerconstants.h
+++ b/src/plugins/projectexplorer/projectexplorerconstants.h
@@ -126,6 +126,8 @@ const char IMPORT_WIZARD_CATEGORY_DISPLAY[] = QT_TRANSLATE_NOOP("ProjectExplorer
 
 // Wizard extra values
 const char PREFERRED_PROJECT_NODE[] = "ProjectExplorer.PreferredProjectNode";
+const char PREFERRED_PROJECT_NODE_PATH[] = "ProjectExplorer.PreferredProjectPath";
+const char PROJECT_POINTER[] = "ProjectExplorer.Project";
 const char PROJECT_KIT_IDS[] = "ProjectExplorer.Profile.Ids";
 
 // Build step lists ids:
diff --git a/src/plugins/projectexplorer/projectfilewizardextension.cpp b/src/plugins/projectexplorer/projectfilewizardextension.cpp
index c7bf50e246..e6bf826e7b 100644
--- a/src/plugins/projectexplorer/projectfilewizardextension.cpp
+++ b/src/plugins/projectexplorer/projectfilewizardextension.cpp
@@ -41,6 +41,7 @@
 #include <texteditor/tabsettings.h>
 #include <texteditor/storagesettings.h>
 #include <projectexplorer/project.h>
+#include <projectexplorer/projecttree.h>
 #include <projectexplorer/editorconfiguration.h>
 #include <utils/mimetypes/mimedatabase.h>
 #
@@ -120,7 +121,8 @@ void ProjectFileWizardExtension::firstExtensionPageShown(
 
     QStringList filePaths;
     ProjectAction projectAction;
-    if (m_context->wizard->kind()== IWizardFactory::ProjectWizard) {
+    const IWizardFactory::WizardKind kind = m_context->wizard->kind();
+    if (kind == IWizardFactory::ProjectWizard) {
         projectAction = AddSubProject;
         filePaths << generatedProjectFilePath(files);
     } else {
@@ -128,13 +130,38 @@ void ProjectFileWizardExtension::firstExtensionPageShown(
         filePaths = Utils::transform(files, &GeneratedFile::path);
     }
 
-    Node *contextNode = extraValues.value(QLatin1String(Constants::PREFERRED_PROJECT_NODE)).value<Node *>();
+    // Static cast from void * to avoid qobject_cast (which needs a valid object) in value().
+    auto contextNode = static_cast<Node *>(extraValues.value(QLatin1String(Constants::PREFERRED_PROJECT_NODE)).value<void *>());
+    auto project = static_cast<Project *>(extraValues.value(Constants::PROJECT_POINTER).value<void *>());
+    const QString path = extraValues.value(Constants::PREFERRED_PROJECT_NODE_PATH).toString();
 
-    m_context->page->initializeProjectTree(contextNode, filePaths, m_context->wizard->kind(),
+    m_context->page->initializeProjectTree(findWizardContextNode(contextNode, project, path),
+                                           filePaths, m_context->wizard->kind(),
                                            projectAction);
+    // Refresh combobox on project tree changes:
+    connect(ProjectTree::instance(), &ProjectTree::treeChanged,
+            m_context->page, [this, project, path, filePaths, kind, projectAction]() {
+        m_context->page->initializeProjectTree(
+                    findWizardContextNode(m_context->page->currentNode(), project, path), filePaths,
+                    kind, projectAction);
+    });
+
     m_context->page->initializeVersionControls();
 }
 
+Node *ProjectFileWizardExtension::findWizardContextNode(Node *contextNode, Project *project,
+                                                        const QString &path)
+{
+    if (contextNode && !ProjectTree::hasNode(contextNode)) {
+        if (SessionManager::projects().contains(project) && project->rootProjectNode()) {
+            contextNode = project->rootProjectNode()->findNode([path](const Node *n) {
+                return path == n->filePath().toString();
+            });
+        }
+    }
+    return contextNode;
+}
+
 QList<QWizardPage *> ProjectFileWizardExtension::extensionPages(const IWizardFactory *wizard)
 {
     if (!m_context)
diff --git a/src/plugins/projectexplorer/projectfilewizardextension.h b/src/plugins/projectexplorer/projectfilewizardextension.h
index fe9d3f8d92..df1c61a650 100644
--- a/src/plugins/projectexplorer/projectfilewizardextension.h
+++ b/src/plugins/projectexplorer/projectfilewizardextension.h
@@ -31,6 +31,8 @@
 
 namespace ProjectExplorer {
 class FolderNode;
+class Node;
+class Project;
 
 namespace Internal {
 
@@ -52,6 +54,7 @@ public slots:
     void firstExtensionPageShown(const QList<Core::GeneratedFile> &files, const QVariantMap &extraValues) override;
 
 private:
+    Node *findWizardContextNode(Node *contextNode, Project *project, const QString &path);
     bool processProject(const QList<Core::GeneratedFile> &files,
                         bool *removeOpenProjectAttribute, QString *errorMessage);
 
diff --git a/src/plugins/projectexplorer/projecttree.cpp b/src/plugins/projectexplorer/projecttree.cpp
index 50b8b65256..fcaf5d8178 100644
--- a/src/plugins/projectexplorer/projecttree.cpp
+++ b/src/plugins/projectexplorer/projecttree.cpp
@@ -72,10 +72,15 @@ ProjectTree::ProjectTree(QObject *parent) : QObject(parent)
 
     connect(SessionManager::instance(), &SessionManager::projectAdded,
             this, &ProjectTree::sessionChanged);
+    connect(SessionManager::instance(), &SessionManager::projectAdded,
+            this, &ProjectTree::treeChanged);
     connect(SessionManager::instance(), &SessionManager::projectRemoved,
             this, &ProjectTree::sessionChanged);
+    connect(SessionManager::instance(), &SessionManager::projectRemoved,
+            this, &ProjectTree::treeChanged);
     connect(SessionManager::instance(), &SessionManager::startupProjectChanged,
             this, &ProjectTree::sessionChanged);
+    connect(this, &ProjectTree::subtreeChanged, this, &ProjectTree::treeChanged);
 }
 
 ProjectTree::~ProjectTree()
@@ -261,7 +266,8 @@ void ProjectTree::updateContext()
 
 void ProjectTree::emitSubtreeChanged(FolderNode *node)
 {
-    emit s_instance->subtreeChanged(node);
+    if (hasNode(node))
+        emit s_instance->subtreeChanged(node);
 }
 
 void ProjectTree::collapseAll()
@@ -377,6 +383,13 @@ void ProjectTree::applyTreeManager(FolderNode *folder)
         f(folder);
 }
 
+bool ProjectTree::hasNode(const Node *node)
+{
+    return Utils::contains(SessionManager::projects(), [node](const Project *p) {
+        return p && p->rootProjectNode() && p->rootProjectNode()->findNode([node](const Node *n) { return n == node; });
+    });
+}
+
 void ProjectTree::hideContextMenu()
 {
     m_focusForContextMenu = nullptr;
diff --git a/src/plugins/projectexplorer/projecttree.h b/src/plugins/projectexplorer/projecttree.h
index eb84adf58b..7f61a2ee78 100644
--- a/src/plugins/projectexplorer/projecttree.h
+++ b/src/plugins/projectexplorer/projecttree.h
@@ -68,6 +68,8 @@ public:
     static void registerTreeManager(const TreeManagerFunction &treeChange);
     static void applyTreeManager(FolderNode *folder);
 
+    static bool hasNode(const Node *node);
+
     void collapseAll();
 
     // for nodes to emit signals, do not call unless you are a node
@@ -83,6 +85,9 @@ signals:
     void aboutToShowContextMenu(ProjectExplorer::Project *project,
                                 ProjectExplorer::Node *node);
 
+    // Emitted on any change to the tree
+    void treeChanged();
+
 private:
     void sessionChanged();
     void focusChanged();