Don`t block main thread when displaying all files removed dialog

Fixes: #8170

6 files changed, 123 insertions, 110 deletions

diff --git a/changelog/unreleased/8170 b/changelog/unreleased/8170
new file mode 100644
index 000000000..466f8cb46
--- /dev/null
+++ b/changelog/unreleased/8170
@@ -0,0 +1,5 @@
+Bugfix: "All Files removed" dialog no longer blocks the application
+
+We fixed a bug where a dialog locked the whole application
+
+https://github.com/owncloud/client/issues/8170
diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index 3cc55a287..2da179c7d 100644
--- a/src/gui/folder.cpp
+++ b/src/gui/folder.cpp
@@ -18,6 +18,7 @@
 #include "account.h"
 #include "accountmanager.h"
 #include "accountstate.h"
+#include "application.h"
 #include "folder.h"
 #include "folderman.h"
 #include "logger.h"
@@ -35,6 +36,7 @@
 #include "csync_exclude.h"
 #include "common/vfs.h"
 #include "creds/abstractcredentials.h"
+#include "settingsdialog.h"
 
 #include <QTimer>
 #include <QUrl>
@@ -92,7 +94,6 @@ Folder::Folder(const FolderDefinition &definition,
     connect(_engine.data(), &SyncEngine::started, this, &Folder::slotSyncStarted, Qt::QueuedConnection);
     connect(_engine.data(), &SyncEngine::finished, this, &Folder::slotSyncFinished, Qt::QueuedConnection);
 
-    //direct connection so the message box is blocking the sync.
     connect(_engine.data(), &SyncEngine::aboutToRemoveAllFiles,
         this, &Folder::slotAboutToRemoveAllFiles);
     connect(_engine.data(), &SyncEngine::transmissionProgress, this, &Folder::slotTransmissionProgress);
@@ -1188,13 +1189,12 @@ bool Folder::virtualFilesEnabled() const
     return _definition.virtualFilesMode != Vfs::Off && !isVfsOnOffSwitchPending();
 }
 
-void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction dir, bool *cancel)
+void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction dir, std::function<void(bool)> callback)
 {
     ConfigFile cfgFile;
     if (!cfgFile.promptDeleteFiles())
         return;
-
-    QString msg = dir == SyncFileItem::Down ? tr("All files in the sync folder '%1' folder were deleted on the server.\n"
+    const QString msg = dir == SyncFileItem::Down ? tr("All files in the sync folder '%1' folder were deleted on the server.\n"
                                                  "These deletes will be synchronized to your local sync folder, making such files "
                                                  "unavailable unless you have a right to restore. \n"
                                                  "If you decide to keep the files, they will be re-synced with the server if you have rights to do so.\n"
@@ -1203,22 +1203,23 @@ void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction dir, bool *cancel
                                                  "synchronized with your server, making such files unavailable unless restored.\n"
                                                  "Are you sure you want to sync those actions with the server?\n"
                                                  "If this was an accident and you decide to keep your files, they will be re-synced from the server.");
-    QMessageBox msgBox(QMessageBox::Warning, tr("Remove All Files?"),
-        msg.arg(shortGuiLocalPath()));
-    msgBox.setWindowFlags(msgBox.windowFlags() | Qt::WindowStaysOnTopHint);
-    msgBox.addButton(tr("Remove all files"), QMessageBox::DestructiveRole);
-    QPushButton *keepBtn = msgBox.addButton(tr("Keep files"), QMessageBox::AcceptRole);
-    if (msgBox.exec() == -1) {
-        *cancel = true;
-        return;
-    }
-    *cancel = msgBox.clickedButton() == keepBtn;
-    if (*cancel) {
-        FileSystem::setFolderMinimumPermissions(path());
-        journalDb()->clearFileTable();
-        _lastEtag.clear();
-        slotScheduleThisFolder();
-    }
+    auto msgBox = new QMessageBox(QMessageBox::Warning, tr("Remove All Files?"),
+        msg.arg(shortGuiLocalPath()), QMessageBox::NoButton, ocApp()->gui()->settingsDialog());
+    msgBox->setAttribute(Qt::WA_DeleteOnClose);
+    msgBox->setWindowFlags(msgBox->windowFlags() | Qt::WindowStaysOnTopHint);
+    msgBox->addButton(tr("Remove all files"), QMessageBox::DestructiveRole);
+    QPushButton *keepBtn = msgBox->addButton(tr("Keep files"), QMessageBox::AcceptRole);
+    connect(msgBox, &QMessageBox::finished, this, [msgBox, keepBtn, callback, this]{
+        const bool cancel = msgBox->clickedButton() == keepBtn;
+        callback(cancel);
+        if (cancel) {
+            FileSystem::setFolderMinimumPermissions(path());
+                journalDb()->clearFileTable();
+                _lastEtag.clear();
+                slotScheduleThisFolder();
+        }
+    });
+    msgBox->open();
 }
 
 void FolderDefinition::save(QSettings &settings, const FolderDefinition &folder)
diff --git a/src/gui/folder.h b/src/gui/folder.h
index 1fca6b002..60c78d6c8 100644
--- a/src/gui/folder.h
+++ b/src/gui/folder.h
@@ -317,7 +317,7 @@ public slots:
     void slotTerminateSync();
 
     // connected to the corresponding signals in the SyncEngine
-    void slotAboutToRemoveAllFiles(SyncFileItem::Direction, bool *);
+    void slotAboutToRemoveAllFiles(SyncFileItem::Direction, std::function<void(bool)> callback);
 
     /**
       * Starts a sync operation
diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index 423cd6dd6..3111843f4 100644
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -636,100 +636,107 @@ void SyncEngine::slotDiscoveryFinished()
     emit transmissionProgress(*_progressInfo);
 
     //    qCInfo(lcEngine) << "Permissions of the root folder: " << _csync_ctx->remote.root_perms.toString();
+    auto finish = [this]{
 
-    if (!_hasNoneFiles && _hasRemoveFile) {
-        qCInfo(lcEngine) << "All the files are going to be changed, asking the user";
-        bool cancel = false;
-        int side = 0; // > 0 means more deleted on the server.  < 0 means more deleted on the client
-        foreach (const auto &it, _syncItems) {
-            if (it->_instruction == CSYNC_INSTRUCTION_REMOVE) {
-                side += it->_direction == SyncFileItem::Down ? 1 : -1;
-            }
+
+        auto databaseFingerprint = _journal->dataFingerprint();
+        // If databaseFingerprint is empty, this means that there was no information in the database
+        // (for example, upgrading from a previous version, or first sync, or server not supporting fingerprint)
+        if (!databaseFingerprint.isEmpty() && _discoveryPhase
+            && _discoveryPhase->_dataFingerprint != databaseFingerprint) {
+            qCInfo(lcEngine) << "data fingerprint changed, assume restore from backup" << databaseFingerprint << _discoveryPhase->_dataFingerprint;
+            restoreOldFiles(_syncItems);
         }
-        emit aboutToRemoveAllFiles(side >= 0 ? SyncFileItem::Down : SyncFileItem::Up, &cancel);
-        if (cancel) {
-            qCInfo(lcEngine) << "User aborted sync";
-            finalize(false);
-            return;
+
+        if (_discoveryPhase->_anotherSyncNeeded && _anotherSyncNeeded == NoFollowUpSync) {
+            _anotherSyncNeeded = ImmediateFollowUp;
         }
-    }
 
-    auto databaseFingerprint = _journal->dataFingerprint();
-    // If databaseFingerprint is empty, this means that there was no information in the database
-    // (for example, upgrading from a previous version, or first sync, or server not supporting fingerprint)
-    if (!databaseFingerprint.isEmpty() && _discoveryPhase
-        && _discoveryPhase->_dataFingerprint != databaseFingerprint) {
-        qCInfo(lcEngine) << "data fingerprint changed, assume restore from backup" << databaseFingerprint << _discoveryPhase->_dataFingerprint;
-        restoreOldFiles(_syncItems);
-    }
+        Q_ASSERT(std::is_sorted(_syncItems.begin(), _syncItems.end()));
 
-    if (_discoveryPhase->_anotherSyncNeeded && _anotherSyncNeeded == NoFollowUpSync) {
-        _anotherSyncNeeded = ImmediateFollowUp;
-    }
+        qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate) #################################################### " << _stopWatch.addLapTime(QStringLiteral("Reconcile (aboutToPropagate)")) << "ms";
 
-    Q_ASSERT(std::is_sorted(_syncItems.begin(), _syncItems.end()));
+        _localDiscoveryPaths.clear();
 
-    qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate) #################################################### " << _stopWatch.addLapTime(QStringLiteral("Reconcile (aboutToPropagate)")) << "ms";
+        // To announce the beginning of the sync
+        emit aboutToPropagate(_syncItems);
 
-    _localDiscoveryPaths.clear();
+        qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate OK) #################################################### "<< _stopWatch.addLapTime(QStringLiteral("Reconcile (aboutToPropagate OK)")) << "ms";
 
-    // To announce the beginning of the sync
-    emit aboutToPropagate(_syncItems);
+        // it's important to do this before ProgressInfo::start(), to announce start of new sync
+        _progressInfo->_status = ProgressInfo::Propagation;
+        emit transmissionProgress(*_progressInfo);
+        _progressInfo->startEstimateUpdates();
 
-    qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate OK) #################################################### "<< _stopWatch.addLapTime(QStringLiteral("Reconcile (aboutToPropagate OK)")) << "ms";
+        // post update phase script: allow to tweak stuff by a custom script in debug mode.
+        if (!qEnvironmentVariableIsEmpty("OWNCLOUD_POST_UPDATE_SCRIPT")) {
+    #ifndef NDEBUG
+            const QString script = qEnvironmentVariable("OWNCLOUD_POST_UPDATE_SCRIPT");
 
-    // it's important to do this before ProgressInfo::start(), to announce start of new sync
-    _progressInfo->_status = ProgressInfo::Propagation;
-    emit transmissionProgress(*_progressInfo);
-    _progressInfo->startEstimateUpdates();
+            qCDebug(lcEngine) << "Post Update Script: " << script;
+            QProcess::execute(script);
+    #else
+            qCWarning(lcEngine) << "**** Attention: POST_UPDATE_SCRIPT installed, but not executed because compiled with NDEBUG";
+    #endif
+        }
 
-    // post update phase script: allow to tweak stuff by a custom script in debug mode.
-    if (!qEnvironmentVariableIsEmpty("OWNCLOUD_POST_UPDATE_SCRIPT")) {
-#ifndef NDEBUG
-        const QString script = qEnvironmentVariable("OWNCLOUD_POST_UPDATE_SCRIPT");
+        // do a database commit
+        _journal->commit(QStringLiteral("post treewalk"));
+
+        _propagator = QSharedPointer<OwncloudPropagator>(
+            new OwncloudPropagator(_account, _localPath, _remotePath, _journal));
+        _propagator->setSyncOptions(_syncOptions);
+        connect(_propagator.data(), &OwncloudPropagator::itemCompleted,
+            this, &SyncEngine::slotItemCompleted);
+        connect(_propagator.data(), &OwncloudPropagator::progress,
+            this, &SyncEngine::slotProgress);
+        connect(_propagator.data(), &OwncloudPropagator::updateFileTotal,
+            this, &SyncEngine::updateFileTotal);
+        connect(_propagator.data(), &OwncloudPropagator::finished, this, &SyncEngine::slotPropagationFinished, Qt::QueuedConnection);
+        connect(_propagator.data(), &OwncloudPropagator::seenLockedFile, this, &SyncEngine::seenLockedFile);
+        connect(_propagator.data(), &OwncloudPropagator::touchedFile, this, &SyncEngine::slotAddTouchedFile);
+        connect(_propagator.data(), &OwncloudPropagator::insufficientLocalStorage, this, &SyncEngine::slotInsufficientLocalStorage);
+        connect(_propagator.data(), &OwncloudPropagator::insufficientRemoteStorage, this, &SyncEngine::slotInsufficientRemoteStorage);
+        connect(_propagator.data(), &OwncloudPropagator::newItem, this, &SyncEngine::slotNewItem);
+
+        // apply the network limits to the propagator
+        setNetworkLimits(_uploadLimit, _downloadLimit);
+
+        deleteStaleDownloadInfos(_syncItems);
+        deleteStaleUploadInfos(_syncItems);
+        deleteStaleErrorBlacklistEntries(_syncItems);
+        _journal->commit(QStringLiteral("post stale entry removal"));
+
+        // Emit the started signal only after the propagator has been set up.
+        if (_needsUpdate)
+            emit(started());
+
+        _propagator->start(_syncItems);
+        _syncItems.clear();
+
+        qCInfo(lcEngine) << "#### Post-Reconcile end #################################################### " << _stopWatch.addLapTime(QStringLiteral("Post-Reconcile Finished")) << "ms";
+    };
 
-        qCDebug(lcEngine) << "Post Update Script: " << script;
-        QProcess::execute(script);
-#else
-        qCWarning(lcEngine) << "**** Attention: POST_UPDATE_SCRIPT installed, but not executed because compiled with NDEBUG";
-#endif
+    if (!_hasNoneFiles && _hasRemoveFile) {
+        qCInfo(lcEngine) << "All the files are going to be changed, asking the user";
+        int side = 0; // > 0 means more deleted on the server.  < 0 means more deleted on the client
+        foreach (const auto &it, _syncItems) {
+            if (it->_instruction == CSYNC_INSTRUCTION_REMOVE) {
+                side += it->_direction == SyncFileItem::Down ? 1 : -1;
+            }
+        }
+        emit aboutToRemoveAllFiles(side >= 0 ? SyncFileItem::Down : SyncFileItem::Up, [this, finish](bool cancel){
+            if (cancel) {
+                qCInfo(lcEngine) << "User aborted sync";
+                finalize(false);
+                return;
+            } else {
+                finish();
+            }
+        });
+        return;
     }
-
-    // do a database commit
-    _journal->commit(QStringLiteral("post treewalk"));
-
-    _propagator = QSharedPointer<OwncloudPropagator>(
-        new OwncloudPropagator(_account, _localPath, _remotePath, _journal));
-    _propagator->setSyncOptions(_syncOptions);
-    connect(_propagator.data(), &OwncloudPropagator::itemCompleted,
-        this, &SyncEngine::slotItemCompleted);
-    connect(_propagator.data(), &OwncloudPropagator::progress,
-        this, &SyncEngine::slotProgress);
-    connect(_propagator.data(), &OwncloudPropagator::updateFileTotal,
-        this, &SyncEngine::updateFileTotal);
-    connect(_propagator.data(), &OwncloudPropagator::finished, this, &SyncEngine::slotPropagationFinished, Qt::QueuedConnection);
-    connect(_propagator.data(), &OwncloudPropagator::seenLockedFile, this, &SyncEngine::seenLockedFile);
-    connect(_propagator.data(), &OwncloudPropagator::touchedFile, this, &SyncEngine::slotAddTouchedFile);
-    connect(_propagator.data(), &OwncloudPropagator::insufficientLocalStorage, this, &SyncEngine::slotInsufficientLocalStorage);
-    connect(_propagator.data(), &OwncloudPropagator::insufficientRemoteStorage, this, &SyncEngine::slotInsufficientRemoteStorage);
-    connect(_propagator.data(), &OwncloudPropagator::newItem, this, &SyncEngine::slotNewItem);
-
-    // apply the network limits to the propagator
-    setNetworkLimits(_uploadLimit, _downloadLimit);
-
-    deleteStaleDownloadInfos(_syncItems);
-    deleteStaleUploadInfos(_syncItems);
-    deleteStaleErrorBlacklistEntries(_syncItems);
-    _journal->commit(QStringLiteral("post stale entry removal"));
-
-    // Emit the started signal only after the propagator has been set up.
-    if (_needsUpdate)
-        emit(started());
-
-    _propagator->start(_syncItems);
-    _syncItems.clear();
-
-    qCInfo(lcEngine) << "#### Post-Reconcile end #################################################### " << _stopWatch.addLapTime(QStringLiteral("Post-Reconcile Finished")) << "ms";
+    finish();
 }
 
 void SyncEngine::slotCleanPollsJobAborted(const QString &error)
diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h
index a2178bf61..4830ec730 100644
--- a/src/libsync/syncengine.h
+++ b/src/libsync/syncengine.h
@@ -159,7 +159,7 @@ signals:
      * This usually happen when the server was reset or something.
      * Set *cancel to true in a slot connected from this signal to abort the sync.
      */
-    void aboutToRemoveAllFiles(SyncFileItem::Direction direction, bool *cancel);
+    void aboutToRemoveAllFiles(SyncFileItem::Direction direction, std::function<void(bool)> f);
 
     // A new folder was discovered and was not synced because of the confirmation feature
     void newBigFolder(const QString &folder, bool isExternal);
diff --git a/test/testallfilesdeleted.cpp b/test/testallfilesdeleted.cpp
index e5505fb0f..4a4580a62 100644
--- a/test/testallfilesdeleted.cpp
+++ b/test/testallfilesdeleted.cpp
@@ -58,11 +58,11 @@ private slots:
         auto initialState = fakeFolder.currentLocalState();
         int aboutToRemoveAllFilesCalled = 0;
         QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveAllFiles,
-            [&](SyncFileItem::Direction dir, bool *cancel) {
+            [&](SyncFileItem::Direction dir, std::function<void(bool)> callback) {
                 QCOMPARE(aboutToRemoveAllFilesCalled, 0);
                 aboutToRemoveAllFilesCalled++;
                 QCOMPARE(dir, deleteOnRemote ? SyncFileItem::Down : SyncFileItem::Up);
-                *cancel = true;
+                callback(true);
                 fakeFolder.syncEngine().journal()->clearFileTable(); // That's what Folder is doing
             });
 
@@ -99,11 +99,11 @@ private slots:
 
         int aboutToRemoveAllFilesCalled = 0;
         QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveAllFiles,
-            [&](SyncFileItem::Direction dir, bool *cancel) {
+            [&](SyncFileItem::Direction dir, std::function<void(bool)> callback) {
                 QCOMPARE(aboutToRemoveAllFilesCalled, 0);
                 aboutToRemoveAllFilesCalled++;
                 QCOMPARE(dir, deleteOnRemote ? SyncFileItem::Down : SyncFileItem::Up);
-                *cancel = false;
+                callback(false);
             });
 
         auto &modifier = deleteOnRemote ? fakeFolder.remoteModifier() : fakeFolder.localModifier();
@@ -158,11 +158,11 @@ private slots:
 
         int aboutToRemoveAllFilesCalled = 0;
         QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveAllFiles,
-            [&](SyncFileItem::Direction dir, bool *cancel) {
+            [&](SyncFileItem::Direction dir, std::function<void(bool)> callback) {
                 QCOMPARE(aboutToRemoveAllFilesCalled, 0);
                 aboutToRemoveAllFilesCalled++;
                 QCOMPARE(dir, SyncFileItem::Down);
-                *cancel = false;
+                callback(false);
             });
 
         // Some small changes
@@ -277,7 +277,7 @@ private slots:
 
         int aboutToRemoveAllFilesCalled = 0;
         QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveAllFiles,
-            [&](SyncFileItem::Direction , bool *) {
+            [&](SyncFileItem::Direction , std::function<void(bool)> ) {
                 aboutToRemoveAllFilesCalled++;
                 QFAIL("should not be called");
             });
@@ -302,7 +302,7 @@ private slots:
 
         int aboutToRemoveAllFilesCalled = 0;
         QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::aboutToRemoveAllFiles,
-            [&](SyncFileItem::Direction , bool *) {
+            [&](SyncFileItem::Direction , std::function<void(bool)>) {
                 aboutToRemoveAllFilesCalled++;
                 QFAIL("should not be called");
             });