KIO DesktopExecParser: simplify code
ClosedPublic

Authored by dfaure on Mar 21 2020, 4:32 PM.

Details

Summary
  • Remove special case for hasSchemeHandler, it's just a hack.

When starting a service that handles a specific scheme, we know it
supports that URL (supportedProtocols returns it).

  • Instead, the broken logic was that if we are in the special case

"username set, no password", which forces a FUSE request *even* for a
supported protocol, and that FUSE request fails, then we don't want to
fallback to kioexec. Using the service directly works just fine.

  • Merge the two for() loops. The first one is "no %u, so only local

files are supported", easy to model in the condition for the bool
'supported'.

  • If we do proceed without kioexec after a fuse error, recheck

request.reply.isError otherwise we end up replacing a URL with a DBus
error message :-)

  • Even further optimization: skip the FUSE request for the case of

a scheme-handler with username. For local, KIO, scheme-handlers,
supported means supported. The hack is only for the other case:
explicitly listed protocols with X-KDE-Protocols like in vlc.desktop

Test Plan

krununittest (which unfortunately doesn't cover all cases),
plus stepping with gdb to see if we make a FUSE request.

Diff Detail

Repository
R241 KIO
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24031
Build 24049: arc lint + arc unit
dfaure created this revision.Mar 21 2020, 4:32 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 21 2020, 4:32 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
dfaure requested review of this revision.Mar 21 2020, 4:32 PM

Neat.

The logic all seems to hold:

We have no urls:

  • if it's a local file, useKioexec is false, and no fuse since "requests" will be empty
  • if it's not a local file, useKioexec is true, and we try fuse; if there's an error with fuse fallback to kioexec and return early

We have urls:

  • if proto is not in supported list, useKioexec is true:
    • if fuseError is true, use kioexec and return early
    • fuseError is false, at least one url can use fuse, go through for loop (on 384)
  • if proto is in supported list, useKioexec is false, which makes fuseError redundant (true/false && false -> false), so kioexec is out:
    • if it's a vlc-like situation, iterate over the urls and check fuse reply stuff
    • if it's a ktelnet-situation, "requests" is always empty, so no fuse is tried

("Complicated" is the least that can be said :D).

And indeed it's not only vlc (continuing the discussion in my other abandoned diff):
$ grep -r X-KDE-Protocols /usr/share/applications/
/usr/share/applications/smplayer.desktop:X-KDE-Protocols=http,ftp,smb
/usr/share/applications/startcenter.desktop:X-KDE-Protocols=file,http,ftp,webdav,webdavs
/usr/share/applications/calc.desktop:X-KDE-Protocols=file,http,ftp,webdav,webdavs
/usr/share/applications/mpv.desktop:X-KDE-Protocols=ftp,http,https,mms,rtmp,rtsp,sftp,smb
/usr/share/applications/vlc.desktop:X-KDE-Protocols=ftp,http,https,mms,rtmp,rtsp,sftp,smb
/usr/share/applications/writer.desktop:X-KDE-Protocols=file,http,ftp,webdav,webdavs

ahmadsamir accepted this revision.Mar 22 2020, 1:25 PM
This revision is now accepted and ready to land.Mar 22 2020, 1:25 PM

Thanks for the detailed review!

dfaure closed this revision.Mar 22 2020, 1:28 PM