From 577624adefc73d8f724a9e7319ac8087d9bae825 Mon Sep 17 00:00:00 2001 From: Daniel Drake Date: Mon, 30 Dec 2013 13:01:28 -0600 Subject: [PATCH] Reduce server grabs during window creation Remove some obvious server grabs from the window creation codepath, also ones that are taken at startup. During startup, there is no need to grab: we install the event handlers before querying for the already-existing windows, so there is no danger that we will 'lose' some window. We might try to create a window twice (if it comes back in the original query and then we get an event for it) but the code is already protected against such conditions. When windows are created later, we also do not need grabs, we just need appropriate error checking as the window may be destroyed at any time (or it may have already been destroyed). The stack tracker is unaffected here - as it listens to CreateNotify and DestroyNotify events and responds directly, the internal stack representation will always be consistent even if the window goes away while we are processing MapRequest or similar. Now that there are no grabs we don't have to worry about explicitly calling display_notify_window after grabs have been dropped. Fold that into meta_window_new_shared(). https://bugzilla.gnome.org/show_bug.cgi?id=721345 --- src/core/display.c | 4 ---- src/core/screen.c | 4 ---- src/core/window.c | 39 ++++++++++++--------------------------- 3 files changed, 12 insertions(+), 35 deletions(-) diff --git a/src/core/display.c b/src/core/display.c index eced28740..2313c8823 100644 --- a/src/core/display.c +++ b/src/core/display.c @@ -825,8 +825,6 @@ meta_display_open (void) enable_compositor (the_display); - meta_display_grab (the_display); - /* Now manage all existing windows */ tmp = the_display->screens; while (tmp != NULL) @@ -894,8 +892,6 @@ meta_display_open (void) meta_idle_monitor_init_dbus (); - meta_display_ungrab (the_display); - /* Done opening new display */ the_display->display_opening = FALSE; diff --git a/src/core/screen.c b/src/core/screen.c index 2fc9bfef2..939412544 100644 --- a/src/core/screen.c +++ b/src/core/screen.c @@ -887,8 +887,6 @@ meta_screen_manage_all_windows (MetaScreen *screen) MetaStackWindow *children; int n_children, i; - meta_display_grab (screen->display); - if (screen->guard_window == None) screen->guard_window = meta_screen_create_guard_window (screen->display->xdisplay, screen); @@ -907,8 +905,6 @@ meta_screen_manage_all_windows (MetaScreen *screen) g_free (children); meta_stack_thaw (screen->stack); - - meta_display_ungrab (screen->display); } /** diff --git a/src/core/window.c b/src/core/window.c index db34afe89..5739367de 100644 --- a/src/core/window.c +++ b/src/core/window.c @@ -1329,12 +1329,6 @@ meta_window_new_shared (MetaDisplay *display, window->constructing = FALSE; - return window; -} - -static void -display_notify_window (MetaDisplay *display, MetaWindow *window) -{ meta_display_notify_window_created (display, window); if (window->wm_state_demands_attention) @@ -1342,6 +1336,8 @@ display_notify_window (MetaDisplay *display, MetaWindow *window) if (window->wm_hints_urgent) g_signal_emit_by_name (window->display, "window-marked-urgent", window); + + return window; } MetaWindow * @@ -1376,15 +1372,12 @@ meta_window_new_for_wayland (MetaDisplay *display, attrs.override_redirect = 0; attrs.screen = scr->xscreen; - /* XXX: Note: In the Wayland case we currently still grab the - * xserver and trap X errors while creating a MetaWindow because we - * will still be making various redundant X requests (passing a - * window xid of None) until we thoroughly audit all the code to - * make sure it knows about non X based clients... + /* XXX: Note: In the Wayland case we currently still trap X errors while + * creating a MetaWindow because we will still be making various redundant + * X requests (passing a window xid of None) until we thoroughly audit all + * the code to make sure it knows about non X based clients... */ - /* Grab server */ - meta_display_grab (display); meta_error_trap_push (display); /* Push a trap over all of window * creation, to reduce XSync() calls */ @@ -1400,14 +1393,12 @@ meta_window_new_for_wayland (MetaDisplay *display, &attrs); meta_error_trap_pop (display); /* pop the XSync()-reducing trap */ - meta_display_ungrab (display); /* XXX: Maybe this could be called in meta_window_new_shared() but * before splitting the X11 specific code out it came after the - * meta_display_ungrab() and we wanted to minimize the risk of + * meta_error_trap_pop() and we wanted to minimize the risk of * breaking something. */ - display_notify_window (window->display, window); return window; } @@ -1434,11 +1425,14 @@ meta_window_new (MetaDisplay *display, return NULL; } - /* Grab server */ - meta_display_grab (display); meta_error_trap_push (display); /* Push a trap over all of window * creation, to reduce XSync() calls */ + /* + * This function executes without any server grabs held. This means that + * the window could have already gone away, or could go away at any point, + * so we must be careful with X error handling. + */ if (!XGetWindowAttributes (display->xdisplay, xwindow, &attrs)) { @@ -1581,20 +1575,11 @@ meta_window_new (MetaDisplay *display, &attrs); meta_error_trap_pop (display); /* pop the XSync()-reducing trap */ - meta_display_ungrab (display); - - /* XXX: Maybe this could be called in meta_window_new_shared() but - * before splitting the X11 specific code out it came after the - * meta_display_ungrab() and we wanted to minimize the risk of - * breaking something. - */ - display_notify_window (display, window); return window; error: meta_error_trap_pop (display); - meta_display_ungrab (display); return NULL; }