GNOME Bugzilla – Bug 608402
Add support for delaying autostart applications
Last modified: 2021-06-14 18:20:21 UTC
Created attachment 152536 [details] [review] Add support for AutostartDelay In Ubuntu, we have quite a few things start in the session now which we start after a delay. This is currently achieved by specifying something like 'sh -c "sleep 30; exec <app>' on the Exec line in the autostart desktop file for each particular application we want to delay. This solution is far from elegant, as it still spawns a shell which sits there until the sleep delay expires. When gnome-screensaver was started from g-s-d, it was started after a delay. This is not the case anymore since it is started as a normal autostart app. I think it would make sense to support something like this natively in gnome-session. I've attached a patch which implements this feature, by supporting a new key called "AutostartDelay" in the desktop files. The feature only permits applications which start in the APPLICATION phase to specify an autostart delay (it doesn't make sense having a delay in earlier phases)
Please prefix the key name with X-GNOME. Also, the egg* changes should go to libegg first, I guess. (didn't really review the patch, though)
Created attachment 152541 [details] [review] libegg patch Here is a patch for libegg for reading integers from desktop files
Review of attachment 152536 [details] [review]: ::: gnome-session/gsm-autostart-app.c @@ +462,3 @@ + if (app->priv->autostart_delay < 0) { + g_warning ("Invalid autostart delay of %d for %s", app->priv->autostart_delay, + gsm_app_peek_id (GSM_APP (app))); Probably better to set it to -1 again if it's invalid (just in case someone adds some "== -1" check later). ::: gnome-session/gsm-manager.c @@ +330,2 @@ error = NULL; + if (gsm_app_is_running (app)) { Shouldn't this check be done in gsm_app_stop? @@ +585,3 @@ + } + + return FALSE; Wouldn't it be simpler to call _start_app again (without the check for autostart delay)? This would imply calling gsm_app_peek_autostart_delay first in _start_app, I guess, and splitting the code in _start_app and _start_app_internal, or something like this. @@ +618,3 @@ + g_timeout_add_seconds (delay, + (GSourceFunc)_autostart_delay_timeout, + app); You want to ref the app, to be 100% sure it still exists on timeout.
Review of attachment 152541 [details] [review]: Probably okay to go in. Dan? ::: libegg/smclient/eggdesktopfile.c @@ +435,3 @@ +egg_desktop_file_get_integer (EggDesktopFile *desktop_file, + const char *key, + GError **error) Interesting indentation :-) ::: libegg/smclient/eggdesktopfile.h @@ +128,3 @@ const char *key, GError **error); +int egg_desktop_file_get_integer (EggDesktopFile *desktop_file, And here too ;-)
(In reply to comment #4) > Review of attachment 152541 [details] [review]: > > Probably okay to go in. Dan? yeah, fix the indentation though.
Thanks, I've committed the libegg change now, with the fixed indentation
Just for the record, this is working wonderfully. Thanks Chris!
This is a little bit of a tangent from this bug report, but for the screensaver case specifically, another idea would be to have screensaver get d-bus activated when 1) the session goes idle 2) the user tries to lock the screen and then not bother starting it at login at all.
(to do comment 8 "right" we'd need to get a feature into D-Bus where it can activate services on signal emission, I think)
Was this fixed finally? (I would like to start to add delays for some apps I run at startup) Thanks
Comment on attachment 152541 [details] [review] libegg patch This got committed a while ago.
Review of attachment 152536 [details] [review]: I reread the patch, and came with additional comments :-) ::: gnome-session/gsm-autostart-app.c @@ +455,3 @@ setup_condition_monitor (app); + if (phase == GSM_MANAGER_PHASE_APPLICATION) { I wouldn't do this check here, but in gsm-manager.c (see other comment). So, always read AutostartDelay. @@ +458,3 @@ + /* Only accept an autostart delay for the application phase */ + app->priv->autostart_delay = egg_desktop_file_get_integer (app->priv->desktop_file, + "AutostartDelay", Hrm, btw, shouldn't this key be prefixed with X-GNOME? ::: gnome-session/gsm-manager.c @@ +568,3 @@ static gboolean +_autostart_delay_timeout (GsmApp *app) +{ This should do nothing if we're in a phase > GSM_MANAGER_PHASE_RUNNING. @@ +614,3 @@ } + delay = gsm_app_peek_autostart_delay (app); This is where the check for GSM_MANAGER_PHASE_APPLICATION should be done, imho. @@ +618,3 @@ + g_timeout_add_seconds (delay, + (GSourceFunc)_autostart_delay_timeout, + app); The other thing I'm wondering is whether we should track those timeouts and properly cancel them on logout. Would be cleaner, but this means yet another list to keep around.
Created attachment 212425 [details] current version of the patch the patch we have currently in Ubuntu (gnome-session 3.2), I don't think it addresses the previous review comments
Created attachment 245793 [details] [review] GSMAutostartApp: Add support for autostarting applications after a delay Make it possible to delay autostarting an application for a time period defined by the "AutostartDelay" key in the applications desktop file Updated for gnome-session 3.8/3.9 and reworked to address the review comments (except for the last comment about keeping track of the timeouts)
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version of gnome-session, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/gnome-session/-/issues/ Thank you for your understanding and your help.