From 16b9aff47caeb5a346ce722d21c52cbde0b46458 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Mon, 4 Oct 2004 20:32:59 +0000 Subject: [PATCH] Fix a variety of focus race conditions in all focus modes, or at least 2004-10-04 Elijah Newren Fix a variety of focus race conditions in all focus modes, or at least make them harder to trigger (fixes #152000) * src/core.[ch] (meta_core_user_lower_and_unfocus): add a timestamp parameter; pass it along to meta_workspace_focus_default_window * src/display.[ch] (meta_display_get_current_time_roundtrip): new function * src/display.c (event_callback): pass a timestamp to the meta_workspace_activate and meta_workspace_focus_default_window function calls * src/frames.c (meta_frames_button_press_event): pass a timestamp to meta_core_user_lower_and_unfocus * src/keybindings.c (handle_activate_workspace): pass a timestamp to meta_workspace_activate, (process_workspace_switch_grab): pass a timestamp to meta_workspace_focus_default_window and meta_workspace_activate, (handle_toggle_desktop): pass a timestamp to meta_workspace_focus_default_window, (do_handle_move_to_workspace): pass a timestamp to meta_workspace_activate_with_focus, (handle_workspace_switch): meta_workspace_activate * src/screen.c (meta_screen_new): pass a timestamp to meta_workspace_activate * src/window.c (meta_window_free): pass a timestamp to meta_workspace_focus_default_window, (idle_calc_showing): don't increment the focus sentinel here, (meta_window_minimize): pass a timestamp to meta_workspace_focus_default_window, (meta_window_client_message), pass a timestamp to meta_workspace_focus_default_window * src/workspace.h (meta_workspace_activate): add timestamp parameter, (meta_workspace_activate_with_focus): add timestamp parameter, (meta_workspace_focus_default_window): add timestamp parameter * src/workspace.c (meta_workspace_focus_mru_window): make this function take a timestamp and use it for meta_window_focus or XSetInputFocus, (meta_workspace_activate_with_focus): make this function take a timestamp and pass it along to meta_window_focus and meta_workspace_focus_default_window, (meta_workspace_activate): make this function take a timestamp and pass it to meta_workspace_activate_with_focus), (meta_workspace_focus_default_window): make this function take a timestamp, warn if its 0 but try to handle that case sanely, and pass the timestamp on to meta_window_focus or meta_workspace_focus_mru_window or XSetInputFocus --- ChangeLog | 55 +++++++++++++++++++++++++++++++++++++++++ src/core.c | 7 ++++-- src/core.h | 3 ++- src/display.c | 33 +++++++++++++++++++++++-- src/display.h | 3 ++- src/frames.c | 4 ++- src/keybindings.c | 18 ++++++++------ src/screen.c | 6 ++--- src/window.c | 27 +++------------------ src/workspace.c | 62 ++++++++++++++++++++++++++++++++--------------- src/workspace.h | 9 ++++--- 11 files changed, 165 insertions(+), 62 deletions(-) diff --git a/ChangeLog b/ChangeLog index 77c32c550..c8db6a9d6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,58 @@ +2004-10-04 Elijah Newren + + Fix a variety of focus race conditions in all focus modes, or at + least make them harder to trigger (fixes #152000) + + * src/core.[ch] (meta_core_user_lower_and_unfocus): add a + timestamp parameter; pass it along to + meta_workspace_focus_default_window + + * src/display.[ch] (meta_display_get_current_time_roundtrip): new + function + + * src/display.c (event_callback): pass a timestamp to the + meta_workspace_activate and meta_workspace_focus_default_window + function calls + + * src/frames.c (meta_frames_button_press_event): pass a timestamp + to meta_core_user_lower_and_unfocus + + * src/keybindings.c (handle_activate_workspace): pass a timestamp + to meta_workspace_activate, (process_workspace_switch_grab): pass + a timestamp to meta_workspace_focus_default_window and + meta_workspace_activate, (handle_toggle_desktop): pass a timestamp + to meta_workspace_focus_default_window, + (do_handle_move_to_workspace): pass a timestamp to + meta_workspace_activate_with_focus, (handle_workspace_switch): + meta_workspace_activate + + * src/screen.c (meta_screen_new): pass a timestamp to + meta_workspace_activate + + * src/window.c (meta_window_free): pass a timestamp to + meta_workspace_focus_default_window, (idle_calc_showing): don't + increment the focus sentinel here, (meta_window_minimize): pass a + timestamp to meta_workspace_focus_default_window, + (meta_window_client_message), pass a timestamp to + meta_workspace_focus_default_window + + * src/workspace.h (meta_workspace_activate): add timestamp + parameter, (meta_workspace_activate_with_focus): add timestamp + parameter, (meta_workspace_focus_default_window): add timestamp + parameter + + * src/workspace.c (meta_workspace_focus_mru_window): make this + function take a timestamp and use it for meta_window_focus or + XSetInputFocus, (meta_workspace_activate_with_focus): make this + function take a timestamp and pass it along to meta_window_focus + and meta_workspace_focus_default_window, + (meta_workspace_activate): make this function take a timestamp and + pass it to meta_workspace_activate_with_focus), + (meta_workspace_focus_default_window): make this function take a + timestamp, warn if its 0 but try to handle that case sanely, and + pass the timestamp on to meta_window_focus or + meta_workspace_focus_mru_window or XSetInputFocus + 2004-09-22 Elijah Newren * src/keybindings.c (process_workspace_switch_grab): Focus the diff --git a/src/core.c b/src/core.c index cf8cc115a..437a00972 100644 --- a/src/core.c +++ b/src/core.c @@ -237,7 +237,8 @@ meta_core_user_raise (Display *xdisplay, void meta_core_user_lower_and_unfocus (Display *xdisplay, - Window frame_xwindow) + Window frame_xwindow, + Time timestamp) { MetaDisplay *display; MetaWindow *window; @@ -277,7 +278,9 @@ meta_core_user_lower_and_unfocus (Display *xdisplay, /* focus on the (new) topmost window */ if (window->has_focus) - meta_workspace_focus_default_window (window->screen->active_workspace, window); + meta_workspace_focus_default_window (window->screen->active_workspace, + window, + timestamp); } } diff --git a/src/core.h b/src/core.h index 0dac163d9..2c7c4e68c 100644 --- a/src/core.h +++ b/src/core.h @@ -61,7 +61,8 @@ void meta_core_user_resize (Display *xdisplay, void meta_core_user_raise (Display *xdisplay, Window frame_xwindow); void meta_core_user_lower_and_unfocus (Display *xdisplay, - Window frame_xwindow); + Window frame_xwindow, + Time timestamp); void meta_core_user_focus (Display *xdisplay, Window frame_xwindow, diff --git a/src/display.c b/src/display.c index 73d584bc5..1214e8d47 100644 --- a/src/display.c +++ b/src/display.c @@ -1099,6 +1099,35 @@ meta_display_get_current_time (MetaDisplay *display) return display->current_time; } +/* Get a timestamp, even if it means a roundtrip */ +guint32 +meta_display_get_current_time_roundtrip (MetaDisplay *display) +{ + guint32 timestamp; + + timestamp = meta_display_get_current_time (display); + if (timestamp == CurrentTime) + { + XEvent property_event; + + /* Using the property XA_PRIMARY because it's safe; nothing + * would use it as a property. The type doesn't matter. + */ + XChangeProperty (display->xdisplay, + display->leader_window, + XA_PRIMARY, XA_STRING, 8, + PropModeAppend, NULL, 0); + XWindowEvent (display->xdisplay, + display->leader_window, + PropertyChangeMask, + &property_event); + + timestamp = property_event.xproperty.time; + } + + return timestamp; +} + static void add_ignored_serial (MetaDisplay *display, unsigned long serial) @@ -2100,7 +2129,7 @@ event_callback (XEvent *event, space); if (workspace) - meta_workspace_activate (workspace); + meta_workspace_activate (workspace, meta_display_get_current_time_roundtrip (display)); else meta_verbose ("Don't know about workspace %d\n", space); } @@ -2129,7 +2158,7 @@ event_callback (XEvent *event, else { meta_screen_unshow_desktop (screen); - meta_workspace_focus_default_window (screen->active_workspace, NULL); + meta_workspace_focus_default_window (screen->active_workspace, NULL, meta_display_get_current_time_roundtrip (display)); } } else if (event->xclient.message_type == diff --git a/src/display.h b/src/display.h index d944d5f1c..abdb7f0f0 100644 --- a/src/display.h +++ b/src/display.h @@ -422,7 +422,8 @@ void meta_display_increment_event_serial (MetaDisplay *display); void meta_display_update_active_window_hint (MetaDisplay *display); -guint32 meta_display_get_current_time (MetaDisplay *display); +guint32 meta_display_get_current_time (MetaDisplay *display); +guint32 meta_display_get_current_time_roundtrip (MetaDisplay *display); /* utility goo */ const char* meta_event_mode_to_string (int m); diff --git a/src/frames.c b/src/frames.c index f37dfd19f..e37e48fa5 100644 --- a/src/frames.c +++ b/src/frames.c @@ -1372,7 +1372,9 @@ meta_frames_button_press_event (GtkWidget *widget, } else if (event->button == 2) { - meta_core_user_lower_and_unfocus (gdk_display, frame->xwindow); + meta_core_user_lower_and_unfocus (gdk_display, + frame->xwindow, + event->time); } else if (event->button == 3) { diff --git a/src/keybindings.c b/src/keybindings.c index 8d364d770..8013982d2 100644 --- a/src/keybindings.c +++ b/src/keybindings.c @@ -2474,7 +2474,7 @@ handle_activate_workspace (MetaDisplay *display, if (workspace) { - meta_workspace_activate (workspace); + meta_workspace_activate (workspace, event->xkey.time); } else { @@ -2680,7 +2680,9 @@ process_workspace_switch_grab (MetaDisplay *display, meta_topic (META_DEBUG_KEYBINDINGS, "Focusing default window on target workspace\n"); - meta_workspace_focus_default_window (target_workspace, NULL); + meta_workspace_focus_default_window (target_workspace, + NULL, + event->xkey.time); return TRUE; /* we already ended the grab */ } @@ -2749,7 +2751,7 @@ process_workspace_switch_grab (MetaDisplay *display, meta_topic (META_DEBUG_KEYBINDINGS, "Activating target workspace\n"); - meta_workspace_activate (target_workspace); + meta_workspace_activate (target_workspace, event->xkey.time); return TRUE; /* we already ended the grab */ } @@ -2760,7 +2762,7 @@ process_workspace_switch_grab (MetaDisplay *display, "Ending workspace tabbing & focusing default window; uninteresting key pressed\n"); workspace = (MetaWorkspace *) meta_ui_tab_popup_get_selected (screen->tab_popup); - meta_workspace_focus_default_window (workspace, NULL); + meta_workspace_focus_default_window (workspace, NULL, event->xkey.time); return FALSE; } @@ -2774,7 +2776,9 @@ handle_toggle_desktop (MetaDisplay *display, if (screen->showing_desktop) { meta_screen_unshow_desktop (screen); - meta_workspace_focus_default_window (screen->active_workspace, NULL); + meta_workspace_focus_default_window (screen->active_workspace, + NULL, + event->xkey.time); } else meta_screen_show_desktop (screen); @@ -3210,7 +3214,7 @@ do_handle_move_to_workspace (MetaDisplay *display, /* Activate second, so the window is never unmapped */ meta_window_change_workspace (window, workspace); if (flip) - meta_workspace_activate_with_focus (workspace, window); + meta_workspace_activate_with_focus (workspace, window, event->xkey.time); } else { @@ -3374,7 +3378,7 @@ handle_workspace_switch (MetaDisplay *display, meta_display_end_grab_op (display, event->xkey.time); } - meta_workspace_activate (next); + meta_workspace_activate (next, event->xkey.time); if (grabbed_before_release) { diff --git a/src/screen.c b/src/screen.c index 9116aae24..35f8e2932 100644 --- a/src/screen.c +++ b/src/screen.c @@ -604,7 +604,7 @@ meta_screen_new (MetaDisplay *display, /* Screens must have at least one workspace at all times, * so create that required workspace. */ - meta_workspace_activate (meta_workspace_new (screen)); + meta_workspace_activate (meta_workspace_new (screen), timestamp); update_num_workspaces (screen); set_workspace_names (screen); @@ -641,7 +641,7 @@ meta_screen_new (MetaDisplay *display, current_workspace); if (space != NULL) - meta_workspace_activate (space); + meta_workspace_activate (space, timestamp); } meta_compositor_manage_screen (screen->display->compositor, @@ -1077,7 +1077,7 @@ update_num_workspaces (MetaScreen *screen) } if (need_change_space) - meta_workspace_activate (last_remaining); + meta_workspace_activate (last_remaining, meta_display_get_current_time_roundtrip (screen->display)); /* Should now be safe to free the workspaces */ tmp = extras; diff --git a/src/window.c b/src/window.c index a3a707eda..9bc5e687b 100644 --- a/src/window.c +++ b/src/window.c @@ -931,7 +931,7 @@ meta_window_free (MetaWindow *window) meta_topic (META_DEBUG_FOCUS, "Focusing default window since we're unmanaging %s\n", window->desc); - meta_workspace_focus_default_window (window->screen->active_workspace, window); + meta_workspace_focus_default_window (window->screen->active_workspace, window, meta_display_get_current_time_roundtrip (window->display)); } else if (window->display->expected_focus_window == window) { @@ -939,7 +939,7 @@ meta_window_free (MetaWindow *window) "Focusing default window since expected focus window freed %s\n", window->desc); window->display->expected_focus_window = NULL; - meta_workspace_focus_default_window (window->screen->active_workspace, window); + meta_workspace_focus_default_window (window->screen->active_workspace, window, meta_display_get_current_time_roundtrip (window->display)); } else { @@ -1487,25 +1487,6 @@ idle_calc_showing (gpointer data) tmp = tmp->next; } - /* for all displays used in the queue, set a sentinel property on - * the root window so that we can ignore EnterNotify events that - * occur before the window maps occur. This avoids a race - * condition. - */ - tmp = should_show; - while (tmp != NULL) - { - MetaWindow *window = tmp->data; - - if (g_slist_find (displays, window->display) == NULL) - { - displays = g_slist_prepend (displays, window->display); - meta_display_increment_focus_sentinel (window->display); - } - - tmp = tmp->next; - } - g_slist_free (copy); g_slist_free (unplaced); @@ -1898,7 +1879,7 @@ meta_window_minimize (MetaWindow *window) meta_topic (META_DEBUG_FOCUS, "Focusing default window due to minimization of focus window %s\n", window->desc); - meta_workspace_focus_default_window (window->screen->active_workspace, window); + meta_workspace_focus_default_window (window->screen->active_workspace, window, meta_display_get_current_time_roundtrip (window->display)); } else { @@ -4021,7 +4002,7 @@ meta_window_client_message (MetaWindow *window, meta_topic (META_DEBUG_FOCUS, "Focusing default window because of minimization of former focus window %s, which was due to a wm_change_state client message\n", window->desc); - meta_workspace_focus_default_window (window->screen->active_workspace, window); + meta_workspace_focus_default_window (window->screen->active_workspace, window, meta_display_get_current_time_roundtrip (window->display)); } } diff --git a/src/workspace.c b/src/workspace.c index cd43661d9..6b03ce406 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -30,7 +30,8 @@ void meta_workspace_queue_calc_showing (MetaWorkspace *workspace); static void set_active_space_hint (MetaScreen *screen); static void meta_workspace_focus_mru_window (MetaWorkspace *workspace, - MetaWindow *not_this_one); + MetaWindow *not_this_one, + Time timestamp); static void maybe_add_to_list (MetaScreen *screen, MetaWindow *window, gpointer data) @@ -270,7 +271,8 @@ meta_workspace_queue_calc_showing (MetaWorkspace *workspace) void meta_workspace_activate_with_focus (MetaWorkspace *workspace, - MetaWindow *focus_this) + MetaWindow *focus_this, + Time timestamp) { MetaWorkspace *old; MetaWindow *move_window; @@ -322,8 +324,7 @@ meta_workspace_activate_with_focus (MetaWorkspace *workspace, if (focus_this) { - meta_window_focus (focus_this, - meta_display_get_current_time (focus_this->display)); + meta_window_focus (focus_this, timestamp); meta_window_raise (focus_this); } else if (move_window) @@ -333,15 +334,15 @@ meta_workspace_activate_with_focus (MetaWorkspace *workspace, else { meta_topic (META_DEBUG_FOCUS, "Focusing default window on new workspace\n"); - meta_workspace_focus_default_window (workspace, NULL); + meta_workspace_focus_default_window (workspace, NULL, timestamp); } } void -meta_workspace_activate (MetaWorkspace *workspace) +meta_workspace_activate (MetaWorkspace *workspace, + Time timestamp) { - meta_workspace_activate_with_focus (workspace, - NULL); + meta_workspace_activate_with_focus (workspace, NULL, timestamp); } int @@ -790,10 +791,18 @@ meta_workspace_get_name (MetaWorkspace *workspace) void meta_workspace_focus_default_window (MetaWorkspace *workspace, - MetaWindow *not_this_one) + MetaWindow *not_this_one, + Time timestamp) { + if (timestamp == CurrentTime) + { + meta_warning ("CurrentTime used to choose focus window; " + "focus window may not be correct.\n"); + } + + if (meta_prefs_get_focus_mode () == META_FOCUS_MODE_CLICK) - meta_workspace_focus_mru_window (workspace, not_this_one); + meta_workspace_focus_mru_window (workspace, not_this_one, timestamp); else { MetaWindow * window; @@ -802,13 +811,28 @@ meta_workspace_focus_default_window (MetaWorkspace *workspace, window->type != META_WINDOW_DOCK && window->type != META_WINDOW_DESKTOP) { - meta_topic (META_DEBUG_FOCUS, - "Focusing mouse window %s\n", window->desc); + if (timestamp == CurrentTime) + { - meta_window_focus (window, meta_display_get_current_time (workspace->screen->display)); + /* We would like for this to never happen. However, if + * it does happen then we kludge since using CurrentTime + * can mean ugly race conditions--and we can avoid these + * by allowing EnterNotify events (which come with + * timestamps) to handle focus. + */ + + meta_topic (META_DEBUG_FOCUS, + "Not focusing mouse window %s because EnterNotify events should handle that\n", window->desc); + } + else + { + meta_topic (META_DEBUG_FOCUS, + "Focusing mouse window %s\n", window->desc); + meta_window_focus (window, timestamp); + } } else if (meta_prefs_get_focus_mode () == META_FOCUS_MODE_SLOPPY) - meta_workspace_focus_mru_window (workspace, not_this_one); + meta_workspace_focus_mru_window (workspace, not_this_one, timestamp); else if (meta_prefs_get_focus_mode () == META_FOCUS_MODE_MOUSE) { meta_topic (META_DEBUG_FOCUS, @@ -817,7 +841,7 @@ meta_workspace_focus_default_window (MetaWorkspace *workspace, XSetInputFocus (workspace->screen->display->xdisplay, workspace->screen->display->no_focus_window, RevertToPointerRoot, - meta_display_get_current_time (workspace->screen->display)); + timestamp); } } } @@ -825,7 +849,8 @@ meta_workspace_focus_default_window (MetaWorkspace *workspace, /* Focus MRU window (or top window if failed) on active workspace */ void meta_workspace_focus_mru_window (MetaWorkspace *workspace, - MetaWindow *not_this_one) + MetaWindow *not_this_one, + Time timestamp) { MetaWindow *window = NULL; GList *tmp; @@ -857,8 +882,7 @@ meta_workspace_focus_mru_window (MetaWorkspace *workspace, meta_topic (META_DEBUG_FOCUS, "Focusing workspace MRU window %s\n", window->desc); - meta_window_focus (window, - meta_display_get_current_time (workspace->screen->display)); + meta_window_focus (window, timestamp); /* Also raise the window if in click-to-focus */ if (meta_prefs_get_focus_mode () == META_FOCUS_MODE_CLICK) @@ -870,6 +894,6 @@ meta_workspace_focus_mru_window (MetaWorkspace *workspace, XSetInputFocus (workspace->screen->display->xdisplay, workspace->screen->display->no_focus_window, RevertToPointerRoot, - meta_display_get_current_time (workspace->screen->display)); + timestamp); } } diff --git a/src/workspace.h b/src/workspace.h index 4dc0df9db..018d1dfbe 100644 --- a/src/workspace.h +++ b/src/workspace.h @@ -63,8 +63,10 @@ void meta_workspace_relocate_windows (MetaWorkspace *workspace, gboolean meta_workspace_contains_window (MetaWorkspace *workspace, MetaWindow *window); void meta_workspace_activate_with_focus (MetaWorkspace *workspace, - MetaWindow *focus_this); -void meta_workspace_activate (MetaWorkspace *workspace); + MetaWindow *focus_this, + Time timestamp); +void meta_workspace_activate (MetaWorkspace *workspace, + Time timestamp); int meta_workspace_index (MetaWorkspace *workspace); GList* meta_workspace_list_windows (MetaWorkspace *workspace); @@ -78,7 +80,8 @@ void meta_workspace_get_work_area_all_xineramas (MetaWorkspace *workspace, MetaRectangle *area); void meta_workspace_focus_default_window (MetaWorkspace *workspace, - MetaWindow *not_this_one); + MetaWindow *not_this_one, + Time timestamp); MetaWorkspace* meta_workspace_get_neighbor (MetaWorkspace *workspace, MetaMotionDirection direction);