Android: Simplify execution of createAvdCommand()

Don't create a separate thread just to blocking execute it
from the caller thread. Use runBlocking with event loop
enabled instead.

Change-Id: I9930d91d25ef4d1af1062570db1cfe20a1c4ca2b
Reviewed-by: Alessandro Portale <alessandro.portale@qt.io>

src/plugins/android/androidavdmanager.cpp

@@ -2,32 +2,25 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
 
 #include "androidavdmanager.h"
+#include "androidconfigurations.h"
 #include "androidtr.h"
 #include "avdmanageroutputparser.h"
 
 #include <coreplugin/icore.h>
 
-#include <projectexplorer/projectexplorerconstants.h>
-
 #include <utils/algorithm.h>
 #include <utils/async.h>
 #include <utils/qtcprocess.h>
-#include <utils/qtcassert.h>
 
 #include <QLoggingCategory>
 #include <QMainWindow>
 #include <QMessageBox>
 
-#include <chrono>
-
 using namespace Utils;
-using namespace std;
 using namespace std::chrono_literals;
 
 namespace Android::Internal {
 
-const int avdCreateTimeoutMs = 30000;
-
 static Q_LOGGING_CATEGORY(avdManagerLog, "qtc.android.avdManager", QtWarningMsg)
 
 /*!
@@ -51,85 +44,6 @@ bool AndroidAvdManager::avdManagerCommand(const QStringList &args, QString *outp
     return false;
 }
 
-static bool checkForTimeout(const chrono::steady_clock::time_point &start,
-                            int msecs = 3000)
-{
-    bool timedOut = false;
-    auto end = chrono::steady_clock::now();
-    if (chrono::duration_cast<chrono::milliseconds>(end-start).count() > msecs)
-        timedOut = true;
-    return timedOut;
-}
-
-static CreateAvdInfo createAvdCommand(const CreateAvdInfo &info)
-{
-    CreateAvdInfo result = info;
-    CommandLine avdManager(androidConfig().avdManagerToolPath(), {"create", "avd", "-n", result.name});
-    avdManager.addArgs({"-k", result.sdkStylePath});
-
-    if (result.sdcardSize > 0)
-        avdManager.addArgs({"-c", QString("%1M").arg(result.sdcardSize)});
-
-    if (!result.deviceDefinition.isEmpty() && result.deviceDefinition != "Custom")
-        avdManager.addArgs({"-d", QString("%1").arg(result.deviceDefinition)});
-
-    if (result.overwrite)
-        avdManager.addArg("-f");
-
-    qCDebug(avdManagerLog).noquote() << "Running AVD Manager command:" << avdManager.toUserOutput();
-    Process proc;
-    proc.setProcessMode(ProcessMode::Writer);
-    proc.setEnvironment(androidConfig().toolsEnvironment());
-    proc.setCommand(avdManager);
-    proc.start();
-    if (!proc.waitForStarted()) {
-        result.error = Tr::tr("Could not start process \"%1\".").arg(avdManager.toUserOutput());
-        return result;
-    }
-    QTC_CHECK(proc.isRunning());
-    proc.write("yes\n"); // yes to "Do you wish to create a custom hardware profile"
-
-    auto start = chrono::steady_clock::now();
-    QString errorOutput;
-    QByteArray question;
-    while (errorOutput.isEmpty()) {
-        proc.waitForReadyRead(500ms);
-        question += proc.readAllRawStandardOutput();
-        if (question.endsWith(QByteArray("]:"))) {
-            // truncate to last line
-            int index = question.lastIndexOf(QByteArray("\n"));
-            if (index != -1)
-                question = question.mid(index);
-            if (question.contains("hw.gpu.enabled"))
-                proc.write("yes\n");
-            else
-                proc.write("\n");
-            question.clear();
-        }
-        // The exit code is always 0, so we need to check stderr
-        // For now assume that any output at all indicates a error
-        errorOutput = QString::fromLocal8Bit(proc.readAllRawStandardError());
-        if (!proc.isRunning())
-            break;
-
-        // For a sane input and command, process should finish before timeout.
-        if (checkForTimeout(start, avdCreateTimeoutMs))
-            result.error = Tr::tr("Cannot create AVD. Command timed out.");
-    }
-
-    result.error = errorOutput;
-    return result;
-}
-
-AndroidAvdManager::AndroidAvdManager() = default;
-
-AndroidAvdManager::~AndroidAvdManager() = default;
-
-QFuture<CreateAvdInfo> AndroidAvdManager::createAvd(CreateAvdInfo info) const
-{
-    return Utils::asyncRun(&createAvdCommand, info);
-}
-
 static void avdConfigEditManufacturerTag(const FilePath &avdPath, bool recoverMode = false)
 {
     if (!avdPath.exists())

src/plugins/android/androidavdmanager.h

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
 #pragma once
 
-#include "androidconfigurations.h"
+#include "androiddeviceinfo.h"
 
 #include <QFuture>
 
@@ -13,10 +13,6 @@ namespace Android::Internal {
 class AndroidAvdManager
 {
 public:
-    AndroidAvdManager();
-    ~AndroidAvdManager();
-
-    QFuture<CreateAvdInfo> createAvd(CreateAvdInfo info) const;
     QFuture<AndroidDeviceInfoList> avdList() const;
 
     QString startAvd(const QString &name) const;

src/plugins/android/androidconfigurations.h

@@ -34,9 +34,6 @@ public:
     QString abi;
     QString deviceDefinition;
     int sdcardSize = 0;
-    QString error; // only used in the return value of createAVD
-    bool overwrite = false;
-    bool cancelled = false;
 };
 
 struct SdkForQtVersions

src/plugins/android/avddialog.cpp

@@ -4,6 +4,7 @@
 #include "avddialog.h"
 #include "androidtr.h"
 #include "androidavdmanager.h"
+#include "androidconfigurations.h"
 #include "androidconstants.h"
 #include "androiddevice.h"
 #include "androidsdkmanager.h"
@@ -16,13 +17,13 @@
 #include <utils/infolabel.h>
 #include <utils/layoutbuilder.h>
 #include <utils/qtcassert.h>
+#include <utils/qtcprocess.h>
 #include <utils/tooltip/tooltip.h>
 #include <utils/utilsicons.h>
 
 #include <QCheckBox>
 #include <QComboBox>
 #include <QDialogButtonBox>
-#include <QFutureWatcher>
 #include <QKeyEvent>
 #include <QLineEdit>
 #include <QLoggingCategory>
@@ -39,8 +40,8 @@ namespace Android::Internal {
 static Q_LOGGING_CATEGORY(avdDialogLog, "qtc.android.avdDialog", QtWarningMsg)
 
 AvdDialog::AvdDialog(QWidget *parent)
-    : QDialog(parent),
+    : QDialog(parent)
-      m_allowedNameChars(QLatin1String("[a-z|A-Z|0-9|._-]*"))
+    , m_allowedNameChars(QLatin1String("[a-z|A-Z|0-9|._-]*"))
 {
     resize(800, 0);
     setWindowTitle(Tr::tr("Create new AVD"));
@@ -129,37 +130,51 @@ int AvdDialog::exec()
             return QDialog::Rejected;
         }
 
-        CreateAvdInfo result;
+        CommandLine cmd(androidConfig().avdManagerToolPath(), {"create", "avd", "-n", name()});
-        result.sdkStylePath = si->sdkStylePath();
+        cmd.addArgs({"-k", si->sdkStylePath()});
-        result.apiLevel = si->apiLevel();
+        if (sdcardSize() > 0)
-        result.name = name();
+            cmd.addArgs({"-c", QString("%1M").arg(sdcardSize())});
-        result.abi = abi();
-        result.deviceDefinition = deviceDefinition();
-        result.sdcardSize = sdcardSize();
-        result.overwrite = m_overwriteCheckBox->isChecked();
 
-        const AndroidAvdManager avdManager;
+        const QString deviceDef = deviceDefinition();
-        QFutureWatcher<CreateAvdInfo> createAvdFutureWatcher;
+        if (!deviceDef.isEmpty() && deviceDef != "Custom")
+            cmd.addArgs({"-d", QString("%1").arg(deviceDef)});
 
-        QEventLoop loop;
+        if (m_overwriteCheckBox->isChecked())
-        QObject::connect(&createAvdFutureWatcher, &QFutureWatcher<CreateAvdInfo>::finished,
+            cmd.addArg("-f");
-                         &loop, &QEventLoop::quit);
-        QObject::connect(&createAvdFutureWatcher, &QFutureWatcher<CreateAvdInfo>::canceled,
-                         &loop, &QEventLoop::quit);
-        createAvdFutureWatcher.setFuture(avdManager.createAvd(result));
-        loop.exec(QEventLoop::ExcludeUserInputEvents);
 
-        const QFuture<CreateAvdInfo> future = createAvdFutureWatcher.future();
+        Process process;
-        if (future.isResultReadyAt(0)) {
+        process.setProcessMode(ProcessMode::Writer);
-            const CreateAvdInfo &info = future.result();
+        process.setEnvironment(androidConfig().toolsEnvironment());
-            if (!info.error.isEmpty()) {
+        process.setCommand(cmd);
-                QMessageBox::warning(Core::ICore::dialogParent(),
+        process.setWriteData("yes\n"); // yes to "Do you wish to create a custom hardware profile"
-                    Tr::tr("Create new AVD"), info.error);
+
-                return QDialog::Rejected;
+        QByteArray buffer;
+        QObject::connect(&process, &Process::readyReadStandardOutput, &process, [&process, &buffer] {
+            // This interaction is needed only if there is no "-d" arg for the avdmanager command.
+            buffer += process.readAllRawStandardOutput();
+            if (buffer.endsWith(QByteArray("]:"))) {
+                // truncate to last line
+                const int index = buffer.lastIndexOf(QByteArray("\n"));
+                if (index != -1)
+                    buffer = buffer.mid(index);
+                if (buffer.contains("hw.gpu.enabled"))
+                    process.write("yes\n");
+                else
+                    process.write("\n");
+                buffer.clear();
             }
-            m_createdAvdInfo = info;
+        });
-            AndroidDeviceManager::instance()->updateAvdsList();
+
+        using namespace std::chrono_literals;
+        process.runBlocking(10s, EventLoopMode::On);
+        if (process.result() != ProcessResult::FinishedWithSuccess) {
+            QMessageBox::warning(Core::ICore::dialogParent(), Tr::tr("Create new AVD"),
+                                 process.exitMessage());
+            return QDialog::Rejected;
         }
+        m_createdAvdInfo = {si->sdkStylePath(), si->apiLevel(), name(), abi(), deviceDef,
+                            sdcardSize()};
+        AndroidDeviceManager::instance()->updateAvdsList();
     }
 
     return execResult;