TaskTree: Unify done / errorOccurred signals

Get rid of TaskTree::errorOccurred signal. Provide additional
DoneResult arg for done signal.

Task-number: QTCREATORBUG-29834
Change-Id: I31a3a0701a14246b01f65303a1295f014c855ecf
Reviewed-by: hjk <hjk@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>

src/libs/solutions/tasking/tasktree.cpp

@@ -985,8 +985,7 @@ public:
     void advanceProgress(int byValue);
     void emitStartedAndProgress();
     void emitProgress();
-    void emitDone();
+    void emitDone(DoneWith result);
-    void emitError();
     QList<TreeStorageBase> addStorages(const QList<TreeStorageBase> &storages);
     void callSetupHandler(TreeStorageBase storage, int storageId) {
         callStorageHandler(storage, storageId, &StorageHandler::m_setupHandler);
@@ -1119,13 +1118,8 @@ void TaskTreePrivate::stop()
     QTC_ASSERT(m_root, return);
     if (!m_root->isRunning())
         return;
-    // TODO: should we have canceled flag (passed to handler)?
-    // Just one done handler with result flag:
-    //   FinishedWithSuccess, FinishedWithError, Canceled, TimedOut.
-    // Canceled either directly by user, or by workflow policy - doesn't matter, in both
-    // cases canceled from outside.
     m_root->stop();
-    emitError();
+    emitDone(DoneWith::Cancel);
 }
 
 void TaskTreePrivate::advanceProgress(int byValue)
@@ -1151,18 +1145,11 @@ void TaskTreePrivate::emitProgress()
     emit q->progressValueChanged(m_progressValue);
 }
 
-void TaskTreePrivate::emitDone()
+void TaskTreePrivate::emitDone(DoneWith result)
 {
     QTC_CHECK(m_progressValue == m_root->taskCount());
     GuardLocker locker(m_guard);
-    emit q->done();
+    emit q->done(result);
-}
-
-void TaskTreePrivate::emitError()
-{
-    QTC_CHECK(m_progressValue == m_root->taskCount());
-    GuardLocker locker(m_guard);
-    emit q->errorOccurred();
 }
 
 QList<TreeStorageBase> TaskTreePrivate::addStorages(const QList<TreeStorageBase> &storages)
@@ -1348,10 +1335,8 @@ SetupResult TaskContainer::continueStart(SetupResult startAction, int nextChild)
             QTC_CHECK(parentContainer->isRunning());
             if (!parentContainer->isStarting())
                 parentContainer->childDone(success);
-        } else if (success) {
-            m_constData.m_taskTreePrivate->emitDone();
         } else {
-            m_constData.m_taskTreePrivate->emitError();
+            m_constData.m_taskTreePrivate->emitDone(success ? DoneWith::Success : DoneWith::Error);
         }
     }
     return groupAction;
@@ -2338,20 +2323,16 @@ bool TaskTree::runBlocking(const QFuture<void> &future)
 
     bool ok = false;
     QEventLoop loop;
-
+    connect(this, &TaskTree::done, &loop, [&loop, &ok](DoneWith result) {
-    const auto finalize = [&loop, &ok](bool success) {
+        ok = result == DoneWith::Success;
-        ok = success;
         // Otherwise, the tasks from inside the running tree that were deleteLater()
         // will be leaked. Refer to the QObject::deleteLater() docs.
         QMetaObject::invokeMethod(&loop, [&loop] { loop.quit(); }, Qt::QueuedConnection);
-    };
+    });
-
     QFutureWatcher<void> watcher;
     connect(&watcher, &QFutureWatcherBase::canceled, this, &TaskTree::stop);
     watcher.setFuture(future);
 
-    connect(this, &TaskTree::done, &loop, [finalize] { finalize(true); });
-    connect(this, &TaskTree::errorOccurred, &loop, [finalize] { finalize(false); });
     QTimer::singleShot(0, this, &TaskTree::start);
 
     loop.exec(QEventLoop::ExcludeUserInputEvents);
@@ -2558,8 +2539,8 @@ void TaskTree::setupStorageHandler(const TreeStorageBase &storage,
 
 TaskTreeTaskAdapter::TaskTreeTaskAdapter()
 {
-    connect(task(), &TaskTree::done, this, [this] { emit done(true); });
+    connect(task(), &TaskTree::done, this,
-    connect(task(), &TaskTree::errorOccurred, this, [this] { emit done(false); });
+            [this](DoneWith result) { emit done(result == DoneWith::Success); });
 }
 
 void TaskTreeTaskAdapter::start()

src/libs/solutions/tasking/tasktree.h

@@ -482,7 +482,7 @@ public:
 
 signals:
     void started();
-    void done();
+    void done(DoneWith result);
     void errorOccurred();
     void progressValueChanged(int value); // updated whenever task finished / skipped / stopped
 

src/libs/utils/filestreamer.cpp

@@ -32,12 +32,10 @@ public:
 
         const GroupItem task = m_filePath.needsDevice() ? remoteTask() : localTask();
         m_taskTree.reset(new TaskTree({task}));
-        const auto finalize = [this](bool success) {
+        connect(m_taskTree.get(), &TaskTree::done, this, [this](DoneWith result) {
             m_taskTree.release()->deleteLater();
-            emit done(success);
+            emit done(result == DoneWith::Success);
-        };
+        });
-        connect(m_taskTree.get(), &TaskTree::done, this, [=] { finalize(true); });
-        connect(m_taskTree.get(), &TaskTree::errorOccurred, this, [=] { finalize(false); });
         m_taskTree->start();
     }
 
@@ -466,14 +464,12 @@ void FileStreamer::start()
     // TODO: Preliminary check if local source exists?
     QTC_ASSERT(!d->m_taskTree, return);
     d->m_taskTree.reset(new TaskTree({d->task()}));
-    const auto finalize = [this](bool success) {
+    connect(d->m_taskTree.get(), &TaskTree::done, this, [this](DoneWith result) {
-        d->m_streamResult = success ? StreamResult::FinishedWithSuccess
+        d->m_streamResult = result == DoneWith::Success ? StreamResult::FinishedWithSuccess
                                                           : StreamResult::FinishedWithError;
         d->m_taskTree.release()->deleteLater();
         emit done();
-    };
+    });
-    connect(d->m_taskTree.get(), &TaskTree::done, this, [=] { finalize(true); });
-    connect(d->m_taskTree.get(), &TaskTree::errorOccurred, this, [=] { finalize(false); });
     d->m_taskTree->start();
 }
 

src/plugins/android/androidsdkdownloader.cpp

@@ -188,12 +188,10 @@ void AndroidSdkDownloader::downloadAndExtractSdk()
     };
 
     m_taskTree.reset(new TaskTree(root));
-    const auto onDone = [this] {
+    connect(m_taskTree.get(), &TaskTree::done, this, [this] {
         m_taskTree.release()->deleteLater();
         m_progressDialog.reset();
-    };
+    });
-    connect(m_taskTree.get(), &TaskTree::done, this, onDone);
-    connect(m_taskTree.get(), &TaskTree::errorOccurred, this, onDone);
     m_taskTree->start();
 }
 

src/plugins/autotest/testcodeparser.cpp

@@ -390,11 +390,11 @@ void TestCodeParser::scanForTests(const QSet<FilePath> &filePaths,
         tasks.append(AsyncTask<TestParseResultPtr>(onSetup, onDone, CallDoneIf::Success));
     }
     m_taskTree.reset(new TaskTree{tasks});
-    const auto onDone = [this] { m_taskTree.release()->deleteLater(); onFinished(true); };
-    const auto onError = [this] { m_taskTree.release()->deleteLater(); onFinished(false); };
     connect(m_taskTree.get(), &TaskTree::started, this, &TestCodeParser::parsingStarted);
-    connect(m_taskTree.get(), &TaskTree::done, this, onDone);
+    connect(m_taskTree.get(), &TaskTree::done, this, [this](DoneWith result) {
-    connect(m_taskTree.get(), &TaskTree::errorOccurred, this, onError);
+        m_taskTree.release()->deleteLater();
+        onFinished(result == DoneWith::Success);
+    });
     if (filteredFiles.size() > 5) {
         auto progress = new TaskProgress(m_taskTree.get());
         progress->setDisplayName(Tr::tr("Scanning for Tests"));

src/plugins/autotest/testrunner.cpp

@@ -455,7 +455,6 @@ void TestRunner::runTestsHelper()
 
     m_taskTree.reset(new TaskTree(tasks));
     connect(m_taskTree.get(), &TaskTree::done, this, &TestRunner::onFinished);
-    connect(m_taskTree.get(), &TaskTree::errorOccurred, this, &TestRunner::onFinished);
 
     auto progress = new TaskProgress(m_taskTree.get());
     progress->setDisplayName(Tr::tr("Running Tests"));

src/plugins/clangtools/documentclangtoolrunner.cpp

@@ -225,7 +225,6 @@ void DocumentClangToolRunner::run()
     cleanup.dismiss();
     m_taskTree.reset(new TaskTree(tasks));
     connect(m_taskTree.get(), &TaskTree::done, this, &DocumentClangToolRunner::finalize);
-    connect(m_taskTree.get(), &TaskTree::errorOccurred, this, &DocumentClangToolRunner::finalize);
     m_taskTree->start();
 }
 

src/plugins/coreplugin/locator/ilocatorfilter.cpp

@@ -486,15 +486,10 @@ void LocatorMatcher::start()
     };
 
     d->m_taskTree->setRecipe(root);
-
+    connect(d->m_taskTree.get(), &TaskTree::done, this, [this](DoneWith result) {
-    const auto onFinish = [this](bool success) {
+        emit done(result == DoneWith::Success);
-        return [this, success] {
-            emit done(success);
         d->m_taskTree.release()->deleteLater();
-        };
+    });
-    };
-    connect(d->m_taskTree.get(), &TaskTree::done, this, onFinish(true));
-    connect(d->m_taskTree.get(), &TaskTree::errorOccurred, this, onFinish(false));
     d->m_taskTree->start();
 }
 

src/plugins/coreplugin/locator/locator.cpp

@@ -396,13 +396,11 @@ void Locator::refresh(const QList<ILocatorFilter *> &filters)
     }
 
     m_taskTree.reset(new TaskTree{tasks});
-    connect(m_taskTree.get(), &TaskTree::done, this, [this] {
+    connect(m_taskTree.get(), &TaskTree::done, this, [this](DoneWith result) {
+        if (result == DoneWith::Success)
             saveSettings();
         m_taskTree.release()->deleteLater();
     });
-    connect(m_taskTree.get(), &TaskTree::errorOccurred, this, [this] {
-        m_taskTree.release()->deleteLater();
-    });
     auto progress = new TaskProgress(m_taskTree.get());
     progress->setDisplayName(Tr::tr("Updating Locator Caches"));
     m_taskTree->start();

src/plugins/coreplugin/plugininstallwizard.cpp

@@ -262,12 +262,10 @@ public:
         };
         m_taskTree.reset(new TaskTree(root));
 
-        const auto onEnd = [this] {
+        connect(m_taskTree.get(), &TaskTree::done, this, [this] {
             m_cancelButton->setVisible(false);
             m_taskTree.release()->deleteLater();
-        };
+        });
-        connect(m_taskTree.get(), &TaskTree::done, this, onEnd);
-        connect(m_taskTree.get(), &TaskTree::errorOccurred, this, onEnd);
 
         m_cancelButton->setVisible(true);
         m_taskTree->start();

src/plugins/coreplugin/progressmanager/taskprogress.cpp

@@ -128,12 +128,9 @@ TaskProgress::TaskProgress(TaskTree *taskTree)
     connect(d->m_taskTree, &TaskTree::progressValueChanged, this, [this](int value) {
         d->advanceProgress(value);
     });
-    connect(d->m_taskTree, &TaskTree::done, this, [this] {
+    connect(d->m_taskTree, &TaskTree::done, this, [this](DoneWith result) {
-        d->m_timer.stop();
-        d->m_futureInterface.reportFinished();
-    });
-    connect(d->m_taskTree, &TaskTree::errorOccurred, this, [this] {
         d->m_timer.stop();
+        if (result != DoneWith::Success)
             d->m_futureInterface.reportCanceled();
         d->m_futureInterface.reportFinished();
     });

src/plugins/ctfvisualizer/ctfvisualizertool.cpp

@@ -219,36 +219,33 @@ void CtfVisualizerTool::loadJson(const QString &fileName)
             m_traceManager->addEvent(asyncPtr->resultAt(index));
         });
     };
-    const auto onDone = [this] {
+    const auto onDone = [this](DoneWith result) {
+        if (result == DoneWith::Success) {
             m_traceManager->updateStatistics();
             if (m_traceManager->isEmpty()) {
-            QMessageBox::warning(Core::ICore::dialogParent(),
+                QMessageBox::warning(Core::ICore::dialogParent(), Tr::tr("CTF Visualizer"),
-                                 Tr::tr("CTF Visualizer"),
                                      Tr::tr("The file does not contain any trace data."));
             } else if (!m_traceManager->errorString().isEmpty()) {
-            QMessageBox::warning(Core::ICore::dialogParent(),
+                QMessageBox::warning(Core::ICore::dialogParent(), Tr::tr("CTF Visualizer"),
-                                 Tr::tr("CTF Visualizer"),
                                      m_traceManager->errorString());
             } else {
                 m_traceManager->finalize();
                 m_perspective.select();
-            zoomControl()->setTrace(m_traceManager->traceBegin(), m_traceManager->traceEnd() + m_traceManager->traceDuration() / 20);
+                const auto end = m_traceManager->traceEnd() + m_traceManager->traceDuration() / 20;
-            zoomControl()->setRange(m_traceManager->traceBegin(), m_traceManager->traceEnd() + m_traceManager->traceDuration() / 20);
+                zoomControl()->setTrace(m_traceManager->traceBegin(), end);
+                zoomControl()->setRange(m_traceManager->traceBegin(), end);
             }
             setAvailableThreads(m_traceManager->getSortedThreads());
-        m_loader.release()->deleteLater();
+        } else {
-    };
+            QMessageBox::warning(Core::ICore::dialogParent(), Tr::tr("CTF Visualizer"),
-    const auto onError = [this] {
-        QMessageBox::warning(Core::ICore::dialogParent(),
-                             Tr::tr("CTF Visualizer"),
                                  Tr::tr("Cannot read the CTF file."));
+        }
         m_loader.release()->deleteLater();
     };
 
     const Group recipe { AsyncTask<json>(onSetup) };
     m_loader.reset(new TaskTree(recipe));
     connect(m_loader.get(), &TaskTree::done, this, onDone);
-    connect(m_loader.get(), &TaskTree::errorOccurred, this, onError);
     auto progress = new TaskProgress(m_loader.get());
     progress->setDisplayName(Tr::tr("Loading CTF File"));
     m_loader->start();

src/plugins/debugger/loadcoredialog.cpp

@@ -173,7 +173,7 @@ int AttachCoreDialog::exec()
     connect(d->buttonBox, &QDialogButtonBox::accepted, this, &AttachCoreDialog::accepted);
     changed();
 
-    connect(&d->taskTree, &TaskTree::done, this, [&]() {
+    connect(&d->taskTree, &TaskTree::done, this, [this] {
         setEnabled(true);
         d->progressIndicator->setVisible(false);
         d->progressLabel->setVisible(false);

src/plugins/diffeditor/diffeditorcontroller.cpp

@@ -110,20 +110,16 @@ void DiffEditorController::requestReload()
 {
     m_document->beginReload();
     m_taskTree.reset(new TaskTree(m_reloadRecipe));
-    connect(m_taskTree.get(), &TaskTree::done, this, [this] { reloadFinished(true); });
+    connect(m_taskTree.get(), &TaskTree::done, this, [this](DoneWith result) {
-    connect(m_taskTree.get(), &TaskTree::errorOccurred, this, [this] { reloadFinished(false); });
+        if (m_taskTree)
+            m_taskTree.release()->deleteLater();
+        m_document->endReload(result == DoneWith::Success);
+    });
     auto progress = new TaskProgress(m_taskTree.get());
     progress->setDisplayName(m_displayName);
     m_taskTree->start();
 }
 
-void DiffEditorController::reloadFinished(bool success)
-{
-    if (m_taskTree)
-        m_taskTree.release()->deleteLater();
-    m_document->endReload(success);
-}
-
 void DiffEditorController::addExtraActions(QMenu *menu, int fileIndex, int chunkIndex,
                                            const ChunkSelection &selection)
 {

src/plugins/diffeditor/diffeditorcontroller.h

@@ -66,7 +66,6 @@ protected:
     void forceContextLineCount(int lines);
 
 private:
-    void reloadFinished(bool success);
     friend class Internal::DiffEditorWidgetController;
     virtual void addExtraActions(QMenu *menu, int fileIndex, int chunkIndex,
                                  const ChunkSelection &selection);

src/plugins/projectexplorer/buildmanager.cpp

@@ -682,7 +682,8 @@ void BuildManager::startBuildQueue()
         topLevel.append(Group(targetTasks));
 
     d->m_taskTree.reset(new TaskTree(Group{topLevel}));
-    const auto endHandler = [](bool success) {
+    const auto onDone = [](DoneWith result) {
+        const bool success = result == DoneWith::Success;
         d->m_taskTree.release()->deleteLater();
 
         if (!success && d->m_progressFutureInterface)
@@ -703,9 +704,7 @@ void BuildManager::startBuildQueue()
             startBuildQueue();
         }
     };
-    connect(d->m_taskTree.get(), &TaskTree::done, instance(), [endHandler] { endHandler(true); });
+    connect(d->m_taskTree.get(), &TaskTree::done, instance(), onDone);
-    connect(d->m_taskTree.get(), &TaskTree::errorOccurred, instance(),
-            [endHandler] { endHandler(false); });
 
     // Progress Reporting
     d->m_progressFutureInterface = new QFutureInterface<void>;

src/plugins/projectexplorer/extracompiler.cpp

@@ -154,12 +154,10 @@ void ExtraCompiler::compileContent(const QByteArray &content)
 
 void ExtraCompiler::compileImpl(const ContentProvider &provider)
 {
-    const auto finalize = [=] {
-        d->m_taskTree.release()->deleteLater();
-    };
     d->m_taskTree.reset(new TaskTree({taskItemImpl(provider)}));
-    connect(d->m_taskTree.get(), &TaskTree::done, this, finalize);
+    connect(d->m_taskTree.get(), &TaskTree::done, this, [this] {
-    connect(d->m_taskTree.get(), &TaskTree::errorOccurred, this, finalize);
+        d->m_taskTree.release()->deleteLater();
+    });
     d->m_taskTree->start();
 }
 

src/plugins/projectexplorer/runcontrol.cpp

@@ -1053,12 +1053,10 @@ void RunControlPrivate::startTaskTree()
 
     m_taskTree.reset(new TaskTree(*m_runRecipe));
     connect(m_taskTree.get(), &TaskTree::started, q, &RunControl::started);
-    const auto finalize = [this] {
+    connect(m_taskTree.get(), &TaskTree::done, this, [this] {
         m_taskTree.release()->deleteLater();
         checkAutoDeleteAndEmitStopped();
-    };
+    });
-    connect(m_taskTree.get(), &TaskTree::done, this, finalize);
-    connect(m_taskTree.get(), &TaskTree::errorOccurred, this, finalize);
     m_taskTree->start();
 }
 

src/plugins/qnx/qnxdeployqtlibrariesdialog.cpp

@@ -300,12 +300,10 @@ void QnxDeployQtLibrariesDialogPrivate::start()
     m_deployProgress->setRange(0, m_deployableFiles.count());
 
     m_taskTree.reset(new TaskTree(deployRecipe()));
-    const auto endHandler = [this] {
+    connect(m_taskTree.get(), &TaskTree::done, this, [this] {
         m_taskTree.release()->deleteLater();
         handleUploadFinished();
-    };
+    });
-    connect(m_taskTree.get(), &TaskTree::done, this, endHandler);
-    connect(m_taskTree.get(), &TaskTree::errorOccurred, this, endHandler);
     m_taskTree->start();
 }
 

src/plugins/updateinfo/updateinfoplugin.cpp

@@ -122,10 +122,6 @@ void UpdateInfoPlugin::startCheckForUpdates()
         process.setCommand({d->m_maintenanceTool, args});
         process.setLowPriority();
     };
-    const auto doCleanup = [this] {
-        d->m_taskTree.release()->deleteLater();
-        checkForUpdatesStopped();
-    };
 
     const auto onUpdateSetup = [doSetup](Process &process) {
         doSetup(process, {"ch", "-g", "*=false,ifw.package.*=true"});
@@ -146,11 +142,12 @@ void UpdateInfoPlugin::startCheckForUpdates()
     }
 
     d->m_taskTree.reset(new TaskTree(Group{tasks}));
-    connect(d->m_taskTree.get(), &TaskTree::done, this, [this, doCleanup] {
+    connect(d->m_taskTree.get(), &TaskTree::done, this, [this](DoneWith result) {
+        if (result == DoneWith::Success)
             checkForUpdatesFinished();
-        doCleanup();
+        d->m_taskTree.release()->deleteLater();
+        checkForUpdatesStopped();
     });
-    connect(d->m_taskTree.get(), &TaskTree::errorOccurred, this, doCleanup);
     d->m_progress = new TaskProgress(d->m_taskTree.get());
     d->m_progress->setHalfLifeTimePerTask(30000); // 30 seconds
     d->m_progress->setDisplayName(Tr::tr("Checking for Updates"));

src/plugins/valgrind/valgrindprocess.cpp

@@ -213,12 +213,10 @@ bool ValgrindProcessPrivate::run()
 {
     m_taskTree.reset(new TaskTree);
     m_taskTree->setRecipe(runRecipe());
-    const auto finalize = [this](bool success) {
+    connect(m_taskTree.get(), &TaskTree::done, this, [this](DoneWith result) {
         m_taskTree.release()->deleteLater();
-        emit q->done(success);
+        emit q->done(result == DoneWith::Success);
-    };
+    });
-    connect(m_taskTree.get(), &TaskTree::done, this, [finalize] { finalize(true); });
-    connect(m_taskTree.get(), &TaskTree::errorOccurred, this, [finalize] { finalize(false); });
     m_taskTree->start();
     return bool(m_taskTree);
 }