Android: Remove AndroidRunnable::{beforeStart,afterFinish}AdbCommands

We have nowadays two ways to pass data from run configurations to
tool that do not require intimate knowledge of the sender:

1. Using RunConfigurationAspects, accessible for all workers in a
   RunControl
2. Using RunWorker::recordData for an individual worker.

This removes the need to use specific fields in a runnable and
means that a tool plugin can be better separated from target plugins.

The approaches are not mutually exclusive, both use an string-ish
id, I chose here to use the same string when using both.

This patch here uses approach 2. for the GammaRay/Android combo.
It also fixes a (harmless) typo (s/POSTSTART/POSTFINISH).

Change-Id: I4048693ca73b17253a39bfcacc9e1880ecf25736
Reviewed-by: Vikas Pachdha <vikas.pachdha@qt.io>

6 files changed, 34 insertions, 31 deletions

diff --git a/src/plugins/android/androidconstants.h b/src/plugins/android/androidconstants.h
index 0015c69ff5..7602f800a9 100644
--- a/src/plugins/android/androidconstants.h
+++ b/src/plugins/android/androidconstants.h
@@ -51,9 +51,11 @@ const char ANDROID_SETTINGS_ID[] = "BB.Android Configurations";
 const char ANDROID_TOOLCHAIN_ID[] = "Qt4ProjectManager.ToolChain.Android";
 const char ANDROIDQT[] = "Qt4ProjectManager.QtVersion.Android";
 
-const char ANDROID_AMSTARTARGS_ASPECT[] = "Android.AmStartArgs";
-const char ANDROID_PRESTARTSHELLCMDLIST_ASPECT[] = "Android.PreStartShellCmdList";
-const char ANDROID_POSTSTARTSHELLCMDLIST_ASPECT[] = "Android.PostStartShellCmdList";
+const char ANDROID_AMSTARTARGS[] = "Android.AmStartArgs";
+// Note: Can be set on RunConfiguration using an aspect and/or
+// the AndroidRunnerWorker using recordData()
+const char ANDROID_PRESTARTSHELLCMDLIST[] = "Android.PreStartShellCmdList";
+const char ANDROID_POSTFINISHSHELLCMDLIST[] = "Android.PostFinishShellCmdList";
 
 const char ANDROID_DEVICE_TYPE[] = "Android.Device.Type";
 const char ANDROID_DEVICE_ID[] = "Android Device";
diff --git a/src/plugins/android/androidrunconfiguration.cpp b/src/plugins/android/androidrunconfiguration.cpp
index ac44a54698..ac1a6858be 100644
--- a/src/plugins/android/androidrunconfiguration.cpp
+++ b/src/plugins/android/androidrunconfiguration.cpp
@@ -111,20 +111,20 @@ AndroidRunConfiguration::AndroidRunConfiguration(Target *target, Core::Id id)
     : RunConfiguration(target, id)
 {
     auto amStartArgsAspect = new BaseStringAspect(this);
-    amStartArgsAspect->setId(Constants::ANDROID_AMSTARTARGS_ASPECT);
+    amStartArgsAspect->setId(Constants::ANDROID_AMSTARTARGS);
     amStartArgsAspect->setSettingsKey("Android.AmStartArgsKey");
     amStartArgsAspect->setLabelText(tr("Activity manager start options:"));
     amStartArgsAspect->setDisplayStyle(BaseStringAspect::LineEditDisplay);
     addExtraAspect(amStartArgsAspect);
 
     auto preStartShellCmdAspect = new BaseStringListAspect(this);
-    preStartShellCmdAspect->setId(Constants::ANDROID_PRESTARTSHELLCMDLIST_ASPECT);
+    preStartShellCmdAspect->setId(Constants::ANDROID_PRESTARTSHELLCMDLIST);
     preStartShellCmdAspect->setSettingsKey("Android.PreStartShellCmdListKey");
     preStartShellCmdAspect->setLabel(tr("Shell commands to run on Android device before application launch."));
     addExtraAspect(preStartShellCmdAspect);
 
     auto postStartShellCmdAspect = new BaseStringListAspect(this);
-    postStartShellCmdAspect->setId(Constants::ANDROID_POSTSTARTSHELLCMDLIST_ASPECT);
+    postStartShellCmdAspect->setId(Constants::ANDROID_POSTFINISHSHELLCMDLIST);
     postStartShellCmdAspect->setSettingsKey("Android.PostStartShellCmdListKey");
     postStartShellCmdAspect->setLabel(tr("Shell commands to run on Android device after application quits."));
     addExtraAspect(postStartShellCmdAspect);
@@ -140,7 +140,7 @@ QWidget *AndroidRunConfiguration::createConfigurationWidget()
     auto widget = new QWidget;
     auto layout = new QFormLayout(widget);
 
-    extraAspect(Constants::ANDROID_AMSTARTARGS_ASPECT)->addToConfigurationLayout(layout);
+    extraAspect(Constants::ANDROID_AMSTARTARGS)->addToConfigurationLayout(layout);
 
     auto warningIconLabel = new QLabel;
     warningIconLabel->setPixmap(Utils::Icons::WARNING.pixmap());
@@ -148,8 +148,8 @@ QWidget *AndroidRunConfiguration::createConfigurationWidget()
     auto warningLabel = new QLabel(tr("If the \"am start\" options conflict, the application might not start."));
     layout->addRow(warningIconLabel, warningLabel);
 
-    extraAspect(Constants::ANDROID_PRESTARTSHELLCMDLIST_ASPECT)->addToConfigurationLayout(layout);
-    extraAspect(Constants::ANDROID_POSTSTARTSHELLCMDLIST_ASPECT)->addToConfigurationLayout(layout);
+    extraAspect(Constants::ANDROID_PRESTARTSHELLCMDLIST)->addToConfigurationLayout(layout);
+    extraAspect(Constants::ANDROID_POSTFINISHSHELLCMDLIST)->addToConfigurationLayout(layout);
 
     auto wrapped = wrapWidget(widget);
     auto detailsWidget = qobject_cast<DetailsWidget *>(wrapped);
diff --git a/src/plugins/android/androidrunnable.h b/src/plugins/android/androidrunnable.h
index e549acf590..f64bd49c29 100644
--- a/src/plugins/android/androidrunnable.h
+++ b/src/plugins/android/androidrunnable.h
@@ -34,8 +34,6 @@ struct ANDROID_EXPORT AndroidRunnable
 {
     AndroidRunnable();
     QString packageName;
-    QStringList beforeStartAdbCommands;
-    QStringList afterFinishAdbCommands;
 
     QString displayName() const { return packageName; }
     static void *staticTypeId;
@@ -43,9 +41,7 @@ struct ANDROID_EXPORT AndroidRunnable
 
 inline bool operator==(const AndroidRunnable &r1, const AndroidRunnable &r2)
 {
-    return r1.packageName == r2.packageName
-        && r1.beforeStartAdbCommands == r2.beforeStartAdbCommands
-        && r1.afterFinishAdbCommands == r2.afterFinishAdbCommands;
+    return r1.packageName == r2.packageName;
 }
 
 inline bool operator!=(const AndroidRunnable &r1, const AndroidRunnable &r2)
diff --git a/src/plugins/android/androidrunner.cpp b/src/plugins/android/androidrunner.cpp
index ce4a0c918c..a1ec0ba528 100644
--- a/src/plugins/android/androidrunner.cpp
+++ b/src/plugins/android/androidrunner.cpp
@@ -130,17 +130,6 @@ AndroidRunner::AndroidRunner(RunControl *runControl,
     QString intent = intentName.isEmpty() ? AndroidManager::intentName(m_target) : intentName;
     m_androidRunnable.packageName = intent.left(intent.indexOf('/'));
 
-    RunConfiguration *rc = runControl->runConfiguration();
-    if (auto aspect = rc->extraAspect(Constants::ANDROID_PRESTARTSHELLCMDLIST_ASPECT)) {
-        for (QString shellCmd : static_cast<BaseStringListAspect *>(aspect)->value())
-            m_androidRunnable.beforeStartAdbCommands.append(QString("shell %1").arg(shellCmd));
-    }
-
-    if (auto aspect = rc->extraAspect(Constants::ANDROID_POSTSTARTSHELLCMDLIST_ASPECT)) {
-        for (QString shellCmd : static_cast<BaseStringListAspect *>(aspect)->value())
-            m_androidRunnable.afterFinishAdbCommands.append(QString("shell %1").arg(shellCmd));
-    }
-
     const int apiLevel = AndroidManager::deviceApiLevel(m_target);
     m_worker.reset(new AndroidRunnerWorker(this, m_androidRunnable));
     m_worker->setIntentName(intent);
diff --git a/src/plugins/android/androidrunnerworker.cpp b/src/plugins/android/androidrunnerworker.cpp
index 05071d1d06..b33d55aef7 100644
--- a/src/plugins/android/androidrunnerworker.cpp
+++ b/src/plugins/android/androidrunnerworker.cpp
@@ -181,8 +181,22 @@ AndroidRunnerWorker::AndroidRunnerWorker(RunWorker *runner, const AndroidRunnabl
     m_deviceSerialNumber = AndroidManager::deviceSerialNumber(target);
     m_apiLevel = AndroidManager::deviceApiLevel(target);
 
-    if (auto aspect = runConfig->extraAspect(Constants::ANDROID_AMSTARTARGS_ASPECT))
+    if (auto aspect = runConfig->extraAspect(Constants::ANDROID_AMSTARTARGS))
         m_amStartExtraArgs = static_cast<BaseStringAspect *>(aspect)->value().split(' ');
+
+    if (auto aspect = runConfig->extraAspect(Constants::ANDROID_PRESTARTSHELLCMDLIST)) {
+        for (const QString &shellCmd : static_cast<BaseStringListAspect *>(aspect)->value())
+            m_beforeStartAdbCommands.append(QString("shell %1").arg(shellCmd));
+    }
+    for (const QString &shellCmd : runner->recordedData(Constants::ANDROID_PRESTARTSHELLCMDLIST).toStringList())
+        m_beforeStartAdbCommands.append(QString("shell %1").arg(shellCmd));
+
+    if (auto aspect = runConfig->extraAspect(Constants::ANDROID_POSTFINISHSHELLCMDLIST)) {
+        for (const QString &shellCmd : static_cast<BaseStringListAspect *>(aspect)->value())
+            m_afterFinishAdbCommands.append(QString("shell %1").arg(shellCmd));
+    }
+    for (const QString &shellCmd : runner->recordedData(Constants::ANDROID_POSTFINISHSHELLCMDLIST).toStringList())
+        m_afterFinishAdbCommands.append(QString("shell %1").arg(shellCmd));
 }
 
 AndroidRunnerWorker::~AndroidRunnerWorker()
@@ -346,7 +360,7 @@ void AndroidRunnerWorker::asyncStartHelper()
     QTC_ASSERT(!m_adbLogcatProcess, /**/);
     m_adbLogcatProcess = std::move(logcatProcess);
 
-    for (const QString &entry: m_androidRunnable.beforeStartAdbCommands)
+    for (const QString &entry : m_beforeStartAdbCommands)
         runAdb(entry.split(' ', QString::SkipEmptyParts));
 
     QStringList args({"shell", "am", "start"});
@@ -398,7 +412,7 @@ void AndroidRunnerWorker::asyncStartHelper()
             emit remoteProcessFinished(tr("Failed to forward C++ debugging ports. Reason: %1.").arg(m_lastRunAdbError));
             return;
         }
-        m_androidRunnable.afterFinishAdbCommands.push_back(removeForward.join(' '));
+        m_afterFinishAdbCommands.push_back(removeForward.join(' '));
     }
 
     if (m_qmlDebugServices != QmlDebug::NoQmlDebugServices) {
@@ -411,7 +425,7 @@ void AndroidRunnerWorker::asyncStartHelper()
                                        .arg(m_lastRunAdbError));
             return;
         }
-        m_androidRunnable.afterFinishAdbCommands.push_back(removeForward.join(' '));
+        m_afterFinishAdbCommands.push_back(removeForward.join(' '));
 
         args << "-e" << "qml_debug" << "true"
              << "-e" << "qmljsdebugger"
@@ -467,7 +481,7 @@ void AndroidRunnerWorker::handleJdbWaiting()
         emit remoteProcessFinished(tr("Failed to forward jdb debugging ports. Reason: %1.").arg(m_lastRunAdbError));
         return;
     }
-    m_androidRunnable.afterFinishAdbCommands.push_back(removeForward.join(' '));
+    m_afterFinishAdbCommands.push_back(removeForward.join(' '));
 
     auto jdbPath = AndroidConfigurations::currentConfig().openJDKLocation().appendPath("bin");
     if (Utils::HostOsInfo::isWindowsHost())
@@ -535,7 +549,7 @@ void AndroidRunnerWorker::onProcessIdChanged(qint64 pid)
         m_gdbServerProcess.reset();
 
         // Run adb commands after application quit.
-        for (const QString &entry: m_androidRunnable.afterFinishAdbCommands)
+        for (const QString &entry: m_afterFinishAdbCommands)
             runAdb(entry.split(' ', QString::SkipEmptyParts));
     } else {
         // In debugging cases this will be funneled to the engine to actually start
diff --git a/src/plugins/android/androidrunnerworker.h b/src/plugins/android/androidrunnerworker.h
index 768b34a19c..1487e71785 100644
--- a/src/plugins/android/androidrunnerworker.h
+++ b/src/plugins/android/androidrunnerworker.h
@@ -87,6 +87,8 @@ protected:
     bool m_isPreNougat = false;
     AndroidRunnable m_androidRunnable;
     QString m_intentName;
+    QStringList m_beforeStartAdbCommands;
+    QStringList m_afterFinishAdbCommands;
     QString m_adb;
     QStringList m_amStartExtraArgs;
     qint64 m_processPID = -1;