Review Request 118933: PlasmaShell: Do not start krunner

Wed Jun 25 16:52:19 UTC 2014

> On June 25, 2014, 11:12 a.m., Marco Martin wrote:
> > Just to recap why it was decided to make it start by the plasma shell and to actually remove the autostart file (part of Ivan's Gsoc last year):
> > krunner being present or not is something that depends from the shell ux, and one shell can decide to not have krunner, and even stopping it if it's already running (active, for instance)
> > (also, the only reason it actually made sense to split the system monitor for it, increasing overhead)
> > relying again on autostart files it forces krunner on every shell, having a process that may not be used at all.
> >
> 
> Vishesh Handa wrote:
>     I do not understand your comment about "having a process which may not be used at all". Without this patch, krunner will always be started.
>     
>     Could you propose an alternate solution for the bug?
>     
>     I can think of the following options -
>     1. We delegate this problem to the future when we actually get shells which do not want krunner, at that time maybe they can close it.
>     2. We modify the "Platform.Application" class to take a dbus-interface name, and check it is registered. Only then should it start the process.
>     
>     I think (1) is a more desirable solution right now, but that's just me.
> 
> Martin Gräßlin wrote:
>     IIRC krunner installs a dbus service and can be started by the dbus daemon - use this as solution (3). If we just ask dbus to start krunner when needed it won't be started again when it's already running.
> 
> Marco Martin wrote:
>     Vishesh: yes, 1) is fine too for me
>     
>     Martin: in this case who would be to invoke the dbus service starting krunner? plasmashell? or may be the global shortcut daemon itself?
> 
> Martin Gräßlin wrote:
>     > in this case who would be to invoke the dbus service starting krunner? plasmashell?
>     
>     yeah, plasmashell.
> 
> Vishesh Handa wrote:
>     @Marco: If (1) is fine with you, then can you please give me a "Ship it".
>     
>     @Martin: Correct me if I misunderstood you - Krunner should only be dbus activated, and not started by any process. When the user needs to use it kglobalaccell dbus-activates it, otherwise the process which needs it launches it?
> 
> Marco Martin wrote:
>     i wanted to understand a bit more what Martin meant...
>     Martin, can you elaborate more?
> 
> Martin Gräßlin wrote:
>     hmm I can try, though I don't know enough of it - d_ed might be the better expert for dbus activation. Krunner installs a dbus service file with that dbus daemon knows that the org.kde.KRunner service exists and which application to start to get this service. If now a process tries to access this service (no idea how that works in detail) dbus daemon just starts the process and the service is around.
>     
>     So in the case plasma-shell - krunner: krunner gets started by plasma-shell "asking" dbus daemon to start krunner. If Krunner is already running, nothing happens, if krunner isn't running it gets started.
> 
> David Edmundson wrote:
>     krunner already has dbus-activation.
>     If anyone queries org.kde.krunner it will start.

Martin, could you please elaborate on how kglobalaccel ties in together with dbus activation? I don't see any relevant code.

- Vishesh

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118933/#review60939
-----------------------------------------------------------

On June 25, 2014, 10:58 a.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118933/
> -----------------------------------------------------------
> 
> (Updated June 25, 2014, 10:58 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
>     PlasmaShell: Do not start krunner
> 
>     Krunner is automatically started via an autostart file. We do not need plasma to
>     start it as well. Without this patch, krunner comes into focus each time
>     plasma is restarted since krunner is already running and executing it
>     then gives it focus.
> 
>     BUG: 336002
> 
> This patch depends on another patch in plasma-workspace/krunner which adds the krunner.desktop file -
> 
> commit 1b570623b1e8df93f20940654e160b35570172ac
> Author: Vishesh Handa <me at vhanda.in>
> Date:   Wed Jun 25 11:38:49 2014 +0200
> 
>     Add a KRunner autostart file
> 
> diff --git a/krunner/CMakeLists.txt b/krunner/CMakeLists.txt
> index 8e625b9..4197827 100644
> --- a/krunner/CMakeLists.txt
> +++ b/krunner/CMakeLists.txt
> @@ -35,6 +35,7 @@ target_link_libraries(krunner
>  
>  install(TARGETS krunner ${INSTALL_TARGETS_DEFAULT_ARGS})
>  install(FILES ${krunner_dbusAppXML} DESTINATION ${DBUS_INTERFACES_INSTALL_DIR} )
> +install(FILES krunner.desktop DESTINATION ${AUTOSTART_INSTALL_DIR})
>  
>  set(CMAKECONFIG_INSTALL_DIR "${CMAKECONFIG_INSTALL_PREFIX}/KRunnerAppDBusInterface")
>  ecm_configure_package_config_file(KRunnerAppDBusInterfaceConfig.cmake.in
> diff --git a/krunner/krunner.desktop b/krunner/krunner.desktop
> new file mode 100644
> index 0000000..2f1f6dc
> --- /dev/null
> +++ b/krunner/krunner.desktop
> @@ -0,0 +1,9 @@
> +[Desktop Entry]
> +Exec=krunner
> +Name=KRunner
> +OnlyShowIn=KDE;
> +Type=Application
> +X-DBUS-StartupType=Unique
> +X-DBUS-ServiceName=org.kde.krunner
> +X-KDE-StartupNotify=false
> +X-KDE-autostart-phase=0
> 
> 
> Diffs
> -----
> 
>   desktoppackage/contents/loader.qml c1ac4a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118933/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140625/10ab1b71/attachment.html>