x11: Drop usage of libstartup-notification

And replace with our own XClientMessageEvent handling. It has the advantage
that we can mix Wayland and X11 startup notifications without always going
through X11 so that libstartup-notification is able to get the full picture.
This will fare much better on no/intermittent X11 availability.

A second advantage is the removed dependency, and that it seems to result
in less code (yay abstractions!).

https://gitlab.gnome.org/GNOME/mutter/merge_requests/540

config.h.meson

@@ -46,9 +46,6 @@
 /* Building with SM support */
 #mesondefine HAVE_SM
 
-/* Building with startup notification support */
-#mesondefine HAVE_STARTUP_NOTIFICATION
-
 /* Path to Xwayland executable */
 #mesondefine XWAYLAND_PATH
 

meson.build

@@ -225,12 +225,6 @@ if have_pango_ft2
   pangoft2_dep = dependency('pangoft2')
 endif
 
-have_startup_notification = get_option('startup_notification')
-if have_startup_notification
-  libstartup_notification_dep = dependency('libstartup-notification-1.0',
-                                           version: libstartup_notification_req)
-endif
-
 have_remote_desktop = get_option('remote_desktop')
 if have_remote_desktop
   libpipewire_dep = dependency('libpipewire-0.2', version: libpipewire_req)
@@ -334,7 +328,6 @@ cdata.set('HAVE_WAYLAND_EGLSTREAM', have_wayland_eglstream)
 cdata.set('HAVE_LIBGUDEV', have_libgudev)
 cdata.set('HAVE_LIBWACOM', have_libwacom)
 cdata.set('HAVE_SM', have_sm)
-cdata.set('HAVE_STARTUP_NOTIFICATION', have_startup_notification)
 cdata.set('HAVE_INTROSPECTION', have_introspection)
 
 xkb_base = xkeyboard_config_dep.get_pkgconfig_variable('xkb_base')
@@ -399,7 +392,6 @@ output = [
   '        gudev.................... ' + have_libgudev.to_string(),
   '        Wacom.................... ' + have_libwacom.to_string(),
   '        SM....................... ' + have_sm.to_string(),
-  '        Startup notification..... ' + have_startup_notification.to_string(),
   '        Introspection............ ' + have_introspection.to_string(),
   '',
   '    Tests:',

src/core/display-private.h

@@ -31,10 +31,6 @@
 #include <X11/extensions/sync.h>
 #include <X11/Xlib.h>
 
-#ifdef HAVE_STARTUP_NOTIFICATION
-#include <libsn/sn.h>
-#endif
-
 #include "clutter/clutter.h"
 #include "core/keybindings-private.h"
 #include "core/meta-gesture-tracker-private.h"

src/core/main.c

@@ -118,11 +118,6 @@ static void prefs_changed_callback (MetaPreference pref,
 static void
 meta_print_compilation_info (void)
 {
-#ifdef HAVE_STARTUP_NOTIFICATION
-  meta_verbose ("Compiled with startup notification\n");
-#else
-  meta_verbose ("Compiled without startup notification\n");
-#endif
 }
 
 /**

src/core/meta-launch-context.c

@@ -125,27 +125,12 @@ meta_launch_context_get_startup_notify_id (GAppLaunchContext *launch_context,
                                            GList             *files)
 {
   MetaLaunchContext *context = META_LAUNCH_CONTEXT (launch_context);
-  MetaDisplay *display = context->display;
   int workspace_idx = -1;
   char *startup_id = NULL;
 
   if (context->workspace)
     workspace_idx = meta_workspace_index (context->workspace);
 
-  if (display->x11_display)
-    {
-      /* If there is a X11 display, we prefer going entirely through
-       * libsn, as SnMonitor expects to keep a view of the full lifetime
-       * of the startup sequence. We can't avoid it when launching and
-       * expect that a "remove" message from a X11 client will be handled.
-       */
-      startup_id =
-        meta_x11_startup_notification_launch (display->x11_display,
-                                              info,
-                                              context->timestamp,
-                                              workspace_idx);
-    }
-
   if (!startup_id)
     {
       const char *application_id = NULL;

src/meson.build

@@ -55,12 +55,6 @@ if have_libgudev
   ]
 endif
 
-if have_startup_notification
-  mutter_pkg_private_deps += [
-    libstartup_notification_dep,
-  ]
-endif
-
 if have_libwacom
   mutter_pkg_private_deps += [
     libwacom_dep,

src/x11/meta-startup-notification-x11.c

@@ -26,266 +26,301 @@
 #include "meta/meta-x11-errors.h"
 #include "x11/meta-x11-display-private.h"
 
-#ifdef HAVE_STARTUP_NOTIFICATION
+#define MAX_MESSAGE_LENGTH 4096
+#define CLIENT_MESSAGE_DATA_LENGTH 20
 
 enum
 {
-  PROP_SEQ_X11_0,
+  MESSAGE_TYPE_NEW,
-  PROP_SEQ_X11_SEQ,
+  MESSAGE_TYPE_REMOVE,
-  N_SEQ_X11_PROPS
-};
-
-struct _MetaStartupSequenceX11
-{
-  MetaStartupSequence parent_instance;
-  SnStartupSequence *seq;
 };
 
 struct _MetaX11StartupNotification
 {
-  SnDisplay *sn_display;
+  Atom atom_net_startup_info_begin;
-  SnMonitorContext *sn_context;
+  Atom atom_net_startup_info;
+  GHashTable *messages;
+  MetaX11Display *x11_display;
 };
 
-static GParamSpec *seq_x11_props[N_SEQ_X11_PROPS];
+typedef struct
-
-G_DEFINE_TYPE (MetaStartupSequenceX11,
-               meta_startup_sequence_x11,
-               META_TYPE_STARTUP_SEQUENCE)
-
-static void
-meta_startup_sequence_x11_complete (MetaStartupSequence *seq)
 {
-  MetaStartupSequenceX11 *seq_x11;
+  Window xwindow;
+  GString *data;
+} StartupMessage;
 
-  seq_x11 = META_STARTUP_SEQUENCE_X11 (seq);
+
-  sn_startup_sequence_complete (seq_x11->seq);
+static StartupMessage *
+startup_message_new (Window window)
+{
+  StartupMessage *message;
+
+  message = g_new0 (StartupMessage, 1);
+  message->xwindow = window;
+  message->data = g_string_new (NULL);
+
+  return message;
 }
 
 static void
-meta_startup_sequence_x11_finalize (GObject *object)
+startup_message_free (StartupMessage *message)
 {
-  MetaStartupSequenceX11 *seq_x11;
+  g_string_free (message->data, TRUE);
-
+  g_free (message);
-  seq_x11 = META_STARTUP_SEQUENCE_X11 (object);
-  sn_startup_sequence_unref (seq_x11->seq);
-
-  G_OBJECT_CLASS (meta_startup_sequence_x11_parent_class)->finalize (object);
 }
 
 static void
-meta_startup_sequence_x11_set_property (GObject      *object,
+skip_whitespace (const gchar **str)
-                                        guint         prop_id,
-                                        const GValue *value,
-                                        GParamSpec   *pspec)
 {
-  MetaStartupSequenceX11 *seq_x11;
+  while ((*str)[0] == ' ')
+    (*str)++;
+}
 
-  seq_x11 = META_STARTUP_SEQUENCE_X11 (object);
+static gchar *
-
+parse_key (const gchar **str)
-  switch (prop_id)
 {
-    case PROP_SEQ_X11_SEQ:
+  const gchar *start = *str;
-      seq_x11->seq = g_value_get_pointer (value);
+
-      sn_startup_sequence_ref (seq_x11->seq);
+  while (*str[0] != '\0' && *str[0] != '=')
+    (*str)++;
+
+  if (start == *str)
+    return NULL;
+
+  return g_strndup (start, *str - start);
+}
+
+static gchar *
+parse_value (const gchar **str)
+{
+  const gchar *end;
+  gboolean escaped = FALSE, quoted = FALSE;
+  GString *value;
+
+  end = *str;
+  value = g_string_new (NULL);
+
+  while (end[0] != '\0')
+    {
+      if (escaped)
+        {
+          g_string_append_c (value, end[0]);
+          escaped = FALSE;
+        }
+      else
+        {
+          if (!quoted && end[0] == ' ')
             break;
-    default:
+          else if (end[0] == '"')
-      G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+            quoted = !quoted;
-      break;
+          else if (end[0] == '\\')
-    }
+            escaped = TRUE;
+          else
+            g_string_append_c (value, end[0]);
         }
 
-static void
+      end++;
-meta_startup_sequence_x11_get_property (GObject    *object,
-                                        guint       prop_id,
-                                        GValue     *value,
-                                        GParamSpec *pspec)
-{
-  MetaStartupSequenceX11 *seq_x11;
-
-  seq_x11 = META_STARTUP_SEQUENCE_X11 (object);
-
-  switch (prop_id)
-    {
-    case PROP_SEQ_X11_SEQ:
-      g_value_set_pointer (value, seq_x11->seq);
-      break;
-    default:
-      G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
-      break;
-    }
     }
 
-static void
+  *str = end;
-meta_startup_sequence_x11_init (MetaStartupSequenceX11 *seq)
+
+  if (value->len == 0)
     {
+      g_string_free (value, TRUE);
+      return NULL;
     }
 
-static void
+  return g_string_free (value, FALSE);
-meta_startup_sequence_x11_class_init (MetaStartupSequenceX11Class *klass)
-{
-  MetaStartupSequenceClass *seq_class;
-  GObjectClass *object_class;
-
-  seq_class = META_STARTUP_SEQUENCE_CLASS (klass);
-  seq_class->complete = meta_startup_sequence_x11_complete;
-
-  object_class = G_OBJECT_CLASS (klass);
-  object_class->finalize = meta_startup_sequence_x11_finalize;
-  object_class->set_property = meta_startup_sequence_x11_set_property;
-  object_class->get_property = meta_startup_sequence_x11_get_property;
-
-  seq_x11_props[PROP_SEQ_X11_SEQ] =
-    g_param_spec_pointer ("seq",
-                          "Sequence",
-                          "Sequence",
-                          G_PARAM_READWRITE |
-                          G_PARAM_CONSTRUCT_ONLY);
-
-  g_object_class_install_properties (object_class, N_SEQ_X11_PROPS,
-                                     seq_x11_props);
 }
 
-static MetaStartupSequence *
+static gboolean
-meta_startup_sequence_x11_new (SnStartupSequence *seq)
+startup_message_parse (StartupMessage  *message,
+                       int             *type,
+                       gchar          **id,
+                       GHashTable     **data)
 {
-  gint64 timestamp;
+  const gchar *str = message->data->str;
 
-  timestamp = sn_startup_sequence_get_timestamp (seq);
+  if (strncmp (str, "new:", 4) == 0)
-  return g_object_new (META_TYPE_STARTUP_SEQUENCE_X11,
+    {
-                       "id", sn_startup_sequence_get_id (seq),
+      *type = MESSAGE_TYPE_NEW;
-                       "icon-name", sn_startup_sequence_get_icon_name (seq),
+      str += 4;
-                       "application-id", sn_startup_sequence_get_application_id (seq),
+    }
-                       "wmclass", sn_startup_sequence_get_wmclass (seq),
+  else if (strncmp (str, "remove:", 7) == 0)
-                       "name", sn_startup_sequence_get_name (seq),
+    {
-                       "workspace", sn_startup_sequence_get_workspace (seq),
+      *type = MESSAGE_TYPE_REMOVE;
-                       "timestamp", timestamp,
+      str += 7;
-                       "seq", seq,
+    }
-                       NULL);
+  else
+    {
+      return FALSE;
     }
 
-static void
+  *data = g_hash_table_new_full (g_str_hash,
-sn_error_trap_push (SnDisplay *sn_display,
+                                 g_str_equal,
-                    Display   *xdisplay)
+                                 g_free,
+                                 g_free);
+  while (str[0])
     {
-  MetaDisplay *display;
+      gchar *key, *value;
 
-  display = meta_display_for_x_display (xdisplay);
+      skip_whitespace (&str);
-  if (display != NULL)
+      key = parse_key (&str);
-    meta_x11_error_trap_push (display->x11_display);
+      if (!key)
-}
-
-static void
-sn_error_trap_pop (SnDisplay *sn_display,
-                   Display   *xdisplay)
-{
-  MetaDisplay *display;
-
-  display = meta_display_for_x_display (xdisplay);
-  if (display != NULL)
-    meta_x11_error_trap_pop (display->x11_display);
-}
-
-static void
-meta_startup_notification_sn_event (SnMonitorEvent *event,
-                                    void           *user_data)
-{
-  MetaX11Display *x11_display = user_data;
-  MetaStartupNotification *sn = x11_display->display->startup_notification;
-  MetaStartupSequence *seq;
-  SnStartupSequence *sequence;
-
-  sequence = sn_monitor_event_get_startup_sequence (event);
-
-  sn_startup_sequence_ref (sequence);
-
-  switch (sn_monitor_event_get_type (event))
-    {
-    case SN_MONITOR_EVENT_INITIATED:
-      {
-        const char *wmclass;
-
-        wmclass = sn_startup_sequence_get_wmclass (sequence);
-
-        meta_topic (META_DEBUG_STARTUP,
-                    "Received startup initiated for %s wmclass %s\n",
-                    sn_startup_sequence_get_id (sequence),
-                    wmclass ? wmclass : "(unset)");
-
-        seq = meta_startup_sequence_x11_new (sequence);
-        meta_startup_notification_add_sequence (sn, seq);
-        g_object_unref (seq);
-      }
         break;
 
-    case SN_MONITOR_EVENT_COMPLETED:
+      str++;
+      value = parse_value (&str);
+
+      if (!value)
         {
-        meta_topic (META_DEBUG_STARTUP,
+          g_free (key);
-                    "Received startup completed for %s\n",
-                    sn_startup_sequence_get_id (sequence));
-
-        seq = meta_startup_notification_lookup_sequence (sn, sn_startup_sequence_get_id (sequence));
-        if (seq)
-          {
-            meta_startup_sequence_complete (seq);
-            meta_startup_notification_remove_sequence (sn, seq);
-          }
-      }
-      break;
-
-    case SN_MONITOR_EVENT_CHANGED:
-      meta_topic (META_DEBUG_STARTUP,
-                  "Received startup changed for %s\n",
-                  sn_startup_sequence_get_id (sequence));
-      break;
-
-    case SN_MONITOR_EVENT_CANCELED:
-      meta_topic (META_DEBUG_STARTUP,
-                  "Received startup canceled for %s\n",
-                  sn_startup_sequence_get_id (sequence));
           break;
         }
 
-  sn_startup_sequence_unref (sequence);
+      g_hash_table_insert (*data, key, value);
+    }
+
+  *id = g_strdup (g_hash_table_lookup (*data, "ID"));
+
+  return TRUE;
+}
+
+static gboolean
+startup_message_add_data (StartupMessage *message,
+                          const gchar    *data)
+{
+  int len;
+
+  len = strnlen (data, CLIENT_MESSAGE_DATA_LENGTH);
+  g_string_append_len (message->data, data, len);
+
+  return (message->data->len > MAX_MESSAGE_LENGTH ||
+          len < CLIENT_MESSAGE_DATA_LENGTH);
 }
-#endif
 
 void
 meta_x11_startup_notification_init (MetaX11Display *x11_display)
 {
-#ifdef HAVE_STARTUP_NOTIFICATION
   MetaX11StartupNotification *x11_sn;
 
   x11_sn = g_new0 (MetaX11StartupNotification, 1);
-  x11_sn->sn_display = sn_display_new (x11_display->xdisplay,
+  x11_sn->atom_net_startup_info_begin = XInternAtom (x11_display->xdisplay,
-                                            sn_error_trap_push,
+                                                     "_NET_STARTUP_INFO_BEGIN",
-                                            sn_error_trap_pop);
+                                                     False);
-  x11_sn->sn_context =
+  x11_sn->atom_net_startup_info = XInternAtom (x11_display->xdisplay,
-    sn_monitor_context_new (x11_sn->sn_display,
+                                               "_NET_STARTUP_INFO",
-                            meta_x11_display_get_screen_number (x11_display),
+                                               False);
-                            meta_startup_notification_sn_event,
+  x11_sn->messages = g_hash_table_new_full (NULL, NULL, NULL,
-                            x11_display,
+                                            (GDestroyNotify) startup_message_free);
-                            NULL);
+  x11_sn->x11_display = x11_display;
-
   x11_display->startup_notification = x11_sn;
-#endif
 }
 
 void
 meta_x11_startup_notification_release (MetaX11Display *x11_display)
 {
-#ifdef HAVE_STARTUP_NOTIFICATION
   MetaX11StartupNotification *x11_sn = x11_display->startup_notification;
 
   x11_display->startup_notification = NULL;
 
   if (x11_sn)
     {
-      sn_monitor_context_unref (x11_sn->sn_context);
+      g_hash_table_unref (x11_sn->messages);
-      sn_display_unref (x11_sn->sn_display);
       g_free (x11_sn);
     }
-#endif
+}
+
+static void
+handle_message (MetaX11StartupNotification *x11_sn,
+                StartupMessage             *message)
+{
+  MetaStartupNotification *sn = x11_sn->x11_display->display->startup_notification;
+  MetaStartupSequence *seq;
+  GHashTable *data;
+  char *id;
+  int type;
+
+  if (message->data->len <= MAX_MESSAGE_LENGTH &&
+      g_utf8_validate (message->data->str, -1, NULL) &&
+      startup_message_parse (message, &type, &id, &data))
+    {
+      if (type == MESSAGE_TYPE_NEW)
+        {
+          uint64_t timestamp = 0;
+          int workspace = -1;
+
+          if (g_hash_table_contains (data, "DESKTOP"))
+            workspace = atoi (g_hash_table_lookup (data, "DESKTOP"));
+          if (g_hash_table_contains (data, "TIMESTAMP"))
+            timestamp = g_ascii_strtoull (g_hash_table_lookup (data, "TIMESTAMP"), NULL, 10);
+
+          seq = g_object_new (META_TYPE_STARTUP_SEQUENCE,
+                              "id", id,
+                              "icon-name", g_hash_table_lookup (data, "ICON_NAME"),
+                              "application-id", g_hash_table_lookup (data, "APPLICATION_ID"),
+                              "wmclass", g_hash_table_lookup (data, "WMCLASS"),
+                              "name", g_hash_table_lookup (data, "NAME"),
+                              "workspace", workspace,
+                              "timestamp", timestamp,
+                              NULL);
+
+          meta_topic (META_DEBUG_STARTUP,
+                      "Received startup initiated for %s wmclass %s\n",
+                      id, (gchar*) g_hash_table_lookup (data, "WMCLASS"));
+
+          meta_startup_notification_add_sequence (sn, seq);
+          g_object_unref (seq);
+        }
+      else if (type == MESSAGE_TYPE_REMOVE)
+        {
+          meta_topic (META_DEBUG_STARTUP,
+                      "Received startup completed for %s\n", id);
+          seq = meta_startup_notification_lookup_sequence (sn, id);
+
+          if (seq)
+            {
+              meta_startup_sequence_complete (seq);
+              meta_startup_notification_remove_sequence (sn, seq);
+            }
+        }
+
+      g_hash_table_unref (data);
+      g_free (id);
+    }
+
+  g_hash_table_remove (x11_sn->messages, GINT_TO_POINTER (message->xwindow));
+}
+
+static gboolean
+handle_startup_notification_event (MetaX11StartupNotification *x11_sn,
+                                   XClientMessageEvent        *client_event)
+{
+  StartupMessage *message;
+
+  if (client_event->message_type == x11_sn->atom_net_startup_info_begin)
+    {
+      message = startup_message_new (client_event->window);
+      g_hash_table_insert (x11_sn->messages,
+                           GINT_TO_POINTER (client_event->window),
+                           message);
+      if (startup_message_add_data (message, client_event->data.b))
+        handle_message (x11_sn, message);
+      return TRUE;
+    }
+  else if (client_event->message_type == x11_sn->atom_net_startup_info)
+    {
+      message = g_hash_table_lookup (x11_sn->messages,
+                                     GINT_TO_POINTER (client_event->window));
+      if (message)
+        {
+          if (startup_message_add_data (message, client_event->data.b))
+            handle_message (x11_sn, message);
+          return TRUE;
+        }
+    }
+
+  return FALSE;
 }
 
 gboolean
@@ -297,74 +332,8 @@ meta_x11_startup_notification_handle_xevent (MetaX11Display *x11_display,
   if (!x11_sn)
     return FALSE;
 
-  return sn_display_process_event (x11_sn->sn_display, xevent);
+  if (xevent->xany.type != ClientMessage)
-}
+    return FALSE;
-
+
-typedef void (* SetAppIdFunc) (SnLauncherContext *context,
+  return handle_startup_notification_event (x11_sn, &xevent->xclient);
-                               const char        *app_id);
-
-gchar *
-meta_x11_startup_notification_launch (MetaX11Display *x11_display,
-                                      GAppInfo       *app_info,
-                                      uint32_t        timestamp,
-                                      int             workspace)
-{
-  gchar *startup_id = NULL;
-#ifdef HAVE_STARTUP_NOTIFICATION
-  MetaX11StartupNotification *x11_sn = x11_display->startup_notification;
-  SnLauncherContext *sn_launcher;
-  int screen;
-
-  screen = meta_x11_display_get_screen_number (x11_display);
-  sn_launcher = sn_launcher_context_new (x11_sn->sn_display, screen);
-
-  sn_launcher_context_set_name (sn_launcher, g_app_info_get_name (app_info));
-  sn_launcher_context_set_workspace (sn_launcher, workspace);
-  sn_launcher_context_set_binary_name (sn_launcher,
-                                       g_app_info_get_executable (app_info));
-
-  if (G_IS_DESKTOP_APP_INFO (app_info))
-    {
-      const char *application_id;
-      SetAppIdFunc func = NULL;
-      GModule *self;
-
-      application_id =
-        g_desktop_app_info_get_filename (G_DESKTOP_APP_INFO (app_info));
-      self = g_module_open (NULL, G_MODULE_BIND_MASK);
-
-      /* This here is a terrible workaround to bypass a libsn bug that is not
-       * likely to get fixed at this point.
-       * sn_launcher_context_set_application_id is correctly defined in the
-       * sn-launcher.h file, but it's mistakenly called
-       * sn_launcher_set_application_id in the C file.
-       *
-       * We look up the symbol instead, but still prefer the correctly named
-       * function, if one were ever to be added.
-       */
-      if (!g_module_symbol (self, "sn_launcher_context_set_application_id",
-                            (gpointer *) &func))
-        {
-          g_module_symbol (self, "sn_launcher_set_application_id",
-                           (gpointer *) &func);
-        }
-
-      if (func)
-        func (sn_launcher, application_id);
-
-      g_module_close (self);
-    }
-
-  sn_launcher_context_initiate (sn_launcher,
-                                g_get_prgname (),
-                                g_app_info_get_name (app_info),
-                                timestamp);
-
-  startup_id = g_strdup (sn_launcher_context_get_startup_id (sn_launcher));
-
-  /* Fire and forget, we have a SnMonitor in addition */
-  sn_launcher_context_unref (sn_launcher);
-#endif /* HAVE_STARTUP_NOTIFICATION */
-
-  return startup_id;
 }

src/x11/meta-startup-notification-x11.h

@@ -36,9 +36,4 @@ void     meta_x11_startup_notification_release (MetaX11Display *x11_display);
 gboolean meta_x11_startup_notification_handle_xevent (MetaX11Display *x11_display,
                                                       XEvent         *xevent);
 
-gchar *  meta_x11_startup_notification_launch (MetaX11Display *x11_display,
-                                               GAppInfo       *app_info,
-                                               uint32_t        timestamp,
-                                               int             workspace);
-
 #endif /* META_X11_STARTUP_NOTIFICATION_H */