CustomExecutableRunConfiguration: Remove nested event loop

Nested event loops are dangerous and in this case lead to a crash.
So change the api of ensureConfigured to not use a nested event loop.
The CustomExecutableRunConfiguration then has to manage the dialog more
explicitly. Also this makes the dialog non modal, since it should then
cope with every situation.

Task-number: QTCREATORBUG-11416
Change-Id: I2af782915c148f8dff1b0df54fdb64aa83f684d2
Reviewed-by: Tobias Hunger <tobias.hunger@digia.com>

8 files changed, 86 insertions, 38 deletions

diff --git a/src/plugins/projectexplorer/projectexplorer.cpp b/src/plugins/projectexplorer/projectexplorer.cpp
index e57a9643f4..2c4d8eb102 100644
--- a/src/plugins/projectexplorer/projectexplorer.cpp
+++ b/src/plugins/projectexplorer/projectexplorer.cpp
@@ -248,6 +248,7 @@ struct ProjectExplorerPluginPrivate {
 
     QString m_lastOpenDirectory;
     QPointer<RunConfiguration> m_delayedRunConfiguration;
+    QList<QPair<RunConfiguration *, ProjectExplorer::RunMode>> m_delayedRunConfigurationForRun;
     bool m_shouldHaveRunConfiguration;
     RunMode m_runMode;
     QString m_projectFilterString;
@@ -1753,14 +1754,25 @@ void ProjectExplorerPlugin::buildStateChanged(Project * pro)
 
 void ProjectExplorerPlugin::executeRunConfiguration(RunConfiguration *runConfiguration, RunMode runMode)
 {
-    QString errorMessage;
-    if (!runConfiguration->ensureConfigured(&errorMessage)) {
-        showRunErrorMessage(errorMessage);
-        return;
+    if (!runConfiguration->isConfigured()) {
+        QString errorMessage;
+        RunConfiguration::ConfigurationState state = runConfiguration->ensureConfigured(&errorMessage);
+
+        if (state == RunConfiguration::UnConfigured) {
+            showRunErrorMessage(errorMessage);
+            return;
+        } else if (state == RunConfiguration::Waiting) {
+            connect(runConfiguration, SIGNAL(configurationFinished()),
+                    this, SLOT(runConfigurationConfigurationFinished()));
+            d->m_delayedRunConfigurationForRun.append(qMakePair(runConfiguration, runMode));
+            return;
+        }
     }
+
     if (IRunControlFactory *runControlFactory = findRunControlFactory(runConfiguration, runMode)) {
         emit aboutToExecuteProject(runConfiguration->target()->project(), runMode);
 
+        QString errorMessage;
         RunControl *control = runControlFactory->create(runConfiguration, runMode, &errorMessage);
         if (!control) {
             showRunErrorMessage(errorMessage);
@@ -1877,6 +1889,22 @@ void ProjectExplorerPlugin::updateContext()
     ICore::updateAdditionalContexts(oldContext, newContext);
 }
 
+void ProjectExplorerPlugin::runConfigurationConfigurationFinished()
+{
+    RunConfiguration *rc = qobject_cast<RunConfiguration *>(sender());
+    ProjectExplorer::RunMode runMode = ProjectExplorer::NoRunMode;
+    for (int i = 0; i < d->m_delayedRunConfigurationForRun.size(); ++i) {
+        if (d->m_delayedRunConfigurationForRun.at(i).first == rc) {
+            runMode = d->m_delayedRunConfigurationForRun.at(i).second;
+            d->m_delayedRunConfigurationForRun.removeAt(i);
+            break;
+        }
+    }
+    if (runMode != ProjectExplorer::NoRunMode
+            && rc->isConfigured())
+        executeRunConfiguration(rc, runMode);
+}
+
 void ProjectExplorerPlugin::setCurrent(Project *project, QString filePath, Node *node)
 {
     if (debug)
diff --git a/src/plugins/projectexplorer/projectexplorer.h b/src/plugins/projectexplorer/projectexplorer.h
index 731a1723eb..b655f2219e 100644
--- a/src/plugins/projectexplorer/projectexplorer.h
+++ b/src/plugins/projectexplorer/projectexplorer.h
@@ -232,6 +232,7 @@ private slots:
     void updateExternalFileWarning();
 
     void updateContext();
+    void runConfigurationConfigurationFinished();
 
 #ifdef WITH_TESTS
     void testAnsiFilterOutputParser_data();
diff --git a/src/plugins/projectexplorer/runconfiguration.cpp b/src/plugins/projectexplorer/runconfiguration.cpp
index 4c848bd9f1..a7b84d3090 100644
--- a/src/plugins/projectexplorer/runconfiguration.cpp
+++ b/src/plugins/projectexplorer/runconfiguration.cpp
@@ -273,13 +273,13 @@ bool RunConfiguration::isConfigured() const
     return true;
 }
 
-bool RunConfiguration::ensureConfigured(QString *errorMessage)
+RunConfiguration::ConfigurationState RunConfiguration::ensureConfigured(QString *errorMessage)
 {
     if (isConfigured())
-        return true;
+        return Configured;
     if (errorMessage)
         *errorMessage = tr("Unknown error.");
-    return false;
+    return UnConfigured;
 }
 
 
diff --git a/src/plugins/projectexplorer/runconfiguration.h b/src/plugins/projectexplorer/runconfiguration.h
index eb74613f6b..f8f9e990ce 100644
--- a/src/plugins/projectexplorer/runconfiguration.h
+++ b/src/plugins/projectexplorer/runconfiguration.h
@@ -163,9 +163,12 @@ public:
     virtual bool isEnabled() const;
     virtual QString disabledReason() const;
     virtual QWidget *createConfigurationWidget() = 0;
+
     virtual bool isConfigured() const;
     // Pop up configuration dialog in case for example the executable is missing.
-    virtual bool ensureConfigured(QString *errorMessage = 0);
+    enum ConfigurationState { Configured, UnConfigured, Waiting };
+    // TODO rename function
+    virtual ConfigurationState ensureConfigured(QString *errorMessage = 0);
 
     Target *target() const;
 
@@ -194,6 +197,7 @@ public:
 signals:
     void enabledChanged();
     void requestRunActionsUpdate();
+    void configurationFinished();
 
 protected:
     RunConfiguration(Target *parent, Core::Id id);
diff --git a/src/plugins/qtsupport/customexecutablerunconfiguration.cpp b/src/plugins/qtsupport/customexecutablerunconfiguration.cpp
index da6652ad69..a5c1b6d3df 100644
--- a/src/plugins/qtsupport/customexecutablerunconfiguration.cpp
+++ b/src/plugins/qtsupport/customexecutablerunconfiguration.cpp
@@ -71,7 +71,8 @@ void CustomExecutableRunConfiguration::ctor()
 CustomExecutableRunConfiguration::CustomExecutableRunConfiguration(Target *parent) :
     LocalApplicationRunConfiguration(parent, Core::Id(CUSTOM_EXECUTABLE_ID)),
     m_workingDirectory(QLatin1String(Constants::DEFAULT_WORKING_DIR)),
-    m_runMode(ProjectExplorer::ApplicationLauncher::Gui)
+    m_runMode(ProjectExplorer::ApplicationLauncher::Gui),
+    m_dialog(0)
 {
     addExtraAspect(new LocalEnvironmentAspect(this));
 
@@ -86,7 +87,8 @@ CustomExecutableRunConfiguration::CustomExecutableRunConfiguration(Target *paren
     m_executable(source->m_executable),
     m_workingDirectory(source->m_workingDirectory),
     m_cmdArguments(source->m_cmdArguments),
-    m_runMode(source->m_runMode)
+    m_runMode(source->m_runMode),
+    m_dialog(0)
 {
     ctor();
 }
@@ -94,6 +96,12 @@ CustomExecutableRunConfiguration::CustomExecutableRunConfiguration(Target *paren
 // Note: Qt4Project deletes all empty customexecrunconfigs for which isConfigured() == false.
 CustomExecutableRunConfiguration::~CustomExecutableRunConfiguration()
 {
+    if (m_dialog) {
+        emit configurationFinished();
+        disconnect(m_dialog, SIGNAL(finished(int)),
+                   this, SLOT(configurationDialogFinished()));
+        delete m_dialog;
+    }
 }
 
 // Dialog embedding the CustomExecutableConfigurationWidget
@@ -150,30 +158,33 @@ void CustomExecutableDialog::accept()
     QDialog::accept();
 }
 
-bool CustomExecutableRunConfiguration::ensureConfigured(QString *errorMessage)
-{
-    if (isConfigured())
-        return validateExecutable(0, errorMessage);
-    CustomExecutableDialog dialog(this, Core::ICore::mainWindow());
-    dialog.setWindowTitle(displayName());
-    const QString oldExecutable = m_executable;
-    const QString oldWorkingDirectory = m_workingDirectory;
-    const QString oldCmdArguments = m_cmdArguments;
-    if (dialog.exec() == QDialog::Accepted)
-        return validateExecutable(0, errorMessage);
-    // User canceled: Hack: Silence the error dialog.
-    if (errorMessage)
-        *errorMessage = QLatin1String("");
-    // Restore values changed by the configuration widget.
-    if (m_executable != oldExecutable
-        || m_workingDirectory != oldWorkingDirectory
-        || m_cmdArguments != oldCmdArguments) {
-        m_executable = oldExecutable;
-        m_workingDirectory = oldWorkingDirectory;
-        m_cmdArguments = oldCmdArguments;
-        emit changed();
+// CustomExecutableRunConfiguration
+
+RunConfiguration::ConfigurationState CustomExecutableRunConfiguration::ensureConfigured(QString *errorMessage)
+{
+    Q_UNUSED(errorMessage)
+    if (m_dialog) {// uhm already shown
+        *errorMessage = QLatin1String(""); // no error dialog
+        m_dialog->activateWindow();
+        m_dialog->raise();
+        return UnConfigured;
     }
-    return false;
+
+    m_dialog = new CustomExecutableDialog(this, Core::ICore::mainWindow());
+    connect(m_dialog, SIGNAL(finished(int)),
+            this, SLOT(configurationDialogFinished()));
+    m_dialog->setWindowTitle(displayName()); // pretty pointless
+    m_dialog->show();
+    return Waiting;
+}
+
+void CustomExecutableRunConfiguration::configurationDialogFinished()
+{
+    disconnect(m_dialog, SIGNAL(finished(int)),
+            this, SLOT(configurationDialogFinished()));
+    m_dialog->deleteLater();
+    m_dialog = 0;
+    emit configurationFinished();
 }
 
 // Search the executable in the path.
diff --git a/src/plugins/qtsupport/customexecutablerunconfiguration.h b/src/plugins/qtsupport/customexecutablerunconfiguration.h
index 38c5f2efeb..314291f1eb 100644
--- a/src/plugins/qtsupport/customexecutablerunconfiguration.h
+++ b/src/plugins/qtsupport/customexecutablerunconfiguration.h
@@ -74,7 +74,7 @@ public:
 
     QVariantMap toMap() const;
 
-    bool ensureConfigured(QString *errorMessage);
+    ConfigurationState ensureConfigured(QString *errorMessage);
 
 signals:
     void changed();
@@ -85,6 +85,8 @@ protected:
     virtual bool fromMap(const QVariantMap &map);
     QString defaultDisplayName() const;
 
+private slots:
+    void configurationDialogFinished();
 private:
     void ctor();
 
@@ -102,6 +104,7 @@ private:
     QString m_workingDirectory;
     QString m_cmdArguments;
     ProjectExplorer::ApplicationLauncher::Mode m_runMode;
+    QWidget *m_dialog;
 };
 
 class CustomExecutableRunConfigurationFactory : public ProjectExplorer::IRunConfigurationFactory
diff --git a/src/plugins/remotelinux/remotelinuxcustomrunconfiguration.cpp b/src/plugins/remotelinux/remotelinuxcustomrunconfiguration.cpp
index e046be1986..2d49f4f36b 100644
--- a/src/plugins/remotelinux/remotelinuxcustomrunconfiguration.cpp
+++ b/src/plugins/remotelinux/remotelinuxcustomrunconfiguration.cpp
@@ -119,16 +119,17 @@ bool RemoteLinuxCustomRunConfiguration::isConfigured() const
     return !m_remoteExecutable.isEmpty();
 }
 
-bool RemoteLinuxCustomRunConfiguration::ensureConfigured(QString *errorMessage)
+ProjectExplorer::RunConfiguration::ConfigurationState
+RemoteLinuxCustomRunConfiguration::ensureConfigured(QString *errorMessage)
 {
     if (!isConfigured()) {
         if (errorMessage) {
             *errorMessage = tr("The remote executable must be set "
                                "in order to run a custom remote run configuration.");
         }
-        return false;
+        return UnConfigured;
     }
-    return true;
+    return Configured;
 }
 
 QWidget *RemoteLinuxCustomRunConfiguration::createConfigurationWidget()
diff --git a/src/plugins/remotelinux/remotelinuxcustomrunconfiguration.h b/src/plugins/remotelinux/remotelinuxcustomrunconfiguration.h
index 97867ad871..87c18b0026 100644
--- a/src/plugins/remotelinux/remotelinuxcustomrunconfiguration.h
+++ b/src/plugins/remotelinux/remotelinuxcustomrunconfiguration.h
@@ -48,7 +48,7 @@ public:
 
     bool isEnabled() const { return true; }
     bool isConfigured() const;
-    bool ensureConfigured(QString *errorMessage);
+    ConfigurationState ensureConfigured(QString *errorMessage);
     QWidget *createConfigurationWidget();
     Utils::OutputFormatter *createOutputFormatter() const;