D19784: Avoid calling QT_LSTAT and accessing recent documents

hoffmannrobert created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. hoffmannrobert requested review of this revision. REVISION SUMMARY Document entries pointing to unavailable network shares freeze the application menu, so avoid calling QT_LSTAT (lsta

D19784: Avoid calling QT_LSTAT and accessing recent documents

ngraham added a comment. Nice, kinda sounds like this fixes https://bugs.kde.org/show_bug.cgi?id=373352. Can you confirm? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D19784 To: hoffmannrobert Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenB

D19784: Avoid calling QT_LSTAT and accessing recent documents

apol added inline comments. INLINE COMMENTS > recentusagemodel.cpp:254 > +// return any useful information for our purposes here. > +url.setScheme(QString()); > +KFileItem fileItem(url); This looks very much like a workaround. How about adding an argument to the KFileItem to skip th

D19784: Avoid calling QT_LSTAT and accessing recent documents

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > recentusagemodel.cpp:261 > > if (!url.isValid() || !(fileItem.isFile() || fileItem.isDir())) { > return QVariant(); How do you expect isFile()

D19784: Avoid calling QT_LSTAT and accessing recent documents

hoffmannrobert updated this revision to Diff 54339. hoffmannrobert added a comment. - Use new KFileItem constructor with skipStat set to true REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19784?vs=53951&id=54339 BRANCH fix_recent_documents_kicker

D19784: Avoid calling QT_LSTAT and accessing recent documents

hoffmannrobert marked 3 inline comments as done. hoffmannrobert added a comment. See https://phabricator.kde.org/D19887 INLINE COMMENTS > dfaure wrote in recentusagemodel.cpp:261 > How do you expect isFile() and isDir() to work without a QT_LSTAT call? A document should always be a file, so

D19784: Avoid calling QT_LSTAT and accessing recent documents

hoffmannrobert marked an inline comment as done. hoffmannrobert added a comment. In D19784#431683 , @ngraham wrote: > Nice, kinda sounds like this fixes https://bugs.kde.org/show_bug.cgi?id=373352. Can you confirm? No, unfortunately it do

D19784: Avoid calling QT_LSTAT and accessing recent documents

hoffmannrobert added a comment. In D19784#434564 , @hoffmannrobert wrote: > In D19784#431683 , @ngraham wrote: > > > Nice, kinda sounds like this fixes https://bugs.kde.org/show_bug.cgi?id=373352.

D19784: Avoid calling QT_LSTAT and accessing recent documents

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. OK so this is about KFileItem::text() and KFileItem::iconName(). Indeed this doesn't need the stat() done by KFileItem's init(). This means the right solution is indeed for KF

D19784: Avoid calling QT_LSTAT and accessing recent documents

dfaure added a comment. > If they point to files on a network drive, and the network or the drive is not responding Well that's exactly the problem with network mounts, and the reason they are a sucky technical solution. KIO's async jobs never have that problem. You will never be

D19784: Avoid calling QT_LSTAT and accessing recent documents

hoffmannrobert added a comment. In D19784#437715 , @dfaure wrote: > ... > Indeed this doesn't need the stat() done by KFileItem's init(). This means the right solution is indeed for KFileItem to do that stat() on demand, and this co

D19784: Avoid calling QT_LSTAT and accessing recent documents

hoffmannrobert updated this revision to Diff 54863. hoffmannrobert added a comment. - Use new KFileItem::SkipMimeTypeDetermination parameter REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19784?vs=54339&id=54863 BRANCH fix_recent_documents_kicker_

D19784: Avoid calling QT_LSTAT and accessing recent documents

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. This needs a KIO version ifdef for KIO >= 5.57. `#if KIO_VERSION >= QT_VERSION_CHECK(5,57,0)` REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D

D19784: Avoid calling QT_LSTAT and accessing recent documents

hoffmannrobert updated this revision to Diff 55115. hoffmannrobert added a comment. - Add KIO_VERSION check, use SkipMimeTypeFromContent REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19784?vs=54863&id=55115 BRANCH fix_recent_documents_kicker_hang

D19784: Avoid calling QT_LSTAT and accessing recent documents

This revision was automatically updated to reflect the committed changes. Closed by commit R119:408f03ca989e: Avoid calling QT_LSTAT and accessing recent documents (authored by hoffmannrobert, committed by dfaure). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.

D19784: Avoid calling QT_LSTAT and accessing recent documents

meven added a comment. Since this patch icons for folders in recent documents have been replaced with the application-octet-stream icon. SkipMimeTypeFromContent should be avoided for local directories. We need a better way to detect if the folder is a network folder before making assum

D19784: Avoid calling QT_LSTAT and accessing recent documents

dfaure added a comment. See KFileItem::isSlow() REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D19784 To: hoffmannrobert, #frameworks, dfaure, #dolphin Cc: meven, apol, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, h

D19784: Avoid calling QT_LSTAT and accessing recent documents

meven added a comment. In D19784#503066 , @dfaure wrote: > See KFileItem::isSlow() Interesting but the initial issue happened when a drive is not mounted and isSlow implementation uses statfs that gives information about ... mounted file