D7404: Run the Baloo runner out of process
davidedmundson created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION 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. 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 REPOSITORY R120 Plasma Workspace BRANCH master REVISION DETAIL https://phabricator.kde.org/D7404 AFFECTED FILES runners/baloo/CMakeLists.txt runners/baloo/baloosearchrunner.cpp runners/baloo/baloosearchrunner.h runners/baloo/dbusutils_p.h runners/baloo/org.kde.baloorunner.service.in runners/baloo/org.kde.krunner1.xml runners/baloo/plasma-runner-baloosearch.desktop To: davidedmundson, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7404: Run the Baloo runner out of process
davidedmundson edited the summary of this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D7404 To: davidedmundson, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7404: Run the Baloo runner out of process
broulik added a comment.
Cool!
INLINE COMMENTS
> baloosearchrunner.cpp:49
> {
> +QApplication app(argc, argv); //KRun needs widgets for some reason..
> +app.setQuitOnLastWindowClosed(false);
`KRun` may spawn `QMessageBox` and the like (which makes me wonder if we really
should make it not desktop settings aware)
> baloosearchrunner.cpp:51
> +app.setQuitOnLastWindowClosed(false);
> +app.setDesktopSettingsAware(false);
> +SearchRunner r;
This is a static and needs to be done before `QApplication` is constructed.
> baloosearchrunner.cpp:68
> +qDBusRegisterMetaType();
> +QDBusConnection::sessionBus().registerService("org.kde.baloorunner");
> +QDBusConnection::sessionBus().registerObject("/runner", this);
Maybe we should name it `org.kde.runners.baloo` so if we group it with others
in the future?
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7404
To: davidedmundson, #plasma
Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart, lukas
D7404: Run the Baloo runner out of process
davidedmundson updated this revision to Diff 18389. davidedmundson added a comment. Kai's comments REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7404?vs=18370&id=18389 BRANCH master REVISION DETAIL https://phabricator.kde.org/D7404 AFFECTED FILES runners/baloo/CMakeLists.txt runners/baloo/baloosearchrunner.cpp runners/baloo/baloosearchrunner.h runners/baloo/dbusutils_p.h runners/baloo/org.kde.baloorunner.service.in runners/baloo/org.kde.krunner1.xml runners/baloo/plasma-runner-baloosearch.desktop To: davidedmundson, #plasma Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7404: Run the Baloo runner out of process
broulik added inline comments.
INLINE COMMENTS
> baloosearchrunner.cpp:51
> +if (!config.fileIndexingEnabled()) {
> +return -1;
> +}
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.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7404
To: davidedmundson, #plasma
Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart, lukas
D7404: Run the Baloo runner out of process
davidedmundson added inline comments. INLINE COMMENTS > broulik wrote in baloosearchrunner.cpp:51 > 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. 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. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D7404 To: davidedmundson, #plasma Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7404: Run the Baloo runner out of process
broulik accepted this revision. broulik added a comment. This revision is now accepted and ready to land. Let's leave it that way REPOSITORY R120 Plasma Workspace BRANCH master REVISION DETAIL https://phabricator.kde.org/D7404 To: davidedmundson, #plasma, broulik Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7404: Run the Baloo runner out of process
This revision was automatically updated to reflect the committed changes. Closed by commit R120:76d41e0831cf: Run the Baloo runner out of process (authored by davidedmundson). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7404?vs=18389&id=18479 REVISION DETAIL https://phabricator.kde.org/D7404 AFFECTED FILES runners/baloo/CMakeLists.txt runners/baloo/baloosearchrunner.cpp runners/baloo/baloosearchrunner.h runners/baloo/dbusutils_p.h runners/baloo/org.kde.baloorunner.service.in runners/baloo/org.kde.krunner1.xml runners/baloo/plasma-runner-baloosearch.desktop To: davidedmundson, #plasma, broulik Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
