AbstractProcessStep: Clean up code

Change-Id: Iae3d1e7cbd02d34da5cebbf0291948dbdd2e95df
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/plugins/android/androidbuildapkstep.cpp

@@ -273,11 +273,6 @@ void AndroidBuildApkStep::setUseGradle(bool b)
     }
 }
 
-bool AndroidBuildApkStep::runInGuiThread() const
-{
-    return true;
-}
-
 bool AndroidBuildApkStep::verboseOutput() const
 {
     return m_verbose;

src/plugins/android/androidbuildapkstep.h

@@ -73,8 +73,6 @@ public:
     bool useGradle() const;
     void setUseGradle(bool b);
 
-    bool runInGuiThread() const override;
-
     QString buildTargetSdk() const;
     void setBuildTargetSdk(const QString &sdk);
 

src/plugins/projectexplorer/abstractprocessstep.cpp

@@ -26,12 +26,12 @@
 #include "abstractprocessstep.h"
 #include "ansifilterparser.h"
 #include "buildstep.h"
-#include "ioutputparser.h"
 #include "project.h"
 #include "task.h"
 
+#include <coreplugin/reaper.h>
+
 #include <utils/qtcassert.h>
-#include <utils/qtcprocess.h>
 
 #include <QTimer>
 #include <QDir>
@@ -84,21 +84,16 @@ using namespace ProjectExplorer;
 
 AbstractProcessStep::AbstractProcessStep(BuildStepList *bsl, Core::Id id) :
     BuildStep(bsl, id)
-{ }
+{
+    m_timer.setInterval(500);
+    connect(&m_timer, &QTimer::timeout, this, &AbstractProcessStep::checkForCancel);
+}
 
 AbstractProcessStep::AbstractProcessStep(BuildStepList *bsl,
                                          AbstractProcessStep *bs) :
     BuildStep(bsl, bs), m_ignoreReturnValue(bs->m_ignoreReturnValue)
 { }
 
-AbstractProcessStep::~AbstractProcessStep()
-{
-    delete m_process;
-    delete m_timer;
-    // do not delete m_futureInterface, we do not own it.
-    delete m_outputParserChain;
-}
-
 /*!
      Deletes all existing output parsers and starts a new chain with the
      given parser.
@@ -108,18 +103,16 @@ AbstractProcessStep::~AbstractProcessStep()
 
 void AbstractProcessStep::setOutputParser(IOutputParser *parser)
 {
-    delete m_outputParserChain;
+    m_outputParserChain.reset(new AnsiFilterParser);
-    m_outputParserChain = new AnsiFilterParser;
     m_outputParserChain->appendOutputParser(parser);
 
-    connect(m_outputParserChain, &IOutputParser::addOutput, this, &AbstractProcessStep::outputAdded);
+    connect(m_outputParserChain.get(), &IOutputParser::addOutput, this, &AbstractProcessStep::outputAdded);
-    connect(m_outputParserChain, &IOutputParser::addTask, this, &AbstractProcessStep::taskAdded);
+    connect(m_outputParserChain.get(), &IOutputParser::addTask, this, &AbstractProcessStep::taskAdded);
 }
 
 /*!
     Appends the given output parser to the existing chain of parsers.
 */
-
 void AbstractProcessStep::appendOutputParser(IOutputParser *parser)
 {
     if (!parser)
@@ -127,12 +120,11 @@ void AbstractProcessStep::appendOutputParser(IOutputParser *parser)
 
     QTC_ASSERT(m_outputParserChain, return);
     m_outputParserChain->appendOutputParser(parser);
-    return;
 }
 
 IOutputParser *AbstractProcessStep::outputParser() const
 {
-    return m_outputParserChain;
+    return m_outputParserChain.get();
 }
 
 void AbstractProcessStep::emitFaultyConfigurationMessage()
@@ -176,7 +168,6 @@ bool AbstractProcessStep::init(QList<const BuildStep *> &earlierSteps)
 
 void AbstractProcessStep::run(QFutureInterface<bool> &fi)
 {
-    m_futureInterface = &fi;
     QDir wd(m_param.effectiveWorkingDirectory());
     if (!wd.exists()) {
         if (!wd.mkpath(wd.absolutePath())) {
@@ -195,35 +186,30 @@ void AbstractProcessStep::run(QFutureInterface<bool> &fi)
         return;
     }
 
-    m_process = new Utils::QtcProcess();
+    m_futureInterface.reset(&fi);
-    if (Utils::HostOsInfo::isWindowsHost())
+
-        m_process->setUseCtrlCStub(true);
+    m_process.reset(new Utils::QtcProcess());
+    m_process->setUseCtrlCStub(Utils::HostOsInfo::isWindowsHost());
     m_process->setWorkingDirectory(wd.absolutePath());
     m_process->setEnvironment(m_param.environment());
+    m_process->setCommand(effectiveCommand, m_param.effectiveArguments());
 
-    connect(m_process, &QProcess::readyReadStandardOutput,
+    connect(m_process.get(), &QProcess::readyReadStandardOutput,
             this, &AbstractProcessStep::processReadyReadStdOutput);
-    connect(m_process, &QProcess::readyReadStandardError,
+    connect(m_process.get(), &QProcess::readyReadStandardError,
             this, &AbstractProcessStep::processReadyReadStdError);
-
+    connect(m_process.get(), static_cast<void (QProcess::*)(int,QProcess::ExitStatus)>(&QProcess::finished),
-    connect(m_process, static_cast<void (QProcess::*)(int,QProcess::ExitStatus)>(&QProcess::finished),
             this, &AbstractProcessStep::slotProcessFinished);
 
-    m_process->setCommand(effectiveCommand, m_param.effectiveArguments());
     m_process->start();
     if (!m_process->waitForStarted()) {
         processStartupFailed();
-        delete m_process;
+        m_process.reset();
-        m_process = nullptr;
         reportRunResult(fi, false);
         return;
     }
     processStarted();
-
+    m_timer.start();
-    m_timer = new QTimer();
-    connect(m_timer, &QTimer::timeout, this, &AbstractProcessStep::checkForCancel);
-    m_timer->start(500);
-    m_killProcess = false;
 }
 
 void AbstractProcessStep::cleanUp()
@@ -232,19 +218,12 @@ void AbstractProcessStep::cleanUp()
     processFinished(m_process->exitCode(), m_process->exitStatus());
     const bool returnValue = processSucceeded(m_process->exitCode(), m_process->exitStatus()) || m_ignoreReturnValue;
 
-    // Clean up output parsers
+    m_outputParserChain.reset();
-    if (m_outputParserChain) {
+    m_process.reset();
-        delete m_outputParserChain;
-        m_outputParserChain = nullptr;
-    }
-
-    // Clean up process
-    delete m_process;
-    m_process = nullptr;
 
     // Report result
-    reportRunResult(*m_futureInterface, returnValue);
+    reportRunResult(*(m_futureInterface.get()), returnValue);
-    m_futureInterface = nullptr;
+    m_futureInterface.reset();
 }
 
 /*!
@@ -298,6 +277,7 @@ void AbstractProcessStep::processStartupFailed()
                    .arg(QDir::toNativeSeparators(m_param.effectiveCommand()),
                         m_param.prettyArguments()),
                    BuildStep::ErrorMessageOutput);
+    m_timer.stop();
 }
 
 /*!
@@ -358,20 +338,14 @@ void AbstractProcessStep::stdError(const QString &line)
 
 QFutureInterface<bool> *AbstractProcessStep::futureInterface() const
 {
-    return m_futureInterface;
+    return m_futureInterface.get();
 }
 
 void AbstractProcessStep::checkForCancel()
 {
-    if (m_futureInterface->isCanceled() && m_timer->isActive()) {
+    if (m_futureInterface->isCanceled() && m_timer.isActive()) {
-        if (!m_killProcess) {
+        m_timer.stop();
-            m_process->terminate();
+        Core::Reaper::reap(m_process.release());
-            m_timer->start(5000);
-            m_killProcess = true;
-        } else {
-            m_process->kill();
-            m_timer->stop();
-        }
     }
 }
 
@@ -437,17 +411,15 @@ void AbstractProcessStep::outputAdded(const QString &string, BuildStep::OutputFo
 
 void AbstractProcessStep::slotProcessFinished(int, QProcess::ExitStatus)
 {
-    m_timer->stop();
+    m_timer.stop();
-    delete m_timer;
-    m_timer = nullptr;
 
-    QString line = QString::fromLocal8Bit(m_process->readAllStandardError());
+    const QString stdErrLine = QString::fromLocal8Bit(m_process->readAllStandardError());
-    if (!line.isEmpty())
+    if (!stdErrLine.isEmpty())
-        stdError(line);
+        stdError(stdErrLine);
 
-    line = QString::fromLocal8Bit(m_process->readAllStandardOutput());
+    const QString stdoutLine = QString::fromLocal8Bit(m_process->readAllStandardOutput());
-    if (!line.isEmpty())
+    if (!stdoutLine.isEmpty())
-        stdOutput(line);
+        stdOutput(stdoutLine);
 
     cleanUp();
 }

src/plugins/projectexplorer/abstractprocessstep.h

@@ -28,29 +28,29 @@
 #include "buildstep.h"
 #include "processparameters.h"
 
-#include <QString>
+#include <projectexplorer/ioutputparser.h>
-#include <QProcess>
 
-QT_BEGIN_NAMESPACE
+#include <utils/qtcprocess.h>
-class QEventLoop;
+
-class QTimer;
+#include <QString>
-QT_END_NAMESPACE
+#include <QTimer>
+
+#include <memory>
 
 namespace Utils { class QtcProcess; }
 namespace ProjectExplorer {
 
 class IOutputParser;
+
 // Documentation inside.
 class PROJECTEXPLORER_EXPORT AbstractProcessStep : public BuildStep
 {
     Q_OBJECT
 
 public:
-    ~AbstractProcessStep() override;
-
     bool init(QList<const BuildStep *> &earlierSteps) override;
     void run(QFutureInterface<bool> &) override;
-    bool runInGuiThread() const  override { return true; }
+    bool runInGuiThread() const  final { return true; }
 
     ProcessParameters *processParameters() { return &m_param; }
 
@@ -88,14 +88,13 @@ private:
 
     void outputAdded(const QString &string, BuildStep::OutputFormat format);
 
-    QTimer *m_timer = nullptr;
+    QTimer m_timer;
-    QFutureInterface<bool> *m_futureInterface = nullptr;
+    std::unique_ptr<QFutureInterface<bool>> m_futureInterface;
+    std::unique_ptr<Utils::QtcProcess> m_process;
+    std::unique_ptr<IOutputParser> m_outputParserChain;
     ProcessParameters m_param;
     bool m_ignoreReturnValue = false;
-    bool m_killProcess = false;
     bool m_skipFlush = false;
-    Utils::QtcProcess *m_process = nullptr;
-    IOutputParser *m_outputParserChain = nullptr;
 };
 
 } // namespace ProjectExplorer