Run the Baloo runner out of process
ClosedPublic

Authored by davidedmundson on Aug 18 2017, 8:54 PM.

Details

Summary

KRunner now supports querying data from running processes rather than
being plugins.

Due the number of crash reports of Baloo in both krunner and more
importantly plasmashell, we can move this out of process to make the UX
better in the event of an issue.

This also means we share the database instance between both krunner and plasmashell.

Test Plan

Searched, typing really quickly
All works as before; including forcing a delay when you only type a few letters
Results are just as fast as before to the human eye (bustle show calls as 0ms)

Tested open with folder and open normally actions
Tested dragging from krunner to dolphin
Tested we had correct icons

Diff Detail

Repository
R120 Plasma Workspace
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
davidedmundson created this revision.Aug 18 2017, 8:54 PM
Restricted Application added a project: Plasma. · View Herald TranscriptAug 18 2017, 8:54 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidedmundson edited the summary of this revision. (Show Details)Aug 18 2017, 8:57 PM

Cool!

runners/baloo/baloosearchrunner.cpp
49

KRun may spawn QMessageBox and the like (which makes me wonder if we really should make it not desktop settings aware)

51

This is a static and needs to be done before QApplication is constructed.

68

Maybe we should name it org.kde.runners.baloo so if we group it with others in the future?

Kai's comments

broulik added inline comments.Aug 19 2017, 1:26 PM
runners/baloo/baloosearchrunner.cpp
47

Won't we end up restarting this service over and over again whilst typing when Baloo is dlsabled? Not sure how bad that is, though.

davidedmundson added inline comments.Aug 19 2017, 1:54 PM
runners/baloo/baloosearchrunner.cpp
47

If baloo is disabled, but the runner is enabled, yes.

Not a huge issue, not ideal either.

Brainstorming options:

  • We just let this tiny app linger doing nothing
  • We don't do DBus autostart, but regular session autostart with X-KDE-autostart-condition
  • We add some sort of smarter Enabled syntax in the Krunner .desktop file like X-KDE-autostart-condition

(most invasiave, but gives the best UI as we can even hide the option for this entry)

  • I could forward AbstractRunner::prepare / teardown (assuming they actually work) Means it'll only happen once per run session.
  • I can make DBusRunner catch errors on the call to GetActions() and turn itself off if gets one.
broulik accepted this revision.Aug 19 2017, 1:57 PM

Let's leave it that way

This revision is now accepted and ready to land.Aug 19 2017, 1:57 PM
This revision was automatically updated to reflect the committed changes.