Android: Do some cleanup

Cleanup after employing task tree...

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

src/plugins/android/androidsdkmanager.cpp

@@ -15,16 +15,13 @@
 #include <utils/layoutbuilder.h>
 #include <utils/outputformatter.h>
 #include <utils/qtcprocess.h>
-#include <utils/qtcassert.h>
-#include <utils/stringutils.h>
 
-#include <QFutureWatcher>
 #include <QDialogButtonBox>
+#include <QLabel>
 #include <QLoggingCategory>
 #include <QMessageBox>
 #include <QPlainTextEdit>
 #include <QProgressBar>
-#include <QReadWriteLock>
 #include <QRegularExpression>
 #include <QTextCodec>
 
@@ -39,8 +36,7 @@ using namespace Utils;
 using namespace std::chrono;
 using namespace std::chrono_literals;
 
-namespace Android {
+namespace Android::Internal {
-namespace Internal {
 
 class QuestionProgressDialog : public QDialog
 {
@@ -124,7 +120,7 @@ static QString sdkRootArg(const AndroidConfig &config)
     return "--sdk_root=" + config.sdkLocation().toString();
 }
 
-static const QRegularExpression &assertionRegExp()
+const QRegularExpression &assertionRegExp()
 {
     static const QRegularExpression theRegExp
         (R"((\(\s*y\s*[\/\\]\s*n\s*\)\s*)(?<mark>[\:\?]))", // (y/N)?
@@ -329,49 +325,11 @@ static GroupItem updateRecipe(const Storage<DialogStorage> &dialogStorage)
     return ProcessTask(onUpdateSetup, onDone);
 }
 
-const int sdkManagerCmdTimeoutS = 60;
-const int sdkManagerOperationTimeoutS = 600;
-
-using SdkCmdPromise = QPromise<AndroidSdkManager::OperationOutput>;
-
-int parseProgress(const QString &out, bool &foundAssertion)
-{
-    int progress = -1;
-    if (out.isEmpty())
-        return progress;
-    static const QRegularExpression reg("(?<progress>\\d*)%");
-    static const QRegularExpression regEndOfLine("[\\n\\r]");
-    const QStringList lines = out.split(regEndOfLine, Qt::SkipEmptyParts);
-    for (const QString &line : lines) {
-        QRegularExpressionMatch match = reg.match(line);
-        if (match.hasMatch()) {
-            progress = match.captured("progress").toInt();
-            if (progress < 0 || progress > 100)
-                progress = -1;
-        }
-        if (!foundAssertion)
-            foundAssertion = assertionRegExp().match(line).hasMatch();
-    }
-    return progress;
-}
-
-void watcherDeleter(QFutureWatcher<void> *watcher)
-{
-    if (!watcher->isFinished() && !watcher->isCanceled())
-        watcher->cancel();
-
-    if (!watcher->isFinished())
-        watcher->waitForFinished();
-
-    delete watcher;
-}
-
 /*!
     Runs the \c sdkmanger tool with arguments \a args. Returns \c true if the command is
     successfully executed. Output is copied into \a output. The function blocks the calling thread.
  */
-static bool sdkManagerCommand(const AndroidConfig &config, const QStringList &args,
+static bool sdkManagerCommand(const AndroidConfig &config, const QStringList &args, QString *output)
-                              QString *output, int timeout = sdkManagerCmdTimeoutS)
 {
     QStringList newArgs = args;
     newArgs.append(sdkRootArg(config));
@@ -382,60 +340,12 @@ static bool sdkManagerCommand(const AndroidConfig &config, const QStringList &ar
     proc.setEnvironment(config.toolsEnvironment());
     proc.setTimeOutMessageBoxEnabled(true);
     proc.setCommand({config.sdkManagerToolPath(), newArgs});
-    proc.runBlocking(seconds(timeout), EventLoopMode::On);
+    proc.runBlocking(60s, EventLoopMode::On);
     if (output)
         *output = proc.allOutput();
     return proc.result() == ProcessResult::FinishedWithSuccess;
 }
 
-/*!
-    Runs the \c sdkmanger tool with arguments \a args. The operation command progress is updated in
-    to the future interface \a fi and \a output is populated with command output. The command listens
-    to cancel signal emmitted by \a sdkManager and kill the commands. The command is also killed
-    after the lapse of \a timeout seconds. The function blocks the calling thread.
- */
-static void sdkManagerCommand(const AndroidConfig &config, const QStringList &args,
-                              AndroidSdkManager &sdkManager, SdkCmdPromise &promise,
-                              AndroidSdkManager::OperationOutput &output, double progressQuota,
-                              bool interruptible = true, int timeout = sdkManagerOperationTimeoutS)
-{
-    QStringList newArgs = args;
-    newArgs.append(sdkRootArg(config));
-    qCDebug(sdkManagerLog).noquote() << "Running SDK Manager command (async):"
-                                     << CommandLine(config.sdkManagerToolPath(), newArgs)
-                                        .toUserOutput();
-    int offset = promise.future().progressValue();
-    Process proc;
-    proc.setEnvironment(config.toolsEnvironment());
-    bool assertionFound = false;
-    proc.setStdOutCallback([offset, progressQuota, &proc, &assertionFound, &promise](const QString &out) {
-        int progressPercent = parseProgress(out, assertionFound);
-        if (assertionFound)
-            proc.stop();
-        if (progressPercent != -1)
-            promise.setProgressValue(offset + qRound((progressPercent / 100.0) * progressQuota));
-    });
-    proc.setStdErrCallback([&output](const QString &err) {
-        output.stdError = err;
-    });
-    if (interruptible) {
-        QObject::connect(&sdkManager, &AndroidSdkManager::cancelActiveOperations, &proc, [&proc] {
-           proc.stop();
-           proc.waitForFinished();
-        });
-    }
-    proc.setCommand({config.sdkManagerToolPath(), newArgs});
-    proc.runBlocking(seconds(timeout), EventLoopMode::On);
-    if (assertionFound) {
-        output.success = false;
-        output.stdOutput = proc.cleanedStdOut();
-        output.stdError = Tr::tr("The operation requires user interaction. "
-                                 "Use the \"sdkmanager\" command-line tool.");
-    } else {
-        output.success = proc.result() == ProcessResult::FinishedWithSuccess;
-    }
-}
-
 class AndroidSdkManagerPrivate
 {
 public:
@@ -453,20 +363,8 @@ public:
     const AndroidSdkPackageList &allPackages();
 
     void parseCommonArguments(QPromise<QString> &promise);
-    void updateInstalled(SdkCmdPromise &fi);
-    void updatePackages(SdkCmdPromise &fi, const InstallationChange &change);
-    void licenseCheck(SdkCmdPromise &fi);
-    void licenseWorkflow(SdkCmdPromise &fi);
 
-    void addWatcher(const QFuture<AndroidSdkManager::OperationOutput> &future);
-    void setLicenseInput(bool acceptLicense);
-
-    std::unique_ptr<QFutureWatcher<void>, decltype(&watcherDeleter)> m_activeOperation;
-
-    QByteArray getUserInput() const;
-    void clearUserInput();
     void reloadSdkPackages();
-    void clearPackages();
 
     void runDialogRecipe(const Storage<DialogStorage> &dialogStorage,
                          const GroupItem &licenseRecipe, const GroupItem &continuationRecipe);
@@ -474,21 +372,13 @@ public:
     AndroidSdkManager &m_sdkManager;
     AndroidSdkPackageList m_allPackages;
     FilePath lastSdkManagerPath;
-    QByteArray m_licenseUserInput;
-    mutable QReadWriteLock m_licenseInputLock;
     bool m_packageListingSuccessful = false;
     TaskTreeRunner m_taskTreeRunner;
 };
 
-AndroidSdkManager::AndroidSdkManager()
+AndroidSdkManager::AndroidSdkManager() : m_d(new AndroidSdkManagerPrivate(*this)) {}
-    : m_d(new AndroidSdkManagerPrivate(*this))
-{
-}
 
-AndroidSdkManager::~AndroidSdkManager()
+AndroidSdkManager::~AndroidSdkManager() = default;
-{
-    cancelOperatons();
-}
 
 SdkPlatformList AndroidSdkManager::installedSdkPlatforms()
 {
@@ -537,14 +427,12 @@ SystemImageList AndroidSdkManager::installedSystemImages()
 {
     const AndroidSdkPackageList list = m_d->filteredPackages(AndroidSdkPackage::AnyValidState,
                                                              AndroidSdkPackage::SdkPlatformPackage);
-    QList<SdkPlatform *> platforms = Utils::static_container_cast<SdkPlatform *>(list);
+    const QList<SdkPlatform *> platforms = Utils::static_container_cast<SdkPlatform *>(list);
-
     SystemImageList result;
     for (SdkPlatform *platform : platforms) {
         if (platform->systemImages().size() > 0)
             result.append(platform->systemImages());
     }
-
     return result;
 }
 
@@ -573,7 +461,6 @@ SdkPlatformList AndroidSdkManager::filteredSdkPlatforms(int minApiLevel,
 {
     const AndroidSdkPackageList list = m_d->filteredPackages(state,
                                                              AndroidSdkPackage::SdkPlatformPackage);
-
     SdkPlatformList result;
     for (AndroidSdkPackage *p : list) {
         auto platform = static_cast<SdkPlatform *>(p);
@@ -608,11 +495,6 @@ void AndroidSdkManager::reloadPackages()
     m_d->reloadSdkPackages();
 }
 
-bool AndroidSdkManager::isBusy() const
-{
-    return m_d->m_activeOperation && !m_d->m_activeOperation->isFinished();
-}
-
 bool AndroidSdkManager::packageListingSuccessful() const
 {
     return m_d->m_packageListingSuccessful;
@@ -623,62 +505,13 @@ QFuture<QString> AndroidSdkManager::availableArguments() const
     return Utils::asyncRun(&AndroidSdkManagerPrivate::parseCommonArguments, m_d.get());
 }
 
-QFuture<AndroidSdkManager::OperationOutput> AndroidSdkManager::updateInstalled()
+AndroidSdkManagerPrivate::AndroidSdkManagerPrivate(AndroidSdkManager &sdkManager)
-{
+    : m_sdkManager(sdkManager)
-    if (isBusy()) {
-        return QFuture<AndroidSdkManager::OperationOutput>();
-    }
-    auto future = Utils::asyncRun(&AndroidSdkManagerPrivate::updateInstalled, m_d.get());
-    m_d->addWatcher(future);
-    return future;
-}
-
-QFuture<AndroidSdkManager::OperationOutput> AndroidSdkManager::updatePackages(const InstallationChange &change)
-{
-    if (isBusy())
-        return QFuture<AndroidSdkManager::OperationOutput>();
-    auto future = Utils::asyncRun(&AndroidSdkManagerPrivate::updatePackages, m_d.get(), change);
-    m_d->addWatcher(future);
-    return future;
-}
-
-QFuture<AndroidSdkManager::OperationOutput> AndroidSdkManager::licenseCheck()
-{
-    if (isBusy())
-        return QFuture<AndroidSdkManager::OperationOutput>();
-    auto future = Utils::asyncRun(&AndroidSdkManagerPrivate::licenseCheck, m_d.get());
-    m_d->addWatcher(future);
-    return future;
-}
-
-QFuture<AndroidSdkManager::OperationOutput> AndroidSdkManager::licenseWorkflow()
-{
-    if (isBusy())
-        return QFuture<AndroidSdkManager::OperationOutput>();
-    auto future = Utils::asyncRun(&AndroidSdkManagerPrivate::licenseWorkflow, m_d.get());
-    m_d->addWatcher(future);
-    return future;
-}
-
-void AndroidSdkManager::cancelOperatons()
-{
-    emit cancelActiveOperations();
-    m_d->m_activeOperation.reset();
-}
-
-void AndroidSdkManager::acceptSdkLicense(bool accept)
-{
-    m_d->setLicenseInput(accept);
-}
-
-AndroidSdkManagerPrivate::AndroidSdkManagerPrivate(AndroidSdkManager &sdkManager):
-    m_activeOperation(nullptr, watcherDeleter),
-    m_sdkManager(sdkManager)
 {}
 
 AndroidSdkManagerPrivate::~AndroidSdkManagerPrivate()
 {
-    clearPackages();
+    qDeleteAll(m_allPackages);
 }
 
 const AndroidSdkPackageList &AndroidSdkManagerPrivate::allPackages()
@@ -690,7 +523,8 @@ const AndroidSdkPackageList &AndroidSdkManagerPrivate::allPackages()
 void AndroidSdkManagerPrivate::reloadSdkPackages()
 {
     emit m_sdkManager.packageReloadBegin();
-    clearPackages();
+    qDeleteAll(m_allPackages);
+    m_allPackages.clear();
 
     lastSdkManagerPath = androidConfig().sdkManagerToolPath();
     m_packageListingSuccessful = false;
@@ -715,194 +549,6 @@ void AndroidSdkManagerPrivate::reloadSdkPackages()
     emit m_sdkManager.packageReloadFinished();
 }
 
-void AndroidSdkManagerPrivate::updateInstalled(SdkCmdPromise &promise)
-{
-    promise.setProgressRange(0, 100);
-    promise.setProgressValue(0);
-    AndroidSdkManager::OperationOutput result;
-    result.type = AndroidSdkManager::UpdateInstalled;
-    result.stdOutput = Tr::tr("Updating installed packages.");
-    promise.addResult(result);
-    QStringList args("--update");
-    args << androidConfig().sdkManagerToolArgs();
-    if (!promise.isCanceled())
-        sdkManagerCommand(androidConfig(), args, m_sdkManager, promise, result, 100);
-    else
-        qCDebug(sdkManagerLog) << "Update: Operation cancelled before start";
-
-    if (result.stdError.isEmpty() && !result.success)
-        result.stdError = Tr::tr("Failed.");
-    result.stdOutput = Tr::tr("Done") + "\n\n";
-    promise.addResult(result);
-    promise.setProgressValue(100);
-}
-
-void AndroidSdkManagerPrivate::updatePackages(SdkCmdPromise &fi, const InstallationChange &change)
-{
-    fi.setProgressRange(0, 100);
-    fi.setProgressValue(0);
-    double progressQuota = 100.0 / change.count();
-    int currentProgress = 0;
-
-    QString installTag = Tr::tr("Installing");
-    QString uninstallTag = Tr::tr("Uninstalling");
-
-    auto doOperation = [&](const QString& packagePath, const QStringList& args,
-            bool isInstall) {
-        AndroidSdkManager::OperationOutput result;
-        result.type = AndroidSdkManager::UpdatePackages;
-        result.stdOutput = QString("%1 %2").arg(isInstall ? installTag : uninstallTag)
-                .arg(packagePath);
-        fi.addResult(result);
-        if (fi.isCanceled())
-            qCDebug(sdkManagerLog) << args << "Update: Operation cancelled before start";
-        else
-            sdkManagerCommand(androidConfig(), args, m_sdkManager, fi, result, progressQuota, isInstall);
-        currentProgress += progressQuota;
-        fi.setProgressValue(currentProgress);
-        if (result.stdError.isEmpty() && !result.success)
-            result.stdError = Tr::tr("Failed");
-        result.stdOutput = Tr::tr("Done") + "\n\n";
-        fi.addResult(result);
-        return fi.isCanceled();
-    };
-
-
-    // Uninstall packages
-    for (const QString &sdkStylePath : change.toUninstall) {
-        // Uninstall operations are not interptible. We don't want to leave half uninstalled.
-        QStringList args;
-        args << "--uninstall" << sdkStylePath << androidConfig().sdkManagerToolArgs();
-        if (doOperation(sdkStylePath, args, false))
-            break;
-    }
-
-    // Install packages
-    for (const QString &sdkStylePath : change.toInstall) {
-        QStringList args(sdkStylePath);
-        args << androidConfig().sdkManagerToolArgs();
-        if (doOperation(sdkStylePath, args, true))
-            break;
-    }
-    fi.setProgressValue(100);
-}
-
-void AndroidSdkManagerPrivate::licenseCheck(SdkCmdPromise &fi)
-{
-    fi.setProgressRange(0, 100);
-    fi.setProgressValue(0);
-    AndroidSdkManager::OperationOutput result;
-    result.type = AndroidSdkManager::LicenseCheck;
-    if (!fi.isCanceled()) {
-        const int timeOutS = 4; // Short timeout as workaround for QTCREATORBUG-25667
-        sdkManagerCommand(androidConfig(), {"--licenses"}, m_sdkManager, fi, result, 100.0, true,
-                          timeOutS);
-    } else {
-        qCDebug(sdkManagerLog) << "Update: Operation cancelled before start";
-    }
-
-    fi.addResult(result);
-    fi.setProgressValue(100);
-}
-
-void AndroidSdkManagerPrivate::licenseWorkflow(SdkCmdPromise &fi)
-{
-    fi.setProgressRange(0, 100);
-    fi.setProgressValue(0);
-
-    AndroidSdkManager::OperationOutput result;
-    result.type = AndroidSdkManager::LicenseWorkflow;
-
-    Process licenseCommand;
-    licenseCommand.setProcessMode(ProcessMode::Writer);
-    licenseCommand.setEnvironment(androidConfig().toolsEnvironment());
-    bool reviewingLicenses = false;
-    licenseCommand.setCommand(CommandLine(androidConfig().sdkManagerToolPath(),
-                                          {"--licenses", sdkRootArg(androidConfig())}));
-    licenseCommand.setUseCtrlCStub(true);
-    licenseCommand.start();
-    QTextCodec *codec = QTextCodec::codecForLocale();
-    int inputCounter = 0, steps = -1;
-    QString licenseTextCache;
-    while (!licenseCommand.waitForFinished(200ms)) {
-        const QString stdOut = codec->toUnicode(licenseCommand.readAllRawStandardOutput());
-        bool assertion = false;
-        if (!stdOut.isEmpty()) {
-            licenseTextCache.append(stdOut);
-            assertion = assertionRegExp().match(licenseTextCache).hasMatch();
-            if (assertion) {
-                if (reviewingLicenses) {
-                    result.stdOutput = licenseTextCache;
-                    fi.addResult(result);
-                }
-                licenseTextCache.clear();
-            }
-        }
-
-        if (reviewingLicenses) {
-            // Check user input
-            QByteArray userInput = getUserInput();
-            if (!userInput.isEmpty()) {
-                clearUserInput();
-                licenseCommand.writeRaw(userInput);
-                ++inputCounter;
-                if (steps != -1)
-                    fi.setProgressValue(qRound((inputCounter / (double)steps) * 100));
-            }
-        } else if (assertion) {
-            // The first assertion is to start reviewing licenses. Always accept.
-            reviewingLicenses = true;
-            static const QRegularExpression reg(R"((\d+\sof\s)(?<steps>\d+))");
-            QRegularExpressionMatch match = reg.match(stdOut);
-            if (match.hasMatch())
-                steps = match.captured("steps").toInt();
-            licenseCommand.write("Y\n");
-        }
-
-        if (fi.isCanceled()) {
-            licenseCommand.terminate();
-            if (!licenseCommand.waitForFinished(300ms)) {
-                licenseCommand.kill();
-                licenseCommand.waitForFinished(200ms);
-            }
-        }
-        if (licenseCommand.state() == QProcess::NotRunning)
-            break;
-    }
-
-    result.success = licenseCommand.exitStatus() == QProcess::NormalExit;
-    if (!result.success)
-        result.stdError = Tr::tr("License command failed.") + "\n\n";
-    fi.addResult(result);
-    fi.setProgressValue(100);
-}
-
-void AndroidSdkManagerPrivate::setLicenseInput(bool acceptLicense)
-{
-    QWriteLocker locker(&m_licenseInputLock);
-    m_licenseUserInput = acceptLicense ? "Y\n" : "n\n";
-}
-
-QByteArray AndroidSdkManagerPrivate::getUserInput() const
-{
-    QReadLocker locker(&m_licenseInputLock);
-    return m_licenseUserInput;
-}
-
-void AndroidSdkManagerPrivate::clearUserInput()
-{
-    QWriteLocker locker(&m_licenseInputLock);
-    m_licenseUserInput.clear();
-}
-
-void AndroidSdkManagerPrivate::addWatcher(const QFuture<AndroidSdkManager::OperationOutput> &future)
-{
-    if (future.isFinished())
-        return;
-    m_activeOperation.reset(new QFutureWatcher<void>());
-    m_activeOperation->setFuture(QFuture<void>(future));
-}
-
 void AndroidSdkManagerPrivate::parseCommonArguments(QPromise<QString> &promise)
 {
     QString argumentDetails;
@@ -923,13 +569,6 @@ void AndroidSdkManagerPrivate::parseCommonArguments(QPromise<QString> &promise)
         promise.addResult(argumentDetails);
 }
 
-void AndroidSdkManagerPrivate::clearPackages()
-{
-    for (AndroidSdkPackage *p : std::as_const(m_allPackages))
-        delete p;
-    m_allPackages.clear();
-}
-
 void AndroidSdkManagerPrivate::runDialogRecipe(const Storage<DialogStorage> &dialogStorage,
                                                const GroupItem &licensesRecipe,
                                                const GroupItem &continuationRecipe)
@@ -991,7 +630,6 @@ void AndroidSdkManager::runUpdate()
     m_d->runDialogRecipe(dialogStorage, licensesRecipe(dialogStorage), updateRecipe(dialogStorage));
 }
 
-} // namespace Internal
+} // namespace Android::Internal
-} // namespace Android
 
 #include "androidsdkmanager.moc"

src/plugins/android/androidsdkmanager.h

@@ -4,18 +4,18 @@
 
 #include "androidsdkpackage.h"
 
-#include <utils/fileutils.h>
+#include <utils/filepath.h>
 
 #include <QObject>
 #include <QFuture>
 
 #include <memory>
 
-namespace Android {
+QT_BEGIN_NAMESPACE
+class QRegularExpression;
+QT_END_MOC_NAMESPACE
 
-class AndroidConfig;
+namespace Android::Internal {
-
-namespace Internal {
 
 class AndroidSdkManagerPrivate;
 
@@ -29,26 +29,10 @@ struct InstallationChange
 class AndroidSdkManager : public QObject
 {
     Q_OBJECT
+
 public:
-    enum CommandType
-    {
-        None,
-        UpdateInstalled,
-        UpdatePackages,
-        LicenseCheck,
-        LicenseWorkflow
-    };
-
-    struct OperationOutput
-    {
-        bool success = false;
-        CommandType type = None;
-        QString stdOutput;
-        QString stdError;
-    };
-
     AndroidSdkManager();
-    ~AndroidSdkManager() override;
+    ~AndroidSdkManager();
 
     SdkPlatformList installedSdkPlatforms();
     const AndroidSdkPackageList &allSdkPackages();
@@ -68,18 +52,10 @@ public:
                                       = AndroidSdkPackage::Installed);
     void refreshPackages();
     void reloadPackages();
-    bool isBusy() const;
 
     bool packageListingSuccessful() const;
 
     QFuture<QString> availableArguments() const;
-    QFuture<OperationOutput> updateInstalled();
-    QFuture<OperationOutput> updatePackages(const InstallationChange &change);
-    QFuture<OperationOutput> licenseCheck();
-    QFuture<OperationOutput> licenseWorkflow();
-
-    void cancelOperatons();
-    void acceptSdkLicense(bool accept);
 
     void runInstallationChange(const InstallationChange &change, const QString &extraMessage = {});
     void runUpdate();
@@ -87,13 +63,12 @@ public:
 signals:
     void packageReloadBegin();
     void packageReloadFinished();
-    void cancelActiveOperations();
 
 private:
     friend class AndroidSdkManagerPrivate;
     std::unique_ptr<AndroidSdkManagerPrivate> m_d;
 };
 
-int parseProgress(const QString &out, bool &foundAssertion);
+const QRegularExpression &assertionRegExp();
-} // namespace Internal
+
-} // namespace Android
+} // namespace Android::Internal

src/plugins/android/androidsdkmanager_test.cpp

@@ -51,6 +51,27 @@ void AndroidSdkManagerTest::testAndroidSdkManagerProgressParser_data()
         << true;
 }
 
+static int parseProgress(const QString &out, bool &foundAssertion)
+{
+    int progress = -1;
+    if (out.isEmpty())
+        return progress;
+    static const QRegularExpression reg("(?<progress>\\d*)%");
+    static const QRegularExpression regEndOfLine("[\\n\\r]");
+    const QStringList lines = out.split(regEndOfLine, Qt::SkipEmptyParts);
+    for (const QString &line : lines) {
+        QRegularExpressionMatch match = reg.match(line);
+        if (match.hasMatch()) {
+            progress = match.captured("progress").toInt();
+            if (progress < 0 || progress > 100)
+                progress = -1;
+        }
+        if (!foundAssertion)
+            foundAssertion = assertionRegExp().match(line).hasMatch();
+    }
+    return progress;
+}
+
 void AndroidSdkManagerTest::testAndroidSdkManagerProgressParser()
 {
     QFETCH(QString, output);

src/plugins/android/androidsdkmanagerwidget.cpp

@@ -7,30 +7,25 @@
 #include "androidsdkmodel.h"
 #include "androidtr.h"
 
-#include <coreplugin/icore.h>
-
 #include <utils/async.h>
 #include <utils/layoutbuilder.h>
-#include <utils/outputformatter.h>
 #include <utils/qtcassert.h>
-#include <utils/utilsicons.h>
 
-#include <QAbstractButton>
+#include <QCheckBox>
 #include <QDialogButtonBox>
-#include <QGuiApplication>
+#include <QHeaderView>
-#include <QLabel>
 #include <QLineEdit>
-#include <QLoggingCategory>
+#include <QPlainTextEdit>
-#include <QMessageBox>
+#include <QPushButton>
+#include <QRadioButton>
 #include <QSortFilterProxyModel>
+#include <QTreeView>
 
 using namespace Utils;
 using namespace std::placeholders;
 
 namespace Android::Internal {
 
-static Q_LOGGING_CATEGORY(androidSdkMgrUiLog, "qtc.android.sdkManagerUi", QtWarningMsg)
-
 class PackageFilterModel : public QSortFilterProxyModel
 {
 public:
@@ -45,10 +40,10 @@ private:
     QString m_searchText;
 };
 
-AndroidSdkManagerWidget::AndroidSdkManagerWidget(AndroidSdkManager *sdkManager, QWidget *parent) :
+AndroidSdkManagerWidget::AndroidSdkManagerWidget(AndroidSdkManager *sdkManager, QWidget *parent)
-    QDialog(parent),
+    : QDialog(parent)
-    m_sdkManager(sdkManager),
+    , m_sdkManager(sdkManager)
-    m_sdkModel(new AndroidSdkModel(m_sdkManager, this))
+    , m_sdkModel(new AndroidSdkModel(m_sdkManager, this))
 {
     QTC_CHECK(sdkManager);
 
@@ -56,9 +51,7 @@ AndroidSdkManagerWidget::AndroidSdkManagerWidget(AndroidSdkManager *sdkManager,
     resize(664, 396);
     setModal(true);
 
-    m_packagesStack = new QWidget;
+    auto packagesView = new QTreeView;
-
-    auto packagesView = new QTreeView(m_packagesStack);
     packagesView->setIndentation(20);
     packagesView->header()->setCascadingSectionResizes(false);
 
@@ -80,38 +73,15 @@ AndroidSdkManagerWidget::AndroidSdkManagerWidget(AndroidSdkManager *sdkManager,
 
     auto optionsButton = new QPushButton(Tr::tr("Advanced Options..."));
 
-    auto searchField = new FancyLineEdit(m_packagesStack);
+    auto searchField = new FancyLineEdit;
     searchField->setPlaceholderText("Filter");
 
     auto expandCheck = new QCheckBox(Tr::tr("Expand All"));
 
-    m_outputStack = new QWidget;
+    m_buttonBox = new QDialogButtonBox;
-    m_operationProgress = new QProgressBar(m_outputStack);
-
-    m_outputEdit = new QPlainTextEdit(m_outputStack);
-    m_outputEdit->setReadOnly(true);
-
-    m_sdkLicenseLabel = new QLabel(Tr::tr("Do you want to accept the Android SDK license?"));
-    m_sdkLicenseLabel->setAlignment(Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter);
-    m_sdkLicenseLabel->hide();
-
-    m_sdkLicenseButtonBox = new QDialogButtonBox(m_outputStack);
-    m_sdkLicenseButtonBox->setEnabled(false);
-    m_sdkLicenseButtonBox->setStandardButtons(QDialogButtonBox::No|QDialogButtonBox::Yes);
-    m_sdkLicenseButtonBox->hide();
-
-    m_buttonBox = new QDialogButtonBox(this);
     m_buttonBox->setStandardButtons(QDialogButtonBox::Apply | QDialogButtonBox::Cancel);
     m_buttonBox->button(QDialogButtonBox::Apply)->setEnabled(false);
 
-    m_viewStack = new QStackedWidget(this);
-    m_viewStack->addWidget(m_packagesStack);
-    m_viewStack->addWidget(m_outputStack);
-    m_viewStack->setCurrentWidget(m_packagesStack);
-
-    m_formatter = new OutputFormatter;
-    m_formatter->setPlainTextEdit(m_outputEdit);
-
     auto proxyModel = new PackageFilterModel(m_sdkModel);
     packagesView->setModel(proxyModel);
     packagesView->header()->setSectionResizeMode(QHeaderView::ResizeToContents);
@@ -122,7 +92,6 @@ AndroidSdkManagerWidget::AndroidSdkManagerWidget(AndroidSdkManager *sdkManager,
     using namespace Layouting;
     Grid {
         searchField, expandCheck, br,
-
         Span(2, packagesView),
         Column {
             updateInstalledButton,
@@ -139,34 +108,13 @@ AndroidSdkManagerWidget::AndroidSdkManagerWidget(AndroidSdkManager *sdkManager,
                 }
             },
             optionsButton
-        },
+        }, br,
-        noMargin
+        Span(3, m_buttonBox)
-    }.attachTo(m_packagesStack);
-
-    Column {
-        m_outputEdit,
-        Row { m_sdkLicenseLabel, m_sdkLicenseButtonBox },
-        m_operationProgress,
-        noMargin
-    }.attachTo(m_outputStack);
-
-    Column {
-        m_viewStack,
-        m_buttonBox
     }.attachTo(this);
 
     connect(m_sdkModel, &AndroidSdkModel::dataChanged, this, [this] {
-        if (m_viewStack->currentWidget() == m_packagesStack)
+        m_buttonBox->button(QDialogButtonBox::Apply)
-            m_buttonBox->button(QDialogButtonBox::Apply)
+            ->setEnabled(m_sdkModel->installationChange().count());
-                ->setEnabled(m_sdkModel->installationChange().count());
-    });
-
-    connect(m_sdkModel, &AndroidSdkModel::modelAboutToBeReset, this,
-            [this, expandCheck] {
-        m_buttonBox->button(QDialogButtonBox::Apply)->setEnabled(false);
-        expandCheck->setChecked(false);
-        cancelPendingOperations();
-        switchView(PackageListing);
     });
 
     connect(expandCheck, &QCheckBox::stateChanged, this, [packagesView](int state) {
@@ -183,8 +131,7 @@ AndroidSdkManagerWidget::AndroidSdkManagerWidget(AndroidSdkManager *sdkManager,
             m_sdkModel->resetSelection();
         }
     });
-    connect(showInstalledRadio, &QRadioButton::toggled,
+    connect(showInstalledRadio, &QRadioButton::toggled, this, [this, proxyModel](bool checked) {
-            this, [this, proxyModel](bool checked) {
         if (checked) {
             proxyModel->setAcceptedPackageState(AndroidSdkPackage::Installed);
             m_sdkModel->resetSelection();
@@ -208,17 +155,17 @@ AndroidSdkManagerWidget::AndroidSdkManagerWidget(AndroidSdkManager *sdkManager,
     connect(m_buttonBox->button(QDialogButtonBox::Apply), &QAbstractButton::clicked, this, [this] {
         m_sdkManager->runInstallationChange(m_sdkModel->installationChange());
     });
-    connect(m_buttonBox, &QDialogButtonBox::rejected, this, &AndroidSdkManagerWidget::onCancel);
+    connect(m_buttonBox, &QDialogButtonBox::rejected, this, &AndroidSdkManagerWidget::reject);
 
-    connect(optionsButton, &QPushButton::clicked,
+    connect(optionsButton, &QPushButton::clicked, this, [this] {
-            this, &AndroidSdkManagerWidget::onSdkManagerOptions);
+        OptionsDialog dlg(m_sdkManager, androidConfig().sdkManagerToolArgs(), this);
-    connect(m_sdkLicenseButtonBox, &QDialogButtonBox::accepted, this, [this] {
+        if (dlg.exec() == QDialog::Accepted) {
-        m_sdkManager->acceptSdkLicense(true);
+            QStringList arguments = dlg.sdkManagerArguments();
-        m_sdkLicenseButtonBox->setEnabled(false); // Wait for next license to enable controls
+            if (arguments != androidConfig().sdkManagerToolArgs()) {
-    });
+                androidConfig().setSdkManagerToolArgs(arguments);
-    connect(m_sdkLicenseButtonBox, &QDialogButtonBox::rejected, this, [this] {
+                m_sdkManager->reloadPackages();
-        m_sdkManager->acceptSdkLicense(false);
+            }
-        m_sdkLicenseButtonBox->setEnabled(false); // Wait for next license to enable controls
+        }
     });
 
     connect(obsoleteCheckBox, &QCheckBox::stateChanged, this, [this](int state) {
@@ -264,211 +211,6 @@ AndroidSdkManagerWidget::AndroidSdkManagerWidget(AndroidSdkManager *sdkManager,
     });
 }
 
-AndroidSdkManagerWidget::~AndroidSdkManagerWidget()
-{
-    if (m_currentOperation)
-        delete m_currentOperation;
-    cancelPendingOperations();
-    delete m_formatter;
-}
-
-void AndroidSdkManagerWidget::licenseCheck()
-{
-    m_formatter->appendMessage(Tr::tr("Checking pending licenses...") + "\n", NormalMessageFormat);
-    m_formatter->appendMessage(Tr::tr("The installation of Android SDK packages may fail if the "
-                                      "respective licenses are not accepted.")
-                                   + "\n",
-                               LogMessageFormat);
-    addPackageFuture(m_sdkManager->licenseCheck());
-}
-
-void AndroidSdkManagerWidget::onCancel()
-{
-    cancelPendingOperations();
-    close();
-}
-
-void AndroidSdkManagerWidget::onOperationResult(int index)
-{
-    QTC_ASSERT(m_currentOperation, return);
-    AndroidSdkManager::OperationOutput result = m_currentOperation->resultAt(index);
-    if (result.type == AndroidSdkManager::LicenseWorkflow) {
-        // Show license controls and enable to user input.
-        m_sdkLicenseLabel->setVisible(true);
-        m_sdkLicenseButtonBox->setVisible(true);
-        m_sdkLicenseButtonBox->setEnabled(true);
-        m_sdkLicenseButtonBox->button(QDialogButtonBox::No)->setDefault(true);
-    }
-    auto breakLine = [](const QString &line) { return line.endsWith("\n") ? line : line + "\n";};
-    if (!result.stdError.isEmpty() && result.type != AndroidSdkManager::LicenseCheck)
-        m_formatter->appendMessage(breakLine(result.stdError), StdErrFormat);
-    if (!result.stdOutput.isEmpty() && result.type != AndroidSdkManager::LicenseCheck)
-        m_formatter->appendMessage(breakLine(result.stdOutput), StdOutFormat);
-    m_outputEdit->ensureCursorVisible();
-}
-
-void AndroidSdkManagerWidget::addPackageFuture(const QFuture<AndroidSdkManager::OperationOutput>
-                                               &future)
-{
-    QTC_ASSERT(!m_currentOperation, return);
-    if (!future.isFinished() || !future.isCanceled()) {
-        m_currentOperation = new QFutureWatcher<AndroidSdkManager::OperationOutput>;
-        connect(m_currentOperation, &QFutureWatcherBase::resultReadyAt,
-                this, &AndroidSdkManagerWidget::onOperationResult);
-        connect(m_currentOperation, &QFutureWatcherBase::finished,
-                this, &AndroidSdkManagerWidget::packageFutureFinished);
-        connect(m_currentOperation, &QFutureWatcherBase::progressValueChanged,
-                this, [this](int value) {
-            m_operationProgress->setValue(value);
-        });
-        m_currentOperation->setFuture(future);
-    } else {
-        qCDebug(androidSdkMgrUiLog) << "Operation canceled/finished before adding to the queue";
-        if (m_sdkManager->isBusy()) {
-            m_formatter->appendMessage(Tr::tr("SDK Manager is busy. Operation cancelled."),
-                                       StdErrFormat);
-        }
-        notifyOperationFinished();
-        switchView(PackageListing);
-    }
-}
-
-void AndroidSdkManagerWidget::updatePackages()
-{
-    if (m_installationChange.count() == 0) {
-        switchView(PackageListing);
-        return;
-    }
-
-    m_formatter->appendMessage(Tr::tr("Installing/Uninstalling selected packages...\n"),
-                               NormalMessageFormat);
-    m_formatter->appendMessage(Tr::tr("Closing the %1 dialog will cancel the running and scheduled SDK "
-                                  "operations.\n").arg(HostOsInfo::isMacHost() ?
-                                                           Tr::tr("preferences") : Tr::tr("options")),
-                               LogMessageFormat);
-
-    addPackageFuture(m_sdkManager->updatePackages(m_installationChange));
-    m_installationChange = {};
-}
-
-void AndroidSdkManagerWidget::updateInstalled()
-{
-    m_formatter->appendMessage(Tr::tr("Updating installed packages...\n"), NormalMessageFormat);
-    m_formatter->appendMessage(Tr::tr("Closing the %1 dialog will cancel the running and scheduled SDK "
-                                  "operations.\n").arg(HostOsInfo::isMacHost() ?
-                                                           Tr::tr("preferences") : Tr::tr("options")),
-                               LogMessageFormat);
-    addPackageFuture(m_sdkManager->updateInstalled());
-}
-
-void AndroidSdkManagerWidget::licenseWorkflow()
-{
-    switchView(LicenseWorkflow);
-    addPackageFuture(m_sdkManager->licenseWorkflow());
-}
-
-void AndroidSdkManagerWidget::notifyOperationFinished()
-{
-    if (!m_currentOperation || m_currentOperation->isFinished()) {
-        QMessageBox::information(this, Tr::tr("Android SDK Changes"),
-                                 Tr::tr("Android SDK operations finished."), QMessageBox::Ok);
-        m_operationProgress->setValue(0);
-        // Once the update/install is done, let's hide the dialog.
-        hide();
-    }
-}
-
-void AndroidSdkManagerWidget::packageFutureFinished()
-{
-    QTC_ASSERT (m_currentOperation, return);
-
-    bool continueWorkflow = true;
-    if (m_currentOperation->isCanceled()) {
-        m_formatter->appendMessage(Tr::tr("Operation cancelled.\n"), StdErrFormat);
-        continueWorkflow = false;
-    }
-    m_operationProgress->setValue(100);
-    int resultCount = m_currentOperation->future().resultCount();
-    if (continueWorkflow && resultCount > 0) {
-        AndroidSdkManager::OperationOutput output = m_currentOperation->resultAt(resultCount -1);
-        AndroidSdkManager::CommandType type = output.type;
-        m_currentOperation->deleteLater();
-        m_currentOperation = nullptr;
-        switch (type) {
-        case AndroidSdkManager::LicenseCheck:
-            if (output.success) {
-                // No assertion was found. Looks like all license are accepted. Go Ahead.
-                if (m_pendingCommand == AndroidSdkManager::UpdatePackages)
-                    updatePackages(); // License workflow can only start when updating packages.
-                else if (m_pendingCommand == AndroidSdkManager::UpdateInstalled)
-                    updateInstalled();
-            } else {
-                // Run license workflow.
-                licenseWorkflow();
-            }
-            break;
-        case AndroidSdkManager::LicenseWorkflow:
-            m_sdkLicenseButtonBox->hide();
-            m_sdkLicenseLabel->hide();
-            if (m_pendingCommand == AndroidSdkManager::UpdatePackages)
-                updatePackages(); // License workflow can only start when updating packages.
-            else if (m_pendingCommand == AndroidSdkManager::UpdateInstalled)
-                updateInstalled();
-            break;
-        case AndroidSdkManager::UpdateInstalled:
-        case AndroidSdkManager::UpdatePackages:
-            notifyOperationFinished();
-            switchView(PackageListing);
-            m_sdkManager->reloadPackages();
-            break;
-        default:
-            break;
-        }
-    } else {
-        m_currentOperation->deleteLater();
-        m_currentOperation = nullptr;
-        switchView(PackageListing);
-        m_sdkManager->reloadPackages();
-    }
-}
-
-void AndroidSdkManagerWidget::cancelPendingOperations()
-{
-    if (!m_sdkManager->isBusy()) {
-        m_formatter->appendMessage(Tr::tr("\nNo pending operations to cancel...\n"),
-                                   NormalMessageFormat);
-        switchView(PackageListing);
-        return;
-    }
-    m_formatter->appendMessage(Tr::tr("\nCancelling pending operations...\n"),
-                               NormalMessageFormat);
-    m_sdkManager->cancelOperatons();
-}
-
-void AndroidSdkManagerWidget::switchView(AndroidSdkManagerWidget::View view)
-{
-    if (m_currentView == PackageListing)
-        m_formatter->clear();
-    m_currentView = view;
-    // We need the buttonBox only in the main listing view, as the license and update
-    // views already have a cancel button.
-    m_buttonBox->button(QDialogButtonBox::Apply)->setVisible(m_currentView == PackageListing);
-    m_operationProgress->setValue(0);
-    m_viewStack->setCurrentWidget(m_currentView == PackageListing ? m_packagesStack : m_outputStack);
-}
-
-void AndroidSdkManagerWidget::onSdkManagerOptions()
-{
-    OptionsDialog dlg(m_sdkManager, androidConfig().sdkManagerToolArgs(), this);
-    if (dlg.exec() == QDialog::Accepted) {
-        QStringList arguments = dlg.sdkManagerArguments();
-        if (arguments != androidConfig().sdkManagerToolArgs()) {
-            androidConfig().setSdkManagerToolArgs(arguments);
-            m_sdkManager->reloadPackages();
-        }
-    }
-}
-
 PackageFilterModel::PackageFilterModel(AndroidSdkModel *sdkModel) :
     QSortFilterProxyModel(sdkModel)
 {
@@ -518,7 +260,8 @@ bool PackageFilterModel::filterAcceptsRow(int sourceRow, const QModelIndex &sour
 }
 
 OptionsDialog::OptionsDialog(AndroidSdkManager *sdkManager, const QStringList &args,
-                             QWidget *parent) : QDialog(parent)
+                             QWidget *parent)
+    : QDialog(parent)
 {
     QTC_CHECK(sdkManager);
     resize(800, 480);

src/plugins/android/androidsdkmanagerwidget.h

@@ -2,38 +2,16 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
 #pragma once
 
-#include "androidconfigurations.h"
 #include "androidsdkmanager.h"
 
 #include <QDialog>
-#include <QFutureWatcher>
-#include <QWidget>
 
 QT_BEGIN_NAMESPACE
+class QDialogButtonBox;
 class QLineEdit;
 class QPlainTextEdit;
 QT_END_NAMESPACE
 
-#include <QtCore/QVariant>
-#include <QtWidgets/QAbstractButton>
-#include <QtWidgets/QApplication>
-#include <QtWidgets/QCheckBox>
-#include <QtWidgets/QComboBox>
-#include <QtWidgets/QDialogButtonBox>
-#include <QtWidgets/QFrame>
-#include <QtWidgets/QGroupBox>
-#include <QtWidgets/QHeaderView>
-#include <QtWidgets/QLabel>
-#include <QtWidgets/QPlainTextEdit>
-#include <QtWidgets/QProgressBar>
-#include <QtWidgets/QPushButton>
-#include <QtWidgets/QRadioButton>
-#include <QtWidgets/QSpacerItem>
-#include <QtWidgets/QStackedWidget>
-#include <QtWidgets/QTreeView>
-#include <QtWidgets/QWidget>
-#include <utils/fancylineedit.h>
-
 namespace Utils { class OutputFormatter; }
 
 namespace Android::Internal {
@@ -44,15 +22,14 @@ class AndroidSdkModel;
 class OptionsDialog : public QDialog
 {
 public:
-    OptionsDialog(AndroidSdkManager *sdkManager, const QStringList &args,
+    OptionsDialog(AndroidSdkManager *sdkManager, const QStringList &args, QWidget *parent = nullptr);
-                  QWidget *parent = nullptr);
     ~OptionsDialog() override;
 
     QStringList sdkManagerArguments() const;
 
 private:
-    QPlainTextEdit *m_argumentDetailsEdit;
+    QPlainTextEdit *m_argumentDetailsEdit = nullptr;
-    QLineEdit *m_argumentsEdit;
+    QLineEdit *m_argumentsEdit = nullptr;
     QFuture<QString> m_optionsFuture;
 };
 
@@ -60,46 +37,13 @@ class AndroidSdkManagerWidget : public QDialog
 {
     Q_OBJECT
 
-    enum View {
-        PackageListing,
-        Operations,
-        LicenseWorkflow
-    };
-
 public:
     AndroidSdkManagerWidget(AndroidSdkManager *sdkManager, QWidget *parent = nullptr);
-    ~AndroidSdkManagerWidget() override;
 
 private:
-    void onCancel();
-    void onOperationResult(int index);
-    void onSdkManagerOptions();
-    void addPackageFuture(const QFuture<AndroidSdkManager::OperationOutput> &future);
-    void licenseCheck();
-    void updatePackages();
-    void updateInstalled();
-    void licenseWorkflow();
-    void notifyOperationFinished();
-    void packageFutureFinished();
-    void cancelPendingOperations();
-    void switchView(View view);
-
-    AndroidSdkManager::CommandType m_pendingCommand = AndroidSdkManager::None;
-    View m_currentView = PackageListing;
     AndroidSdkManager *m_sdkManager = nullptr;
     AndroidSdkModel *m_sdkModel = nullptr;
-    Utils::OutputFormatter *m_formatter = nullptr;
+    QDialogButtonBox *m_buttonBox = nullptr;
-    QFutureWatcher<AndroidSdkManager::OperationOutput> *m_currentOperation = nullptr;
-
-    InstallationChange m_installationChange;
-    QStackedWidget *m_viewStack;
-    QWidget *m_packagesStack;
-    QWidget *m_outputStack;
-    QProgressBar *m_operationProgress;
-    QPlainTextEdit *m_outputEdit;
-    QLabel *m_sdkLicenseLabel;
-    QDialogButtonBox *m_sdkLicenseButtonBox;
-    QDialogButtonBox *m_buttonBox;
 };
 
 } // Android::Internal

src/plugins/android/androidsdkmodel.cpp

@@ -292,18 +292,14 @@ void AndroidSdkModel::refreshData()
             m_tools << p;
     }
     Utils::sort(m_sdkPlatforms, [](const SdkPlatform *p1, const SdkPlatform *p2) {
-       return p1->apiLevel() > p2->apiLevel();
+        return p1->apiLevel() > p2->apiLevel();
     });
 
     Utils::sort(m_tools, [](const AndroidSdkPackage *p1, const AndroidSdkPackage *p2) {
-       if (p1->state() == p2->state()) {
+        if (p1->state() == p2->state())
-           if (p1->type() ==  p2->type())
+            return p1->type() == p2->type() ? p1->revision() > p2->revision() : p1->type() > p2->type();
-               return p1->revision() > p2->revision();
+        else
-           else
+            return p1->state() < p2->state();
-               return p1->type() > p2->type();
-       } else {
-           return p1->state() < p2->state();
-       }
     });
 }
 

src/plugins/android/androidsettingswidget.cpp

@@ -34,6 +34,7 @@
 #include <QDir>
 #include <QFileDialog>
 #include <QGroupBox>
+#include <QGuiApplication>
 #include <QList>
 #include <QListWidget>
 #include <QLoggingCategory>