D26407: KFileItem: Improve isSlow to not block when a network mount is unresponsive, make SkipMimeTypeFromContent skip only on slow fs

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> meven wrote in kfileitem.cpp:787
> I did not mean to avoid android, just surfaced the comment in KMountPoint::currentMountPoints regarding its limitation to make it clear why we need an else block here in the first place.

OK, but I still read "can only mean" like a possibly incorrect assumption.

I suggest s/can only mean/for instance/

> kfileitem.cpp:771
> +        // refresh cached mountpoints data
> +        mountPoints = KMountPoint::currentMountPoints();
> +    }

You forgot to update lastMountPointUpdate here.

And to avoid calling currentDateTime() twice (this is a relatively slow method), make sure to put the result into a local var used in both places.

> kfileitem.cpp:1246
> +    // avoid potential blocking stat on network mount
> +    if (d->m_bSkipMimeTypeFromContent && d->isSlow()) {
>          return false;

Did you mean || ?

Otherwise this changes the behaviour also on fast local paths (when SkipMimeTypeFromContent is set).

Then again, D19887 <https://phabricator.kde.org/D19887> (which introduced this if) was apparently about network mounts.

> kmountpoint.h:66
> +         */
> +        Ptr findByPathDirectly(const QString &path) const;
> +

I don't really like the name, it's non-obvious until reading the documentation.
Maybe it should be findByPath(path, KMountPoint::DontResolveSymlinks) ?
(with a 2-args findByPath overload and a KF6 TODO to merge the two)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D26407

To: meven, #frameworks, ngraham, broulik, dfaure
Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200112/dfe1fbd8/attachment.html>