Wizards: Do not crash when adding files to existing project

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>

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))
+    // Use static cast from void * to avoid qobject_cast (which needs a valid object) in value()
-            .value<Node *>();
+    // in the following code:
-    initializeProjectTree(contextNode, files, kind,
+    auto contextNode = findWizardContextNode(static_cast<Node *>(m_wizard->value(Constants::PREFERRED_PROJECT_NODE).value<void *>()));
-                          isProject ? AddSubProject : AddNewFile);
+    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();

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);
 

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()));
+    // store void pointer to avoid QVariant to use qobject_cast, which might core-dump when trying
-    if (ProjectTree::currentProject()) {
+    // to access meta data on an object that get deleted in the meantime:
-        QList<Id> profileIds = Utils::transform(ProjectTree::currentProject()->targets(), &Target::id);
+    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);

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:

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)

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);
 

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;

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();