Merge lp:~kalgasnik/lightdm-gtk-greeter/pre2.0-bugfixes into lp:lightdm-gtk-greeter

[Andrew P.]

Fixes:
1. Some regressions after merging background branch (greeter do not close spawned processes correctly)
2. Using INDICATOR_OBJECT_SIGNAL_ENTRY_SCROLLED signal instead of obsolete "scroll-entry"
3. Invalid loop in indicator_menu_show_cb()
4. lp1398619 (upstart). "configure" options:

  --enable-at-spi-command[=/command/to/execute]
    Command to launch at-spi bus.
    Enabled by default, command: "/usr/lib/at-spi2-core/at-spi-bus-launcher --launch-immediately"

  --enable-indicator-services-command[=/command/to/execute]
    Command to launch indicators service.
    Enabled by default, command: "upstart --user --startup-event indicator-services-start"

  --enable-indicator-services is obsolete now.

Required changes:
  Remove "--enable-indicator-services" from configure command line.
  Specify correct (old) value of --enable-indicator-services-command option for systems without installed "systemd-sysv" package:
  --enable-indicator-services-command="init --user --startup-event indicator-services-start"

[Sean Davis (bluesabre)]

Looks good to me, thanks!

review: Approve

=== modified file 'configure.ac'
--- configure.ac	2014-08-10 12:48:13 +0000
+++ configure.ac	2015-01-26 21:11:48 +0000
@@ -36,6 +36,8 @@
 INDICATOR_PKG=indicator3-0.4
 IDO_PKG=libido3-0.1
 
+dnl ###########################################################################
+
 AC_ARG_ENABLE([libindicator],
     AC_HELP_STRING([--enable-libindicator], [Enable libindicator support])
     AC_HELP_STRING([--disable-libindicator], [Disable libindicator support]),
@@ -67,6 +69,8 @@
     AC_MSG_RESULT([disabled])
 ])
 
+dnl ###########################################################################
+
 AC_ARG_ENABLE([libido],
     AC_HELP_STRING([--enable-libido], [Enable libido support])
     AC_HELP_STRING([--disable-libido], [Disable libido support]),
@@ -86,23 +90,46 @@
     AC_MSG_RESULT([disabled])
 ])
 
-AC_ARG_ENABLE([indicator-services],
-    AC_HELP_STRING([--enable-indicator-services], [Try to start indicator services])
-    AC_HELP_STRING([--disable-indicator-services], [Do not start indicator services]),
-            [], [enable_indicator_services=no])
-
-AS_IF([test "x$enable_indicator_services" != "xno" && test "x$have_libindicator" = "xyes"], [
-    AC_DEFINE([START_INDICATOR_SERVICES], [], [Try to start indicator-services])
-])
+dnl ###########################################################################
 
 AC_ARG_WITH([libxklavier], AS_HELP_STRING([--with-libxklavier], [Use libxklavier to manage layouts (instead of LightDM API)]))
 
 AS_IF([test "x$with_libxklavier" = "xyes"],
-      [
-       PKG_CHECK_MODULES([LIBXKLAVIER], [libxklavier], [have_xklavier=yes])
-       AC_DEFINE([HAVE_LIBXKLAVIER], [1], [Define if "libxklavier" is present])
-      ],
-      [])
+[
+    PKG_CHECK_MODULES([LIBXKLAVIER], [libxklavier], [have_xklavier=yes])
+    AC_DEFINE([HAVE_LIBXKLAVIER], [1], [Define if "libxklavier" is present])
+],
+[])
+
+dnl ###########################################################################
+
+AC_ARG_ENABLE([at-spi-command],
+    AC_HELP_STRING([--enable-at-spi-command[=command]], [Try to start at-spi service]])
+    AC_HELP_STRING([--disable-at-spi-command], [Do not start at-spi service]),
+    [], [])
+
+AS_IF([test "x$enable_at_spi_command" != "xno"],
+[
+    if test "x$enable_at_spi_command" = "xyes" || test "x$enable_at_spi_command" = "x"; then
+        enable_at_spi_command="/usr/lib/at-spi2-core/at-spi-bus-launcher --launch-immediately"
+    fi
+    AC_DEFINE_UNQUOTED([AT_SPI_COMMAND], ["$enable_at_spi_command"], [Command to start at-spi service])
+])
+
+dnl ###########################################################################
+
+AC_ARG_ENABLE([indicator-services-command],
+    AC_HELP_STRING([--enable-indicator-services-command[=command]], [Try to start indicators service]])
+    AC_HELP_STRING([--disable-indicator-services-command], [Do not start indicators service]),
+    [], [])
+
+AS_IF([test "x$enable_indicator_services_command" != "xno"],
+[
+    if test "x$enable_indicator_services_command" = "xyes" || test "x$enable_indicator_services_command" = "x"; then
+        enable_indicator_services_command="upstart --user --startup-event indicator-services-start"
+    fi
+    AC_DEFINE_UNQUOTED([INDICATOR_SERVICES_COMMAND], ["$enable_indicator_services_command"], [Command to start indicators service])
+])
 
 dnl ###########################################################################
 dnl Internationalization

=== modified file 'src/lightdm-gtk-greeter.c'
--- src/lightdm-gtk-greeter.c	2015-01-17 13:45:58 +0000
+++ src/lightdm-gtk-greeter.c	2015-01-26 21:11:48 +0000
@@ -53,7 +53,6 @@
 #include "src/lightdm-gtk-greeter-css-fallback.h"
 #include "src/lightdm-gtk-greeter-css-application.h"
 
-
 static LightDMGreeter *greeter;
 
 /* State file */
@@ -61,10 +60,14 @@
 static gchar *state_filename;
 static void save_state_file (void);
 
-/* Terminating */
-GSList *pids_to_close = NULL;
+/* List of spawned processes */
+static GSList *pids_to_close = NULL;
+static GPid spawn_argv_pid (gchar **argv, GSpawnFlags flags, gint *pfd, GError **perror);
+#if defined(AT_SPI_COMMAND) || defined(INDICATOR_SERVICES_COMMAND)
+static GPid spawn_line_pid (const gchar *line, GSpawnFlags flags, GError **perror);
+#endif
+static void close_pid (GPid pid, gboolean remove);
 static void sigterm_cb (gpointer user_data);
-static void close_pid (GPid *pid, gboolean remove);
 
 /* Screen window */
 static GtkOverlay *screen_overlay;
@@ -148,6 +151,8 @@
 /* External command (keyboard, reader) */
 typedef struct
 {
+    const gchar *name;
+
     gchar **argv;
     gint argc;
 
@@ -156,8 +161,9 @@
     GtkWidget *widget;
 } MenuCommand;
 
-static MenuCommand *menu_command_parse (const gchar *value, GtkWidget *menu_item);
-static MenuCommand *menu_command_parse_extended (const gchar *value, GtkWidget *menu_item,
+static MenuCommand *menu_command_parse (const gchar *name, const gchar *value, GtkWidget *menu_item);
+static MenuCommand *menu_command_parse_extended (const gchar *name,
+                                                 const gchar *value, GtkWidget *menu_item,
                                                  const gchar *xid_app, const gchar *xid_arg);
 static gboolean menu_command_run (MenuCommand *command);
 static gboolean menu_command_stop (MenuCommand *command);
@@ -349,6 +355,79 @@
 
 /* Terminating */
 
+static GPid
+spawn_argv_pid (gchar **argv, GSpawnFlags flags, gint *pfd, GError **perror)
+{
+    GPid pid = 0;
+    GError *error = NULL;
+    gboolean spawned = FALSE;
+
+    if (pfd)
+        spawned = g_spawn_async_with_pipes (NULL, argv, NULL, flags, NULL, NULL, &pid, NULL, pfd, NULL, perror);
+    else
+        spawned = g_spawn_async (NULL, argv, NULL, flags, NULL, NULL, &pid, &error);
+
+    if (spawned)
+    {
+        pids_to_close = g_slist_prepend (pids_to_close, GINT_TO_POINTER (pid));
+        g_debug ("[PIDs] Command executed (#%d): %s", pid, argv[0]);
+    }
+    else if (perror)
+    {
+        *perror = error;
+    }
+    else
+    {
+        g_warning ("[PIDs] Failed to execute command: %s", argv[0]);
+        g_clear_error (&error);
+    }
+
+    return pid;
+}
+
+#if defined(AT_SPI_COMMAND) || defined(INDICATOR_SERVICES_COMMAND)
+static GPid
+spawn_line_pid (const gchar *line, GSpawnFlags flags, GError **perror)
+{
+    gint argc = 0;
+    gchar **argv = NULL;
+    GError *error = NULL;
+
+    if (g_shell_parse_argv (line, &argc, &argv, &error))
+    {
+        GPid pid = spawn_argv_pid (argv, flags, NULL, perror);
+        g_strfreev (argv);
+        return pid;
+    }
+    else if (!perror && error)
+    {
+        g_warning ("[PIDs] Failed to parse command line: %s, %s", error->message, line);
+        g_clear_error (&error);
+    }
+    else if (error)
+        *perror = error;
+
+    return 0;
+}
+#endif
+
+static void
+close_pid (GPid pid, gboolean remove)
+{
+    if (!pid)
+        return;
+
+    if (remove)
+        pids_to_close = g_slist_remove (pids_to_close, GINT_TO_POINTER (pid));
+
+    if (kill (pid, SIGTERM) == 0)
+        g_debug ("[PIDs] Process terminated: #%d", pid);
+    else
+        g_warning ("[PIDs] Failed to terminate process #%d: %s", pid, g_strerror (errno));
+
+    waitpid (pid, NULL, 0);
+}
+
 static void
 sigterm_cb (gpointer user_data)
 {
@@ -358,20 +437,6 @@
     gtk_main_quit ();
 }
 
-static void
-close_pid (GPid *ppid, gboolean remove)
-{
-    if (!ppid || !*ppid)
-        return;
-
-    if (remove)
-        pids_to_close = g_slist_remove (pids_to_close, GINT_TO_POINTER (*ppid));
-
-    kill (*ppid, SIGTERM);
-    waitpid (*ppid, NULL, 0);
-    *ppid = 0;
-}
-
 /* Power window */
 
 static gboolean
@@ -682,13 +747,14 @@
 /* MenuCommand */
 
 static MenuCommand*
-menu_command_parse (const gchar *value, GtkWidget *menu_item)
+menu_command_parse (const gchar *name, const gchar *value, GtkWidget *menu_item)
 {
-    return menu_command_parse_extended (value, menu_item, NULL, NULL);
+    return menu_command_parse_extended (name, value, menu_item, NULL, NULL);
 }
 
 static MenuCommand*
-menu_command_parse_extended (const gchar *value,    /* String to parse */
+menu_command_parse_extended (const gchar *name,
+                             const gchar *value,    /* String to parse */
                              GtkWidget *menu_item,  /* Menu item to connect */
                              const gchar *xid_app,  /* Program that have "xembed" mode support */
                              const gchar *xid_arg)  /* Argument that must be added to command line */
@@ -703,7 +769,7 @@
     if (!g_shell_parse_argv (value, &argc, &argv, &error))
     {
         if (error)
-            g_warning ("Failed to parse command line: %s", error->message);
+            g_warning ("[Command/%s] Failed to parse command line: %s", name, error->message);
         g_clear_error (&error);
         return NULL;
     }
@@ -737,7 +803,7 @@
             else
             {
                 if (error)
-                    g_warning ("Failed to parse command line: %s", error->message);
+                    g_warning ("[Command/%s] Failed to parse command line: %s", name, error->message);
                 g_clear_error (&error);
             }
             g_free (new_value);
@@ -749,6 +815,7 @@
             gtk_overlay_add_overlay (screen_overlay, command->widget);
         }
     }
+    command->name = name;
     return command;
 }
 
@@ -758,14 +825,17 @@
     g_return_val_if_fail (command && g_strv_length (command->argv), FALSE);
 
     GError *error = NULL;
-    gboolean spawned = FALSE;
+    command->pid = 0;
+
+    g_debug ("[Command/%s] Running command", command->name);
+
     if (command->widget)
     {
         GtkSocket* socket = NULL;
         gint out_fd = 0;
+        GPid pid = spawn_argv_pid (command->argv, G_SPAWN_SEARCH_PATH, &out_fd, &error);
 
-        if (g_spawn_async_with_pipes (NULL, command->argv, NULL, G_SPAWN_SEARCH_PATH,
-                                      NULL, NULL, &command->pid, NULL, &out_fd, NULL, &error))
+        if (pid && out_fd)
         {
             gchar* text = NULL;
             GIOChannel* out_channel = g_io_channel_unix_new (out_fd);
@@ -782,37 +852,36 @@
                     gtk_container_add (GTK_CONTAINER (command->widget), GTK_WIDGET (socket));
                     gtk_socket_add_id (socket, id);
                     gtk_widget_show_all (GTK_WIDGET (command->widget));
-                    spawned = TRUE;
+
+                    command->pid = pid;
                 }
                 else
-                    g_warning ("Failed to get '%s' socket: unrecognized output", command->argv[0]);
+                    g_warning ("[Command/%s] Failed to get '%s' socket for: unrecognized output",
+                               command->name, command->argv[0]);
 
                 g_free (text);
             }
         }
+
+        if (!command->pid && pid)
+            close_pid (pid, TRUE);
     }
     else
     {
-        spawned = g_spawn_async (NULL, command->argv, NULL,
-                                 G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD,
-                                 NULL, NULL, &command->pid, &error);
-        if (spawned)
+        command->pid = spawn_argv_pid (command->argv, G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD, NULL, &error);
+        if (command->pid)
             g_child_watch_add (command->pid, (GChildWatchFunc)menu_command_terminated_cb, command);
     }
 
-    if (spawned)
-    {
-        pids_to_close = g_slist_prepend (pids_to_close, GINT_TO_POINTER (command->pid));
-    }
-    else
+    if (!command->pid)
     {
         if (error)
-            g_warning ("Command spawning error: '%s'", error->message);
-        command->pid = 0;
+            g_warning ("[Command/%s] Failed to run: %s", command->name, error->message);
         gtk_check_menu_item_set_active (GTK_CHECK_MENU_ITEM (command->menu_item), FALSE);
     }
     g_clear_error (&error);
-    return spawned;
+
+    return command->pid;
 }
 
 static gboolean
@@ -822,7 +891,9 @@
 
     if (command->pid)
     {
-        close_pid (&command->pid, TRUE);
+        g_debug ("[Command/%s] Stopping command", command->name);
+        close_pid (command->pid, TRUE);
+        command->pid = 0;
         if (command->menu_item)
             gtk_check_menu_item_set_active (GTK_CHECK_MENU_ITEM (command->menu_item), FALSE);
         if (command->widget)
@@ -1156,8 +1227,7 @@
 
     g_return_val_if_fail (INDICATOR_IS_OBJECT (io), FALSE);
 
-    g_signal_emit_by_name (io, "scroll", 1, event->direction);
-    g_signal_emit_by_name (io, "scroll-entry", entry, 1, event->direction);
+    g_signal_emit_by_name(io, INDICATOR_OBJECT_SIGNAL_ENTRY_SCROLLED, entry, 1, event->direction);
 
     return FALSE;
 }
@@ -1275,7 +1345,7 @@
     {
         /* Close any open menus instead of opening one */
         entries = indicator_object_get_entries (io);
-        for (lp = entries; lp; lp = g_list_next (entry))
+        for (lp = entries; lp; lp = g_list_next (lp))
         {
             entrydata = lp->data;
             gtk_menu_popdown (entrydata->menu);
@@ -1434,18 +1504,6 @@
             /* Don't allow virtual file systems? */
             greeter_set_env ("GIO_USE_VFS", "local");
             greeter_set_env ("GVFS_DISABLE_FUSE", "1");
-
-            #ifdef START_INDICATOR_SERVICES
-            gchar *INDICATORS_CMD[] = {"init", "--user", "--startup-event", "indicator-services-start", NULL};
-            GError *error = NULL;
-            GPid pid = 0;
-
-            if (g_spawn_async (NULL, INDICATORS_CMD, NULL, G_SPAWN_SEARCH_PATH, NULL, NULL, &pid, &error))
-                pids_to_close = g_slist_prepend (pids_to_close, GINT_TO_POINTER (pid));
-            else
-                g_warning ("[Greeter] Failed to run 'init --startup-event indicator-services-start': %s", error->message);
-            g_clear_error (&error);
-            #endif
             inited = TRUE;
         }
 
@@ -2553,7 +2611,7 @@
     bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8");
     textdomain (GETTEXT_PACKAGE);
 
-    g_unix_signal_add (SIGTERM, (GSourceFunc)sigterm_cb, pids_to_close);
+    g_unix_signal_add (SIGTERM, (GSourceFunc)sigterm_cb, NULL);
 
     config = g_key_file_new ();
     g_key_file_load_from_file (config, CONFIG_FILE, G_KEY_FILE_NONE, &error);
@@ -2692,6 +2750,13 @@
         g_object_set (gtk_settings_get_default (), "gtk-xft-rgba", value, NULL);
     g_free (value);
 
+    #ifdef AT_SPI_COMMAND
+    spawn_line_pid (AT_SPI_COMMAND, G_SPAWN_SEARCH_PATH, NULL);
+    #endif
+
+    #ifdef INDICATOR_SERVICES_COMMAND
+    spawn_line_pid (INDICATOR_SERVICES_COMMAND, G_SPAWN_SEARCH_PATH, NULL);
+    #endif
 
     builder = gtk_builder_new ();
     if (!gtk_builder_add_from_string (builder, lightdm_gtk_greeter_ui,
@@ -2865,30 +2930,16 @@
     }
 
     value = g_key_file_get_value (config, "greeter", "keyboard", NULL);
-    a11y_keyboard_command = menu_command_parse_extended (value, keyboard_menuitem, "onboard", "--xid");
+    a11y_keyboard_command = menu_command_parse_extended ("keyboard", value, keyboard_menuitem, "onboard", "--xid");
     g_free (value);
 
     gtk_widget_set_visible (keyboard_menuitem, a11y_keyboard_command != NULL);
 
     value = g_key_file_get_value (config, "greeter", "reader", NULL);
-    a11y_reader_command = menu_command_parse (value, reader_menuitem);
+    a11y_reader_command = menu_command_parse ("reader", value, reader_menuitem);
     gtk_widget_set_visible (reader_menuitem, a11y_reader_command != NULL);
     g_free (value);
 
-    if (a11y_keyboard_command || a11y_reader_command)
-    {
-        gchar *AT_SPI_CMD[] = {"/usr/lib/at-spi2-core/at-spi-bus-launcher", "--launch-immediately", NULL};
-        GPid pid = 0;
-        if (g_spawn_async (NULL, AT_SPI_CMD, NULL, G_SPAWN_SEARCH_PATH, NULL, NULL, &pid, &error))
-        {
-            g_debug ("[Greeter] AT-SPI launched");
-            pids_to_close = g_slist_prepend (pids_to_close, GINT_TO_POINTER (pid));
-        }
-        else
-            g_warning ("[Greeter] Failed to launch AT-SPI: %s", error->message);
-        g_clear_error (&error);
-    }
-
     /* Power menu */
     if (gtk_widget_get_visible (power_menuitem))
     {