Merge lp:~albertomilone/unity-settings-daemon/non-blocking-touch-mapping-wily into lp:unity-settings-daemon

[Alberto Milone]

Make touchscreen mapping non-blocking, and kill it if it takes more than 3 seconds; finally, in case of failure, try again after a few seconds. This helps on resume from S3 (LP: #1471708).

Make touchscreen mapping non-blocking, and kill it if it takes more than 3 seconds; finally, in case of failure, try again after a few seconds. This helps on resume from S3 (LP: #1471708).

[Iain Lane (laney)]

Basic logic mostly fine (not tested), some comments inline. I hope they make sense.

Style for this file is to not include {} when not required - i.e. if there's only one statement in that branch.

review: Needs Fixing

[lp:~albertomilone/unity-settings-daemon/non-blocking-touch-mapping-wily]

4091. By Alberto Milone on 2015-08-12

Keep track of processes and retries dinamically

[Iain Lane (laney)]

I think this is fine, thanks. Just some small comments to fix. Feel free to upload once that is done.

review: Approve

[lp:~albertomilone/unity-settings-daemon/non-blocking-touch-mapping-wily]

4092. By Alberto Milone on 2015-08-25

Add whitespaces and remove duplicate message

=== modified file 'plugins/xrandr/gsd-xrandr-manager.c'
--- plugins/xrandr/gsd-xrandr-manager.c	2014-11-27 10:32:03 +0000
+++ plugins/xrandr/gsd-xrandr-manager.c	2015-08-25 08:21:49 +0000
@@ -129,6 +129,13 @@
         gchar *main_touchscreen_name;
 };
 
+typedef struct {
+        int mapping_pid;
+        guint mapping_kill_id;
+        guint mapping_retry_id;
+        gboolean mapping_killed;
+} TouchMappingPrivate;
+
 static const GsdRRRotation possible_rotations[] = {
         GSD_RR_ROTATION_0,
         GSD_RR_ROTATION_90,
@@ -157,9 +164,14 @@
 
 static FILE *log_file;
 
+/* Wait before retrying */
+static const int RETRY_TIMEOUT = 5;
+/* Wait before timing out */
+static const int MAPPING_TIMEOUT = 3;
+
 static GsdRROutput * input_info_find_size_match (GsdXrandrManager *manager, GsdRRScreen *rr_screen);
-static int map_touch_to_output (GsdRRScreen *screen, int device_id, GsdRROutputInfo *output);
-static void do_touchscreen_mapping (GsdXrandrManager *manager);
+static int map_touch_to_output (GsdXrandrManager *manager, GsdRROutputInfo *output);
+static gboolean do_touchscreen_mapping (GsdXrandrManager *manager);
 
 static void
 log_open (void)
@@ -2179,37 +2191,163 @@
         XFreeDeviceList (device_info);
 }
 
+/* Kill a mapping process */
+static gboolean
+cb_mapping_child_kill (gpointer data)
+{
+        int kill_status;
+
+        gchar *message = NULL;
+        TouchMappingPrivate *mapping_data = (TouchMappingPrivate*) data;
+
+        message = g_strdup_printf ("Killing touchscreen mapping process %d after %d second(s) timeout...",
+                                    mapping_data->mapping_pid, MAPPING_TIMEOUT);
+
+        if (!message) {
+                g_error ("Failed to allocate memory to log the killing of the mapping process");
+                goto out;
+        }
+
+        g_warning ("%s", message);
+
+        g_free (message);
+
+        kill_status = kill (mapping_data->mapping_pid, SIGTERM);
+
+        /* Mark as killed */
+        mapping_data->mapping_killed = TRUE;
+
+        g_debug ("Kill status %d...", kill_status);
+
+        if (kill_status != 0)
+                g_error ("Failed to kill mapping process: %s", strerror (errno));
+
+out:
+        return G_SOURCE_REMOVE;
+}
+
+/* Clean up spawned processes when they are done */
+static void
+cb_mapping_child_watch (GPid  pid,
+                        gint  status,
+                        gpointer data)
+{
+        TouchMappingPrivate *mapping_data = (TouchMappingPrivate*) data;
+
+        g_debug ("Cleaning up spawned mapping");
+
+        /* Close pid */
+        g_spawn_close_pid (pid);
+
+        /* No need to kill a process that ended */
+        if (mapping_data->mapping_kill_id > 0) {
+                g_debug ("Cancelling killing of process that ended");
+                g_source_remove (mapping_data->mapping_kill_id);
+        }
+
+        /* No need to retry if it succeeded */
+        if (!mapping_data->mapping_killed) {
+                g_debug ("Cancelling retry id %d", mapping_data->mapping_retry_id);
+                g_source_remove (mapping_data->mapping_retry_id);
+        }
+
+        /* Free mapping data */
+        g_slice_free (TouchMappingPrivate, mapping_data);
+        mapping_data = NULL;
+}
+
 static int
-map_touch_to_output (GsdRRScreen *screen, int device_id,
-                     GsdRROutputInfo *output)
+map_touch_to_output (GsdXrandrManager *manager, GsdRROutputInfo *output)
 {
-        int status = 0;
-        char command[100];
+        TouchMappingPrivate *mapping_data = NULL;
+        gchar *command_str = NULL;
+        GError **error = NULL;
+        gboolean success = FALSE;
+        gchar **command = NULL;
+
+        GsdXrandrManagerPrivate *priv = manager->priv;
         gchar *name = gsd_rr_output_info_get_name (output);
 
         if (!name) {
                 g_debug ("Failure to map screen with missing name");
-                status = 1;
                 goto out;
         }
 
-        if (gsd_rr_output_info_is_active(output)) {
+        if (gsd_rr_output_info_is_active (output)) {
                 g_debug ("Mapping touchscreen %d onto output %s",
-                         device_id, name);
-                sprintf (command, "xinput  --map-to-output %d %s",
-                         device_id, name);
-                status = system (command);
+                         priv->main_touchscreen_id, name);
+
+                command_str = g_strdup_printf ("/usr/bin/xinput --map-to-output %d %s",
+                                               priv->main_touchscreen_id, name);
+
+                if (!command_str)
+                        goto out;
+
+                if (!g_shell_parse_argv (command_str, NULL, &command, NULL))
+                        goto out;
+
+                /* Each spawned process gets its own mapping data */
+                mapping_data = g_slice_new (TouchMappingPrivate);
+                if (!mapping_data) {
+                        g_error ("Touchscreen mapping resource allocation failed");
+                        goto out;
+                }
+                /* Initialise mapping data */
+                mapping_data->mapping_pid = -1;
+                mapping_data->mapping_kill_id = 0;
+                mapping_data->mapping_retry_id = 0;
+                mapping_data->mapping_killed = FALSE;
+
+                success = g_spawn_async (NULL,
+                                         command,
+                                         NULL,
+                                         G_SPAWN_DO_NOT_REAP_CHILD,
+                                         NULL,
+                                         NULL,
+                                         &(mapping_data->mapping_pid),
+                                         error);
+                g_strfreev (command);
+
+                if (success) {
+                        g_debug ("Touchscreen mapping spawn succeeded");
+
+                        /* Clean up after child is done */
+                        g_child_watch_add (mapping_data->mapping_pid, (GChildWatchFunc) cb_mapping_child_watch,
+                                           mapping_data);
+                        /* Kill the child after n seconds */
+                        mapping_data->mapping_kill_id = g_timeout_add_seconds (MAPPING_TIMEOUT,
+                                                                               (GSourceFunc) cb_mapping_child_kill,
+                                                                               mapping_data);
+                        /* Set potential retry */
+                        g_debug ("Retrying in %d second(s)", RETRY_TIMEOUT+MAPPING_TIMEOUT);
+                        mapping_data->mapping_retry_id = g_timeout_add_seconds (RETRY_TIMEOUT+MAPPING_TIMEOUT,
+                                                                                (GSourceFunc) do_touchscreen_mapping,
+                                                                                manager);
+                        g_debug ("Retry id: %d", mapping_data->mapping_retry_id);
+                }
+                else {
+                        g_error ("Touchscreen mapping failed");
+                        if (error != NULL)
+                                g_error ("%s", (*error)->message);
+                        g_clear_error (error);
+
+                        /* Free mapping data */
+                        g_slice_free(TouchMappingPrivate, mapping_data);
+                        mapping_data = NULL;
+                }
         }
         else {
                 g_debug ("No need to map %d onto output %s. The output is off",
-                         device_id, name);
+                         priv->main_touchscreen_id, name);
         }
 
 out:
-        return (status == 0);
+        g_free (command_str);
+
+        return success;
 }
 
-static void
+static gboolean
 do_touchscreen_mapping (GsdXrandrManager *manager)
 {
         GsdXrandrManagerPrivate *priv = manager->priv;
@@ -2231,9 +2369,7 @@
         if (priv->main_touchscreen_id != -1) {
                 /* Set initial mapping */
                 g_debug ("Setting initial touchscreen mapping");
-                map_touch_to_output (screen,
-                                     priv->main_touchscreen_id,
-                                     laptop_output);
+                map_touch_to_output (manager, laptop_output);
         }
         else {
                 g_debug ("No main touchscreen detected");
@@ -2241,6 +2377,8 @@
 
 out:
         g_object_unref (current);
+
+        return G_SOURCE_REMOVE;
 }
 
 gboolean