Android: Flatten then AndroidRunnerWorker hierarchy again

The difference was the selection of findProcessPID vs
findProcessPIDPreNougat functions, done by a flag now
passed to and used inside a combined findProcessPID function.

Change-Id: I738cdac1a81302c2207f9bc3c74c7cf916ca4089
Reviewed-by: BogDan Vatra <bogdan@kdab.com>

src/plugins/android/androidrunner.cpp

@@ -147,26 +147,24 @@ AndroidRunner::AndroidRunner(RunControl *runControl,
     }
 
     const int apiLevel = AndroidManager::deviceApiLevel(m_target);
-    if (apiLevel > 23)
+    m_worker.reset(new AndroidRunnerWorker(runControl, m_androidRunnable));
-        m_worker.reset(new AndroidRunnerWorker(runControl, m_androidRunnable));
+    m_worker->setIsPreNougat(apiLevel <= 23);
-    else
-        m_worker.reset(new AndroidRunnerWorkerPreNougat(runControl, m_androidRunnable));
     m_worker->setExtraAppParams(extraAppParams);
     m_worker->setExtraEnvVars(extraEnvVars);
 
     m_worker->moveToThread(&m_thread);
 
-    connect(this, &AndroidRunner::asyncStart, m_worker.data(), &AndroidRunnerWorkerBase::asyncStart);
+    connect(this, &AndroidRunner::asyncStart, m_worker.data(), &AndroidRunnerWorker::asyncStart);
-    connect(this, &AndroidRunner::asyncStop, m_worker.data(), &AndroidRunnerWorkerBase::asyncStop);
+    connect(this, &AndroidRunner::asyncStop, m_worker.data(), &AndroidRunnerWorker::asyncStop);
     connect(this, &AndroidRunner::androidDeviceInfoChanged,
-            m_worker.data(), &AndroidRunnerWorkerBase::setAndroidDeviceInfo);
+            m_worker.data(), &AndroidRunnerWorker::setAndroidDeviceInfo);
-    connect(m_worker.data(), &AndroidRunnerWorkerBase::remoteProcessStarted,
+    connect(m_worker.data(), &AndroidRunnerWorker::remoteProcessStarted,
             this, &AndroidRunner::handleRemoteProcessStarted);
-    connect(m_worker.data(), &AndroidRunnerWorkerBase::remoteProcessFinished,
+    connect(m_worker.data(), &AndroidRunnerWorker::remoteProcessFinished,
             this, &AndroidRunner::handleRemoteProcessFinished);
-    connect(m_worker.data(), &AndroidRunnerWorkerBase::remoteOutput,
+    connect(m_worker.data(), &AndroidRunnerWorker::remoteOutput,
             this, &AndroidRunner::remoteOutput);
-    connect(m_worker.data(), &AndroidRunnerWorkerBase::remoteErrorOutput,
+    connect(m_worker.data(), &AndroidRunnerWorker::remoteErrorOutput,
             this, &AndroidRunner::remoteErrorOutput);
 
     connect(&m_outputParser, &QmlDebug::QmlOutputParser::waitingForConnectionOnPort,

src/plugins/android/androidrunner.h

@@ -43,7 +43,7 @@
 namespace Android {
 namespace Internal {
 
-class AndroidRunnerWorkerBase;
+class AndroidRunnerWorker;
 
 class AndroidRunner : public ProjectExplorer::RunWorker
 {
@@ -84,7 +84,7 @@ private:
     QString m_launchedAVDName;
     QThread m_thread;
     QTimer m_checkAVDTimer;
-    QScopedPointer<AndroidRunnerWorkerBase> m_worker;
+    QScopedPointer<AndroidRunnerWorker> m_worker;
     QPointer<ProjectExplorer::Target> m_target;
     Utils::Port m_gdbServerPort;
     QUrl m_qmlServer;

src/plugins/android/androidrunnerworker.cpp

@@ -98,41 +98,28 @@ static qint64 extractPID(const QByteArray &output, const QString &packageName)
     return pid;
 }
 
-void findProcessPIDPreNougat(QFutureInterface<qint64> &fi, const QString &adbPath,
+static void findProcessPID(QFutureInterface<qint64> &fi, const QString &adbPath,
-                    QStringList selector, const QString &packageName)
+                           QStringList selector, const QString &packageName,
+                           bool preNougat)
 {
     if (packageName.isEmpty())
         return;
 
     qint64 processPID = -1;
     chrono::high_resolution_clock::time_point start = chrono::high_resolution_clock::now();
+
+    selector.append("shell");
+    selector.append(preNougat ? pidScriptPreNougat : pidScript.arg(packageName));
+
     do {
         QThread::msleep(200);
-        const QByteArray out = Utils::SynchronousProcess()
+        const QByteArray out = Utils::SynchronousProcess().runBlocking(adbPath, selector).allRawOutput();
-                .runBlocking(adbPath, selector << QStringLiteral("shell") << pidScriptPreNougat)
+        if (preNougat) {
-                .allRawOutput();
+            processPID = extractPID(out, packageName);
-        processPID = extractPID(out, packageName);
+        } else {
-    } while (processPID == -1 && !isTimedOut(start) && !fi.isCanceled());
+            if (!out.isEmpty())
-
+                processPID = out.trimmed().toLongLong();
-    if (!fi.isCanceled())
+        }
-        fi.reportResult(processPID);
-}
-
-void findProcessPID(QFutureInterface<qint64> &fi, const QString &adbPath,
-                    QStringList selector, const QString &packageName)
-{
-    if (packageName.isEmpty())
-        return;
-
-    qint64 processPID = -1;
-    chrono::high_resolution_clock::time_point start = chrono::high_resolution_clock::now();
-    do {
-        QThread::msleep(200);
-        const QByteArray out = Utils::SynchronousProcess()
-                .runBlocking(adbPath, selector << QStringLiteral("shell") << pidScript.arg(packageName))
-                .allRawOutput();
-        if (!out.isEmpty())
-            processPID = out.trimmed().toLongLong();
     } while (processPID == -1 && !isTimedOut(start) && !fi.isCanceled());
 
     if (!fi.isCanceled())
@@ -150,7 +137,7 @@ static void deleter(QProcess *p)
     p->deleteLater();
 }
 
-AndroidRunnerWorkerBase::AndroidRunnerWorkerBase(RunControl *runControl, const AndroidRunnable &runnable)
+AndroidRunnerWorker::AndroidRunnerWorker(RunControl *runControl, const AndroidRunnable &runnable)
     : m_androidRunnable(runnable)
     , m_adbLogcatProcess(nullptr, deleter)
     , m_psIsAlive(nullptr, deleter)
@@ -192,7 +179,7 @@ AndroidRunnerWorkerBase::AndroidRunnerWorkerBase(RunControl *runControl, const A
     m_apiLevel = AndroidManager::deviceApiLevel(target);
 }
 
-AndroidRunnerWorkerBase::~AndroidRunnerWorkerBase()
+AndroidRunnerWorker::~AndroidRunnerWorker()
 {
     if (m_processPID != -1)
         forceStop();
@@ -201,7 +188,7 @@ AndroidRunnerWorkerBase::~AndroidRunnerWorkerBase()
         m_pidFinder.cancel();
 }
 
-bool AndroidRunnerWorkerBase::adbShellAmNeedsQuotes()
+bool AndroidRunnerWorker::adbShellAmNeedsQuotes()
 {
     // Between Android SDK Tools version 24.3.1 and 24.3.4 the quoting
     // needs for the 'adb shell am start ...' parameters changed.
@@ -223,7 +210,7 @@ bool AndroidRunnerWorkerBase::adbShellAmNeedsQuotes()
     return !oldSdk;
 }
 
-bool AndroidRunnerWorkerBase::runAdb(const QStringList &args, int timeoutS)
+bool AndroidRunnerWorker::runAdb(const QStringList &args, int timeoutS)
 {
     Utils::SynchronousProcess adb;
     adb.setTimeoutS(timeoutS);
@@ -233,18 +220,18 @@ bool AndroidRunnerWorkerBase::runAdb(const QStringList &args, int timeoutS)
     return response.result == Utils::SynchronousProcessResponse::Finished;
 }
 
-void AndroidRunnerWorkerBase::adbKill(qint64 pid)
+void AndroidRunnerWorker::adbKill(qint64 pid)
 {
     runAdb({"shell", "kill", "-9", QString::number(pid)});
     runAdb({"shell", "run-as", m_androidRunnable.packageName, "kill", "-9", QString::number(pid)});
 }
 
-QStringList AndroidRunnerWorkerBase::selector() const
+QStringList AndroidRunnerWorker::selector() const
 {
     return AndroidDeviceInfo::adbSelector(m_deviceSerialNumber);
 }
 
-void AndroidRunnerWorkerBase::forceStop()
+void AndroidRunnerWorker::forceStop()
 {
     runAdb({"shell", "am", "force-stop", m_androidRunnable.packageName}, 30);
 
@@ -259,19 +246,19 @@ void AndroidRunnerWorkerBase::forceStop()
     }
 }
 
-void AndroidRunnerWorkerBase::logcatReadStandardError()
+void AndroidRunnerWorker::logcatReadStandardError()
 {
     if (m_processPID != -1)
         logcatProcess(m_adbLogcatProcess->readAllStandardError(), m_stderrBuffer, true);
 }
 
-void AndroidRunnerWorkerBase::logcatReadStandardOutput()
+void AndroidRunnerWorker::logcatReadStandardOutput()
 {
     if (m_processPID != -1)
         logcatProcess(m_adbLogcatProcess->readAllStandardOutput(), m_stdoutBuffer, false);
 }
 
-void AndroidRunnerWorkerBase::logcatProcess(const QByteArray &text, QByteArray &buffer, bool onlyError)
+void AndroidRunnerWorker::logcatProcess(const QByteArray &text, QByteArray &buffer, bool onlyError)
 {
     QList<QByteArray> lines = text.split('\n');
     // lines always contains at least one item
@@ -332,22 +319,22 @@ void AndroidRunnerWorkerBase::logcatProcess(const QByteArray &text, QByteArray &
     }
 }
 
-void AndroidRunnerWorkerBase::setAndroidDeviceInfo(const AndroidDeviceInfo &info)
+void AndroidRunnerWorker::setAndroidDeviceInfo(const AndroidDeviceInfo &info)
 {
     m_deviceSerialNumber = info.serialNumber;
     m_apiLevel = info.sdk;
 }
 
-void AndroidRunnerWorkerBase::asyncStart()
+void AndroidRunnerWorker::asyncStartHelper()
 {
     forceStop();
 
     // Start the logcat process before app starts.
     std::unique_ptr<QProcess, Deleter> logcatProcess(new QProcess, deleter);
     connect(logcatProcess.get(), &QProcess::readyReadStandardOutput,
-            this, &AndroidRunnerWorkerBase::logcatReadStandardOutput);
+            this, &AndroidRunnerWorker::logcatReadStandardOutput);
     connect(logcatProcess.get(), &QProcess::readyReadStandardError,
-            this, &AndroidRunnerWorkerBase::logcatReadStandardError);
+            this, &AndroidRunnerWorker::logcatReadStandardError);
     // Its assumed that the device or avd returned by selector() is online.
     logcatProcess->start(m_adb, selector() << "logcat");
     QTC_ASSERT(!m_adbLogcatProcess, /**/);
@@ -444,7 +431,16 @@ void AndroidRunnerWorkerBase::asyncStart()
     }
 }
 
-void AndroidRunnerWorkerBase::asyncStop()
+void AndroidRunnerWorker::asyncStart()
+{
+    asyncStartHelper();
+
+    m_pidFinder = Utils::onResultReady(Utils::runAsync(findProcessPID, m_adb, selector(),
+                                                       m_androidRunnable.packageName, m_isPreNougat),
+                                       bind(&AndroidRunnerWorker::onProcessIdChanged, this, _1));
+}
+
+void AndroidRunnerWorker::asyncStop()
 {
     if (!m_pidFinder.isFinished())
         m_pidFinder.cancel();
@@ -456,7 +452,7 @@ void AndroidRunnerWorkerBase::asyncStop()
     m_gdbServerProcess.reset();
 }
 
-void AndroidRunnerWorkerBase::handleJdbWaiting()
+void AndroidRunnerWorker::handleJdbWaiting()
 {
     QStringList removeForward{"forward", "--remove", "tcp:" + m_localJdbServerPort.toString()};
     runAdb(removeForward);
@@ -485,7 +481,7 @@ void AndroidRunnerWorkerBase::handleJdbWaiting()
     m_jdbProcess = std::move(jdbProcess);
 }
 
-void AndroidRunnerWorkerBase::handleJdbSettled()
+void AndroidRunnerWorker::handleJdbSettled()
 {
     auto waitForCommand = [&]() {
         for (int i= 0; i < 5 && m_jdbProcess->state() == QProcess::Running; ++i) {
@@ -518,7 +514,7 @@ void AndroidRunnerWorkerBase::handleJdbSettled()
     emit remoteProcessFinished(tr("Can't attach jdb to the running application").arg(m_lastRunAdbError));
 }
 
-void AndroidRunnerWorkerBase::onProcessIdChanged(qint64 pid)
+void AndroidRunnerWorker::onProcessIdChanged(qint64 pid)
 {
     // Don't write to m_psProc from a different thread
     QTC_ASSERT(QThread::currentThread() == thread(), return);
@@ -544,49 +540,22 @@ void AndroidRunnerWorkerBase::onProcessIdChanged(qint64 pid)
         m_psIsAlive.reset(new QProcess);
         m_psIsAlive->setProcessChannelMode(QProcess::MergedChannels);
         connect(m_psIsAlive.get(), static_cast<void(QProcess::*)(int)>(&QProcess::finished),
-                this, bind(&AndroidRunnerWorkerBase::onProcessIdChanged, this, -1));
+                this, bind(&AndroidRunnerWorker::onProcessIdChanged, this, -1));
         m_psIsAlive->start(m_adb, selector() << QStringLiteral("shell")
                            << pidPollingScript.arg(m_processPID));
     }
 }
 
-void AndroidRunnerWorkerBase::setExtraEnvVars(const Utils::Environment &extraEnvVars)
+void AndroidRunnerWorker::setExtraEnvVars(const Utils::Environment &extraEnvVars)
 {
     m_extraEnvVars = extraEnvVars;
 }
 
-void AndroidRunnerWorkerBase::setExtraAppParams(const QString &extraAppParams)
+void AndroidRunnerWorker::setExtraAppParams(const QString &extraAppParams)
 {
     m_extraAppParams = extraAppParams;
 }
 
 
-AndroidRunnerWorker::AndroidRunnerWorker(RunControl *runControl, const AndroidRunnable &runnable)
-    : AndroidRunnerWorkerBase(runControl, runnable)
-{
-}
-
-void AndroidRunnerWorker::asyncStart()
-{
-    AndroidRunnerWorkerBase::asyncStart();
-    m_pidFinder = Utils::onResultReady(Utils::runAsync(&findProcessPID, m_adb, selector(),
-                                                       m_androidRunnable.packageName),
-                                       bind(&AndroidRunnerWorker::onProcessIdChanged, this, _1));
-}
-
-AndroidRunnerWorkerPreNougat::AndroidRunnerWorkerPreNougat(RunControl *runControl, const AndroidRunnable &runnable)
-    : AndroidRunnerWorkerBase(runControl, runnable)
-{
-}
-
-void AndroidRunnerWorkerPreNougat::asyncStart()
-{
-    AndroidRunnerWorkerBase::asyncStart();
-    m_pidFinder = Utils::onResultReady(Utils::runAsync(&findProcessPIDPreNougat, m_adb, selector(),
-                                                       m_androidRunnable.packageName),
-                                       bind(&AndroidRunnerWorkerPreNougat::onProcessIdChanged, this, _1));
-
-}
-
 } // namespace Internal
 } // namespace Android

src/plugins/android/androidrunnerworker.h

@@ -29,7 +29,6 @@
 #include <qmldebug/qmldebugcommandlinearguments.h>
 
 #include <QFuture>
-#include <QTcpSocket>
 
 #include "androidrunnable.h"
 
@@ -45,12 +44,12 @@ namespace Internal {
 
 const int MIN_SOCKET_HANDSHAKE_PORT = 20001;
 
-class AndroidRunnerWorkerBase : public QObject
+class AndroidRunnerWorker : public QObject
 {
     Q_OBJECT
 public:
-    AndroidRunnerWorkerBase(ProjectExplorer::RunControl *runControl, const AndroidRunnable &runnable);
+    AndroidRunnerWorker(ProjectExplorer::RunControl *runControl, const AndroidRunnable &runnable);
-    ~AndroidRunnerWorkerBase() override;
+    ~AndroidRunnerWorker() override;
     bool adbShellAmNeedsQuotes();
     bool runAdb(const QStringList &args, int timeoutS = 10);
     void adbKill(qint64 pid);
@@ -62,11 +61,12 @@ public:
     void setAndroidDeviceInfo(const AndroidDeviceInfo &info);
     void setExtraEnvVars(const Utils::Environment &extraEnvVars);
     void setExtraAppParams(const QString &extraAppParams);
+    void setIsPreNougat(bool isPreNougat) { m_isPreNougat = isPreNougat; }
 
-    virtual void asyncStart();
+    void asyncStart();
-    virtual void asyncStop();
+    void asyncStop();
-    virtual void handleJdbWaiting();
+    void handleJdbWaiting();
-    virtual void handleJdbSettled();
+    void handleJdbSettled();
 
 signals:
     void remoteProcessStarted(Utils::Port gdbServerPort, const QUrl &qmlServer, int pid);
@@ -76,15 +76,18 @@ signals:
     void remoteErrorOutput(const QString &output);
 
 protected:
+    void asyncStartHelper();
+
     enum class JDBState {
         Idle,
         Waiting,
         Settled
     };
-    virtual void onProcessIdChanged(qint64 pid);
+    void onProcessIdChanged(qint64 pid);
     using Deleter = void (*)(QProcess *);
 
     // Create the processes and timer in the worker thread, for correct thread affinity
+    bool m_isPreNougat = false;
     AndroidRunnable m_androidRunnable;
     QString m_adb;
     qint64 m_processPID = -1;
@@ -110,23 +113,5 @@ protected:
     Utils::Environment m_extraEnvVars;
 };
 
-
-class AndroidRunnerWorker : public AndroidRunnerWorkerBase
-{
-    Q_OBJECT
-public:
-    AndroidRunnerWorker(ProjectExplorer::RunControl *runControl, const AndroidRunnable &runnable);
-    void asyncStart() override;
-};
-
-
-class AndroidRunnerWorkerPreNougat : public AndroidRunnerWorkerBase
-{
-    Q_OBJECT
-public:
-    AndroidRunnerWorkerPreNougat(ProjectExplorer::RunControl *runControl, const AndroidRunnable &runnable);
-    void asyncStart() override;
-};
-
 } // namespace Internal
 } // namespace Android