Improve safety of VcsCommand

Amends 6e85ff9f4b

Using "invokeMethod(m_outputWindow, ..." has the disadvantage that it
crashes if m_outputWindow was deleted. This is nicely handled when
connecting to signals, so switch back to signals.

Change-Id: I6a681ac48a86536fa8e69e42d3c61ffa9d30c3d5
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Orgad Shaneh <orgads@gmail.com>

src/libs/utils/shellcommand.cpp

@@ -318,7 +318,7 @@ void ShellCommand::runCommand(SynchronousProcess &proc,
     }
 
     if (!(d->m_flags & SuppressCommandLogging))
-        appendCommand(dir, command);
+        emit appendCommand(dir, command);
 
     proc.setCommand(command);
     if ((d->m_flags & FullySynchronously)
@@ -333,9 +333,9 @@ void ShellCommand::runCommand(SynchronousProcess &proc,
         // Success/Fail message in appropriate window?
         if (proc.result() == QtcProcess::FinishedWithSuccess) {
             if (d->m_flags & ShowSuccessMessage)
-                appendMessage(proc.exitMessage());
+                emit appendMessage(proc.exitMessage());
         } else if (!(d->m_flags & SuppressFailMessage)) {
-            appendError(proc.exitMessage());
+            emit appendError(proc.exitMessage());
         }
     }
 }
@@ -360,14 +360,14 @@ void ShellCommand::runFullySynchronous(SynchronousProcess &process,
     if (!d->m_aborted) {
         const QString stdErr = process.stdErr();
         if (!stdErr.isEmpty() && !(d->m_flags & SuppressStdErr))
-            append(stdErr);
+            emit append(stdErr);
 
         const QString stdOut = process.stdOut();
         if (!stdOut.isEmpty() && d->m_flags & ShowStdOut) {
             if (d->m_flags & SilentOutput)
-                appendSilently(stdOut);
+                emit appendSilently(stdOut);
             else
-                append(stdOut);
+                emit append(stdOut);
         }
     }
 }

src/libs/utils/shellcommand.h

@@ -138,21 +138,17 @@ signals:
 
     void terminate(); // Internal
 
+    void append(const QString &text);
+    void appendSilently(const QString &text);
+    void appendError(const QString &text);
+    void appendCommand(const QString &workingDirectory, const Utils::CommandLine &command);
+    void appendMessage(const QString &text);
+
 protected:
     virtual void addTask(QFuture<void> &future);
     int timeoutS() const;
     QString workDirectory(const QString &wd) const;
 
-    // Below methods are called directly from other threads
-    virtual void append(const QString &text) { Q_UNUSED(text) }
-    virtual void appendSilently(const QString &text) { Q_UNUSED(text) }
-    virtual void appendError(const QString &text) { Q_UNUSED(text) }
-    virtual void appendCommand(const QString &workingDirectory, const Utils::CommandLine &command) {
-        Q_UNUSED(workingDirectory)
-        Q_UNUSED(command)
-    }
-    virtual void appendMessage(const QString &text) { Q_UNUSED(text) }
-
 private:
     void run(QFutureInterface<void> &future);
 

src/plugins/vcsbase/vcscommand.cpp

@@ -42,7 +42,6 @@ VcsCommand::VcsCommand(const QString &workingDirectory, const Environment &envir
 {
     VcsOutputWindow::setRepository(workingDirectory);
     setDisableUnixTerminal();
-    m_outputWindow = VcsOutputWindow::instance();
     m_sshPrompt = VcsBase::sshPrompt();
 
     connect(this, &VcsCommand::started, this, [this] {
@@ -53,6 +52,15 @@ VcsCommand::VcsCommand(const QString &workingDirectory, const Environment &envir
         if (flags() & ExpectRepoChanges)
             Utils::GlobalFileChangeBlocker::instance()->forceBlocked(false);
     });
+
+    VcsOutputWindow *outputWindow = VcsOutputWindow::instance();
+    connect(this, &ShellCommand::append, outputWindow, [outputWindow](const QString &t) {
+        outputWindow->append(t);
+    });
+    connect(this, &ShellCommand::appendSilently, outputWindow, &VcsOutputWindow::appendSilently);
+    connect(this, &ShellCommand::appendError, outputWindow, &VcsOutputWindow::appendError);
+    connect(this, &ShellCommand::appendCommand, outputWindow, &VcsOutputWindow::appendCommand);
+    connect(this, &ShellCommand::appendMessage, outputWindow, &VcsOutputWindow::appendMessage);
 }
 
 const Environment VcsCommand::processEnvironment() const
@@ -70,41 +78,6 @@ void VcsCommand::runCommand(SynchronousProcess &proc,
     emitRepositoryChanged(workingDirectory);
 }
 
-void VcsCommand::append(const QString &text)
-{
-    QMetaObject::invokeMethod(m_outputWindow, [this, text] {
-        m_outputWindow->append(text);
-    });
-}
-
-void VcsCommand::appendSilently(const QString &text)
-{
-    QMetaObject::invokeMethod(m_outputWindow, [this, text] {
-        m_outputWindow->appendSilently(text);
-    });
-}
-
-void VcsCommand::appendError(const QString &text)
-{
-    QMetaObject::invokeMethod(m_outputWindow, [this, text] {
-        m_outputWindow->appendError(text);
-    });
-}
-
-void VcsCommand::appendCommand(const QString &workingDirectory, const Utils::CommandLine &command)
-{
-    QMetaObject::invokeMethod(m_outputWindow, [this, workingDirectory, command] {
-        m_outputWindow->appendCommand(workingDirectory, command);
-    });
-}
-
-void VcsCommand::appendMessage(const QString &text)
-{
-    QMetaObject::invokeMethod(m_outputWindow, [this, text] {
-        m_outputWindow->appendMessage(text);
-    });
-}
-
 void VcsCommand::emitRepositoryChanged(const QString &workingDirectory)
 {
     if (m_preventRepositoryChanged || !(flags() & VcsCommand::ExpectRepoChanges))

src/plugins/vcsbase/vcscommand.h

@@ -51,13 +51,6 @@ public:
                     const Utils::CommandLine &command,
                     const QString &workDirectory = {}) override;
 
-protected:
-    void append(const QString &text) override;
-    void appendSilently(const QString &text) override;
-    void appendError(const QString &text) override;
-    void appendCommand(const QString &workingDirectory, const Utils::CommandLine &command) override;
-    void appendMessage(const QString &text) override;
-
 private:
     void emitRepositoryChanged(const QString &workingDirectory);
 
@@ -65,7 +58,6 @@ private:
 
     QString m_sshPrompt;
     bool m_preventRepositoryChanged;
-    VcsOutputWindow *m_outputWindow = nullptr;
 };
 
 } // namespace VcsBase