mirror of
https://invent.kde.org/system/dolphin
synced 2024-08-22 01:40:02 +00:00
Avoid KJob::exec() in DolphinView
This commit replaces an error-prone usage of KIO::StatJob::exec() in DolphinView with the recommended KIO::StatJob::start(). The containing method DolphinView::statusBarText() is changed to be a method without return value: requestStatusBarText() The new status bar text is instead returned through a new setStatusBarText() signal that will be emitted asynchronously if necessary. The calling code that deals with status bar text is refactored to correctly work despite the new asynchronicity. The helper method calculateItemCount() is moved into requestStatusBarText() and some other code from requestStatusBarText() is moved into a new helper method emitStatusBarText(). The documentation of KIO::KJob::exec() explains why it should be avoided. A reproducible crash is the reason for this commit.
This commit is contained in:
parent
7eae6bba5f
commit
a825e1bd74
|
@ -85,6 +85,10 @@ DolphinPart::DolphinPart(QWidget* parentWidget, QObject* parent,
|
|||
this, &DolphinPart::slotItemActivated);
|
||||
connect(m_view, &DolphinView::itemsActivated,
|
||||
this, &DolphinPart::slotItemsActivated);
|
||||
connect(m_view, &DolphinView::statusBarTextChanged, this, [this](const QString &text) {
|
||||
const QString escapedText = Qt::convertFromPlainText(text);
|
||||
Q_EMIT ReadOnlyPart::setStatusBarText(QStringLiteral("<qt>%1</qt>").arg(escapedText));
|
||||
});
|
||||
connect(m_view, &DolphinView::tabRequested,
|
||||
this, &DolphinPart::createNewWindow);
|
||||
connect(m_view, &DolphinView::requestContextMenu,
|
||||
|
@ -597,8 +601,7 @@ void DolphinPart::updateNewMenu()
|
|||
|
||||
void DolphinPart::updateStatusBar()
|
||||
{
|
||||
const QString escapedText = Qt::convertFromPlainText(m_view->statusBarText());
|
||||
Q_EMIT ReadOnlyPart::setStatusBarText(QStringLiteral("<qt>%1</qt>").arg(escapedText));
|
||||
m_view->requestStatusBarText();
|
||||
}
|
||||
|
||||
void DolphinPart::updateProgress(int percent)
|
||||
|
|
|
@ -165,6 +165,10 @@ DolphinViewContainer::DolphinViewContainer(const QUrl& url, QWidget* parent) :
|
|||
m_statusBar, &DolphinStatusBar::setText);
|
||||
connect(m_view, &DolphinView::operationCompletedMessage,
|
||||
m_statusBar, &DolphinStatusBar::setText);
|
||||
connect(m_view, &DolphinView::statusBarTextChanged,
|
||||
m_statusBar, &DolphinStatusBar::setDefaultText);
|
||||
connect(m_view, &DolphinView::statusBarTextChanged,
|
||||
m_statusBar, &DolphinStatusBar::resetToDefaultText);
|
||||
connect(m_statusBar, &DolphinStatusBar::stopPressed,
|
||||
this, &DolphinViewContainer::stopDirectoryLoading);
|
||||
connect(m_statusBar, &DolphinStatusBar::zoomLevelChanged,
|
||||
|
@ -544,10 +548,7 @@ void DolphinViewContainer::delayedStatusBarUpdate()
|
|||
void DolphinViewContainer::updateStatusBar()
|
||||
{
|
||||
m_statusBarTimestamp.start();
|
||||
|
||||
const QString text = m_view->statusBarText();
|
||||
m_statusBar->setDefaultText(text);
|
||||
m_statusBar->resetToDefaultText();
|
||||
m_view->requestStatusBarText();
|
||||
}
|
||||
|
||||
void DolphinViewContainer::updateDirectoryLoadingProgress(int percent)
|
||||
|
|
|
@ -528,17 +528,18 @@ QStringList DolphinView::mimeTypeFilters() const
|
|||
return m_model->mimeTypeFilters();
|
||||
}
|
||||
|
||||
QString DolphinView::statusBarText() const
|
||||
void DolphinView::requestStatusBarText()
|
||||
{
|
||||
QString summary;
|
||||
QString foldersText;
|
||||
QString filesText;
|
||||
|
||||
int folderCount = 0;
|
||||
int fileCount = 0;
|
||||
KIO::filesize_t totalFileSize = 0;
|
||||
if (m_statJobForStatusBarText) {
|
||||
// Kill the pending request.
|
||||
m_statJobForStatusBarText->kill();
|
||||
}
|
||||
|
||||
if (m_container->controller()->selectionManager()->hasSelection()) {
|
||||
int folderCount = 0;
|
||||
int fileCount = 0;
|
||||
KIO::filesize_t totalFileSize = 0;
|
||||
|
||||
// Give a summary of the status of the selected files
|
||||
const KFileItemList list = selectedItems();
|
||||
for (const KFileItem& item : list) {
|
||||
|
@ -552,14 +553,37 @@ QString DolphinView::statusBarText() const
|
|||
|
||||
if (folderCount + fileCount == 1) {
|
||||
// If only one item is selected, show info about it
|
||||
return list.first().getStatusBarInfo();
|
||||
Q_EMIT statusBarTextChanged(list.first().getStatusBarInfo());
|
||||
} else {
|
||||
// At least 2 items are selected
|
||||
foldersText = i18ncp("@info:status", "1 Folder selected", "%1 Folders selected", folderCount);
|
||||
filesText = i18ncp("@info:status", "1 File selected", "%1 Files selected", fileCount);
|
||||
emitStatusBarText(folderCount, fileCount, totalFileSize, HasSelection);
|
||||
}
|
||||
} else { // has no selection
|
||||
if (!m_model->rootItem().url().isValid()) {
|
||||
return;
|
||||
}
|
||||
|
||||
m_statJobForStatusBarText = KIO::statDetails(m_model->rootItem().url(),
|
||||
KIO::StatJob::SourceSide, KIO::StatRecursiveSize, KIO::HideProgressInfo);
|
||||
connect(m_statJobForStatusBarText, &KJob::result,
|
||||
this, &DolphinView::slotStatJobResult);
|
||||
m_statJobForStatusBarText->start();
|
||||
}
|
||||
}
|
||||
|
||||
void DolphinView::emitStatusBarText(const int folderCount, const int fileCount,
|
||||
KIO::filesize_t totalFileSize, const Selection selection)
|
||||
{
|
||||
QString foldersText;
|
||||
QString filesText;
|
||||
QString summary;
|
||||
|
||||
if (selection == HasSelection) {
|
||||
// At least 2 items are selected because the case of 1 selected item is handled in
|
||||
// DolphinView::requestStatusBarText().
|
||||
foldersText = i18ncp("@info:status", "1 Folder selected", "%1 Folders selected", folderCount);
|
||||
filesText = i18ncp("@info:status", "1 File selected", "%1 Files selected", fileCount);
|
||||
} else {
|
||||
calculateItemCount(fileCount, folderCount, totalFileSize);
|
||||
foldersText = i18ncp("@info:status", "1 Folder", "%1 Folders", folderCount);
|
||||
filesText = i18ncp("@info:status", "1 File", "%1 Files", fileCount);
|
||||
}
|
||||
|
@ -577,8 +601,7 @@ QString DolphinView::statusBarText() const
|
|||
} else {
|
||||
summary = i18nc("@info:status", "0 Folders, 0 Files");
|
||||
}
|
||||
|
||||
return summary;
|
||||
Q_EMIT statusBarTextChanged(summary);
|
||||
}
|
||||
|
||||
QList<QAction*> DolphinView::versionControlActions(const KFileItemList& items) const
|
||||
|
@ -1271,6 +1294,36 @@ void DolphinView::emitSelectionChangedSignal()
|
|||
Q_EMIT selectionChanged(selectedItems());
|
||||
}
|
||||
|
||||
void DolphinView::slotStatJobResult(KJob *job)
|
||||
{
|
||||
int folderCount = 0;
|
||||
int fileCount = 0;
|
||||
KIO::filesize_t totalFileSize = 0;
|
||||
bool countFileSize = true;
|
||||
|
||||
const auto entry = static_cast<KIO::StatJob *>(job)->statResult();
|
||||
if (entry.contains(KIO::UDSEntry::UDS_RECURSIVE_SIZE)) {
|
||||
// We have a precomputed value.
|
||||
totalFileSize = static_cast<KIO::filesize_t>(
|
||||
entry.numberValue(KIO::UDSEntry::UDS_RECURSIVE_SIZE));
|
||||
countFileSize = false;
|
||||
}
|
||||
|
||||
const int itemCount = m_model->count();
|
||||
for (int i = 0; i < itemCount; ++i) {
|
||||
const KFileItem item = m_model->fileItem(i);
|
||||
if (item.isDir()) {
|
||||
++folderCount;
|
||||
} else {
|
||||
++fileCount;
|
||||
if (countFileSize) {
|
||||
totalFileSize += item.size();
|
||||
}
|
||||
}
|
||||
}
|
||||
emitStatusBarText(folderCount, fileCount, totalFileSize, NoSelection);
|
||||
}
|
||||
|
||||
void DolphinView::updateSortRole(const QByteArray& role)
|
||||
{
|
||||
ViewProperties props(viewPropertiesUrl());
|
||||
|
@ -1537,40 +1590,6 @@ void DolphinView::hideToolTip(const ToolTipManager::HideBehavior behavior)
|
|||
#endif
|
||||
}
|
||||
|
||||
void DolphinView::calculateItemCount(int& fileCount,
|
||||
int& folderCount,
|
||||
KIO::filesize_t& totalFileSize) const
|
||||
{
|
||||
const int itemCount = m_model->count();
|
||||
|
||||
bool countFileSize = true;
|
||||
|
||||
if (!m_model->rootItem().url().isValid()) {
|
||||
return;
|
||||
}
|
||||
|
||||
// In case we have a precomputed value
|
||||
const auto job = KIO::statDetails(m_model->rootItem().url(), KIO::StatJob::SourceSide, KIO::StatRecursiveSize, KIO::HideProgressInfo);
|
||||
job->exec();
|
||||
const auto entry = job->statResult();
|
||||
if (entry.contains(KIO::UDSEntry::UDS_RECURSIVE_SIZE)) {
|
||||
totalFileSize = static_cast<KIO::filesize_t>(entry.numberValue(KIO::UDSEntry::UDS_RECURSIVE_SIZE));
|
||||
countFileSize = false;
|
||||
}
|
||||
|
||||
for (int i = 0; i < itemCount; ++i) {
|
||||
const KFileItem item = m_model->fileItem(i);
|
||||
if (item.isDir()) {
|
||||
++folderCount;
|
||||
} else {
|
||||
++fileCount;
|
||||
if (countFileSize) {
|
||||
totalFileSize += item.size();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void DolphinView::slotTwoClicksRenamingTimerTimeout()
|
||||
{
|
||||
const KItemListSelectionManager* selectionManager = m_container->controller()->selectionManager();
|
||||
|
|
|
@ -19,6 +19,7 @@
|
|||
#include <kparts/part.h>
|
||||
|
||||
#include <QMimeData>
|
||||
#include <QPointer>
|
||||
#include <QUrl>
|
||||
#include <QWidget>
|
||||
|
||||
|
@ -240,10 +241,13 @@ public:
|
|||
QStringList mimeTypeFilters() const;
|
||||
|
||||
/**
|
||||
* Returns a textual representation of the state of the current
|
||||
* Tells the view to generate an updated status bar text. The result
|
||||
* is returned through the statusBarTextChanged(QString statusBarText) signal.
|
||||
* It will carry a textual representation of the state of the current
|
||||
* folder or selected items, suitable for use in the status bar.
|
||||
* Any pending requests of status bar text are killed.
|
||||
*/
|
||||
QString statusBarText() const;
|
||||
void requestStatusBarText();
|
||||
|
||||
/**
|
||||
* Returns the version control actions that are provided for the items \p items.
|
||||
|
@ -450,6 +454,10 @@ signals:
|
|||
/** Is emitted if the 'grouped sorting' property has been changed. */
|
||||
void groupedSortingChanged(bool groupedSorting);
|
||||
|
||||
/** Is emmited in reaction to a requestStatusBarText() call.
|
||||
* @see requestStatusBarText() */
|
||||
void statusBarTextChanged(QString statusBarText);
|
||||
|
||||
/** Is emitted if the sorting by name, size or date has been changed. */
|
||||
void sortRoleChanged(const QByteArray& role);
|
||||
|
||||
|
@ -642,6 +650,15 @@ private slots:
|
|||
*/
|
||||
void emitSelectionChangedSignal();
|
||||
|
||||
/**
|
||||
* Helper method for DolphinView::requestStatusBarText().
|
||||
* Calculates the amount of folders and files and their total size in
|
||||
* response to a KStatJob::result(), then calls emitStatusBarText().
|
||||
* @see requestStatusBarText()
|
||||
* @see emitStatusBarText()
|
||||
*/
|
||||
void slotStatJobResult(KJob *job);
|
||||
|
||||
/**
|
||||
* Updates the view properties of the current URL to the
|
||||
* sorting given by \a role.
|
||||
|
@ -735,16 +752,6 @@ private slots:
|
|||
*/
|
||||
void slotDirectoryRedirection(const QUrl& oldUrl, const QUrl& newUrl);
|
||||
|
||||
/**
|
||||
* Calculates the number of currently shown files into
|
||||
* \a fileCount and the number of folders into \a folderCount.
|
||||
* The size of all files is written into \a totalFileSize.
|
||||
* It is recommend using this method instead of asking the
|
||||
* directory lister or the model directly, as it takes
|
||||
* filtering and hierarchical previews into account.
|
||||
*/
|
||||
void calculateItemCount(int& fileCount, int& folderCount, KIO::filesize_t& totalFileSize) const;
|
||||
|
||||
void slotTwoClicksRenamingTimerTimeout();
|
||||
|
||||
private:
|
||||
|
@ -769,6 +776,21 @@ private:
|
|||
*/
|
||||
void applyModeToView();
|
||||
|
||||
enum Selection {
|
||||
HasSelection,
|
||||
NoSelection
|
||||
};
|
||||
/**
|
||||
* Helper method for DolphinView::requestStatusBarText().
|
||||
* Generates the status bar text from the parameters and
|
||||
* then emits statusBarTextChanged().
|
||||
* @param totalFileSize the sum of the sizes of the files
|
||||
* @param selection if HasSelection is passed, the emitted status bar text will say
|
||||
* that the folders and files which are counted here are selected.
|
||||
*/
|
||||
void emitStatusBarText(const int folderCount, const int fileCount,
|
||||
KIO::filesize_t totalFileSize, const Selection selection);
|
||||
|
||||
/**
|
||||
* Helper method for DolphinView::paste() and DolphinView::pasteIntoFolder().
|
||||
* Pastes the clipboard data into the URL \a url.
|
||||
|
@ -829,6 +851,8 @@ private:
|
|||
Mode m_mode;
|
||||
QList<QByteArray> m_visibleRoles;
|
||||
|
||||
QPointer<KIO::StatJob> m_statJobForStatusBarText;
|
||||
|
||||
QVBoxLayout* m_topLayout;
|
||||
|
||||
KFileItemModel* m_model;
|
||||
|
|
Loading…
Reference in a new issue