From dc149ec5e52f52c514cf362603d05ba8eea506b8 Mon Sep 17 00:00:00 2001 From: Jin Liu Date: Thu, 29 Feb 2024 23:13:47 +0000 Subject: [PATCH] DragAndDropHelper::updateDropAction: use StatJob for remote URLs When dragging onto tabs/Places from a remote URL, we don't process the QDropEvent immediately, but start a StatJob and process the event when it finishes. Also, the result of the StatJob is cached for 30 seconds, to avoid starting duplicate jobs. --- src/dolphintabwidget.cpp | 4 +- src/dolphintabwidget.h | 3 ++ src/panels/places/placespanel.cpp | 4 +- src/panels/places/placespanel.h | 3 ++ src/views/draganddrophelper.cpp | 79 ++++++++++++++++++++++++++++--- src/views/draganddrophelper.h | 58 ++++++++++++++++++----- 6 files changed, 129 insertions(+), 22 deletions(-) diff --git a/src/dolphintabwidget.cpp b/src/dolphintabwidget.cpp index f80b94ea7c..32e251f02b 100644 --- a/src/dolphintabwidget.cpp +++ b/src/dolphintabwidget.cpp @@ -9,7 +9,6 @@ #include "dolphin_generalsettings.h" #include "dolphintabbar.h" #include "dolphinviewcontainer.h" -#include "views/draganddrophelper.h" #include #include @@ -26,6 +25,7 @@ DolphinTabWidget::DolphinTabWidget(DolphinNavigatorsWidgetAction *navigatorsWidg : QTabWidget(parent) , m_lastViewedTab(nullptr) , m_navigatorsWidget{navigatorsWidget} + , m_dragAndDropHelper{this} { KAcceleratorManager::setNoAccel(this); @@ -394,7 +394,7 @@ void DolphinTabWidget::tabDragMoveEvent(int index, QDragMoveEvent *event) { if (index >= 0) { DolphinView *view = tabPageAt(index)->activeViewContainer()->view(); - DragAndDropHelper::updateDropAction(event, view->url()); + m_dragAndDropHelper.updateDropAction(event, view->url()); } } diff --git a/src/dolphintabwidget.h b/src/dolphintabwidget.h index a28a6bea1f..a02c8fb841 100644 --- a/src/dolphintabwidget.h +++ b/src/dolphintabwidget.h @@ -9,6 +9,7 @@ #include "dolphinnavigatorswidgetaction.h" #include "dolphintabpage.h" +#include "views/draganddrophelper.h" #include #include @@ -276,6 +277,8 @@ private: */ const std::optional viewShowingItem(const QUrl &item) const; + DragAndDropHelper m_dragAndDropHelper; + private: QPointer m_lastViewedTab; QPointer m_navigatorsWidget; diff --git a/src/panels/places/placespanel.cpp b/src/panels/places/placespanel.cpp index ba3451bd5e..eaf2642eb2 100644 --- a/src/panels/places/placespanel.cpp +++ b/src/panels/places/placespanel.cpp @@ -15,7 +15,6 @@ #include "dolphin_placespanelsettings.h" #include "dolphinplacesmodelsingleton.h" #include "settings/dolphinsettingsdialog.h" -#include "views/draganddrophelper.h" #include #include @@ -32,6 +31,7 @@ PlacesPanel::PlacesPanel(QWidget *parent) : KFilePlacesView(parent) + , m_dragAndDropHelper(this) { setDropOnPlaceEnabled(true); connect(this, &PlacesPanel::urlsDropped, this, &PlacesPanel::slotUrlsDropped); @@ -161,7 +161,7 @@ void PlacesPanel::dragMoveEvent(QDragMoveEvent *event) if (!url.isValid() || !KProtocolManager::supportsWriting(url)) { event->setDropAction(Qt::IgnoreAction); } else { - DragAndDropHelper::updateDropAction(event, url); + m_dragAndDropHelper.updateDropAction(event, url); } } } diff --git a/src/panels/places/placespanel.h b/src/panels/places/placespanel.h index cbd5fc224c..d21e7d64e5 100644 --- a/src/panels/places/placespanel.h +++ b/src/panels/places/placespanel.h @@ -10,6 +10,7 @@ #define PLACESPANEL_H #include "panels/panel.h" +#include "views/draganddrophelper.h" #include #include @@ -78,6 +79,8 @@ private: QAction *m_configureTrashAction; QAction *m_openInSplitView; QAction *m_lockPanelsAction; + + DragAndDropHelper m_dragAndDropHelper; }; #endif // PLACESPANEL_H diff --git a/src/views/draganddrophelper.cpp b/src/views/draganddrophelper.cpp index 7b9949df4d..7163507a67 100644 --- a/src/views/draganddrophelper.cpp +++ b/src/views/draganddrophelper.cpp @@ -73,20 +73,85 @@ bool DragAndDropHelper::supportsDropping(const KFileItem &destItem) return (destItem.isDir() && destItem.isWritable()) || destItem.isDesktopFile(); } +DragAndDropHelper::DragAndDropHelper(QObject *parent) + : QObject(parent) +{ + m_destItemCacheInvalidationTimer.setSingleShot(true); + m_destItemCacheInvalidationTimer.setInterval(30000); + connect(&m_destItemCacheInvalidationTimer, &QTimer::timeout, this, [this]() { + m_destItemCache = KFileItem(); + }); +} + void DragAndDropHelper::updateDropAction(QDropEvent *event, const QUrl &destUrl) { + auto processEvent = [this](QDropEvent *event) { + if (supportsDropping(m_destItemCache)) { + event->setDropAction(event->proposedAction()); + event->accept(); + } else { + event->setDropAction(Qt::IgnoreAction); + event->ignore(); + } + }; + + m_lastUndecidedEvent = nullptr; + if (urlListMatchesUrl(event->mimeData()->urls(), destUrl)) { event->setDropAction(Qt::IgnoreAction); event->ignore(); + return; } - KFileItem item(destUrl); - if (!item.isLocalFile() || supportsDropping(item)) { - event->setDropAction(event->proposedAction()); - event->accept(); - } else { - event->setDropAction(Qt::IgnoreAction); - event->ignore(); + + if (destUrl == m_destItemCache.url()) { + // We already received events for this URL, and already have the + // stat result cached because: + // 1. it's a local file, and we already called KFileItem(destUrl) + // 2. it's a remote file, and StatJob finished + processEvent(event); + return; } + + if (m_statJob) { + if (destUrl == m_statJobUrl) { + // We already received events for this URL. Still waiting for + // the stat result. StatJob will process the event when it finishes. + m_lastUndecidedEvent = event; + return; + } + + // We are waiting for the stat result of a different URL. Cancel. + m_statJob->kill(); + m_statJob = nullptr; + m_statJobUrl.clear(); + } + + if (destUrl.isLocalFile()) { + // New local URL. KFileItem will stat on demand. + m_destItemCache = KFileItem(destUrl); + m_destItemCacheInvalidationTimer.start(); + processEvent(event); + return; + } + + // New remote URL. Start a StatJob and process the event when it finishes. + m_lastUndecidedEvent = event; + m_statJob = KIO::stat(destUrl, KIO::StatJob::SourceSide, KIO::StatDetail::StatBasic, KIO::JobFlag::HideProgressInfo); + m_statJobUrl = destUrl; + connect(m_statJob, &KIO::StatJob::result, this, [this, processEvent](KJob *job) { + KIO::StatJob *statJob = static_cast(job); + + m_destItemCache = KFileItem(statJob->statResult(), m_statJobUrl); + m_destItemCacheInvalidationTimer.start(); + + if (m_lastUndecidedEvent) { + processEvent(m_lastUndecidedEvent); + m_lastUndecidedEvent = nullptr; + } + + m_statJob = nullptr; + m_statJobUrl.clear(); + }); } void DragAndDropHelper::clearUrlListMatchesUrlCache() diff --git a/src/views/draganddrophelper.h b/src/views/draganddrophelper.h index 73043febc2..df3b49966a 100644 --- a/src/views/draganddrophelper.h +++ b/src/views/draganddrophelper.h @@ -11,9 +11,11 @@ #include "dolphin_export.h" #include +#include #include #include +#include #include class QDropEvent; @@ -24,7 +26,7 @@ namespace KIO class DropJob; } -class DOLPHIN_EXPORT DragAndDropHelper +class DOLPHIN_EXPORT DragAndDropHelper : public QObject { public: /** @@ -51,16 +53,6 @@ public: */ static bool supportsDropping(const KFileItem &destItem); - /** - * Updates the drop action according to whether the destination supports dropping. - * If supportsDropping(destUrl), set dropAction = proposedAction. Otherwise, set - * dropAction = Qt::IgnoreAction. - * - * @param event Drop event. - * @param destUrl Destination URL. - */ - static void updateDropAction(QDropEvent *event, const QUrl &destUrl); - /** * @return True if destUrl is contained in the urls parameter. */ @@ -84,11 +76,55 @@ public: */ static void clearUrlListMatchesUrlCache(); + DragAndDropHelper(QObject *parent); + + /** + * Updates the drop action according to whether the destination supports dropping. + * If supportsDropping(destUrl), set dropAction = proposedAction. Otherwise, set + * dropAction = Qt::IgnoreAction. + * + * @param event Drop event. + * @param destUrl Destination URL. + */ + void updateDropAction(QDropEvent *event, const QUrl &destUrl); + private: /** * Stores the results of the expensive checks made in urlListMatchesUrl. */ static QHash m_urlListMatchesUrlCache; + + /** + * When updateDropAction() is called with a remote URL, we create a StatJob to + * check if the destination is a directory or a desktop file. We cache the result + * here to avoid doing the stat again on subsequent calls to updateDropAction(). + */ + KFileItem m_destItemCache; + + /** + * Only keep the cache for 30 seconds, because the stat of the destUrl might change. + */ + QTimer m_destItemCacheInvalidationTimer; + + /** + * A StatJob on-fly to fill the cache for a remote URL. We shouldn't create more + * than one StatJob at a time, so we keep a pointer to the current one. + */ + KIO::StatJob *m_statJob = nullptr; + + /** + * The URL for which the StatJob is running. + * Note: We can't use m_statJob->url() because StatJob might resolve the URL to be + * different from what we passed into stat(). E.g. "mtp:" is resolved + * to "mtp:" + */ + QUrl m_statJobUrl; + + /** + * The last event we received in updateDropAction(), but can't react to yet, + * because a StatJob is on-fly. + */ + QDropEvent *m_lastUndecidedEvent = nullptr; }; #endif