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

Fixes: #8170

src/gui/folder.cpp

@@ -35,6 +35,7 @@
 #include "csync_exclude.h"
 #include "common/vfs.h"
 #include "creds/abstractcredentials.h"
+#include "settingsdialog.h"
 
 #include <QTimer>
 #include <QUrl>
@@ -92,7 +93,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);
@@ -1226,37 +1226,37 @@ 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' 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 restore the files, they will be re-synced with the server if you have rights to do so.\n"
                                                  "If you decide to delete the files, they will be unavailable to you, unless you are the owner.")
-                                            : tr("All files got deleted from your local sync folder '%1'.\n"
-                                                 "These files will be deleted from the server and will not be available on your other devices if they "
-                                                 "will not be restored.\n"
-                                                 "If this action was unintended you can restore the lost data now.");
-    QMessageBox msgBox(QMessageBox::Warning, tr("Delete all files?"),
-        msg.arg(shortGuiLocalPath()));
-    msgBox.setWindowFlags(msgBox.windowFlags() | Qt::WindowStaysOnTopHint);
-    msgBox.addButton(tr("Delete all files"), QMessageBox::DestructiveRole);
-    QPushButton *keepBtn = msgBox.addButton(tr("Restore deleted files"), QMessageBox::AcceptRole);
-    if (msgBox.exec() == -1) {
-        *cancel = true;
-        return;
-    }
-    *cancel = msgBox.clickedButton() == keepBtn;
-    if (*cancel) {
-        FileSystem::setFolderMinimumPermissions(path());
-        journalDb()->clearFileTable();
-        _lastEtag.clear();
-        slotScheduleThisFolder();
-    }
+                                            : tr("All the files in your local sync folder '%1' were deleted. These deletes will be "
+                                                 "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.");
+    auto msgBox = new QMessageBox(QMessageBox::Warning, tr("Remove All Files?"),
+        msg.arg(shortGuiLocalPath()), QMessageBox::NoButton);
+    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)

src/gui/folder.h

@@ -311,7 +311,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

src/libsync/syncengine.cpp

@@ -647,99 +647,103 @@ void SyncEngine::slotDiscoveryFinished()
     emit transmissionProgress(*_progressInfo);
 
     //    qCInfo(lcEngine) << "Permissions of the root folder: " << _csync_ctx->remote.root_perms.toString();
-
-    ConfigFile cfgFile;
-    if (!_hasNoneFiles && _hasRemoveFile && cfgFile.promptDeleteFiles()) {
-        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;
-            }
-        }
-        emit aboutToRemoveAllFiles(side >= 0 ? SyncFileItem::Down : SyncFileItem::Up, &cancel);
-        if (cancel) {
-            qCInfo(lcEngine) << "User aborted sync";
-            finalize(false);
-            return;
+    auto finish = [this]{
+        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);
         }
-    }
 
-    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);
-    }
+        if (_discoveryPhase->_anotherSyncNeeded && _anotherSyncNeeded == NoFollowUpSync) {
+            _anotherSyncNeeded = ImmediateFollowUp;
+        }
 
-    if (_discoveryPhase->_anotherSyncNeeded && _anotherSyncNeeded == NoFollowUpSync) {
-        _anotherSyncNeeded = ImmediateFollowUp;
-    }
+        Q_ASSERT(std::is_sorted(_syncItems.begin(), _syncItems.end()));
 
-    Q_ASSERT(std::is_sorted(_syncItems.begin(), _syncItems.end()));
+        qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate) #################################################### " << _stopWatch.addLapTime(QStringLiteral("Reconcile (aboutToPropagate)")) << "ms";
 
-    qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate) #################################################### " << _stopWatch.addLapTime(QLatin1String("Reconcile (aboutToPropagate)")) << "ms";
+        _localDiscoveryPaths.clear();
 
-    _localDiscoveryPaths.clear();
+        // To announce the beginning of the sync
+        emit aboutToPropagate(_syncItems);
 
-    // To announce the beginning of the sync
-    emit aboutToPropagate(_syncItems);
+        qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate OK) #################################################### "<< _stopWatch.addLapTime(QStringLiteral("Reconcile (aboutToPropagate OK)")) << "ms";
 
-    qCInfo(lcEngine) << "#### Reconcile (aboutToPropagate OK) #################################################### "<< _stopWatch.addLapTime(QLatin1String("Reconcile (aboutToPropagate OK)")) << "ms";
+        // it's important to do this before ProgressInfo::start(), to announce start of new sync
+        _progressInfo->_status = ProgressInfo::Propagation;
+        emit transmissionProgress(*_progressInfo);
+        _progressInfo->startEstimateUpdates();
 
-    // it's important to do this before ProgressInfo::start(), to announce start of new sync
-    _progressInfo->_status = ProgressInfo::Propagation;
-    emit transmissionProgress(*_progressInfo);
-    _progressInfo->startEstimateUpdates();
+        // 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");
 
-    // post update phase script: allow to tweak stuff by a custom script in debug mode.
-    if (!qEnvironmentVariableIsEmpty("OWNCLOUD_POST_UPDATE_SCRIPT")) {
-#ifndef NDEBUG
-        QString script = qgetenv("OWNCLOUD_POST_UPDATE_SCRIPT");
+            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
+        }
 
-        qCDebug(lcEngine) << "Post Update Script: " << script;
-        QProcess::execute(script.toUtf8());
-#else
-        qCWarning(lcEngine) << "**** Attention: POST_UPDATE_SCRIPT installed, but not executed because compiled with NDEBUG";
-#endif
+        // 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::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";
+    };
+
+    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("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::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("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(QLatin1String("Post-Reconcile Finished")) << "ms";
+    finish();
 }
 
 void SyncEngine::slotCleanPollsJobAborted(const QString &error)

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);

test/testallfilesdeleted.cpp

@@ -61,11 +61,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
             });
 
@@ -102,11 +102,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();
@@ -161,11 +161,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
@@ -280,7 +280,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");
             });
@@ -305,7 +305,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");
             });