Make bell and libstartup-notification bits work without X11

https://bugzilla.gnome.org/show_bug.cgi?id=759538

src/core/bell.c

@@ -52,13 +52,73 @@
 #include "window-private.h"
 #include "util-private.h"
 #include "compositor/compositor-private.h"
-#include "x11/meta-x11-display-private.h"
-#include <meta/prefs.h>
 #include <meta/compositor.h>
 #ifdef HAVE_LIBCANBERRA
 #include <canberra-gtk.h>
 #endif
 
+G_DEFINE_TYPE (MetaBell, meta_bell, G_TYPE_OBJECT)
+
+enum
+{
+  IS_AUDIBLE_CHANGED,
+  LAST_SIGNAL
+};
+
+static guint bell_signals [LAST_SIGNAL] = { 0 };
+
+static void
+prefs_changed_callback (MetaPreference pref,
+                        gpointer       data)
+{
+  MetaBell *bell = data;
+
+  if (pref == META_PREF_AUDIBLE_BELL)
+    {
+      g_signal_emit (bell, bell_signals[IS_AUDIBLE_CHANGED], 0,
+                     meta_prefs_bell_is_audible ());
+    }
+}
+
+static void
+meta_bell_finalize (GObject *object)
+{
+  MetaBell *bell = META_BELL (object);
+
+  meta_prefs_remove_listener (prefs_changed_callback, bell);
+
+  G_OBJECT_CLASS (meta_bell_parent_class)->finalize (object);
+}
+
+static void
+meta_bell_class_init (MetaBellClass *klass)
+{
+  GObjectClass *object_class = G_OBJECT_CLASS (klass);
+
+  object_class->finalize = meta_bell_finalize;
+
+  bell_signals[IS_AUDIBLE_CHANGED] =
+    g_signal_new ("is-audible-changed",
+                  G_TYPE_FROM_CLASS (klass),
+                  G_SIGNAL_RUN_LAST,
+                  0,
+                  NULL, NULL, NULL,
+                  G_TYPE_NONE, 1,
+                  G_TYPE_BOOLEAN);
+}
+
+static void
+meta_bell_init (MetaBell *bell)
+{
+  meta_prefs_add_listener (prefs_changed_callback, bell);
+}
+
+MetaBell *
+meta_bell_new (MetaDisplay *display)
+{
+  return g_object_new (META_TYPE_BELL, NULL);
+}
+
 /**
  * bell_flash_fullscreen:
  * @display: The display the event came in on
@@ -226,72 +286,6 @@ meta_bell_notify (MetaDisplay *display,
   return TRUE;
 }
 
-void
-meta_bell_set_audible (MetaDisplay *display, gboolean audible)
-{
-  MetaX11Display *x11_display = display->x11_display;
-#ifdef HAVE_LIBCANBERRA
-  /* When we are playing sounds using libcanberra support, we handle the
-   * bell whether its an audible bell or a visible bell */
-  gboolean enable_system_bell = FALSE;
-#else
-  gboolean enable_system_bell = audible;
-#endif /* HAVE_LIBCANBERRA */
-
-  XkbChangeEnabledControls (x11_display->xdisplay,
-                            XkbUseCoreKbd,
-                            XkbAudibleBellMask,
-                            enable_system_bell ? XkbAudibleBellMask : 0);
-}
-
-gboolean
-meta_bell_init (MetaDisplay *display)
-{
-  int xkb_base_error_type, xkb_opcode;
-  MetaX11Display *x11_display = display->x11_display;
-
-  if (!XkbQueryExtension (x11_display->xdisplay, &xkb_opcode,
-			  &display->xkb_base_event_type,
-			  &xkb_base_error_type,
-			  NULL, NULL))
-    {
-      display->xkb_base_event_type = -1;
-      g_message ("could not find XKB extension.");
-      return FALSE;
-    }
-  else
-    {
-      unsigned int mask = XkbBellNotifyMask;
-      gboolean visual_bell_auto_reset = FALSE;
-      /* TRUE if and when non-broken version is available */
-      XkbSelectEvents (x11_display->xdisplay,
-		       XkbUseCoreKbd,
-		       XkbBellNotifyMask,
-		       XkbBellNotifyMask);
-      meta_bell_set_audible (display, meta_prefs_bell_is_audible ());
-      if (visual_bell_auto_reset) {
-        XkbSetAutoResetControls (x11_display->xdisplay,
-				 XkbAudibleBellMask,
-				 &mask,
-				 &mask);
-      }
-      return TRUE;
-    }
-  return FALSE;
-}
-
-void
-meta_bell_shutdown (MetaDisplay *display)
-{
-  MetaX11Display *x11_display = display->x11_display;
-
-  /* TODO: persist initial bell state in display, reset here */
-  XkbChangeEnabledControls (x11_display->xdisplay,
-			    XkbUseCoreKbd,
-			    XkbAudibleBellMask,
-			    XkbAudibleBellMask);
-}
-
 /**
  * meta_bell_notify_frame_destroy:
  * @frame: The frame which is being destroyed

src/core/bell.h

@@ -17,11 +17,19 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <X11/Xlib.h>
-#include <X11/XKBlib.h>
 #include "display-private.h"
 #include "frame.h"
 
+struct _MetaBell
+{
+  GObject parent;
+};
+
+#define META_TYPE_BELL (meta_bell_get_type ())
+G_DECLARE_FINAL_TYPE (MetaBell, meta_bell, META, BELL, GObject)
+
+MetaBell * meta_bell_new (MetaDisplay *display);
+
 /**
  * meta_bell_notify:
  * @display: The display the bell event came in on
@@ -34,48 +42,6 @@
 gboolean meta_bell_notify (MetaDisplay *display,
                            MetaWindow  *window);
 
-/**
- * meta_bell_set_audible:
- * @display: The display we're configuring
- * @audible: True for an audible bell, false for a visual bell
- *
- * Turns the bell to audible or visual. This tells X what to do, but
- * not Mutter; you will need to set the "visual bell" pref for that.
- *
- * If the configure script found we had no XKB, this is a no-op.
- */
-void meta_bell_set_audible (MetaDisplay *display, gboolean audible);
-
-/**
- * meta_bell_init:
- * @display: The display which is opening
- *
- * Initialises the bell subsystem. This involves intialising
- * XKB (which, despite being a keyboard extension, is the
- * place to look for bell notifications), then asking it
- * to send us bell notifications, and then also switching
- * off the audible bell if we're using a visual one ourselves.
- *
- * \bug There is a line of code that's never run that tells
- * XKB to reset the bell status after we quit. Bill H said
- * (<http://bugzilla.gnome.org/show_bug.cgi?id=99886#c12>)
- * that XFree86's implementation is broken so we shouldn't
- * call it, but that was in 2002. Is it working now?
- */
-gboolean meta_bell_init (MetaDisplay *display);
-
-/**
- * meta_bell_shutdown:
- * @display: The display which is closing
- *
- * Shuts down the bell subsystem.
- *
- * \bug This is never called! If we had XkbSetAutoResetControls
- * enabled in meta_bell_init(), this wouldn't be a problem, but
- * we don't.
- */
-void meta_bell_shutdown (MetaDisplay *display);
-
 /**
  * meta_bell_notify_frame_destroy:
  * @frame: The frame which is being destroyed

src/core/display-private.h

@@ -48,6 +48,7 @@
 
 #include <X11/extensions/sync.h>
 
+typedef struct _MetaBell       MetaBell;
 typedef struct _MetaStack      MetaStack;
 typedef struct _MetaUISlave    MetaUISlave;
 
@@ -205,9 +206,6 @@ struct _MetaDisplay
    * to avoid some race conditions on EnterNotify events
    */
   int         sentinel_counter;
-
-  int         xkb_base_event_type;
-  guint32     last_bell_time;
   int	      grab_resize_timeout_id;
 
   MetaKeyBindingManager key_binding_manager;
@@ -254,6 +252,8 @@ struct _MetaDisplay
   MetaDisplayCorner starting_corner;
   guint vertical_workspaces : 1;
   guint workspace_layout_overridden : 1;
+
+  MetaBell *bell;
 };
 
 struct _MetaDisplayClass

src/core/display.c

@@ -765,7 +765,6 @@ meta_display_open (void)
 
   display->grab_resize_timeout_id = 0;
   display->grab_have_keyboard = FALSE;
-  display->last_bell_time = 0;
 
   display->grab_op = META_GRAB_OP_NONE;
   display->grab_window = NULL;
@@ -804,7 +803,6 @@ meta_display_open (void)
 
   meta_display_set_cursor (display, META_CURSOR_DEFAULT);
 
-
   /* This is the default layout extracted from default
    * variable values in update_num_workspaces ()
    * This can be overriden using _NET_DESKTOP_LAYOUT in
@@ -824,6 +822,12 @@ meta_display_open (void)
 
   reload_logical_monitors (display);
 
+  display->startup_notification = meta_startup_notification_get (display);
+  g_signal_connect (display->startup_notification, "changed",
+                    G_CALLBACK (on_startup_notification_changed), display);
+
+  display->bell = meta_bell_new (display);
+
   x11_display = meta_x11_display_new (display, &error);
   g_assert (x11_display != NULL); /* Required, for now */
   display->x11_display = x11_display;
@@ -834,8 +838,6 @@ meta_display_open (void)
   display->stack = meta_stack_new (display);
   display->stack_tracker = meta_stack_tracker_new (display);
 
-  meta_bell_init (display);
-
   display->last_focus_time = timestamp;
   display->last_user_time = timestamp;
   display->compositor = NULL;
@@ -846,10 +848,6 @@ meta_display_open (void)
                           display->x11_display->atom__NET_ACTIVE_WINDOW,
                           &old_active_xwindow);
 
-  display->startup_notification = meta_startup_notification_get (display);
-  g_signal_connect (display->startup_notification, "changed",
-                    G_CALLBACK (on_startup_notification_changed), display);
-
   enable_compositor (display);
 
   if (display->x11_display)
@@ -1015,7 +1013,6 @@ meta_display_close (MetaDisplay *display,
 
   meta_display_remove_autoraise_callback (display);
 
-  g_clear_object (&display->startup_notification);
   g_clear_object (&display->gesture_tracker);
 
   g_clear_pointer (&display->stack, (GDestroyNotify) meta_stack_free);
@@ -1056,6 +1053,9 @@ meta_display_close (MetaDisplay *display,
 
   meta_display_shutdown_keys (display);
 
+  g_clear_object (&display->bell);
+  g_clear_object (&display->startup_notification);
+
   g_object_unref (display);
   the_display = NULL;
 
@@ -2564,12 +2564,8 @@ prefs_changed_callback (MetaPreference pref,
 {
   MetaDisplay *display = data;
 
-  if (pref == META_PREF_AUDIBLE_BELL)
+  if (pref == META_PREF_CURSOR_THEME ||
-    {
+      pref == META_PREF_CURSOR_SIZE)
-      meta_bell_set_audible (display, meta_prefs_bell_is_audible ());
-    }
-  else if (pref == META_PREF_CURSOR_THEME ||
-           pref == META_PREF_CURSOR_SIZE)
     {
       meta_display_reload_cursor (display);
     }

src/core/startup-notification.c

@@ -528,11 +528,6 @@ meta_startup_notification_finalize (GObject *object)
 {
   MetaStartupNotification *sn = META_STARTUP_NOTIFICATION (object);
 
-#ifdef HAVE_STARTUP_NOTIFICATION
-  sn_monitor_context_unref (sn->sn_context);
-  sn_display_unref (sn->sn_display);
-#endif
-
   if (sn->startup_sequence_timeout)
     g_source_remove (sn->startup_sequence_timeout);
 
@@ -660,6 +655,33 @@ meta_startup_notification_sn_event (SnMonitorEvent *event,
 
   sn_startup_sequence_unref (sequence);
 }
+
+static void
+on_x11_display_opened (MetaStartupNotification *sn)
+{
+  MetaX11Display *x11_display = sn->display->x11_display;
+
+  sn->sn_display = sn_display_new (x11_display->xdisplay,
+                                   sn_error_trap_push,
+                                   sn_error_trap_pop);
+
+  sn->sn_context =
+    sn_monitor_context_new (sn->sn_display,
+                            meta_x11_display_get_screen_number (x11_display),
+                            meta_startup_notification_sn_event,
+                            sn,
+                            NULL);
+}
+
+static void
+on_x11_display_closing (MetaStartupNotification *sn)
+{
+  sn_monitor_context_unref (sn->sn_context);
+  sn->sn_context = NULL;
+
+  sn_display_unref (sn->sn_display);
+  sn->sn_display = NULL;
+}
 #endif
 
 static void
@@ -670,16 +692,22 @@ meta_startup_notification_constructed (GObject *object)
   g_assert (sn->display != NULL);
 
 #ifdef HAVE_STARTUP_NOTIFICATION
-  sn->sn_display = sn_display_new (sn->display->x11_display->xdisplay,
+  sn->sn_display = NULL;
-                                   sn_error_trap_push,
+  sn->sn_context = NULL;
-                                   sn_error_trap_pop);
+
-  sn->sn_context =
+  g_signal_connect_object (sn->display,
-    sn_monitor_context_new (sn->sn_display,
+                          "x11-display-opened",
-                            meta_ui_get_screen_number (),
+                           G_CALLBACK (on_x11_display_opened),
-                            meta_startup_notification_sn_event,
+                           sn,
-                            sn,
+                           G_CONNECT_SWAPPED);
-                            NULL);
+
+  g_signal_connect_object (sn->display,
+                           "x11-display-closing",
+                           G_CALLBACK (on_x11_display_closing),
+                           sn,
+                           G_CONNECT_SWAPPED);
 #endif
+
   sn->startup_sequences = NULL;
   sn->startup_sequence_timeout = 0;
 }
@@ -753,6 +781,9 @@ meta_startup_notification_get_sequences (MetaStartupNotification *sn)
 #ifdef HAVE_STARTUP_NOTIFICATION
   GSList *l;
 
+  if (!sn->sn_display)
+    return sequences;
+
   /* We return a list of SnStartupSequences here */
   for (l = sn->startup_sequences; l; l = l->next)
     {

src/x11/events.c

@@ -24,6 +24,7 @@
 #include "x11/events.h"
 
 #include <X11/Xatom.h>
+#include <X11/XKBlib.h>
 #include <X11/extensions/Xdamage.h>
 #include <X11/extensions/shape.h>
 
@@ -1166,23 +1167,24 @@ process_selection_clear (MetaX11Display *x11_display,
 }
 
 static void
-notify_bell (MetaDisplay *display,
+notify_bell (MetaX11Display *x11_display,
-             XkbAnyEvent *xkb_ev)
+             XkbAnyEvent    *xkb_ev)
 {
+  MetaDisplay *display = x11_display->display;
   XkbBellNotifyEvent *xkb_bell_event = (XkbBellNotifyEvent*) xkb_ev;
   MetaWindow *window;
 
-  window = meta_x11_display_lookup_x_window (display->x11_display,
+  window = meta_x11_display_lookup_x_window (x11_display,
                                              xkb_bell_event->window);
   if (!window && display->focus_window && display->focus_window->frame)
     window = display->focus_window;
 
-  display->last_bell_time = xkb_ev->time;
+  x11_display->last_bell_time = xkb_ev->time;
   if (!meta_bell_notify (display, window) &&
       meta_prefs_bell_is_audible ())
     {
       /* Force a classic bell if the libcanberra bell failed. */
-      XkbForceDeviceBell (display->x11_display->xdisplay,
+      XkbForceDeviceBell (x11_display->xdisplay,
                           xkb_bell_event->device,
                           xkb_bell_event->bell_class,
                           xkb_bell_event->bell_id,
@@ -1656,17 +1658,17 @@ handle_other_xevent (MetaX11Display *x11_display,
       }
       break;
     default:
-      if (event->type == display->xkb_base_event_type)
+      if (event->type == x11_display->xkb_base_event_type)
         {
           XkbAnyEvent *xkb_ev = (XkbAnyEvent *) event;
 
           switch (xkb_ev->xkb_type)
             {
             case XkbBellNotify:
-              if (XSERVER_TIME_IS_BEFORE(display->last_bell_time,
+              if (XSERVER_TIME_IS_BEFORE(x11_display->last_bell_time,
                                          xkb_ev->time - 100))
                 {
-                  notify_bell (display, xkb_ev);
+                  notify_bell (x11_display, xkb_ev);
                 }
               break;
             default:

src/x11/meta-x11-display-private.h

@@ -110,6 +110,9 @@ struct _MetaX11Display
   /* Managed by group-props.c */
   MetaGroupPropHooks *group_prop_hooks;
 
+  int xkb_base_event_type;
+  guint32 last_bell_time;
+
   MetaAlarmFilter alarm_filter;
   gpointer alarm_filter_data;
 

src/x11/meta-x11-display.c

@@ -38,6 +38,7 @@
 #include <unistd.h>
 
 #include <X11/Xatom.h>
+#include <X11/XKBlib.h>
 #ifdef HAVE_RANDR
 #include <X11/extensions/Xrandr.h>
 #endif
@@ -402,6 +403,97 @@ query_xi_extension (MetaX11Display *x11_display)
     meta_fatal ("X server doesn't have the XInput extension, version 2.2 or newer\n");
 }
 
+/*
+ * Initialises the bell subsystem. This involves intialising
+ * XKB (which, despite being a keyboard extension, is the
+ * place to look for bell notifications), then asking it
+ * to send us bell notifications, and then also switching
+ * off the audible bell if we're using a visual one ourselves.
+ *
+ * \bug There is a line of code that's never run that tells
+ * XKB to reset the bell status after we quit. Bill H said
+ * (<http://bugzilla.gnome.org/show_bug.cgi?id=99886#c12>)
+ * that XFree86's implementation is broken so we shouldn't
+ * call it, but that was in 2002. Is it working now?
+ */
+static void
+init_x11_bell (MetaX11Display *x11_display)
+{
+  int xkb_base_error_type, xkb_opcode;
+
+  if (!XkbQueryExtension (x11_display->xdisplay, &xkb_opcode,
+                          &x11_display->xkb_base_event_type,
+                          &xkb_base_error_type,
+                          NULL, NULL))
+    {
+      x11_display->xkb_base_event_type = -1;
+      meta_warning ("could not find XKB extension.");
+    }
+  else
+    {
+      unsigned int mask = XkbBellNotifyMask;
+      gboolean visual_bell_auto_reset = FALSE;
+      /* TRUE if and when non-broken version is available */
+      XkbSelectEvents (x11_display->xdisplay,
+                       XkbUseCoreKbd,
+                       XkbBellNotifyMask,
+                       XkbBellNotifyMask);
+
+      if (visual_bell_auto_reset)
+        {
+          XkbSetAutoResetControls (x11_display->xdisplay,
+                                   XkbAudibleBellMask,
+                                   &mask,
+                                   &mask);
+        }
+    }
+}
+
+/*
+ * \bug This is never called! If we had XkbSetAutoResetControls
+ * enabled in meta_x11_bell_init(), this wouldn't be a problem,
+ * but we don't.
+ */
+G_GNUC_UNUSED static void
+shutdown_x11_bell (MetaX11Display *x11_display)
+{
+  /* TODO: persist initial bell state in display, reset here */
+  XkbChangeEnabledControls (x11_display->xdisplay,
+                            XkbUseCoreKbd,
+                            XkbAudibleBellMask,
+                            XkbAudibleBellMask);
+}
+
+/*
+ * Turns the bell to audible or visual. This tells X what to do, but
+ * not Mutter; you will need to set the "visual bell" pref for that.
+ */
+static void
+set_x11_bell_is_audible (MetaX11Display *x11_display,
+                         gboolean is_audible)
+{
+#ifdef HAVE_LIBCANBERRA
+  /* When we are playing sounds using libcanberra support, we handle the
+   * bell whether its an audible bell or a visible bell */
+  gboolean enable_system_bell = FALSE;
+#else
+  gboolean enable_system_bell = is_audible;
+#endif /* HAVE_LIBCANBERRA */
+
+  XkbChangeEnabledControls (x11_display->xdisplay,
+                            XkbUseCoreKbd,
+                            XkbAudibleBellMask,
+                            enable_system_bell ? XkbAudibleBellMask : 0);
+}
+
+static void
+on_is_audible_changed (MetaBell       *bell,
+                       gboolean        is_audible,
+                       MetaX11Display *x11_display)
+{
+  set_x11_bell_is_audible (x11_display, is_audible);
+}
+
 static void
 set_desktop_geometry_hint (MetaX11Display *x11_display)
 {
@@ -957,6 +1049,7 @@ meta_x11_display_new (MetaDisplay *display, GError **error)
   x11_display->timestamp_pinging_window = None;
   x11_display->wm_sn_selection_window = None;
 
+  x11_display->last_bell_time = 0;
   x11_display->focus_serial = 0;
   x11_display->server_focus_window = None;
   x11_display->server_focus_serial = 0;
@@ -1105,6 +1198,14 @@ meta_x11_display_new (MetaDisplay *display, GError **error)
 
   meta_prefs_add_listener (prefs_changed_callback, x11_display);
 
+  init_x11_bell (x11_display);
+
+  g_signal_connect_object (display->bell, "is-audible-changed",
+                           G_CALLBACK (on_is_audible_changed),
+                           x11_display, 0);
+
+  set_x11_bell_is_audible (x11_display, meta_prefs_bell_is_audible ());
+
   return x11_display;
 }