gnome-settings-daemon r578 - in trunk: . plugins/common plugins/keybindings plugins/media-keys

[behdad svn gnome org]

Author: behdad
Date: Mon Nov  3 21:28:52 2008
New Revision: 578
URL: http://svn.gnome.org/viewvc/gnome-settings-daemon?rev=578&view=rev

Log:
2008-11-03  Behdad Esfahbod  <behdad gnome org>

        * plugins/common/gsd-keygrab.c (setup_modifiers), (grab_key_real),
        (grab_key_unsafe):
        * plugins/common/gsd-keygrab.h:
        * plugins/keybindings/gsd-keybindings-manager.c
        (binding_register_keys):
        * plugins/media-keys/gsd-media-keys-manager.c (update_kbd_cb),
        (init_kbd):
        Don't trap errors around grab_key (bug #559164)

        Such that we can do a single gdk_flush for multiple keys.
        The only downside is that we cannot write out in the warning
        which key is being accessed by another app.  Not that we really
        care.

Modified:
   trunk/ChangeLog
   trunk/plugins/common/gsd-keygrab.c
   trunk/plugins/common/gsd-keygrab.h
   trunk/plugins/keybindings/gsd-keybindings-manager.c
   trunk/plugins/media-keys/gsd-media-keys-manager.c

Modified: trunk/plugins/common/gsd-keygrab.c
==============================================================================
--- trunk/plugins/common/gsd-keygrab.c	(original)
+++ trunk/plugins/common/gsd-keygrab.c	Mon Nov  3 21:28:52 2008
@@ -42,7 +42,7 @@
 static GdkModifierType gsd_used_mods = 0;
 
 static void
-setup_modifiers ()
+setup_modifiers (void)
 {
         if (gsd_used_mods == 0 || gsd_ignored_mods == 0) {
                 GdkModifierType dynmods;
@@ -67,13 +67,12 @@
 	}
 }
 
-static gboolean
+static void
 grab_key_real (guint      keycode,
                GdkWindow *root,
                gboolean   grab,
                int        mask)
 {
-        gdk_error_trap_push ();
         if (grab) {
                 XGrabKey (GDK_DISPLAY (),
                           keycode,
@@ -88,21 +87,33 @@
                             mask,
                             GDK_WINDOW_XID (root));
         }
-
-        gdk_flush ();
-        return gdk_error_trap_pop () == 0;
 }
 
 /* Grab the key. In order to ignore GSD_IGNORED_MODS we need to grab
  * all combinations of the ignored modifiers and those actually used
  * for the binding (if any).
  *
- * inspired by all_combinations from gnome-panel/gnome-panel/global-keys.c */
+ * inspired by all_combinations from gnome-panel/gnome-panel/global-keys.c
+ *
+ * This may generate X errors.  The correct way to use this is like:
+ *
+ *        gdk_error_trap_push ();
+ *
+ *        grab_key_unsafe (key, grab, screens);
+ *
+ *        gdk_flush ();
+ *        if (gdk_error_trap_pop ())
+ *                g_warning ("Grab failed, another application may already have access to key '%u'",
+ *                           key->keycode);
+ *
+ * This is not done in the function itself, to allow doing multiple grab_key
+ * operations with one flush only.
+ */
 #define N_BITS 32
 void
-grab_key (Key                 *key,
-          gboolean             grab,
-          GSList              *screens)
+grab_key_unsafe (Key                 *key,
+                 gboolean             grab,
+                 GSList              *screens)
 {
         int   indexes[N_BITS]; /* indexes of bits we need to flip */
         int   i;
@@ -141,14 +152,10 @@
 
                 for (l = screens; l; l = l->next) {
                         GdkScreen *screen = l->data;
-                        if (!grab_key_real (key->keycode,
-                                            gdk_screen_get_root_window (screen),
-                                            grab,
-                                            result | key->state)) {
-                                g_warning ("Grab failed, another application may already have access to key '%u'",
-                                           key->keycode);
-                                return;
-                        }
+                        grab_key_real (key->keycode,
+                                       gdk_screen_get_root_window (screen),
+                                       grab,
+                                       result | key->state);
                 }
         }
 }

Modified: trunk/plugins/common/gsd-keygrab.h
==============================================================================
--- trunk/plugins/common/gsd-keygrab.h	(original)
+++ trunk/plugins/common/gsd-keygrab.h	Mon Nov  3 21:28:52 2008
@@ -32,7 +32,7 @@
 } Key;
 
 
-void	        grab_key	(Key     *key,
+void	        grab_key_unsafe	(Key     *key,
 		        	 gboolean grab,
 			         GSList  *screens);
 

Modified: trunk/plugins/keybindings/gsd-keybindings-manager.c
==============================================================================
--- trunk/plugins/keybindings/gsd-keybindings-manager.c	(original)
+++ trunk/plugins/keybindings/gsd-keybindings-manager.c	Mon Nov  3 21:28:52 2008
@@ -255,6 +255,7 @@
 binding_register_keys (GsdKeybindingsManager *manager)
 {
         GSList *li;
+        gboolean need_flush = FALSE;
 
         gdk_error_trap_push ();
 
@@ -266,10 +267,11 @@
                     binding->previous_key.state != binding->key.state) {
                         /* Ungrab key if it changed and not clashing with previously set binding */
                         if (! key_already_used (manager, binding)) {
+                                need_flush = TRUE;
                                 if (binding->previous_key.keycode) {
-                                        grab_key (&binding->previous_key, FALSE, manager->priv->screens);
+                                        grab_key_unsafe (&binding->previous_key, FALSE, manager->priv->screens);
                                 }
-                                grab_key (&binding->key, TRUE, manager->priv->screens);
+                                grab_key_unsafe (&binding->key, TRUE, manager->priv->screens);
 
                                 binding->previous_key.keysym = binding->key.keysym;
                                 binding->previous_key.state = binding->key.state;
@@ -278,8 +280,11 @@
                                 g_warning ("Key binding (%s) is already in use", binding->binding_str);
                 }
         }
-        gdk_flush ();
-        gdk_error_trap_pop ();
+
+        if (need_flush)
+                gdk_flush ();
+        if (gdk_error_trap_pop ())
+                g_warning ("Grab failed for some keys, another application may already have access the them.");
 }
 
 extern char **environ;

Modified: trunk/plugins/media-keys/gsd-media-keys-manager.c
==============================================================================
--- trunk/plugins/media-keys/gsd-media-keys-manager.c	(original)
+++ trunk/plugins/media-keys/gsd-media-keys-manager.c	Mon Nov  3 21:28:52 2008
@@ -276,9 +276,12 @@
                GsdMediaKeysManager *manager)
 {
         int      i;
+        gboolean need_flush = TRUE;
 
         g_return_if_fail (entry->key != NULL);
 
+        gdk_error_trap_push ();
+
         /* Find the key that was modified */
         for (i = 0; i < HANDLED_KEYS; i++) {
                 if (strcmp (entry->key, keys[i].gconf_key) == 0) {
@@ -286,7 +289,8 @@
                         Key  *key;
 
                         if (keys[i].key != NULL) {
-                                grab_key (keys[i].key, FALSE, manager->priv->screens);
+                                need_flush = TRUE;
+                                grab_key_unsafe (keys[i].key, FALSE, manager->priv->screens);
                         }
 
                         g_free (keys[i].key);
@@ -308,7 +312,8 @@
                                 break;
                         }
 
-                        grab_key (key, TRUE, manager->priv->screens);
+                        need_flush = TRUE;
+                        grab_key_unsafe (key, TRUE, manager->priv->screens);
                         keys[i].key = key;
 
                         g_free (tmp);
@@ -316,15 +321,23 @@
                         break;
                 }
         }
+
+        if (need_flush)
+                gdk_flush ();
+        if (gdk_error_trap_pop ())
+                g_warning ("Grab failed for some keys, another application may already have access the them.");
 }
 
 static void
 init_kbd (GsdMediaKeysManager *manager)
 {
         int i;
+        gboolean need_flush = FALSE;
 
         gnome_settings_profile_start (NULL);
 
+        gdk_error_trap_push ();
+
         for (i = 0; i < HANDLED_KEYS; i++) {
                 char *tmp;
                 Key  *key;
@@ -340,6 +353,7 @@
                 tmp = gconf_client_get_string (manager->priv->conf_client,
                                                keys[i].gconf_key,
                                                NULL);
+
                 if (!is_valid_shortcut (tmp)) {
                         g_debug ("Not a valid shortcut: '%s'", tmp);
                         g_free (tmp);
@@ -367,9 +381,15 @@
 
                 keys[i].key = key;
 
-                grab_key (key, TRUE, manager->priv->screens);
+                need_flush = TRUE;
+                grab_key_unsafe (key, TRUE, manager->priv->screens);
         }
 
+        if (need_flush)
+                gdk_flush ();
+        if (gdk_error_trap_pop ())
+                g_warning ("Grab failed for some keys, another application may already have access the them.");
+
         gnome_settings_profile_end (NULL);
 }