Skip to content

Commit

Permalink
fix: thumbnails and hidden files cause dead loops
Browse files Browse the repository at this point in the history 
The datachanged signal emited by thumbnail will lead model refresh by .hidden file. 
The model refresh file info also update thumbnail and emit datachanged again.
This becomes a dead cycle.

Add flag refreshfile to control whether refresh file info in model refresh.
Add roles on emiting datachanged signal.

Log: 

Bug: https://pms.uniontech.com/bug-view-211109.html
  • Loading branch information
@Clauszy @deepin-bot
Clauszy authored and deepin-bot[bot] committed Jul 28, 2023
1 parent 2fc4890 commit 2c878c3
Show file tree
Hide file tree
Showing 26 changed files with 89 additions and 79 deletions.
4 changes: 2 additions & 2 deletions src/plugins/desktop/core/ddplugin-canvas/hook/canvasmodelhook.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ bool CanvasModelHook::dataRested(QList<QUrl> *urls, void *extData) const
return CanvasModelRunHook(hook_CanvasModel_DataRested, urls, extData);
}

bool CanvasModelHook::dataChanged(const QUrl &url, void *extData) const
bool CanvasModelHook::dataChanged(const QUrl &url, const QVector<int> &roles, void *extData) const
{
return CanvasModelRunHook(hook_CanvasModel_DataChanged, url, extData);
return CanvasModelRunHook(hook_CanvasModel_DataChanged, url, roles, extData);
}

bool CanvasModelHook::dropMimeData(const QMimeData *data, const QUrl &dir, Qt::DropAction action, void *extData) const
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/desktop/core/ddplugin-canvas/hook/canvasmodelhook.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class CanvasModelHook : public QObject, public ModelHookInterface
bool dataRemoved(const QUrl &url, void *extData = nullptr) const override; // must return false
bool dataRenamed(const QUrl &oldUrl, const QUrl &newUrl, void *extData = nullptr) const override;
bool dataRested(QList<QUrl> *urls, void *extData = nullptr) const override; // must return false
bool dataChanged(const QUrl &url, void *extData = nullptr) const override; // must return false
bool dataChanged(const QUrl &url, const QVector<int> &roles, void *extData = nullptr) const override; // must return false
bool dropMimeData(const QMimeData *data, const QUrl &dir, Qt::DropAction action, void *extData = nullptr) const override;
bool mimeData(const QList<QUrl> &urls, QMimeData *out, void *extData = nullptr) const override;
bool mimeTypes (QStringList *types, void *extData = nullptr) const override;
Expand Down
21 changes: 11 additions & 10 deletions src/plugins/desktop/core/ddplugin-canvas/model/canvasmodelfilter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ bool CanvasModelFilter::resetFilter(QList<QUrl> &urls)
return false;
}

bool CanvasModelFilter::updateFilter(const QUrl &url)
bool CanvasModelFilter::updateFilter(const QUrl &url, const QVector<int> &roles)
{
return false;
}
Expand All @@ -48,11 +48,8 @@ bool HiddenFileFilter::insertFilter(const QUrl &url)
if (model->showHiddenFiles())
return false;

if (auto info = FileCreator->createFileInfo(url)) {
//! fouce refresh
info->refresh();
if (auto info = FileCreator->createFileInfo(url))
return info->isAttributes(OptInfoType::kIsHidden);
}

return false;
}
Expand All @@ -74,13 +71,17 @@ bool HiddenFileFilter::resetFilter(QList<QUrl> &urls)
return false;
}

bool HiddenFileFilter::updateFilter(const QUrl &url)
bool HiddenFileFilter::updateFilter(const QUrl &url, const QVector<int> &roles)
{
// the filemanager hidden attr changed.
// get file that removed form .hidden if do not show hidden file.
if (!model->showHiddenFiles() && url.fileName() == ".hidden") {
qDebug() << "refresh by hidden changed.";
model->refresh(model->rootIndex());
// just refresh model if file content changed.
if (roles.contains(Global::kItemCreateFileInfoRole)) {
// get file that removed form .hidden if do not show hidden file.
if (!model->showHiddenFiles() && url.fileName() == ".hidden") {
qDebug() << "refresh by hidden changed.";
// do not refresh file info and wait 100ms to let the file atrr changed signal to refresh file
model->refresh(model->rootIndex(), false, 100, false);
}
}

return false;
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/desktop/core/ddplugin-canvas/model/canvasmodelfilter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class CanvasModelFilter
explicit CanvasModelFilter(CanvasProxyModel *m);
virtual bool insertFilter(const QUrl &url);
virtual bool resetFilter(QList<QUrl> &urls);
virtual bool updateFilter(const QUrl &url);
virtual bool updateFilter(const QUrl &url, const QVector<int> &roles = {});
virtual bool removeFilter(const QUrl &url);
virtual bool renameFilter(const QUrl &oldUrl, const QUrl &newUrl);
protected:
Expand All @@ -32,7 +32,7 @@ class HiddenFileFilter : public CanvasModelFilter
using CanvasModelFilter::CanvasModelFilter;
bool insertFilter(const QUrl &url) override;
bool resetFilter(QList<QUrl> &urls) override;
bool updateFilter(const QUrl &url) override;
bool updateFilter(const QUrl &url, const QVector<int> &roles = {}) override;
bool renameFilter(const QUrl &oldUrl, const QUrl &newUrl) override;
};

Expand Down
22 changes: 11 additions & 11 deletions src/plugins/desktop/core/ddplugin-canvas/model/canvasproxymodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,14 @@ bool CanvasProxyModelPrivate::resetFilter(QList<QUrl> &urls)
return ret;
}

bool CanvasProxyModelPrivate::updateFilter(const QUrl &url)
bool CanvasProxyModelPrivate::updateFilter(const QUrl &url, const QVector<int> &roles)
{
// these filters is like Notifier.
// so it will don't interrupt when some one return true.
bool ret = false;
auto unused = std::all_of(modelFilters.begin(), modelFilters.end(),
[&ret, &url](const QSharedPointer<CanvasModelFilter> &filter) {
ret |= filter->updateFilter(url);
[&ret, &url, &roles](const QSharedPointer<CanvasModelFilter> &filter) {
ret |= filter->updateFilter(url, roles);
return true;
});
Q_UNUSED(unused);
Expand Down Expand Up @@ -406,13 +406,13 @@ bool CanvasProxyModelPrivate::doSort(QList<QUrl> &files) const
return true;
}

void CanvasProxyModelPrivate::doRefresh(bool global)
void CanvasProxyModelPrivate::doRefresh(bool global, bool refreshFile)
{
if (global) {
srcModel->refresh(srcModel->rootIndex());
} else {
// refresh all file info
{
if (refreshFile) {
// do not emit data changed signal, just refresh file info.
QSignalBlocker blocker(srcModel);
srcModel->update();
Expand All @@ -436,12 +436,12 @@ void CanvasProxyModelPrivate::sourceDataChanged(const QModelIndex &sourceTopleft
// find items in this model
for (int i = begin; i <= end; ++i) {
auto url = srcModel->fileUrl(srcModel->index(i));
if (hookIfs && hookIfs->dataChanged(url)) {
if (hookIfs && hookIfs->dataChanged(url, roles)) {
qWarning() << "invalid module: dataChanged returns true.";
}

// canvas filter
updateFilter(url);
updateFilter(url, roles);

auto cur = q->index(url);
if (cur.isValid())
Expand Down Expand Up @@ -829,7 +829,7 @@ void CanvasProxyModel::update()
emit dataChanged(createIndex(0, 0), createIndex(rowCount(rootIndex()) - 1, 0));
}

void CanvasProxyModel::refresh(const QModelIndex &parent, bool global, int ms)
void CanvasProxyModel::refresh(const QModelIndex &parent, bool global, int ms, bool refreshFile)
{
d->isNotMixDirAndFile = !Application::instance()->appAttribute(Application::kFileAndDirMixedSort).toBool();

Expand All @@ -840,12 +840,12 @@ void CanvasProxyModel::refresh(const QModelIndex &parent, bool global, int ms)
d->refreshTimer->stop();

if (ms < 1) {
d->doRefresh(global);
d->doRefresh(global, refreshFile);
} else {
d->refreshTimer.reset(new QTimer);
d->refreshTimer->setSingleShot(true);
connect(d->refreshTimer.get(), &QTimer::timeout, this, [this, global]() {
d->doRefresh(global);
connect(d->refreshTimer.get(), &QTimer::timeout, this, [this, global, refreshFile]() {
d->doRefresh(global, refreshFile);
});

d->refreshTimer->start(ms);
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/desktop/core/ddplugin-canvas/model/canvasproxymodel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class CanvasProxyModel : public QAbstractProxyModel
public slots:
bool sort();
void update();
void refresh(const QModelIndex &parent, bool global = false, int ms = 50);
void refresh(const QModelIndex &parent, bool global = false, int ms = 50, bool refreshFile = true);
void setShowHiddenFiles(bool show);
bool fetch(const QUrl &url); //show \a url if exsited
bool take(const QUrl &url); // hide \a url
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/desktop/core/ddplugin-canvas/model/canvasproxymodel_p.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class CanvasProxyModelPrivate : public QObject
bool doSort(QList<QUrl> &files) const;
bool lessThan(const QUrl &left, const QUrl &right) const;
public slots:
void doRefresh(bool global);
void doRefresh(bool global, bool refreshFile);
void sourceDataChanged(const QModelIndex &sourceTopleft,
const QModelIndex &sourceBottomright,
const QVector<int> &roles);
Expand All @@ -44,7 +44,7 @@ public slots:
public:
bool insertFilter(const QUrl &url);
bool resetFilter(QList<QUrl> &urls);
bool updateFilter(const QUrl &url);
bool updateFilter(const QUrl &url , const QVector<int> &roles);
bool removeFilter(const QUrl &url);
bool renameFilter(const QUrl &oldUrl, const QUrl &newUrl);

Expand Down
17 changes: 13 additions & 4 deletions src/plugins/desktop/core/ddplugin-canvas/model/fileinfomodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,12 @@ void FileInfoModelPrivate::insertData(const QUrl &url)
int row = -1;
{
QReadLocker lk(&lock);
if (Q_UNLIKELY(fileMap.contains(url))) {
qWarning() << "file exists:" << url;
if (auto cur = fileMap.value(url)) {
lk.unlock();
qInfo() << "the file to insert is existed" << url;
cur->refresh(); // refresh fileinfo.
const QModelIndex &index = q->index(url);
emit q->dataChanged(index, index);
return;
}
row = fileList.count();
Expand Down Expand Up @@ -157,7 +161,12 @@ void FileInfoModelPrivate::replaceData(const QUrl &oldUrl, const QUrl &newUrl)
removeData(oldUrl);
lk.relock();
position = fileList.indexOf(newUrl);
auto cur = fileMap.value(newUrl);
lk.unlock();

// refresh file
cur->refresh();
qInfo() << "move file" << oldUrl << "to overwritte" << newUrl;
} else {
fileList.replace(position, newUrl);
fileMap.remove(oldUrl);
Expand Down Expand Up @@ -190,7 +199,7 @@ void FileInfoModelPrivate::updateData(const QUrl &url)
if (Q_UNLIKELY(!index.isValid()))
return;

emit q->dataChanged(index, index);
emit q->dataChanged(index, index, {Global::kItemCreateFileInfoRole});
}

void FileInfoModelPrivate::dataUpdated(const QUrl &url, const bool isLinkOrg)
Expand Down Expand Up @@ -237,7 +246,7 @@ void FileInfoModelPrivate::thumbUpdated(const QUrl &url, const QString &thumb)
if (Q_UNLIKELY(!index.isValid()))
return;

emit q->dataChanged(index, index);
emit q->dataChanged(index, index, {kItemIconRole});
}

FileInfoModel::FileInfoModel(QObject *parent)
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/desktop/core/ddplugin-canvas/model/modelhookinterface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ bool ModelHookInterface::dataRested(QList<QUrl> *urls, void *extData) const
return false;
}

bool ModelHookInterface::dataChanged(const QUrl &url, void *extData) const
bool ModelHookInterface::dataChanged(const QUrl &url, const QVector<int> &roles, void *extData) const
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/desktop/core/ddplugin-canvas/model/modelhookinterface.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class ModelHookInterface
virtual bool dataRemoved(const QUrl &url, void *extData = nullptr) const; // must return false
virtual bool dataRenamed(const QUrl &oldUrl, const QUrl &newUrl, void *extData = nullptr) const;
virtual bool dataRested(QList<QUrl> *urls, void *extData = nullptr) const; // must return false
virtual bool dataChanged(const QUrl &url, void *extData = nullptr) const; // must return false
virtual bool dataChanged(const QUrl &url, const QVector<int> &roles, void *extData = nullptr) const; // must return false
virtual bool dropMimeData(const QMimeData *data, const QUrl &dir, Qt::DropAction action, void *extData = nullptr) const;
virtual bool mimeData(const QList<QUrl> &urls, QMimeData *out, void *extData = nullptr) const;
virtual bool mimeTypes (QStringList *types, void *extData = nullptr) const;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/desktop/ddplugin-organizer/broker/organizerbroker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class OrganizerBroker : public QObject
signals:

public slots:
virtual void refreshModel(bool global, int ms) = 0;
virtual void refreshModel(bool global, int ms, bool file) = 0;
virtual QString gridPoint(const QUrl &item, QPoint *point) = 0;
virtual QRect visualRect(const QString &id, const QUrl &item) = 0;
virtual QAbstractItemView * view(const QString &id) = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/desktop/ddplugin-organizer/mode/normalized/normalizedmodebroker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ NormalizedModeBroker::NormalizedModeBroker(NormalizedMode *parent)
Q_ASSERT(mode);
}

void NormalizedModeBroker::refreshModel(bool global, int ms)
void NormalizedModeBroker::refreshModel(bool global, int ms, bool file)
{
if (auto m = mode->getModel())
m->refresh(m->rootIndex(), global, ms);
m->refresh(m->rootIndex(), global, ms, file);
}

QString NormalizedModeBroker::gridPoint(const QUrl &item, QPoint *point)
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/desktop/ddplugin-organizer/mode/normalized/normalizedmodebroker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class NormalizedModeBroker : public OrganizerBroker
public:
explicit NormalizedModeBroker(NormalizedMode *parent = nullptr);
public slots:
void refreshModel(bool global, int ms) override;
void refreshModel(bool global, int ms, bool file) override;
QString gridPoint(const QUrl &item, QPoint *point) override;
QRect visualRect(const QString &id, const QUrl &item) override;
QAbstractItemView *view(const QString &id) override;
Expand Down
19 changes: 12 additions & 7 deletions src/plugins/desktop/ddplugin-organizer/models/collectionmodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ void CollectionModelPrivate::sourceDataChanged(const QModelIndex &sourceTopleft,
auto cur = q->index(url);

if (handler)
handler->acceptUpdate(url);
handler->acceptUpdate(url, roles);

if (cur.isValid())
idxs << cur;
Expand Down Expand Up @@ -262,14 +262,19 @@ void CollectionModelPrivate::clearRenameReuired()
waitForRenameFile.clear();
}

void CollectionModelPrivate::doRefresh(bool global)
void CollectionModelPrivate::doRefresh(bool global, bool file)
{
if (global) {
shell->refresh(shell->rootIndex());
} else {
if (file) {
// do not emit data changed signal, just refresh file info.
QSignalBlocker blocker(q);
q->update();
}

sourceAboutToBeReset();
sourceReset();
q->update();
}
}

Expand Down Expand Up @@ -369,7 +374,7 @@ QUrl CollectionModel::fileUrl(const QModelIndex &index) const
return d->fileList.at(index.row());
}

void CollectionModel::refresh(const QModelIndex &parent, bool global, int ms)
void CollectionModel::refresh(const QModelIndex &parent, bool global, int ms, bool file)
{
if (parent != rootIndex())
return;
Expand All @@ -378,12 +383,12 @@ void CollectionModel::refresh(const QModelIndex &parent, bool global, int ms)
d->refreshTimer->stop();

if (ms < 1) {
d->doRefresh(global);
d->doRefresh(global, file);
} else {
d->refreshTimer.reset(new QTimer);
d->refreshTimer->setSingleShot(true);
connect(d->refreshTimer.get(), &QTimer::timeout, this, [this, global]() {
d->doRefresh(global);
connect(d->refreshTimer.get(), &QTimer::timeout, this, [this, global, file]() {
d->doRefresh(global, file);
});

d->refreshTimer->start(ms);
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/desktop/ddplugin-organizer/models/collectionmodel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class CollectionModel : public QAbstractProxyModel
FileInfoPointer fileInfo(const QModelIndex &index) const;
QList<QUrl> files() const;
QUrl fileUrl(const QModelIndex &index) const;
void refresh(const QModelIndex &parent, bool global = false, int ms = 50);
void refresh(const QModelIndex &parent, bool global = false, int ms = 50, bool file = true);
void update();
bool fetch(const QList<QUrl> &urls);
bool take(const QList<QUrl> &urls);
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/desktop/ddplugin-organizer/models/collectionmodel_p.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class CollectionModelPrivate : public QObject
void reset();
void clearMapping();
void createMapping();
void doRefresh(bool global);
void doRefresh(bool global, bool file);
public slots:
void sourceDataChanged(const QModelIndex &sourceTopleft,
const QModelIndex &sourceBottomright,
Expand Down
Loading

0 comments on commit 2c878c3

Please sign in to comment.