Android: Do some refactoring for AvdDialog class

Move some logic for avd creation to the AvdDialog, and some
refactoring and simplification.

Change-Id: Id65e586ab1c0e9e898a04f07d7707371f20da649
Reviewed-by: Alessandro Portale <alessandro.portale@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

src/plugins/android/androiddevice.cpp

@@ -604,50 +604,20 @@ AndroidDeviceFactory::AndroidDeviceFactory()
 
 IDevice::Ptr AndroidDeviceFactory::create() const
 {
-    AndroidSdkManager sdkManager = AndroidSdkManager(m_androidConfig);
+    AvdDialog dialog = AvdDialog(m_androidConfig, Core::ICore::dialogParent());
-    const CreateAvdInfo info = AvdDialog::gatherCreateAVDInfo(Core::ICore::dialogParent(),
+    if (dialog.exec() != QDialog::Accepted)
-                                                              &sdkManager, m_androidConfig);
+        return ProjectExplorer::IDevice::Ptr();
-    if (!info.isValid()) {
+
-        if (!info.cancelled) {
+    const ProjectExplorer::IDevice::Ptr dev = dialog.device();
-            AndroidDeviceWidget::criticalDialog(
+    const AndroidDevice *androidDev = static_cast<AndroidDevice*>(dev.data());
-                        QObject::tr("The returned device info is invalid."));
+    if (androidDev) {
-        }
+        qCDebug(androidDeviceLog, "Created new Android AVD id \"%s\".",
-        return nullptr;
+                qPrintable(androidDev->avdName()));
+    } else {
+        AndroidDeviceWidget::criticalDialog(
+                    QObject::tr("The device info returned from AvdDialog is invalid."));
     }
 
-    const AndroidAvdManager avdManager = AndroidAvdManager(m_androidConfig);
-    QFutureWatcher<CreateAvdInfo> createAvdFutureWatcher;
-    createAvdFutureWatcher.setFuture(avdManager.createAvd(info));
-
-    QEventLoop loop;
-    QObject::connect(&createAvdFutureWatcher, &QFutureWatcher<CreateAvdInfo>::finished,
-                     &loop, &QEventLoop::quit);
-    QObject::connect(&createAvdFutureWatcher, &QFutureWatcher<CreateAvdInfo>::canceled,
-                     &loop, &QEventLoop::quit);
-    loop.exec(QEventLoop::ExcludeUserInputEvents);
-
-    QFuture<CreateAvdInfo> future = createAvdFutureWatcher.future();
-    if (!(future.isResultReadyAt(0) && future.result().isValid())) {
-        AndroidDeviceWidget::criticalDialog(QObject::tr("The device info returned by "
-                        "avdmanager tool is invalid for the device name \"%1\".").arg(info.name));
-        return nullptr;
-    }
-
-    const CreateAvdInfo newAvdInfo = createAvdFutureWatcher.result();
-
-    AndroidDevice *dev = new AndroidDevice();
-    const Utils::Id deviceId = AndroidDevice::idFromAvdInfo(newAvdInfo);
-    dev->setupId(IDevice::AutoDetected, deviceId);
-    dev->setMachineType(IDevice::Emulator);
-    dev->setDisplayName(newAvdInfo.name);
-    dev->setDeviceState(IDevice::DeviceConnected);
-    dev->setExtraData(Constants::AndroidAvdName, newAvdInfo.name);
-    dev->setExtraData(Constants::AndroidCpuAbi, {newAvdInfo.abi});
-    dev->setExtraData(Constants::AndroidSdk, newAvdInfo.systemImage->apiLevel());
-    dev->setExtraData(Constants::AndroidAvdSdcard, QString("%1 MB").arg(newAvdInfo.sdcardSize));
-    dev->setExtraData(Constants::AndroidAvdDevice, newAvdInfo.deviceDefinition);
-
-    qCDebug(androidDeviceLog, "Created new Android AVD id \"%s\".", qPrintable(newAvdInfo.name));
     return IDevice::Ptr(dev);
 }
 

src/plugins/android/avddialog.cpp

@@ -26,6 +26,8 @@
 #include "avddialog.h"
 #include "androidsdkmanager.h"
 #include "androidavdmanager.h"
+#include "androiddevice.h"
+#include "androidconstants.h"
 
 #include <projectexplorer/projectexplorerconstants.h>
 #include <utils/algorithm.h>
@@ -33,6 +35,7 @@
 #include <utils/utilsicons.h>
 #include <utils/qtcassert.h>
 
+#include <QFutureWatcher>
 #include <QKeyEvent>
 #include <QMessageBox>
 #include <QToolTip>
@@ -46,29 +49,35 @@ namespace {
 static Q_LOGGING_CATEGORY(avdDialogLog, "qtc.android.avdDialog", QtWarningMsg)
 }
 
-AvdDialog::AvdDialog(int minApiLevel, AndroidSdkManager *sdkManager, const QStringList &abis,
+AvdDialog::AvdDialog(const AndroidConfig &config, QWidget *parent)
-                     const AndroidConfig &config, QWidget *parent) :
+    : QDialog(parent),
-    QDialog(parent),
+      m_sdkManager(m_androidConfig),
-    m_sdkManager(sdkManager),
+      m_allowedNameChars(QLatin1String("[a-z|A-Z|0-9|._-]*")),
-    m_minApiLevel(minApiLevel),
+      m_androidConfig(config)
-    m_allowedNameChars(QLatin1String("[a-z|A-Z|0-9|._-]*")),
-    m_androidConfig(config)
 {
-    QTC_CHECK(m_sdkManager);
     m_avdDialog.setupUi(this);
     m_hideTipTimer.setInterval(2000);
     m_hideTipTimer.setSingleShot(true);
 
-    if (abis.isEmpty()) {
+    connect(&m_hideTipTimer, &QTimer::timeout, this, &Utils::ToolTip::hide);
-        m_avdDialog.abiComboBox->addItems(QStringList({
+    connect(m_avdDialog.deviceDefinitionTypeComboBox,
-                                               ProjectExplorer::Constants::ANDROID_ABI_X86,
+            QOverload<int>::of(&QComboBox::currentIndexChanged),
-                                               ProjectExplorer::Constants::ANDROID_ABI_X86_64,
+            this, &AvdDialog::updateDeviceDefinitionComboBox);
-                                               ProjectExplorer::Constants::ANDROID_ABI_ARMEABI_V7A,
+    connect(m_avdDialog.abiComboBox,
-                                               ProjectExplorer::Constants::ANDROID_ABI_ARMEABI,
+            QOverload<int>::of(&QComboBox::currentIndexChanged),
-                                               ProjectExplorer::Constants::ANDROID_ABI_ARM64_V8A}));
+            this, &AvdDialog::updateApiLevelComboBox);
-    } else {
+
-        m_avdDialog.abiComboBox->addItems(abis);
+    deviceTypeToStringMap.insert(AvdDialog::Phone, "Phone");
-    }
+    deviceTypeToStringMap.insert(AvdDialog::Tablet, "Tablet");
+    deviceTypeToStringMap.insert(AvdDialog::Automotive, "Automotive");
+    deviceTypeToStringMap.insert(AvdDialog::TV, "TV");
+    deviceTypeToStringMap.insert(AvdDialog::Wear, "Wear");
+
+    m_avdDialog.abiComboBox->addItems(QStringList({
+                                           ProjectExplorer::Constants::ANDROID_ABI_X86,
+                                           ProjectExplorer::Constants::ANDROID_ABI_X86_64,
+                                           ProjectExplorer::Constants::ANDROID_ABI_ARMEABI_V7A,
+                                           ProjectExplorer::Constants::ANDROID_ABI_ARM64_V8A}));
 
     auto v = new QRegularExpressionValidator(m_allowedNameChars, this);
     m_avdDialog.nameLineEdit->setValidator(v);
@@ -77,46 +86,70 @@ AvdDialog::AvdDialog(int minApiLevel, AndroidSdkManager *sdkManager, const QStri
     m_avdDialog.warningText->setType(Utils::InfoLabel::Warning);
     m_avdDialog.warningText->setElideMode(Qt::ElideNone);
 
-    connect(&m_hideTipTimer, &QTimer::timeout, this, []() { Utils::ToolTip::hide(); });
-
     parseDeviceDefinitionsList();
-    for (const QString &type : DeviceTypeToStringMap)
+    for (const QString &type : deviceTypeToStringMap)
         m_avdDialog.deviceDefinitionTypeComboBox->addItem(type);
 
-    connect(m_avdDialog.deviceDefinitionTypeComboBox,
-            QOverload<int>::of(&QComboBox::currentIndexChanged),
-            this,
-            &AvdDialog::updateDeviceDefinitionComboBox);
-    connect(m_avdDialog.abiComboBox,
-            QOverload<int>::of(&QComboBox::currentIndexChanged),
-            this, &AvdDialog::updateApiLevelComboBox);
-
-    m_avdDialog.deviceDefinitionTypeComboBox->setCurrentIndex(1); // Set Phone type as default index
     updateApiLevelComboBox();
 }
 
+int AvdDialog::exec()
+{
+    const int execResult = QDialog::exec();
+    if (execResult == QDialog::Accepted) {
+        CreateAvdInfo result;
+        result.systemImage = systemImage();
+        result.name = name();
+        result.abi = abi();
+        result.deviceDefinition = deviceDefinition();
+        result.sdcardSize = sdcardSize();
+        result.overwrite = m_avdDialog.overwriteCheckBox->isChecked();
+
+        const AndroidAvdManager avdManager = AndroidAvdManager(m_androidConfig);
+        QFutureWatcher<CreateAvdInfo> createAvdFutureWatcher;
+        createAvdFutureWatcher.setFuture(avdManager.createAvd(result));
+
+        QEventLoop loop;
+        QObject::connect(&createAvdFutureWatcher, &QFutureWatcher<CreateAvdInfo>::finished,
+                         &loop, &QEventLoop::quit);
+        QObject::connect(&createAvdFutureWatcher, &QFutureWatcher<CreateAvdInfo>::canceled,
+                         &loop, &QEventLoop::quit);
+        loop.exec(QEventLoop::ExcludeUserInputEvents);
+
+        const QFuture<CreateAvdInfo> future = createAvdFutureWatcher.future();
+        if (future.isResultReadyAt(0))
+            m_createdAvdInfo = future.result();
+    }
+
+    return execResult;
+}
+
 bool AvdDialog::isValid() const
 {
     return !name().isEmpty() && systemImage() && systemImage()->isValid() && !abi().isEmpty();
 }
 
-CreateAvdInfo AvdDialog::gatherCreateAVDInfo(QWidget *parent, AndroidSdkManager *sdkManager,
+ProjectExplorer::IDevice::Ptr AvdDialog::device() const
-                                             const AndroidConfig &config, int minApiLevel, const QStringList &abis)
 {
-    CreateAvdInfo result;
+    AndroidDevice *dev = new AndroidDevice();
-    AvdDialog d(minApiLevel, sdkManager, abis, config, parent);
+    const Utils::Id deviceId = AndroidDevice::idFromAvdInfo(m_createdAvdInfo);
-    result.cancelled = (d.exec() != QDialog::Accepted);
+    using namespace ProjectExplorer;
-    if (result.cancelled || !d.isValid())
+    dev->setupId(IDevice::AutoDetected, deviceId);
-        return result;
+    dev->setMachineType(IDevice::Emulator);
+    dev->setDisplayName(m_createdAvdInfo.name);
+    dev->setDeviceState(IDevice::DeviceConnected);
+    dev->setExtraData(Constants::AndroidAvdName, m_createdAvdInfo.name);
+    dev->setExtraData(Constants::AndroidCpuAbi, {m_createdAvdInfo.abi});
+    if (!m_createdAvdInfo.systemImage) {
+        qCWarning(avdDialogLog) << "System image of the created AVD is nullptr";
+        return IDevice::Ptr();
+    }
+    dev->setExtraData(Constants::AndroidSdk, m_createdAvdInfo.systemImage->apiLevel());
+    dev->setExtraData(Constants::AndroidAvdSdcard, QString("%1 MB")
+                      .arg(m_createdAvdInfo.sdcardSize));
+    dev->setExtraData(Constants::AndroidAvdDevice, m_createdAvdInfo.deviceDefinition);
 
-    result.systemImage = d.systemImage();
+    return IDevice::Ptr(dev);
-    result.name = d.name();
-    result.abi = d.abi();
-    result.deviceDefinition = d.deviceDefinition();
-    result.sdcardSize = d.sdcardSize();
-    result.overwrite = d.m_avdDialog.overwriteCheckBox->isChecked();
-
-    return result;
 }
 
 AvdDialog::DeviceType AvdDialog::tagToDeviceType(const QString &type_tag)
@@ -175,7 +208,7 @@ void AvdDialog::parseDeviceDefinitionsList()
 
 void AvdDialog::updateDeviceDefinitionComboBox()
 {
-    DeviceType curDeviceType = DeviceTypeToStringMap.key(
+    DeviceType curDeviceType = deviceTypeToStringMap.key(
         m_avdDialog.deviceDefinitionTypeComboBox->currentText());
 
     m_avdDialog.deviceDefinitionComboBox->clear();
@@ -215,8 +248,8 @@ int AvdDialog::sdcardSize() const
 
 void AvdDialog::updateApiLevelComboBox()
 {
-    SystemImageList installedSystemImages = m_sdkManager->installedSystemImages();
+    SystemImageList installedSystemImages = m_sdkManager.installedSystemImages();
-    DeviceType curDeviceType = DeviceTypeToStringMap.key(
+    DeviceType curDeviceType = deviceTypeToStringMap.key(
         m_avdDialog.deviceDefinitionTypeComboBox->currentText());
 
     QString selectedAbi = abi();
@@ -250,8 +283,7 @@ void AvdDialog::updateApiLevelComboBox()
         m_avdDialog.warningText->setVisible(true);
         m_avdDialog.warningText->setText(
             tr("Cannot create a new AVD. No suitable Android system image is installed.<br/>"
-               "Install a system image of at least API version %1 from the SDK Manager tab.")
+               "Install a system image for the intended Android version from the SDK Manager."));
-                .arg(m_minApiLevel));
         m_avdDialog.buttonBox->button(QDialogButtonBox::Ok)->setEnabled(false);
     } else if (filteredList.isEmpty()) {
         m_avdDialog.targetApiComboBox->setEnabled(false);

src/plugins/android/avddialog.h

@@ -41,21 +41,27 @@ class AvdDialog : public QDialog
 {
     Q_OBJECT
 public:
-    explicit AvdDialog(int minApiLevel,
+    explicit AvdDialog(const AndroidConfig &config, QWidget *parent = nullptr);
-                       AndroidSdkManager *sdkManager,
+    int exec() override;
-                       const QStringList &abis,
-                       const AndroidConfig &config,
-                       QWidget *parent = nullptr);
 
-    enum DeviceType { TV, Phone, Wear, Tablet, Automotive, PhoneOrTablet };
+    enum DeviceType { Phone, Tablet, Automotive, TV, Wear, PhoneOrTablet };
 
-    const QMap<DeviceType, QString> DeviceTypeToStringMap{
+    ProjectExplorer::IDevice::Ptr device() const;
-        {TV,            "TV"},
+
-        {Phone,         "Phone"},
+    const SystemImage *systemImage() const;
-        {Wear,          "Wear"},
+    QString name() const;
-        {Tablet,        "Tablet"},
+    QString abi() const;
-        {Automotive,    "Automotive"}
+    QString deviceDefinition() const;
-    };
+    int sdcardSize() const;
+    bool isValid() const;
+
+private:
+    void parseDeviceDefinitionsList();
+    void updateDeviceDefinitionComboBox();
+    void updateApiLevelComboBox();
+    bool eventFilter(QObject *obj, QEvent *event) override;
+
+    static AvdDialog::DeviceType tagToDeviceType(const QString &type_tag);
 
     struct DeviceDefinitionStruct
     {
@@ -64,30 +70,14 @@ public:
         DeviceType deviceType;
     };
 
-    const SystemImage *systemImage() const;
-    QString name() const;
-    QString abi() const;
-    QString deviceDefinition() const;
-    int sdcardSize() const;
-    bool isValid() const;
-    static AvdDialog::DeviceType tagToDeviceType(const QString &type_tag);
-    static CreateAvdInfo gatherCreateAVDInfo(QWidget *parent, AndroidSdkManager *sdkManager,
-                                             const AndroidConfig &config,
-                                             int minApiLevel = 0, const QStringList &abis = {});
-
-private:
-    void parseDeviceDefinitionsList();
-    void updateDeviceDefinitionComboBox();
-    void updateApiLevelComboBox();
-    bool eventFilter(QObject *obj, QEvent *event) override;
-
     Ui::AddNewAVDDialog m_avdDialog;
-    AndroidSdkManager *m_sdkManager;
+    AndroidSdkManager m_sdkManager;
-    int m_minApiLevel;
+    CreateAvdInfo m_createdAvdInfo;
     QTimer m_hideTipTimer;
     QRegularExpression m_allowedNameChars;
     QList<DeviceDefinitionStruct> m_deviceDefinitionsList;
     AndroidConfig m_androidConfig;
+    QMap<AvdDialog::DeviceType, QString> deviceTypeToStringMap;
 };
 }
 }