Optimizations for managing new windows (do not all take effect if

2002-10-21  Havoc Pennington  <hp@redhat.com>

	Optimizations for managing new windows (do not all take effect if
	METACITY_DEBUG=1). Bug #96404

	* src/keybindings.c (meta_change_keygrab): use error trap nesting
	and conditionalize on meta_is_verbose() to avoid a ton of XSync

	* src/display.c (meta_change_button_grab): ditto

	Throughout: move to new error trap setup to save on XSync calls,
	new setup is:

	* src/errors.c (meta_error_trap_push_with_return): new function,
	an error trap that needs to care about return value and thus
	sync even if an outer trap still exists
	(meta_error_trap_pop_with_return): new function
	(meta_error_trap_pop): add "last_request_was_roundtrip"
	argument allowing us to avoid XSync() if we just did
	a GetProperty or whatever.

	* src/util.c (meta_warning): flush the warning file descriptor

	* src/Makefile.am (INCLUDES): define G_LOG_DOMAIN

19 files changed, 365 insertions, 229 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index d8a67982..fe789c24 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,7 +1,7 @@
 
 SUBDIRS=wm-tester tools themes
 
-INCLUDES=@METACITY_CFLAGS@ -DMETACITY_LIBEXECDIR=\"$(libexecdir)\" -DHOST_ALIAS=\"@HOST_ALIAS@\" -DMETACITY_LOCALEDIR=\"$(prefix)/@DATADIRNAME@/locale\" -DMETACITY_PKGDATADIR=\"$(pkgdatadir)\" -DMETACITY_DATADIR=\"$(datadir)\"
+INCLUDES=@METACITY_CFLAGS@ -DMETACITY_LIBEXECDIR=\"$(libexecdir)\" -DHOST_ALIAS=\"@HOST_ALIAS@\" -DMETACITY_LOCALEDIR=\"$(prefix)/@DATADIRNAME@/locale\" -DMETACITY_PKGDATADIR=\"$(pkgdatadir)\" -DMETACITY_DATADIR=\"$(datadir)\" -DG_LOG_DOMAIN=\"metacity\"
 
 EGGFILES=			\
 	eggaccelerators.c	\
diff --git a/src/delete.c b/src/delete.c
index 447e8214..c8a38047 100644
--- a/src/delete.c
+++ b/src/delete.c
@@ -371,7 +371,7 @@ meta_window_delete (MetaWindow  *window,
                   window->desc);
       XKillClient (window->display->xdisplay, window->xwindow);
     }
-  meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
 
   meta_display_ping_window (window->display,
                             window,
@@ -438,7 +438,7 @@ meta_window_kill (MetaWindow *window)
               window->desc);
   meta_error_trap_push (window->display);
   XKillClient (window->display->xdisplay, window->xwindow);
-  meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
 }
 
 void
diff --git a/src/display.c b/src/display.c
index a120fdb3..efe29298 100644
--- a/src/display.c
+++ b/src/display.c
@@ -84,7 +84,7 @@ static void    update_window_grab_modifiers (MetaDisplay *display);
 static void    prefs_changed_callback    (MetaPreference pref,
                                           void          *data);
 
-static int
+static void
 set_utf8_string_hint (MetaDisplay *display,
                       Window xwindow,
                       Atom atom,
@@ -95,7 +95,7 @@ set_utf8_string_hint (MetaDisplay *display,
                    xwindow, atom,
                    display->atom_utf8_string, 
                    8, PropModeReplace, (guchar*) val, strlen (val));
-  return meta_error_trap_pop (display);
+  meta_error_trap_pop (display, FALSE);
 }
 
 static void
@@ -258,6 +258,7 @@ meta_display_open (const char *name)
    */
   display->name = g_strdup (XDisplayName (name));
   display->xdisplay = xdisplay;
+  display->error_trap_synced_at_last_pop = TRUE;
   display->error_traps = 0;
   display->error_trap_handler = NULL;
   display->server_grab_count = 0;
@@ -497,7 +498,7 @@ meta_display_open (const char *name)
     /* Force a new FocusIn (does this work?) */
     XSetInputFocus (display->xdisplay, focus, ret_to, CurrentTime);
     
-    meta_error_trap_pop (display);
+    meta_error_trap_pop (display, FALSE);
   }
   
   meta_display_ungrab (display);  
@@ -667,10 +668,18 @@ meta_display_screen_for_xwindow (MetaDisplay *display,
                                  Window       xwindow)
 {
   XWindowAttributes attr;
-
+  int result;
+  
   meta_error_trap_push (display);
-  XGetWindowAttributes (display->xdisplay, xwindow, &attr);
-  if (meta_error_trap_pop (display) != Success)
+  attr.screen = NULL;
+  result = XGetWindowAttributes (display->xdisplay, xwindow, &attr);
+  meta_error_trap_pop (display, TRUE);
+
+  /* Note, XGetWindowAttributes is on all kinds of crack
+   * and returns 1 on success 0 on failure, rather than Success
+   * on success.
+   */
+  if (result == 0 || attr.screen == NULL)
     return NULL;
   
   return meta_display_screen_for_x_screen (display, attr.screen);
@@ -930,7 +939,7 @@ window_raise_with_delay_callback (void *data)
 				   window->xwindow,
 				   &root, &child,
 				   &root_x, &root_y, &x, &y, &mask);
-      meta_error_trap_pop (window->display);
+      meta_error_trap_pop (window->display, TRUE);
 
       if ((window->frame && POINT_IN_RECT (root_x, root_y, window->frame->rect)) ||
 	  (window->frame == NULL && POINT_IN_RECT (root_x, root_y, window->rect)))
@@ -1340,7 +1349,7 @@ event_callback (XEvent   *event,
                             window->frame->xwindow);
               meta_error_trap_push (display);
               meta_window_destroy_frame (window->frame->window);
-              meta_error_trap_pop (display);
+              meta_error_trap_pop (display, FALSE);
             }
           else
             {
@@ -1452,7 +1461,7 @@ event_callback (XEvent   *event,
           meta_error_trap_push (display);
           XConfigureWindow (display->xdisplay, event->xconfigurerequest.window,
                             xwcm, &xwc);
-          meta_error_trap_pop (display);
+          meta_error_trap_pop (display, FALSE);
         }
       else
         {
@@ -2043,7 +2052,7 @@ meta_spew_event (MetaDisplay *display,
         meta_error_trap_push (display);
         str = XGetAtomName (display->xdisplay,
                             event->xproperty.atom);
-        meta_error_trap_pop (display);
+        meta_error_trap_pop (display, TRUE);
 
         if (event->xproperty.state == PropertyNewValue)
           state = "PropertyNewValue";
@@ -2077,7 +2086,7 @@ meta_spew_event (MetaDisplay *display,
         meta_error_trap_push (display);
         str = XGetAtomName (display->xdisplay,
                             event->xclient.message_type);
-        meta_error_trap_pop (display);
+        meta_error_trap_pop (display, TRUE);
         extra = g_strdup_printf ("type: %s format: %d\n",
                                  str ? str : "(unknown atom)",
                                  event->xclient.format);
@@ -2266,10 +2275,9 @@ meta_display_set_grab_op_cursor (MetaDisplay *display,
 #define GRAB_MASK (PointerMotionMask | PointerMotionHintMask |  \
                    ButtonPressMask | ButtonReleaseMask)
 
-  meta_error_trap_push (display);
-
   if (change_pointer)
     {
+      meta_error_trap_push_with_return (display);
       XChangeActivePointerGrab (display->xdisplay,
                                 GRAB_MASK,
                                 cursor,
@@ -2277,11 +2285,20 @@ meta_display_set_grab_op_cursor (MetaDisplay *display,
 
       meta_topic (META_DEBUG_WINDOW_OPS,
                   "Changed pointer with XChangeActivePointerGrab()\n");
+
+      if (meta_error_trap_pop_with_return (display, FALSE) != Success)
+        {
+          meta_topic (META_DEBUG_WINDOW_OPS,
+                      "Error trapped from XChangeActivePointerGrab()\n");
+          if (display->grab_have_pointer)
+            display->grab_have_pointer = FALSE;
+        }
     }
   else
     {
       g_assert (screen != NULL);
-      
+
+      meta_error_trap_push (display);
       if (XGrabPointer (display->xdisplay,
                         grab_xwindow,
                         False,
@@ -2295,15 +2312,9 @@ meta_display_set_grab_op_cursor (MetaDisplay *display,
           meta_topic (META_DEBUG_WINDOW_OPS,
                       "XGrabPointer() returned GrabSuccess\n");
         }
-    }  
-
-  if (meta_error_trap_pop (display) != Success)
-    {
-      meta_topic (META_DEBUG_WINDOW_OPS,
-                  "Error trapped from XGrabPointer()\n");
-      if (display->grab_have_pointer)
-        display->grab_have_pointer = FALSE;
+      meta_error_trap_pop (display, TRUE);
     }
+
 #undef GRAB_MASK
   
   if (cursor != None)
@@ -2501,12 +2512,12 @@ meta_change_button_grab (MetaDisplay *display,
                          int          modmask)
 {
   int ignored_mask;
-
+  
+  meta_error_trap_push (display);
+  
   ignored_mask = 0;
   while (ignored_mask < (int) display->ignored_modifier_mask)
     {
-      int result;
-      
       if (ignored_mask & ~(display->ignored_modifier_mask))
         {
           /* Not a combination of ignored modifiers
@@ -2515,8 +2526,10 @@ meta_change_button_grab (MetaDisplay *display,
           ++ignored_mask;
           continue;
         }
-  
-      meta_error_trap_push (display);
+
+      if (meta_is_debugging ())
+        meta_error_trap_push_with_return (display);
+      
       if (grab)
         XGrabButton (display->xdisplay, button, modmask | ignored_mask,
                      xwindow, False,
@@ -2528,15 +2541,23 @@ meta_change_button_grab (MetaDisplay *display,
       else
         XUngrabButton (display->xdisplay, button, modmask | ignored_mask,
                        xwindow);
-        
-      result = meta_error_trap_pop (display);
 
-      if (result != Success)
-        meta_verbose ("Failed to grab button %d with mask 0x%x for window 0x%lx error code %d\n",
-                      button, modmask | ignored_mask, xwindow, result);
+      if (meta_is_debugging ())
+        {
+          int result;
+          
+          result = meta_error_trap_pop_with_return (display, FALSE);
+          
+          if (result != Success)
+            meta_verbose ("Failed to %s button %d with mask 0x%x for window 0x%lx error code %d\n",
+                          grab ? "grab" : "ungrab",
+                          button, modmask | ignored_mask, xwindow, result);
+        }
       
       ++ignored_mask;
     }
+
+  meta_error_trap_pop (display, FALSE);
 }
 
 void
@@ -2681,7 +2702,7 @@ meta_display_update_active_window_hint (MetaDisplay *display)
                        display->atom_net_active_window,
                        XA_WINDOW,
                        32, PropModeReplace, (guchar*) data, 2);
-      meta_error_trap_pop (display);
+      meta_error_trap_pop (display, FALSE);
 
       tmp = tmp->next;
     }
@@ -3191,7 +3212,7 @@ convert_property (MetaDisplay *display,
   conversion_targets[2] = display->atom_timestamp;
   conversion_targets[3] = display->atom_version;
 
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   if (target == display->atom_targets)
     XChangeProperty (display->xdisplay, w, property,
 		     XA_ATOM, 32, PropModeReplace,
@@ -3206,16 +3227,17 @@ convert_property (MetaDisplay *display,
 		     (unsigned char *)icccm_version, 2);
   else
     {
-      meta_error_trap_pop (display);
+      meta_error_trap_pop_with_return (display, FALSE);
       return FALSE;
     }
   
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, FALSE) != Success)
     return FALSE;
 
   /* Be sure the PropertyNotify has arrived so we
    * can send SelectionNotify
    */
+  /* FIXME the error trap pop synced anyway, right? */
   XSync (display->xdisplay, False);
 
   return TRUE;
@@ -3240,7 +3262,7 @@ process_selection_request (MetaDisplay   *display,
       meta_error_trap_push (display);
       str = XGetAtomName (display->xdisplay,
                           event->xselectionrequest.selection);
-      meta_error_trap_pop (display);
+      meta_error_trap_pop (display, TRUE);
       
       meta_verbose ("Selection request with selection %s window 0x%lx not a WM_Sn selection we recognize\n",
                     str ? str : "(bad atom)", event->xselectionrequest.owner);
@@ -3267,13 +3289,18 @@ process_selection_request (MetaDisplay   *display,
           unsigned long num, rest;
           unsigned char *data;
 
-          meta_error_trap_push (display);
-          XGetWindowProperty (display->xdisplay,
-                              event->xselectionrequest.requestor,
-                              event->xselectionrequest.property, 0, 256, False,
-                              display->atom_atom_pair,
-                              &type, &format, &num, &rest, &data);
-          if (meta_error_trap_pop (display) == Success)
+          meta_error_trap_push_with_return (display);
+          if (XGetWindowProperty (display->xdisplay,
+                                  event->xselectionrequest.requestor,
+                                  event->xselectionrequest.property, 0, 256, False,
+                                  display->atom_atom_pair,
+                                  &type, &format, &num, &rest, &data) != Success)
+            {
+              meta_error_trap_pop_with_return (display, TRUE);
+              return;
+            }
+          
+          if (meta_error_trap_pop_with_return (display, TRUE) == Success)
             {              
               /* FIXME: to be 100% correct, should deal with rest > 0,
                * but since we have 4 possible targets, we will hardly ever
@@ -3296,7 +3323,7 @@ process_selection_request (MetaDisplay   *display,
                                event->xselectionrequest.property,
                                display->atom_atom_pair,
                                32, PropModeReplace, data, num);
-              meta_error_trap_pop (display);
+              meta_error_trap_pop (display, FALSE);
               meta_XFree (data);
             }
         }
@@ -3350,7 +3377,7 @@ process_selection_clear (MetaDisplay   *display,
     meta_error_trap_push (display);
     str = XGetAtomName (display->xdisplay,
                         event->xselectionclear.selection);
-    meta_error_trap_pop (display);
+    meta_error_trap_pop (display, TRUE);
 
     meta_verbose ("Selection clear with selection %s window 0x%lx not a WM_Sn selection we recognize\n",
                   str ? str : "(bad atom)", event->xselectionclear.window);
diff --git a/src/display.h b/src/display.h
index f97e0fc7..82a2d324 100644
--- a/src/display.h
+++ b/src/display.h
@@ -167,6 +167,7 @@ struct _MetaDisplay
   guint static_gravity_works : 1;
   
   /*< private-ish >*/
+  guint error_trap_synced_at_last_pop : 1;
   MetaEventQueue *events;
   GSList *screens;
   GHashTable *window_ids;
diff --git a/src/errors.c b/src/errors.c
index 0ae29465..d558de03 100644
--- a/src/errors.c
+++ b/src/errors.c
@@ -26,6 +26,8 @@
 #include <stdlib.h>
 #include <gdk/gdk.h>
 
+static int sync_count = 0;
+
 static int x_error_handler    (Display     *display,
                                XErrorEvent *error);
 static int x_io_error_handler (Display     *display);
@@ -37,13 +39,20 @@ meta_errors_init (void)
   XSetIOErrorHandler (x_io_error_handler);
 }
 
-void
-meta_error_trap_push (MetaDisplay *display)
+static void
+meta_error_trap_push_internal (MetaDisplay *display,
+                               gboolean     need_sync)
 {
   /* GDK resets the error handler on each push */
   int (* old_error_handler) (Display     *,
                              XErrorEvent *);
 
+  if (need_sync)
+    {
+      XSync (display->xdisplay, False);
+      ++sync_count;
+    }
+  
   gdk_error_trap_push ();
 
   /* old_error_handler will just be equal to x_error_handler
@@ -60,17 +69,23 @@ meta_error_trap_push (MetaDisplay *display)
     }
 
   display->error_traps += 1;
+
+  meta_topic (META_DEBUG_ERRORS, "%d traps remain\n", display->error_traps);
 }
 
-int
-meta_error_trap_pop (MetaDisplay *display)
+static int
+meta_error_trap_pop_internal  (MetaDisplay *display,
+                               gboolean     need_sync)
 {
   int result;
 
   g_assert (display->error_traps > 0);
-  
-  /* just use GDK trap, but we do the sync since GDK doesn't */
-  XSync (display->xdisplay, False);
+
+  if (need_sync)
+    {
+      XSync (display->xdisplay, False);
+      ++sync_count;
+    }
 
   result = gdk_error_trap_pop ();
 
@@ -92,9 +107,75 @@ meta_error_trap_pop (MetaDisplay *display)
       display->error_trap_handler = NULL;
     }
 
+  meta_topic (META_DEBUG_ERRORS, "%d traps\n", display->error_traps);
+  
   return result;
 }
 
+void
+meta_error_trap_push (MetaDisplay *display)
+{
+  meta_error_trap_push_internal (display, FALSE);
+}
+
+void
+meta_error_trap_pop (MetaDisplay *display,
+                     gboolean     last_request_was_roundtrip)
+{
+  gboolean need_sync;
+
+  /* we only have to sync when popping the outermost trap */
+  need_sync = (display->error_traps == 1 && !last_request_was_roundtrip);
+
+  if (need_sync)
+    meta_topic (META_DEBUG_SYNC, "%d: Syncing on error_trap_pop, traps = %d, roundtrip = %d\n",
+                sync_count, display->error_traps, last_request_was_roundtrip);
+
+  display->error_trap_synced_at_last_pop = need_sync || last_request_was_roundtrip;
+  
+  meta_error_trap_pop_internal (display, need_sync);
+}
+
+void
+meta_error_trap_push_with_return (MetaDisplay *display)
+{
+  gboolean need_sync;
+
+  /* We don't sync on push_with_return if there are no traps
+   * currently, because we assume that any errors were either covered
+   * by a previous pop, or were fatal.
+   *
+   * More generally, we don't sync if we were synchronized last time
+   * we popped. This is known to be the case if there are no traps,
+   * but we also keep a flag so we know whether it's the case otherwise.
+   */
+
+  if (!display->error_trap_synced_at_last_pop)
+    need_sync = TRUE;
+  else
+    need_sync = FALSE;
+
+  if (need_sync)
+    meta_topic (META_DEBUG_SYNC, "%d: Syncing on error_trap_push_with_return, traps = %d\n",
+                sync_count, display->error_traps);
+  
+  meta_error_trap_push_internal (display, FALSE);
+}
+
+int
+meta_error_trap_pop_with_return  (MetaDisplay *display,
+                                  gboolean     last_request_was_roundtrip)
+{
+  if (!last_request_was_roundtrip)
+    meta_topic (META_DEBUG_SYNC, "%d: Syncing on error_trap_pop_with_return, traps = %d, roundtrip = %d\n",
+                sync_count, display->error_traps, last_request_was_roundtrip);
+
+  display->error_trap_synced_at_last_pop = TRUE;
+  
+  return meta_error_trap_pop_internal (display,
+                                       !last_request_was_roundtrip);
+}
+
 static int
 x_error_handler (Display     *xdisplay,
                  XErrorEvent *error)
diff --git a/src/errors.h b/src/errors.h
index b1a90e4f..e7c8f389 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -25,9 +25,15 @@
 #include "util.h"
 #include "display.h"
 
-void     meta_errors_init     (void);
-void     meta_error_trap_push (MetaDisplay *display);
+void      meta_errors_init     (void);
+void      meta_error_trap_push (MetaDisplay *display);
+void      meta_error_trap_pop  (MetaDisplay *display,
+                                gboolean     last_request_was_roundtrip);
+
+void      meta_error_trap_push_with_return (MetaDisplay *display);
 /* returns X error code, or 0 for no error */
-int      meta_error_trap_pop  (MetaDisplay *display);
+int       meta_error_trap_pop_with_return  (MetaDisplay *display,
+                                            gboolean     last_request_was_roundtrip);
+
 
 #endif
diff --git a/src/frame.c b/src/frame.c
index dbca8937..eddd4049 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -124,7 +124,7 @@ meta_window_ensure_frame (MetaWindow *window)
                    window->rect.x,
                    window->rect.y);
   /* FIXME handle this error */
-  meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
   
   /* stick frame to the window */
   window->frame = frame;
@@ -180,7 +180,7 @@ meta_window_destroy_frame (MetaWindow *window)
                    /* FIXME where to put it back depends on the gravity */
                    window->frame->rect.x,
                    window->frame->rect.y);
-  meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
 
   meta_display_unregister_x_window (window->display,
                                     frame->xwindow);
diff --git a/src/iconcache.c b/src/iconcache.c
index 392f002f..388df519 100644
--- a/src/iconcache.c
+++ b/src/iconcache.c
@@ -231,7 +231,7 @@ read_rgb_icon (MetaDisplay   *display,
   gulong *best_mini;
   int mini_w, mini_h;
 
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   type = None;
   data = NULL;
   result = XGetWindowProperty (display->xdisplay,
@@ -241,7 +241,7 @@ read_rgb_icon (MetaDisplay   *display,
 			       False, XA_CARDINAL, &type, &format, &nitems,
 			       &bytes_after, ((guchar **)&data));
 
-  err = meta_error_trap_pop (display);
+  err = meta_error_trap_pop_with_return (display, TRUE);
 
   if (err != Success ||
       result != Success)
@@ -406,7 +406,7 @@ try_pixmap_and_mask (MetaDisplay *display,
                                               w, h);
     }
 
-  meta_error_trap_pop (display);
+  meta_error_trap_pop (display, FALSE);
 
   if (mask)
     {
@@ -470,7 +470,7 @@ get_kwm_win_icon (MetaDisplay *display,
   *pixmap = None;
   *mask = None;
 
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   icons = NULL;
   result = XGetWindowProperty (display->xdisplay, xwindow,
                                display->atom_kwm_win_icon,
@@ -480,7 +480,7 @@ get_kwm_win_icon (MetaDisplay *display,
 			       &type, &format, &nitems,
 			       &bytes_after, (guchar **)&icons);
 
-  err = meta_error_trap_pop (display);
+  err = meta_error_trap_pop_with_return (display, TRUE);
   if (err != Success ||
       result != Success)
     return;
diff --git a/src/keybindings.c b/src/keybindings.c
index 6b08296e..80bb068b 100644
--- a/src/keybindings.c
+++ b/src/keybindings.c
@@ -799,7 +799,6 @@ meta_change_keygrab (MetaDisplay *display,
                      int          keycode,
                      int          modmask)
 {
-  int result;
   int ignored_mask;
 
   /* Grab keycode/modmask, together with
@@ -812,6 +811,9 @@ meta_change_keygrab (MetaDisplay *display,
               grab ? "Grabbing" : "Ungrabbing",
               keysym_name (keysym),
               modmask, xwindow);
+
+  /* efficiency, avoid so many XSync() */
+  meta_error_trap_push (display);
   
   ignored_mask = 0;
   while (ignored_mask < (int) display->ignored_modifier_mask)
@@ -824,8 +826,9 @@ meta_change_keygrab (MetaDisplay *display,
           ++ignored_mask;
           continue;
         }
-      
-      meta_error_trap_push (display);
+
+      if (meta_is_debugging ())
+        meta_error_trap_push_with_return (display);
       if (grab)
         XGrabKey (display->xdisplay, keycode,
                   modmask | ignored_mask,
@@ -836,21 +839,28 @@ meta_change_keygrab (MetaDisplay *display,
         XUngrabKey (display->xdisplay, keycode,
                     modmask | ignored_mask,
                     xwindow);
-      
-      result = meta_error_trap_pop (display);
 
-      if (grab && result != Success)
-        {      
-          if (result == BadAccess)
-            meta_warning (_("Some other program is already using the key %s with modifiers %x as a binding\n"), keysym_name (keysym), modmask | ignored_mask);
-          else
-            meta_topic (META_DEBUG_KEYBINDINGS,
-                        "Failed to grab key %s with modifiers %x\n",
-                        keysym_name (keysym), modmask | ignored_mask);
+      if (meta_is_debugging ())
+        {
+          int result;
+          
+          result = meta_error_trap_pop_with_return (display, FALSE);
+          
+          if (grab && result != Success)
+            {      
+              if (result == BadAccess)
+                meta_warning (_("Some other program is already using the key %s with modifiers %x as a binding\n"), keysym_name (keysym), modmask | ignored_mask);
+              else
+                meta_topic (META_DEBUG_KEYBINDINGS,
+                            "Failed to grab key %s with modifiers %x\n",
+                            keysym_name (keysym), modmask | ignored_mask);
+            }
         }
 
       ++ignored_mask;
     }
+
+  meta_error_trap_pop (display, FALSE);
 }
 
 static void
@@ -894,12 +904,12 @@ ungrab_all_keys (MetaDisplay *display,
 {
   int result;
   
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
 
   XUngrabKey (display->xdisplay, AnyKey, AnyModifier,
               xwindow);
       
-  result = meta_error_trap_pop (display);
+  result = meta_error_trap_pop_with_return (display, FALSE);
   
   if (result != Success)    
     meta_topic (META_DEBUG_KEYBINDINGS,
@@ -984,19 +994,26 @@ grab_keyboard (MetaDisplay *display,
   /* Grab the keyboard, so we get key releases and all key
    * presses
    */
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
 
-  XGrabKeyboard (display->xdisplay,
-                 xwindow, True,
-                 GrabModeAsync, GrabModeAsync,
-                 meta_display_get_current_time (display));
-       
-  result = meta_error_trap_pop (display);
-  if (result != Success)
+  if (XGrabKeyboard (display->xdisplay,
+                     xwindow, True,
+                     GrabModeAsync, GrabModeAsync,
+                     meta_display_get_current_time (display)) != GrabSuccess)
     {
+      meta_error_trap_pop_with_return (display, TRUE);
       meta_topic (META_DEBUG_KEYBINDINGS,
-                  "XGrabKeyboard() failed\n");
-      return FALSE;
+                  "XGrabKeyboard() returned failure\n");
+    }
+  else
+    {
+      result = meta_error_trap_pop_with_return (display, TRUE);
+      if (result != Success)
+        {
+          meta_topic (META_DEBUG_KEYBINDINGS,
+                      "XGrabKeyboard() resulted in an error\n");
+          return FALSE;
+        }
     }
        
   meta_topic (META_DEBUG_KEYBINDINGS, "Grabbed all keys\n");
@@ -1017,7 +1034,7 @@ ungrab_keyboard (MetaDisplay *display)
               "Ungrabbing keyboard with timestamp %lu\n",
               timestamp);
   XUngrabKeyboard (display->xdisplay, timestamp);
-  meta_error_trap_pop (display);
+  meta_error_trap_pop (display, FALSE);
 }
 
 gboolean
diff --git a/src/main.c b/src/main.c
index 81aca432..55ecf624 100644
--- a/src/main.c
+++ b/src/main.c
@@ -284,6 +284,9 @@ main (int argc, char **argv)
   g_log_set_handler (NULL,
                      G_LOG_LEVEL_MASK | G_LOG_FLAG_FATAL | G_LOG_FLAG_RECURSION,
                      log_handler, NULL);
+  g_log_set_handler (G_LOG_DOMAIN,
+                     G_LOG_LEVEL_MASK | G_LOG_FLAG_FATAL | G_LOG_FLAG_RECURSION,
+                     log_handler, NULL);
   g_log_set_handler ("Gtk",
                      G_LOG_LEVEL_MASK | G_LOG_FLAG_FATAL | G_LOG_FLAG_RECURSION,
                      log_handler, NULL);
diff --git a/src/screen.c b/src/screen.c
index 4e98eed8..c4139034 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -404,11 +404,11 @@ meta_screen_new (MetaDisplay *display,
         }
 
       /* We want to find out when the current selection owner dies */
-      meta_error_trap_push (display);
+      meta_error_trap_push_with_return (display);
       attrs.event_mask = StructureNotifyMask;
       XChangeWindowAttributes (xdisplay,
                                current_wm_sn_owner, CWEventMask, &attrs);
-      if (meta_error_trap_pop (display) != Success)
+      if (meta_error_trap_pop_with_return (display, FALSE) != Success)
         current_wm_sn_owner = None; /* don't wait for it to die later on */
     }
 
@@ -477,7 +477,7 @@ meta_screen_new (MetaDisplay *display,
     }
   
   /* select our root window events */
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
 
   /* We need to or with the existing event mask since
    * gtk+ may be interested in other events.
@@ -492,7 +492,7 @@ meta_screen_new (MetaDisplay *display,
                 KeyPressMask | KeyReleaseMask |
                 FocusChangeMask | StructureNotifyMask |
 		attr.your_event_mask);
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, FALSE) != Success)
     {
       meta_warning (_("Screen %d on display \"%s\" already has a window manager\n"),
                     number, display->name);
@@ -608,9 +608,9 @@ meta_screen_free (MetaScreen *screen)
 
   meta_stack_free (screen->stack);
 
-  meta_error_trap_push (screen->display);
+  meta_error_trap_push_with_return (screen->display);
   XSelectInput (screen->display->xdisplay, screen->xroot, 0);
-  if (meta_error_trap_pop (screen->display) != Success)
+  if (meta_error_trap_pop_with_return (screen->display, FALSE) != Success)
     meta_warning (_("Could not release screen %d on display \"%s\"\n"),
                   screen->number, screen->display->name);
 
@@ -638,13 +638,13 @@ meta_screen_manage_all_windows (MetaScreen *screen)
   /* Must grab server to avoid obvious race condition */
   meta_display_grab (screen->display);
 
-  meta_error_trap_push (screen->display);
+  meta_error_trap_push_with_return (screen->display);
   
   XQueryTree (screen->display->xdisplay,
               screen->xroot,
               &ignored1, &ignored2, &children, &n_children);
 
-  if (meta_error_trap_pop (screen->display))
+  if (meta_error_trap_pop_with_return (screen->display, TRUE) != Success)
     {
       meta_display_ungrab (screen->display);
       return;
@@ -844,14 +844,14 @@ meta_screen_get_workspace_by_index (MetaScreen  *screen,
   return NULL;
 }
 
-static int
+static void
 set_number_of_spaces_hint (MetaScreen *screen,
 			   int         n_spaces)
 {
   unsigned long data[1];
 
   if (screen->closing > 0)
-    return 0;
+    return;
 
   data[0] = n_spaces;
 
@@ -862,7 +862,7 @@ set_number_of_spaces_hint (MetaScreen *screen,
                    screen->display->atom_net_number_of_desktops,
                    XA_CARDINAL,
                    32, PropModeReplace, (guchar*) data, 1);
-  return meta_error_trap_pop (screen->display);
+  meta_error_trap_pop (screen->display, FALSE);
 }
 
 static void
@@ -1411,7 +1411,7 @@ meta_create_offscreen_window (Display *xdisplay,
                         &attrs);
 }
 
-static int
+static void
 set_work_area_hint (MetaScreen *screen)
 {
   int num_workspaces;
@@ -1448,7 +1448,7 @@ set_work_area_hint (MetaScreen *screen)
 		   XA_CARDINAL, 32, PropModeReplace,
 		   (guchar*) data, num_workspaces*4);
   g_free (data);
-  return meta_error_trap_pop (screen->display);
+  meta_error_trap_pop (screen->display, FALSE);
 }
 
 static gboolean
@@ -1564,7 +1564,7 @@ update_showing_desktop_hint (MetaScreen *screen)
                    screen->display->atom_net_showing_desktop,
                    XA_CARDINAL,
                    32, PropModeReplace, (guchar*) data, 1);
-  meta_error_trap_pop (screen->display);
+  meta_error_trap_pop (screen->display, FALSE);
 }
 
 static void
diff --git a/src/stack.c b/src/stack.c
index 558ffad1..7018283d 100644
--- a/src/stack.c
+++ b/src/stack.c
@@ -940,13 +940,13 @@ raise_window_relative_to_managed_windows (MetaScreen *screen,
    * or restack any windows before using the XQueryTree results.
    */
   
-  meta_error_trap_push (screen->display);
+  meta_error_trap_push_with_return (screen->display);
   
   XQueryTree (screen->display->xdisplay,
               screen->xroot,
               &ignored1, &ignored2, &children, &n_children);
 
-  if (meta_error_trap_pop (screen->display))
+  if (meta_error_trap_pop_with_return (screen->display, TRUE) != Success)
     {
       meta_topic (META_DEBUG_STACK,
                   "Error querying root children to raise window 0x%lx\n",
@@ -991,7 +991,7 @@ raise_window_relative_to_managed_windows (MetaScreen *screen,
                             xwindow,
                             CWSibling | CWStackMode,
                             &changes);
-          meta_error_trap_pop (screen->display);
+          meta_error_trap_pop (screen->display, FALSE);
 
           break;
         }
@@ -1007,7 +1007,7 @@ raise_window_relative_to_managed_windows (MetaScreen *screen,
       meta_error_trap_push (screen->display);
       XLowerWindow (screen->display->xdisplay,
                     xwindow);
-      meta_error_trap_pop (screen->display);
+      meta_error_trap_pop (screen->display, FALSE);
     }
   
   if (children)
@@ -1176,7 +1176,7 @@ meta_stack_sync_to_server (MetaStack *stack)
         }
     }
 
-  meta_error_trap_pop (stack->screen->display);
+  meta_error_trap_pop (stack->screen->display, FALSE);
   /* on error, a window was destroyed; it should eventually
    * get removed from the stacking list when we unmanage it
    * and we'll fix stacking at that time.
diff --git a/src/util.c b/src/util.c
index 03ebf2d0..c6b6b7e9 100644
--- a/src/util.c
+++ b/src/util.c
@@ -254,6 +254,10 @@ topic_name (MetaDebugTopic topic)
       return "XINERAMA";
     case META_DEBUG_KEYBINDINGS:
       return "KEYBINDINGS";
+    case META_DEBUG_SYNC:
+      return "SYNC";
+    case META_DEBUG_ERRORS:
+      return "ERRORS";
     }
 
   return "Window manager";
@@ -335,6 +339,8 @@ meta_warning (const char *format, ...)
   if (no_prefix == 0)
     utf8_fputs (_("Window manager warning: "), out);
   utf8_fputs (str, out);
+
+  fflush (out);
   
   g_free (str);
 }
diff --git a/src/util.h b/src/util.h
index c45cb707..07ebc42e 100644
--- a/src/util.h
+++ b/src/util.h
@@ -58,7 +58,9 @@ typedef enum
   META_DEBUG_PLACEMENT    = 1 << 9,
   META_DEBUG_PING         = 1 << 10,
   META_DEBUG_XINERAMA     = 1 << 11,
-  META_DEBUG_KEYBINDINGS  = 1 << 12
+  META_DEBUG_KEYBINDINGS  = 1 << 12,
+  META_DEBUG_SYNC         = 1 << 13,
+  META_DEBUG_ERRORS       = 1 << 14
 
 } MetaDebugTopic;
 
diff --git a/src/window.c b/src/window.c
index a03e755f..8466690e 100644
--- a/src/window.c
+++ b/src/window.c
@@ -2,6 +2,7 @@
 
 /* 
  * Copyright (C) 2001 Havoc Pennington, Anders Carlsson
+ * Copyright (C) 2002 Red Hat, Inc.
  * 
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -70,29 +71,29 @@ static void constrain_position (MetaWindow        *window,
                                 int               *new_x,
                                 int               *new_y);
 
-static int      update_size_hints         (MetaWindow     *window);
-static int      update_title              (MetaWindow     *window);
-static int      update_protocols          (MetaWindow     *window);
-static int      update_wm_hints           (MetaWindow     *window);
+static void     update_size_hints         (MetaWindow     *window);
+static void     update_title              (MetaWindow     *window);
+static void     update_protocols          (MetaWindow     *window);
+static void     update_wm_hints           (MetaWindow     *window);
 static void     update_net_wm_state       (MetaWindow     *window);
 static void     update_mwm_hints          (MetaWindow     *window);
-static int      update_wm_class           (MetaWindow     *window);
-static int      update_transient_for      (MetaWindow     *window);
+static void     update_wm_class           (MetaWindow     *window);
+static void     update_transient_for      (MetaWindow     *window);
 static void     update_sm_hints           (MetaWindow     *window);
 static void     update_role               (MetaWindow     *window);
 static void     update_net_wm_type        (MetaWindow     *window);
-static int      update_initial_workspace  (MetaWindow     *window);
-static int      update_icon_name          (MetaWindow     *window);
-static int      update_icon               (MetaWindow     *window);
+static void     update_initial_workspace  (MetaWindow     *window);
+static void     update_icon_name          (MetaWindow     *window);
+static void     update_icon               (MetaWindow     *window);
 static void     redraw_icon               (MetaWindow     *window); 
 static void     update_struts             (MetaWindow     *window);
 static void     recalc_window_type        (MetaWindow     *window);
 static void     recalc_window_features    (MetaWindow     *window);
 static void     recalc_do_not_cover_struts(MetaWindow     *window);
 static void     invalidate_work_areas     (MetaWindow *window);
-static int      set_wm_state              (MetaWindow     *window,
+static void     set_wm_state              (MetaWindow     *window,
                                            int             state);
-static int      set_net_wm_state          (MetaWindow     *window);
+static void     set_net_wm_state          (MetaWindow     *window);
 static void     send_configure_notify     (MetaWindow     *window);
 static gboolean process_property_notify   (MetaWindow     *window,
                                            XPropertyEvent *event);
@@ -177,16 +178,20 @@ meta_window_new (MetaDisplay *display, Window xwindow,
   
   /* Grab server */
   meta_display_grab (display);
+  meta_error_trap_push (display); /* Push a trap over all of window
+                                   * creation, to reduce XSync() calls
+                                   */
   
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   
   XGetWindowAttributes (display->xdisplay,
                         xwindow, &attrs);
 
-  if (meta_error_trap_pop (display))
+  if (meta_error_trap_pop_with_return (display, TRUE) != Success)
     {
       meta_verbose ("Failed to get attributes for window 0x%lx\n",
                     xwindow);
+      meta_error_trap_pop (display, TRUE);
       meta_display_ungrab (display);
       return NULL;
     }
@@ -194,6 +199,7 @@ meta_window_new (MetaDisplay *display, Window xwindow,
   if (attrs.override_redirect)
     {
       meta_verbose ("Deciding not to manage override_redirect window 0x%lx\n", xwindow);
+      meta_error_trap_pop (display, TRUE);
       meta_display_ungrab (display);
       return NULL;
     }
@@ -223,6 +229,7 @@ meta_window_new (MetaDisplay *display, Window xwindow,
             (state == IconicState || state == NormalState)))
         {
           meta_verbose ("Deciding not to manage unmapped or unviewable window 0x%lx\n", xwindow);
+          meta_error_trap_pop (display, TRUE);
           meta_display_ungrab (display);
           return NULL;
         }
@@ -232,7 +239,7 @@ meta_window_new (MetaDisplay *display, Window xwindow,
                     wm_state_to_string (existing_wm_state));
     }
   
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   
   XAddToSaveSet (display->xdisplay, xwindow);
 
@@ -258,10 +265,11 @@ meta_window_new (MetaDisplay *display, Window xwindow,
                                &set_attrs);
     }
   
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, FALSE) != Success)
     {
       meta_verbose ("Window 0x%lx disappeared just as we tried to manage it\n",
                     xwindow);
+      meta_error_trap_pop (display, FALSE);
       meta_display_ungrab (display);
       return NULL;
     }
@@ -645,7 +653,8 @@ meta_window_new (MetaDisplay *display, Window xwindow,
     meta_screen_unshow_desktop (window->screen);
   
   meta_window_queue_calc_showing (window);
-  
+
+  meta_error_trap_pop (display, FALSE); /* pop the XSync()-reducing trap */
   meta_display_ungrab (display);
   
   return window;
@@ -908,7 +917,7 @@ meta_window_free (MetaWindow  *window)
                        window->xwindow,
                        window->display->atom_net_wm_state);
       set_wm_state (window, WithdrawnState);
-      meta_error_trap_pop (window->display);
+      meta_error_trap_pop (window->display, FALSE);
     }
   else
     {
@@ -919,7 +928,7 @@ meta_window_free (MetaWindow  *window)
         {
           meta_error_trap_push (window->display);
           set_wm_state (window, NormalState);
-          meta_error_trap_pop (window->display);
+          meta_error_trap_pop (window->display, FALSE);
         }
 
       /* And we need to be sure the window is mapped so other WMs
@@ -928,7 +937,7 @@ meta_window_free (MetaWindow  *window)
       meta_error_trap_push (window->display);
       XMapWindow (window->display->xdisplay,
                   window->xwindow);
-      meta_error_trap_pop (window->display);
+      meta_error_trap_pop (window->display, FALSE);
     }
 
   meta_window_ungrab_keys (window);
@@ -955,7 +964,7 @@ meta_window_free (MetaWindow  *window)
                 window->xwindow,
                 NoEventMask);
   
-  meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
 
   if (window->icon)
     g_object_unref (G_OBJECT (window->icon));
@@ -978,7 +987,7 @@ meta_window_free (MetaWindow  *window)
   g_free (window);
 }
 
-static int
+static void
 set_wm_state (MetaWindow *window,
               int         state)
 {
@@ -998,10 +1007,10 @@ set_wm_state (MetaWindow *window,
                    window->display->atom_wm_state,
                    window->display->atom_wm_state,
                    32, PropModeReplace, (guchar*) data, 2);
-  return meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
 }
 
-static int
+static void
 set_net_wm_state (MetaWindow *window)
 {
   int i;
@@ -1063,7 +1072,7 @@ set_net_wm_state (MetaWindow *window)
                    window->display->atom_net_wm_state,
                    XA_ATOM,
                    32, PropModeReplace, (guchar*) data, i);
-  return meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
 }
 
 /* FIXME rename this, it makes it sound like map state is relevant */
@@ -1441,7 +1450,7 @@ meta_window_show (MetaWindow *window)
           window->unmaps_pending += 1;
           meta_error_trap_push (window->display);
           XUnmapWindow (window->display->xdisplay, window->xwindow);
-          meta_error_trap_pop (window->display);
+          meta_error_trap_pop (window->display, FALSE);
         }
 
       if (!window->iconic)
@@ -1459,7 +1468,7 @@ meta_window_show (MetaWindow *window)
           window->mapped = TRUE;
           meta_error_trap_push (window->display);
           XMapWindow (window->display->xdisplay, window->xwindow);
-          meta_error_trap_pop (window->display);
+          meta_error_trap_pop (window->display, FALSE);
           did_show = TRUE;
         }      
       
@@ -1545,7 +1554,7 @@ meta_window_hide (MetaWindow *window)
       window->unmaps_pending += 1;
       meta_error_trap_push (window->display);      
       XUnmapWindow (window->display->xdisplay, window->xwindow);
-      meta_error_trap_pop (window->display);
+      meta_error_trap_pop (window->display, FALSE);
       did_hide = TRUE;
     }
 
@@ -2370,7 +2379,7 @@ meta_window_move_resize_internal (MetaWindow  *window,
                         window->xwindow,
                         mask,
                         &values);
-      meta_error_trap_pop (window->display);
+      meta_error_trap_pop (window->display, FALSE);
     }
 
   if (!configure_frame_first && window->frame)
@@ -2866,7 +2875,7 @@ meta_window_focus (MetaWindow  *window,
           window->display->expected_focus_window = window;
         }
       
-      meta_error_trap_pop (window->display);
+      meta_error_trap_pop (window->display, FALSE);
     }
 }
 
@@ -2958,7 +2967,7 @@ meta_window_get_net_wm_desktop (MetaWindow *window)
     return meta_workspace_index (window->workspaces->data);
 }
 
-int
+void
 meta_window_set_current_workspace_hint (MetaWindow *window)
 {
   /* FIXME if on more than one workspace, we claim to be "sticky",
@@ -2969,7 +2978,7 @@ meta_window_set_current_workspace_hint (MetaWindow *window)
   if (window->workspaces == NULL)
     {
       /* this happens when unmanaging windows */      
-      return Success;
+      return;
     }
   
   data[0] = meta_window_get_net_wm_desktop (window);
@@ -2982,7 +2991,7 @@ meta_window_set_current_workspace_hint (MetaWindow *window)
                    window->display->atom_net_wm_desktop,
                    XA_CARDINAL,
                    32, PropModeReplace, (guchar*) data, 1);
-  return meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
 }
 
 void
@@ -3036,7 +3045,7 @@ meta_window_send_icccm_message (MetaWindow *window,
     meta_error_trap_push (window->display);
     XSendEvent (window->display->xdisplay,
                 window->xwindow, False, 0, (XEvent*) &ev);
-    meta_error_trap_pop (window->display);
+    meta_error_trap_pop (window->display, FALSE);
 }
 
 gboolean
@@ -3308,14 +3317,14 @@ meta_window_client_message (MetaWindow *window,
           char *str1;
           char *str2;
 
-          meta_error_trap_push (display);
+          meta_error_trap_push_with_return (display);
           str1 = XGetAtomName (display->xdisplay, first);
-          if (meta_error_trap_pop (display))
+          if (meta_error_trap_pop_with_return (display, TRUE) != Success)
             str1 = NULL;
 
-          meta_error_trap_push (display);
+          meta_error_trap_push_with_return (display);
           str2 = XGetAtomName (display->xdisplay, second); 
-          if (meta_error_trap_pop (display))
+          if (meta_error_trap_pop_with_return (display, TRUE) != Success)
             str2 = NULL;
           
           meta_verbose ("Request to change _NET_WM_STATE action %ld atom1: %s atom2: %s\n",
@@ -3528,7 +3537,7 @@ meta_window_client_message (MetaWindow *window,
                              &query_root_x, &query_root_y,
                              &x, &y,
                              &mask);
-              meta_error_trap_pop (window->display);
+              meta_error_trap_pop (window->display, TRUE);
 
               if (mask & Button1Mask)
                 button = 1;
@@ -3591,7 +3600,7 @@ meta_window_client_message (MetaWindow *window,
                            window->xwindow, window->display->atom_win_hints,
                            XA_CARDINAL, 32, PropModeReplace,
                            (unsigned char *)data, 1);
-          meta_error_trap_pop (window->display);
+          meta_error_trap_pop (window->display, FALSE);
         }
       else
         {
@@ -3603,7 +3612,7 @@ meta_window_client_message (MetaWindow *window,
           meta_error_trap_push (window->display);
           XDeleteProperty (window->display->xdisplay,
                            window->xwindow, window->display->atom_win_hints);
-          meta_error_trap_pop (window->display);
+          meta_error_trap_pop (window->display, FALSE);
         }
       
       return TRUE;
@@ -3705,7 +3714,7 @@ meta_window_notify_focus (MetaWindow *window,
           meta_error_trap_push (window->display);
           XInstallColormap (window->display->xdisplay,
                             window->colormap);
-          meta_error_trap_pop (window->display);
+          meta_error_trap_pop (window->display, FALSE);
 
           /* move into FOCUSED_WINDOW layer */
           meta_window_update_layer (window);
@@ -3741,7 +3750,7 @@ meta_window_notify_focus (MetaWindow *window,
           meta_error_trap_push (window->display);
           XUninstallColormap (window->display->xdisplay,
                               window->colormap);
-          meta_error_trap_pop (window->display);
+          meta_error_trap_pop (window->display, FALSE);
 
           /* move out of FOCUSED_WINDOW layer */
           meta_window_update_layer (window);
@@ -3922,7 +3931,7 @@ send_configure_notify (MetaWindow *window)
   XSendEvent (window->display->xdisplay,
               window->xwindow,
               False, StructureNotifyMask, &event);
-  meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
 }
 
 #define FLAG_TOGGLED_ON(old,new,flag) \
@@ -3985,7 +3994,7 @@ spew_size_hints_differences (const XSizeHints *old,
                 old->win_gravity, new->win_gravity);  
 }
 
-static int
+static void
 update_size_hints (MetaWindow *window)
 {
   int x, y, w, h;
@@ -4013,7 +4022,8 @@ update_size_hints (MetaWindow *window)
                      window->xwindow,
                      &window->size_hints,
                      &supplied);
-
+  meta_error_trap_pop (window->display, TRUE);
+  
   /* as far as I can tell, "supplied" is just
    * to check whether we had old-style normal hints
    * without gravity, base size as returned by
@@ -4165,17 +4175,13 @@ update_size_hints (MetaWindow *window)
   recalc_window_features (window);
 
   spew_size_hints_differences (&old_hints, &window->size_hints);
-  
-  return meta_error_trap_pop (window->display);
 }
 
-static int
+static void
 update_title (MetaWindow *window)
 {
   char *str;
   
-  meta_error_trap_push (window->display);
-  
   if (window->title)
     {
       g_free (window->title);
@@ -4222,11 +4228,9 @@ update_title (MetaWindow *window)
     meta_ui_set_frame_title (window->screen->ui,
                              window->frame->xwindow,
                              window->title);
-  
-  return meta_error_trap_pop (window->display);
 }
 
-static int
+static void
 update_protocols (MetaWindow *window)
 {
   Atom *protocols = NULL;
@@ -4263,10 +4267,10 @@ update_protocols (MetaWindow *window)
                 window->desc, window->take_focus, window->delete_window,
                 window->net_wm_ping);
   
-  return meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, TRUE);
 }
 
-static int
+static void
 update_wm_hints (MetaWindow *window)
 {
   XWMHints *hints;
@@ -4285,6 +4289,9 @@ update_wm_hints (MetaWindow *window)
   
   hints = XGetWMHints (window->display->xdisplay,
                        window->xwindow);
+
+  meta_error_trap_pop (window->display, TRUE);
+  
   if (hints)
     {
       if (hints->flags & InputHint)
@@ -4324,8 +4331,6 @@ update_wm_hints (MetaWindow *window)
       /* ensure this window is listed in the group for this group leader */
       meta_window_get_group (window);
     }
-  
-  return meta_error_trap_pop (window->display);
 }
 
 static void
@@ -4530,7 +4535,7 @@ meta_window_get_icon_geometry (MetaWindow    *window,
   return FALSE;
 }
 
-static int
+static void
 update_wm_class (MetaWindow *window)
 {
   XClassHint ch;
@@ -4543,14 +4548,14 @@ update_wm_class (MetaWindow *window)
   window->res_class = NULL;
   window->res_name = NULL;
 
-  meta_error_trap_push (window->display);
-
   ch.res_name = NULL;
   ch.res_class = NULL;
 
+  meta_error_trap_push (window->display);
   XGetClassHint (window->display->xdisplay,
                  window->xwindow,
                  &ch);
+  meta_error_trap_pop (window->display, TRUE);
 
   if (ch.res_name)
     {
@@ -4568,8 +4573,6 @@ update_wm_class (MetaWindow *window)
                 window->desc,
                 window->res_class ? window->res_class : "(null)",
                 window->res_name ? window->res_name : "(null)");
-  
-  return meta_error_trap_pop (window->display);
 }
 
 static Window
@@ -4683,7 +4686,7 @@ update_role (MetaWindow *window)
                 window->desc, window->role ? window->role : "null");
 }
 
-static int
+static void
 update_transient_for (MetaWindow *window)
 {
   Window w;
@@ -4693,6 +4696,7 @@ update_transient_for (MetaWindow *window)
   XGetTransientForHint (window->display->xdisplay,
                         window->xwindow,
                         &w);
+  meta_error_trap_pop (window->display, TRUE);
   window->xtransient_for = w;
 
   window->transient_parent_is_root_window =
@@ -4709,8 +4713,6 @@ update_transient_for (MetaWindow *window)
 
   /* update stacking constraints */
   meta_stack_update_transient (window->screen->stack, window);
-  
-  return meta_error_trap_pop (window->display);
 }
 
 static char*
@@ -4740,7 +4742,7 @@ get_text_property (MetaDisplay *display,
       meta_verbose ("XGetTextProperty() failed\n");
     }
   
-  meta_error_trap_pop (display);
+  meta_error_trap_pop (display, TRUE);
 
   return retval;
 }
@@ -4836,7 +4838,7 @@ update_net_wm_type (MetaWindow *window)
         {
           meta_error_trap_push (window->display);
           str = XGetAtomName (window->display->xdisplay, window->type_atom);
-          meta_error_trap_pop (window->display);
+          meta_error_trap_pop (window->display, TRUE);
         }
 
       meta_verbose ("Window %s type atom %s\n", window->desc,
@@ -4849,7 +4851,7 @@ update_net_wm_type (MetaWindow *window)
   recalc_window_type (window);
 }
 
-static int
+static void
 update_initial_workspace (MetaWindow *window)
 {
   gulong val = 0;
@@ -4883,17 +4885,13 @@ update_initial_workspace (MetaWindow *window)
                   "Read legacy GNOME workspace prop %d for %s\n",
                   window->initial_workspace, window->desc);
     }
-
-  return Success;
 }
 
-static int
+static void
 update_icon_name (MetaWindow *window)
 {
   char *str;
   
-  meta_error_trap_push (window->display);
-  
   if (window->icon_name)
     {
       g_free (window->icon_name);
@@ -4929,11 +4927,9 @@ update_icon_name (MetaWindow *window)
   
   if (window->icon_name == NULL)
     window->icon_name = g_strdup ("");
-  
-  return meta_error_trap_pop (window->display);
 }
 
-static int
+static void
 update_icon (MetaWindow *window)
 {
   GdkPixbuf *icon;
@@ -4965,8 +4961,6 @@ update_icon (MetaWindow *window)
   
   g_assert (window->icon);
   g_assert (window->mini_icon);
-  
-  return Success;
 }
 
 static void
@@ -5228,15 +5222,13 @@ recalc_window_type (MetaWindow *window)
     }
 }
 
-static int
+static void
 set_allowed_actions_hint (MetaWindow *window)
 {
 #define MAX_N_ACTIONS 8
   unsigned long data[MAX_N_ACTIONS];
   int i;
 
-
-
   i = 0;
   if (window->has_close_func)
     {
@@ -5284,7 +5276,7 @@ set_allowed_actions_hint (MetaWindow *window)
                    window->display->atom_net_wm_allowed_actions,
                    XA_ATOM,
                    32, PropModeReplace, (guchar*) data, i);
-  return meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
 #undef MAX_N_ACTIONS
 }
 
@@ -6000,7 +5992,7 @@ window_query_root_pointer (MetaWindow *window,
                  &win_y_return,
                  &mask_return);
 
-  meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, TRUE);
 
   if (x)
     *x = root_x_return;
@@ -6284,7 +6276,7 @@ meta_window_set_gravity (MetaWindow *window,
                            CWWinGravity,
                            &attrs);
   
-  meta_error_trap_pop (window->display);
+  meta_error_trap_pop (window->display, FALSE);
 }
 
 void
@@ -6553,7 +6545,7 @@ warp_pointer (MetaWindow *window,
         return FALSE;
     }
 
-  meta_error_trap_push (window->display);
+  meta_error_trap_push_with_return (window->display);
   
   XWarpPointer (window->display->xdisplay,
                 None,
@@ -6562,7 +6554,7 @@ warp_pointer (MetaWindow *window,
                 *x,
                 *y);
 
-  if (meta_error_trap_pop (window->display))
+  if (meta_error_trap_pop_with_return (window->display, FALSE) != Success)
     {
       meta_verbose ("Failed to warp pointer for window %s\n", window->desc);
       return FALSE;
diff --git a/src/window.h b/src/window.h
index a89f878e..fd69bbdc 100644
--- a/src/window.h
+++ b/src/window.h
@@ -372,7 +372,7 @@ gboolean meta_window_client_message    (MetaWindow *window,
 gboolean meta_window_notify_focus      (MetaWindow *window,
                                         XEvent     *event);
 
-int      meta_window_set_current_workspace_hint (MetaWindow *window);
+void     meta_window_set_current_workspace_hint (MetaWindow *window);
 
 unsigned long meta_window_get_net_wm_desktop (MetaWindow *window);
 
diff --git a/src/wm-tester/test-gravity.c b/src/wm-tester/test-gravity.c
index 0773af22..10c4bcbe 100644
--- a/src/wm-tester/test-gravity.c
+++ b/src/wm-tester/test-gravity.c
@@ -24,7 +24,8 @@ double screen_y_fraction[3] = { 0, 0.5, 1.0 };
 int screen_width;
 int screen_height;
 
-void calculate_position (int i, int *x, int *y)
+static void
+calculate_position (int i, int *x, int *y)
 {
   if (i == 9)
     {
diff --git a/src/workspace.c b/src/workspace.c
index 6def6904..a627dd0f 100644
--- a/src/workspace.c
+++ b/src/workspace.c
@@ -27,7 +27,7 @@
 
 void meta_workspace_queue_calc_showing  (MetaWorkspace *workspace);
 
-static int set_active_space_hint      (MetaScreen *screen);
+static void set_active_space_hint      (MetaScreen *screen);
 
 MetaWorkspace*
 meta_workspace_new (MetaScreen *screen)
@@ -279,7 +279,7 @@ meta_workspace_list_windows (MetaWorkspace *workspace)
   return workspace_windows;
 }
 
-static int
+static void
 set_active_space_hint (MetaScreen *screen)
 {
   unsigned long data[1];
@@ -293,7 +293,7 @@ set_active_space_hint (MetaScreen *screen)
                    screen->display->atom_net_current_desktop,
                    XA_CARDINAL,
                    32, PropModeReplace, (guchar*) data, 1);
-  return meta_error_trap_pop (screen->display);
+  meta_error_trap_pop (screen->display, FALSE);
 }
 
 void
diff --git a/src/xprops.c b/src/xprops.c
index 7b4ba84a..c57c8be9 100644
--- a/src/xprops.c
+++ b/src/xprops.c
@@ -49,7 +49,7 @@ check_type_and_format (MetaDisplay *display,
   type_name = XGetAtomName (display->xdisplay, type);
   expected_name = XGetAtomName (display->xdisplay, expected_type);
   prop_name = XGetAtomName (display->xdisplay, xatom);
-  meta_error_trap_pop (display);
+  meta_error_trap_pop (display, TRUE);
 
   meta_warning (_("Window 0x%lx has property %s that was expected to have type %s format %d and actually has type %s format %d n_items %d\n"),
                 xwindow,
@@ -85,18 +85,18 @@ meta_prop_get_atom_list (MetaDisplay *display,
   *atoms_p = NULL;
   *n_atoms_p = 0;
   
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   if (XGetWindowProperty (display->xdisplay, xwindow, xatom,
                           0, G_MAXLONG,
                           False, XA_ATOM, &type, &format, &n_atoms,
                           &bytes_after, (guchar **)&atoms) != Success ||
       type == None)
     {
-      meta_error_trap_pop (display);
+      meta_error_trap_pop_with_return (display, TRUE);
       return FALSE;
     }
 
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, TRUE) != Success)
     return FALSE;
     
   if (!check_type_and_format (display, xwindow, xatom, 32, XA_ATOM,
@@ -128,18 +128,18 @@ meta_prop_get_cardinal_list (MetaDisplay *display,
   *cardinals_p = NULL;
   *n_cardinals_p = 0;
   
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   if (XGetWindowProperty (display->xdisplay, xwindow, xatom,
                           0, G_MAXLONG,
                           False, XA_CARDINAL, &type, &format, &n_cardinals,
                           &bytes_after, (guchar **)&cardinals) != Success ||
       type == None)
     {
-      meta_error_trap_pop (display);
+      meta_error_trap_pop_with_return (display, TRUE);
       return FALSE;
     }
 
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, TRUE) != Success)
     return FALSE;
 
   if (!check_type_and_format (display, xwindow, xatom, 32, XA_CARDINAL,
@@ -174,18 +174,18 @@ meta_prop_get_motif_hints (MetaDisplay   *display,
 
   hints = NULL;
   n_items = 0;
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   if (XGetWindowProperty (display->xdisplay, xwindow, xatom,
                           0, MAX_ITEMS,
                           False, AnyPropertyType, &type, &format, &n_items,
                           &bytes_after, (guchar **)&hints) != Success ||
       type == None)
     {
-      meta_error_trap_pop (display);
+      meta_error_trap_pop_with_return (display, TRUE);
       return FALSE;
     }
 
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, TRUE) != Success)
     return FALSE;
   
   if (type == None || n_items <= 0)
@@ -225,18 +225,18 @@ meta_prop_get_latin1_string (MetaDisplay   *display,
   
   *str_p = NULL;
   
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   if (XGetWindowProperty (display->xdisplay, xwindow, xatom,
                           0, G_MAXLONG,
                           False, XA_STRING, &type, &format, &n_items,
                           &bytes_after, (guchar **)&str) != Success ||
       type == None)
     {
-      meta_error_trap_pop (display);
+      meta_error_trap_pop_with_return (display, TRUE);
       return FALSE;
     }
 
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, TRUE) != Success)
     return FALSE;
 
   if (!check_type_and_format (display, xwindow, xatom, 8, XA_STRING,
@@ -265,7 +265,7 @@ meta_prop_get_utf8_string (MetaDisplay   *display,
   
   *str_p = NULL;
   
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   if (XGetWindowProperty (display->xdisplay, xwindow, xatom,
                           0, G_MAXLONG,
                           False, display->atom_utf8_string,
@@ -273,11 +273,11 @@ meta_prop_get_utf8_string (MetaDisplay   *display,
                           &bytes_after, (guchar **)&str) != Success ||
       type == None)
     {
-      meta_error_trap_pop (display);
+      meta_error_trap_pop_with_return (display, TRUE);
       return FALSE;
     }
 
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, TRUE) != Success)
     return FALSE;
 
   if (!check_type_and_format (display, xwindow, xatom, 8,
@@ -326,7 +326,7 @@ meta_prop_get_utf8_list (MetaDisplay   *display,
   *str_p = NULL;
   *n_str_p = 0;
   
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   if (XGetWindowProperty (display->xdisplay, xwindow, xatom,
                           0, G_MAXLONG,
                           False, display->atom_utf8_string,
@@ -334,11 +334,11 @@ meta_prop_get_utf8_list (MetaDisplay   *display,
                           &bytes_after, (guchar **)&val) != Success ||
       type == None)
     {
-      meta_error_trap_pop (display);
+      meta_error_trap_pop_with_return (display, TRUE);
       return FALSE;
     }
 
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, TRUE) != Success)
     return FALSE;
 
   if (!check_type_and_format (display, xwindow, xatom, 8,
@@ -377,7 +377,7 @@ meta_prop_get_utf8_list (MetaDisplay   *display,
 
           meta_error_trap_push (display);
           name = XGetAtomName (display->xdisplay, xatom);
-          meta_error_trap_pop (display);
+          meta_error_trap_pop (display, TRUE);
           meta_warning (_("Property %s on window 0x%lx contained invalid UTF-8 for item %d in the list\n"),
                         name, xwindow, i);
           meta_XFree (name);
@@ -415,18 +415,18 @@ meta_prop_get_window (MetaDisplay   *display,
 
   *window_p = None;
   
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   if (XGetWindowProperty (display->xdisplay, xwindow, xatom,
                           0, G_MAXLONG,
                           False, XA_WINDOW, &type, &format, &n_items,
                           &bytes_after, (guchar **)&window) != Success ||
       type == None)
     {
-      meta_error_trap_pop (display);
+      meta_error_trap_pop_with_return (display, TRUE);
       return FALSE;
     }
 
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, TRUE) != Success)
     return FALSE;
 
   if (!check_type_and_format (display, xwindow, xatom, 32, XA_WINDOW,
@@ -469,18 +469,18 @@ meta_prop_get_cardinal_with_atom_type (MetaDisplay   *display,
 
   *cardinal_p = 0;
   
-  meta_error_trap_push (display);
+  meta_error_trap_push_with_return (display);
   if (XGetWindowProperty (display->xdisplay, xwindow, xatom,
                           0, G_MAXLONG,
                           False, prop_type, &type, &format, &n_items,
                           &bytes_after, (guchar **)&cardinal) != Success ||
       type == None)
     {
-      meta_error_trap_pop (display);
+      meta_error_trap_pop_with_return (display, TRUE);
       return FALSE;
     }
 
-  if (meta_error_trap_pop (display) != Success)
+  if (meta_error_trap_pop_with_return (display, TRUE) != Success)
     return FALSE;
 
   if (!check_type_and_format (display, xwindow, xatom, 32, prop_type,