Review Request 126831: WIP: Fix QFileDialog::openUrl() for remote files

[Alex Richardson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126831/
-----------------------------------------------------------

(Updated Jan. 24, 2016, 11:23 p.m.)


Status
------

This change has been discarded.


Review request for KDE Frameworks.


Bugs: 345253
    https://bugs.kde.org/show_bug.cgi?id=345253


Repository: kio


Description
-------

WIP: Fix QFileDialog::openUrl() for remote files

QFileDialog doesn't make an attempt to determine whether remote URLs are files or directories so it [just passes the file URL to the platform file dialog](http://code.woboq.org/qt5/qtbase/src/widgets/dialogs/qfiledialog.cpp.html#_ZL17_qt_get_directoryRK4QUrl) which currently causes an error popup and a nonexistent directory to be selected

I will clean up the code and remove qDebug() if this is the correct approach to fixing https://bugs.kde.org/show_bug.cgi?id=345253
I prefer fixing this here instead of in KDEPlatformFileDialog as this means that other calls KFileWidget::setUrl() are also fixed.

Should be a better solution than https://git.reviewboard.kde.org/r/126821/


Problem is there are a lot of KIO::stat() calls going on which can be quite slow for remote files. Any suggestions how to fix them?


Diffs
-----

  src/filewidgets/kfilewidget.cpp 868928b3f6511c7890e4cf4ba09edc68048649e7 

Diff: https://git.reviewboard.kde.org/r/126831/diff/


Testing
-------

kwrite sftp://user@host/home/user/foo.txt, press open button, directory is shown and foo.txt is selected.


However, there are quite a lot of stats going on for a single QFileDialog::getOpenUrl(), probably we should optimize this.
A total of 8 KIO::stat() calls are made when opening a file, I would expect only 2: original URL to find out if it is a directory or a file
and then the selected file to check if it can be opened.

Debug output:
```
startDir QUrl("")
for QUrl("") -> QUrl("file:///auto/homes/alr48") recentDirClass "" fileName ""
startDir after getStartUrl() QUrl("file:///auto/homes/alr48") file name ""
KIO::stat() QUrl("file:///auto/homes/alr48")
stat of QUrl("file:///auto/homes/alr48") -> statRes true isDir true
KIO::stat() QUrl("sftp://user@host/home/user/foo.txt")
stat of QUrl("sftp://user@host/home/user/foo.txt") -> statRes true isDir false
statJob -> directory QUrl("sftp://user@host/home/user") filename "foo.txt"
File passed to KFileWidget::setUrl(), expected directory: QUrl("sftp://user@host/home/user/foo.txt")
KIO::stat() QUrl("sftp://user@host/home/user/")
stat of QUrl("sftp://user@host/home/user/") -> statRes true isDir true
KIO::stat() QUrl("sftp://user@host/home/user/foo.txt")
stat of QUrl("sftp://user@host/home/user/foo.txt") -> statRes true isDir false
statJob -> directory QUrl("sftp://user@host/home/user") filename "foo.txt"
File passed to KFileWidget::setUrl(), expected directory: QUrl("sftp://user@host/home/user/foo.txt")
KIO::stat() QUrl("sftp://user@host/home/user/")
stat of QUrl("sftp://user@host/home/user/") -> statRes true isDir true
KIO::stat() QUrl("sftp://user@host/home/user/foo.txt")
stat of QUrl("sftp://user@host/home/user/foo.txt") -> statRes true isDir false
statJob -> directory QUrl("sftp://user@host/home/user") filename "foo.txt"
File passed to KFileWidget::setUrl(), expected directory: QUrl("sftp://user@host/home/user/foo.txt")
KIO::stat() QUrl("sftp://user@host/home/user/")
stat of QUrl("sftp://user@host/home/user/") -> statRes true isDir true
KIO::stat() QUrl("sftp://user@host/home/user/bar.txt")
```


First StatJob stats QUrl("$HOME") and comes from the constructor with QUrl(), we can remove that by skipping it if url is empty.

```
1	KIO::stat	statjob.cpp	180	0x7fce732928c8	
2	KFileWidget::KFileWidget	kfilewidget.cpp	643	0x7fce663f1e05	
3	KDEPlatformFileDialog::KDEPlatformFileDialog	kdeplatformfiledialoghelper.cpp	96	0x7fce666b0678	
4	KDEPlatformFileDialogHelper::KDEPlatformFileDialogHelper	kdeplatformfiledialoghelper.cpp	201	0x7fce666b0dbd	
5	KdePlatformTheme::createPlatformDialogHelper	kdeplatformtheme.cpp	273	0x7fce666a63bc	
6	QDialogPrivate::platformHelper	qdialog.cpp	94	0x7fce759eaecc	
7	QFileDialogPrivate::platformFileDialogHelper	qfiledialog_p.h	112	0x7fce75a03f98	
8	QFileDialogPrivate::init	qfiledialog.cpp	2800	0x7fce759fa737	
9	QFileDialog::QFileDialog	qfiledialog.cpp	373	0x7fce759f169d	
10	QFileDialog::getOpenFileUrls	qfiledialog.cpp	2271	0x7fce759f7da3	
11	KWrite::slotOpen	kwrite.cpp	250	0x41a0c1	
12	KWrite::qt_static_metacall	moc_kwrite.cpp	131	0x41ea10	
13	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
14	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
15	QAction::triggered	moc_qaction.cpp	368	0x7fce7574ae7a	
16	QAction::activate	qaction.cpp	1163	0x7fce7574a28a	
17	QAction::trigger	qaction.h	177	0x7fce7574b821	
18	QToolButton::nextCheckState	qtoolbutton.cpp	954	0x7fce759a6bd3	
19	QAbstractButtonPrivate::click	qabstractbutton.cpp	515	0x7fce758aab1c	
20	QAbstractButton::mouseReleaseEvent	qabstractbutton.cpp	1131	0x7fce758ac08c	
...	<More>				
```


Second one for QUrl("sftp://user@host/home/user/foo.txt") has this backtrace:

```
1	KIO::stat	statjob.cpp	180	0x7fce732928c8	
2	KFileWidget::setUrl	kfilewidget.cpp	1476	0x7fce663f6e6c	
3	KDEPlatformFileDialog::setDirectory	kdeplatformfiledialoghelper.cpp	189	0x7fce666b0ce0	
4	KDEPlatformFileDialogHelper::setDirectory	kdeplatformfiledialoghelper.cpp	344	0x7fce666b1c06	
5	QFileDialogPrivate::setDirectory_sys	qfiledialog_p.h	368	0x7fce75a043a1	
6	QFileDialog::setDirectoryUrl	qfiledialog.cpp	976	0x7fce759f3cbb	
7	QFileDialogPrivate::init	qfiledialog.cpp	2806	0x7fce759fa7cd	
8	QFileDialog::QFileDialog	qfiledialog.cpp	373	0x7fce759f169d	
9	QFileDialog::getOpenFileUrls	qfiledialog.cpp	2271	0x7fce759f7da3	
10	KWrite::slotOpen	kwrite.cpp	250	0x41a0c1	
11	KWrite::qt_static_metacall	moc_kwrite.cpp	131	0x41ea10	
12	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
13	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
14	QAction::triggered	moc_qaction.cpp	368	0x7fce7574ae7a	
15	QAction::activate	qaction.cpp	1163	0x7fce7574a28a	
16	QAction::trigger	qaction.h	177	0x7fce7574b821	
17	QToolButton::nextCheckState	qtoolbutton.cpp	954	0x7fce759a6bd3	
18	QAbstractButtonPrivate::click	qabstractbutton.cpp	515	0x7fce758aab1c	
19	QAbstractButton::mouseReleaseEvent	qabstractbutton.cpp	1131	0x7fce758ac08c	
20	QToolButton::mouseReleaseEvent	qtoolbutton.cpp	609	0x7fce759a5839	
...	<More>		
```

next one on QUrl("sftp://user@host/home/user/") (caused by an url change here, possibly block signals on d->ops during setUrl()?):

```
1	KIO::stat	statjob.cpp	180	0x7fce732928c8	
2	KFileWidget::setUrl	kfilewidget.cpp	1476	0x7fce663f6e6c	
3	KFileWidgetPrivate::_k_enterUrl	kfilewidget.cpp	1545	0x7fce663f759a	
4	KFileWidget::qt_static_metacall	moc_kfilewidget.cpp	176	0x7fce663fe3e2	
5	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
6	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
7	KUrlNavigator::urlChanged	moc_kurlnavigator.cpp	299	0x7fce66439dcf	
8	KUrlNavigator::setLocationUrl	kurlnavigator.cpp	1080	0x7fce66438eb4	
9	KFileWidgetPrivate::_k_urlEntered	kfilewidget.cpp	1515	0x7fce663f73a7	
10	KFileWidget::qt_static_metacall	moc_kfilewidget.cpp	175	0x7fce663fe3bf	
11	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
12	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
13	KDirOperator::urlEntered	moc_kdiroperator.cpp	586	0x7fce663de2b3	
14	KDirOperator::setUrl	kdiroperator.cpp	1039	0x7fce663d2599	
15	KFileWidget::setUrl	kfilewidget.cpp	1491	0x7fce663f7207	
16	KDEPlatformFileDialog::setDirectory	kdeplatformfiledialoghelper.cpp	189	0x7fce666b0ce0	
17	KDEPlatformFileDialogHelper::setDirectory	kdeplatformfiledialoghelper.cpp	344	0x7fce666b1c06	
18	QFileDialogPrivate::setDirectory_sys	qfiledialog_p.h	368	0x7fce75a043a1	
19	QFileDialog::setDirectoryUrl	qfiledialog.cpp	976	0x7fce759f3cbb	
20	QFileDialogPrivate::init	qfiledialog.cpp	2806	0x7fce759fa7cd	
...	<More>				
```

and again QUrl("sftp://user@host/home/user/foo.txt") (why is QFileDialogPrivate::restoreFromSettings() setting an URL if one was explicitly requested):

```
1	KIO::stat	statjob.cpp	180	0x7fce732928c8	
2	KFileWidget::setUrl	kfilewidget.cpp	1476	0x7fce663f6e6c	
3	KDEPlatformFileDialog::setDirectory	kdeplatformfiledialoghelper.cpp	189	0x7fce666b0ce0	
4	KDEPlatformFileDialogHelper::setDirectory	kdeplatformfiledialoghelper.cpp	344	0x7fce666b1c06	
5	QFileDialogPrivate::setDirectory_sys	qfiledialog_p.h	368	0x7fce75a043a1	
6	QFileDialog::setDirectoryUrl	qfiledialog.cpp	976	0x7fce759f3cbb	
7	QFileDialogPrivate::restoreFromSettings	qfiledialog.cpp	2716	0x7fce759f9c71	
8	QFileDialogPrivate::init	qfiledialog.cpp	2812	0x7fce759fa817	
9	QFileDialog::QFileDialog	qfiledialog.cpp	373	0x7fce759f169d	
10	QFileDialog::getOpenFileUrls	qfiledialog.cpp	2271	0x7fce759f7da3	
11	KWrite::slotOpen	kwrite.cpp	250	0x41a0c1	
12	KWrite::qt_static_metacall	moc_kwrite.cpp	131	0x41ea10	
13	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
14	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
15	QAction::triggered	moc_qaction.cpp	368	0x7fce7574ae7a	
16	QAction::activate	qaction.cpp	1163	0x7fce7574a28a	
17	QAction::trigger	qaction.h	177	0x7fce7574b821	
18	QToolButton::nextCheckState	qtoolbutton.cpp	954	0x7fce759a6bd3	
19	QAbstractButtonPrivate::click	qabstractbutton.cpp	515	0x7fce758aab1c	
20	QAbstractButton::mouseReleaseEvent	qabstractbutton.cpp	1131	0x7fce758ac08c	
...	<More>				
```

which seems to cause another call to KFileWidget::setUrl() (block signals?). Stats QUrl("sftp://user@host/home/user/") as before:

```
1	KIO::stat	statjob.cpp	180	0x7fce732928c8	
2	KFileWidget::setUrl	kfilewidget.cpp	1476	0x7fce663f6e6c	
3	KFileWidgetPrivate::_k_enterUrl	kfilewidget.cpp	1545	0x7fce663f759a	
4	KFileWidget::qt_static_metacall	moc_kfilewidget.cpp	176	0x7fce663fe3e2	
5	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
6	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
7	KUrlNavigator::urlChanged	moc_kurlnavigator.cpp	299	0x7fce66439dcf	
8	KUrlNavigator::setLocationUrl	kurlnavigator.cpp	1080	0x7fce66438eb4	
9	KFileWidgetPrivate::_k_urlEntered	kfilewidget.cpp	1515	0x7fce663f73a7	
10	KFileWidget::qt_static_metacall	moc_kfilewidget.cpp	175	0x7fce663fe3bf	
11	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
12	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
13	KDirOperator::urlEntered	moc_kdiroperator.cpp	586	0x7fce663de2b3	
14	KDirOperator::setUrl	kdiroperator.cpp	1039	0x7fce663d2599	
15	KFileWidget::setUrl	kfilewidget.cpp	1491	0x7fce663f7207	
16	KDEPlatformFileDialog::setDirectory	kdeplatformfiledialoghelper.cpp	189	0x7fce666b0ce0	
17	KDEPlatformFileDialogHelper::setDirectory	kdeplatformfiledialoghelper.cpp	344	0x7fce666b1c06	
18	QFileDialogPrivate::setDirectory_sys	qfiledialog_p.h	368	0x7fce75a043a1	
19	QFileDialog::setDirectoryUrl	qfiledialog.cpp	976	0x7fce759f3cbb	
20	QFileDialogPrivate::restoreFromSettings	qfiledialog.cpp	2716	0x7fce759f9c71	
21	QFileDialogPrivate::init	qfiledialog.cpp	2812	0x7fce759fa817	
22	QFileDialog::QFileDialog	qfiledialog.cpp	373	0x7fce759f169d	
23	QFileDialog::getOpenFileUrls	qfiledialog.cpp	2271	0x7fce759f7da3	
24	KWrite::slotOpen	kwrite.cpp	250	0x41a0c1	
25	KWrite::qt_static_metacall	moc_kwrite.cpp	131	0x41ea10	
...	<More>				
```

And now KDEPlatformFileDialogHelper::show() sets the URL to QUrl("sftp://user@host/home/user/foo.txt") again

```
1	KIO::stat	statjob.cpp	180	0x7fce732928c8	
2	KFileWidget::setUrl	kfilewidget.cpp	1476	0x7fce663f6e6c	
3	KDEPlatformFileDialog::setDirectory	kdeplatformfiledialoghelper.cpp	189	0x7fce666b0ce0	
4	KDEPlatformFileDialogHelper::setDirectory	kdeplatformfiledialoghelper.cpp	344	0x7fce666b1c06	
5	KDEPlatformFileDialogHelper::initializeDialog	kdeplatformfiledialoghelper.cpp	238	0x7fce666b147f	
6	KDEPlatformFileDialogHelper::show	kdeplatformfiledialoghelper.cpp	310	0x7fce666b1a36	
7	QDialogPrivate::setNativeDialogVisible	qdialog.cpp	129	0x7fce759eb10c	
8	QFileDialog::setVisible	qfiledialog.cpp	842	0x7fce759f3571	
9	QWidget::show	qwidget.cpp	7728	0x7fce757a8921	
10	QDialog::exec	qdialog.cpp	533	0x7fce759eb8c7	
11	QFileDialog::getOpenFileUrls	qfiledialog.cpp	2275	0x7fce759f7e08	
12	KWrite::slotOpen	kwrite.cpp	250	0x41a0c1	
13	KWrite::qt_static_metacall	moc_kwrite.cpp	131	0x41ea10	
14	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
15	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
16	QAction::triggered	moc_qaction.cpp	368	0x7fce7574ae7a	
17	QAction::activate	qaction.cpp	1163	0x7fce7574a28a	
18	QAction::trigger	qaction.h	177	0x7fce7574b821	
19	QToolButton::nextCheckState	qtoolbutton.cpp	954	0x7fce759a6bd3	
20	QAbstractButtonPrivate::click	qabstractbutton.cpp	515	0x7fce758aab1c	
...	<More>				
```

causing another call (possibly fixable by blocking signals). Stats QUrl("sftp://user@host/home/user/") again:

```
1	KIO::stat	statjob.cpp	180	0x7fce732928c8	
2	KFileWidget::setUrl	kfilewidget.cpp	1476	0x7fce663f6e6c	
3	KFileWidgetPrivate::_k_enterUrl	kfilewidget.cpp	1545	0x7fce663f759a	
4	KFileWidget::qt_static_metacall	moc_kfilewidget.cpp	176	0x7fce663fe3e2	
5	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
6	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
7	KUrlNavigator::urlChanged	moc_kurlnavigator.cpp	299	0x7fce66439dcf	
8	KUrlNavigator::setLocationUrl	kurlnavigator.cpp	1080	0x7fce66438eb4	
9	KFileWidgetPrivate::_k_urlEntered	kfilewidget.cpp	1515	0x7fce663f73a7	
10	KFileWidget::qt_static_metacall	moc_kfilewidget.cpp	175	0x7fce663fe3bf	
11	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
12	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
13	KDirOperator::urlEntered	moc_kdiroperator.cpp	586	0x7fce663de2b3	
14	KDirOperator::setUrl	kdiroperator.cpp	1039	0x7fce663d2599	
15	KFileWidget::setUrl	kfilewidget.cpp	1491	0x7fce663f7207	
16	KDEPlatformFileDialog::setDirectory	kdeplatformfiledialoghelper.cpp	189	0x7fce666b0ce0	
17	KDEPlatformFileDialogHelper::setDirectory	kdeplatformfiledialoghelper.cpp	344	0x7fce666b1c06	
18	KDEPlatformFileDialogHelper::initializeDialog	kdeplatformfiledialoghelper.cpp	238	0x7fce666b147f	
19	KDEPlatformFileDialogHelper::show	kdeplatformfiledialoghelper.cpp	310	0x7fce666b1a36	
20	QDialogPrivate::setNativeDialogVisible	qdialog.cpp	129	0x7fce759eb10c	
21	QFileDialog::setVisible	qfiledialog.cpp	842	0x7fce759f3571	
22	QWidget::show	qwidget.cpp	7728	0x7fce757a8921	
23	QDialog::exec	qdialog.cpp	533	0x7fce759eb8c7	
24	QFileDialog::getOpenFileUrls	qfiledialog.cpp	2275	0x7fce759f7e08	
25	KWrite::slotOpen	kwrite.cpp	250	0x41a0c1	
...	<More>	
```


Finally, when selecting a new file (but this one is required as we need to check whether the new file can be opened)
Stats QUrl("sftp://user@host/home/user/bar.txt")
```
1	KIO::stat	statjob.cpp	180	0x7fce732928c8	
2	KFileWidget::slotOk	kfilewidget.cpp	993	0x7fce663f4005	
3	KFileWidget::qt_static_metacall	moc_kfilewidget.cpp	171	0x7fce663fe357	
4	QMetaObject::activate	qobject.cpp	3730	0x7fce7490fc54	
5	QMetaObject::activate	qobject.cpp	3595	0x7fce7490f442	
6	QAbstractButton::clicked	moc_qabstractbutton.cpp	306	0x7fce75bec92a	
7	QAbstractButtonPrivate::emitClicked	qabstractbutton.cpp	533	0x7fce758aac1f	
8	QAbstractButtonPrivate::click	qabstractbutton.cpp	526	0x7fce758aabae	
9	QAbstractButton::mouseReleaseEvent	qabstractbutton.cpp	1131	0x7fce758ac08c	
10	QWidget::event	qwidget.cpp	8738	0x7fce757ab018	
11	QAbstractButton::event	qabstractbutton.cpp	1088	0x7fce758abeca	
12	QPushButton::event	qpushbutton.cpp	673	0x7fce7596c071	
13	QApplicationPrivate::notify_helper	qapplication.cpp	3712	0x7fce7575b278	
14	QApplication::notify	qapplication.cpp	3270	0x7fce75758fdf	
15	QCoreApplication::notifyInternal2	qcoreapplication.cpp	1013	0x7fce748cf642	
16	QCoreApplication::sendSpontaneousEvent	qcoreapplication.h	230	0x7fce7575e226	
17	QApplicationPrivate::sendMouseEvent	qapplication.cpp	2765	0x7fce75757997	
18	QWidgetWindow::handleMouseEvent	qwidgetwindow.cpp	554	0x7fce757d7a99	
19	QWidgetWindow::event	qwidgetwindow.cpp	210	0x7fce757d671f	
20	QApplicationPrivate::notify_helper	qapplication.cpp	3712	0x7fce7575b278	
...	<More>				
```


Thanks,

Alex Richardson

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160124/e2150199/attachment-0001.html>