FolderStatusModel: fix potential assert

OCC::FolderStatusModel::slotUpdateDirectories: ASSERT: "parentInfo->_fetching" in file /home/olivier/kdegit/owncloud/mirall/src/gui/folderstatusmodel.cpp, line 599 This can happen if the structure of a folder is change while the user expands the root folder. In this case, resetSubs() is called which resets _fetching to false. Instead, we need to keep a pointer to the job so we can abort it by deleting it.
owncloud · Mar 28, 2018 · d69936e · d69936e
1 parent 1ac45be
commit d69936e
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 10 deletions.
diff --git a/src/gui/folderstatusmodel.cpp b/src/gui/folderstatusmodel.cpp
@@ -526,7 +526,7 @@ bool FolderStatusModel::canFetchMore(const QModelIndex &parent) const
         return false;
     }
     auto info = infoForIndex(parent);
-    if (!info || info->_fetched || info->_fetching)
+    if (!info || info->_fetched || info->_fetchingJob)
         return false;
     if (info->_hasError) {
         // Keep showing the error to the user, it will be hidden when the account reconnects
@@ -540,10 +540,9 @@ void FolderStatusModel::fetchMore(const QModelIndex &parent)
 {
     auto info = infoForIndex(parent);
 
-    if (!info || info->_fetched || info->_fetching)
+    if (!info || info->_fetched || info->_fetchingJob)
         return;
     info->resetSubs(this, parent);
-    info->_fetching = true;
     QString path = info->_folder->remotePath();
     if (info->_path != QLatin1String("/")) {
         if (!path.endsWith(QLatin1Char('/'))) {
@@ -552,6 +551,7 @@ void FolderStatusModel::fetchMore(const QModelIndex &parent)
         path += info->_path;
     }
     LsColJob *job = new LsColJob(_accountState->account(), path, this);
+    info->_fetchingJob = job;
     job->setProperties(QList<QByteArray>() << "resourcetype"
                                            << "http://owncloud.org/ns:size"
                                            << "http://owncloud.org/ns:permissions");
@@ -596,18 +596,18 @@ void FolderStatusModel::slotUpdateDirectories(const QStringList &list)
     if (!parentInfo) {
         return;
     }
-    ASSERT(parentInfo->_fetching); // we should only get a result if we were doing a fetch
+    ASSERT(parentInfo->_fetchingJob == job);
     ASSERT(parentInfo->_subs.isEmpty());
 
     if (parentInfo->hasLabel()) {
         beginRemoveRows(idx, 0, 0);
-        parentInfo->_lastErrorString.clear();
         parentInfo->_hasError = false;
         parentInfo->_fetchingLabel = false;
         endRemoveRows();
     }
 
-    parentInfo->_fetching = false;
+    parentInfo->_lastErrorString.clear();
+    parentInfo->_fetchingJob = nullptr;
     parentInfo->_fetched = true;
 
     QUrl url = parentInfo->_folder->remoteUrl();
@@ -1197,7 +1197,7 @@ void FolderStatusModel::slotShowFetchProgress()
         if (it.value().elapsed() > 800) {
             auto idx = it.key();
             auto *info = infoForIndex(idx);
-            if (info && info->_fetching) {
+            if (info && info->_fetchingJob) {
                 bool add = !info->hasLabel();
                 if (add) {
                     beginInsertRows(idx, 0, 0);
@@ -1220,7 +1220,7 @@ bool FolderStatusModel::SubFolderInfo::hasLabel() const
 void FolderStatusModel::SubFolderInfo::resetSubs(FolderStatusModel *model, QModelIndex index)
 {
     _fetched = false;
-    _fetching = false;
+    delete _fetchingJob;
     if (hasLabel()) {
         model->beginRemoveRows(index, 0, 0);
         _fetchingLabel = false;

diff --git a/src/gui/folderstatusmodel.h b/src/gui/folderstatusmodel.h
@@ -20,6 +20,7 @@
 #include <QLoggingCategory>
 #include <QVector>
 #include <QElapsedTimer>
+#include <QPointer>
 
 class QNetworkReply;
 namespace OCC {
@@ -28,6 +29,7 @@ Q_DECLARE_LOGGING_CATEGORY(lcFolderStatus)
 
 class Folder;
 class ProgressInfo;
+class LsColJob;
 
 /**
  * @brief The FolderStatusModel class
@@ -59,7 +61,6 @@ class FolderStatusModel : public QAbstractItemModel
             , _size(0)
             , _isExternal(false)
             , _fetched(false)
-            , _fetching(false)
             , _hasError(false)
             , _fetchingLabel(false)
             , _isUndecided(false)
@@ -75,7 +76,7 @@ class FolderStatusModel : public QAbstractItemModel
         bool _isExternal;
 
         bool _fetched; // If we did the LSCOL for this folder already
-        bool _fetching; // Whether a LSCOL job is currently running
+        QPointer<LsColJob> _fetchingJob; // Currently running LsColJob
         bool _hasError; // If the last fetching job ended in an error
         QString _lastErrorString;
         bool _fetchingLabel; // Whether a 'fetching in progress' label is shown.