From 50312dd0e89daf704fb2849dc25bd332aa07eebe Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sun, 20 Feb 2005 17:14:16 +0000 Subject: [PATCH] Big patch to cover about 6 different issues in order to correct rare 2005-02-20 Elijah Newren Big patch to cover about 6 different issues in order to correct rare problems with timestamps (make sure window selected in tasklist actually gets focus, sanity check timestamps to avoid rogue apps hosing the system, correct the updating of net_wm_user_time, correctly handle timestamps of 0 when comparing xserver timestamps for those who have had their systems up for over 25 days or so, add some debugging information to verbose logs, some code cleanups). Fixes all issues listed in #167358. * src/display.h: (struct _MetaDisplay): clarify comment on last_focus_time, introduce a new variable--last_user_time, (XSERVER_TIME_IS_BEFORE macro): put this functionality into a separate macro and then introduce a new macro with this name that uses the old one but adds additional special-case checks for timestamps that are 0, (comment to meta_display_set_input_focus_window): add information about how last_user_time should be used in this function * src/display.c (santiy_check_timestamps): new function, (meta_display_open): intialize display->last_user_time, (meta_display_get_current_time_roundtrip): use the timestamp, which is known to be good, in order to sanity_check_timestamps, (event_callback): use the new meta_window_ste_user_time() function in order to correct problems, use the timestamp of KeyPress and ButtonPress events, which are known to be good, in order to sanity_check_timestamps, (timestamp_too_old): new function for common behavior of meta_display_focus_the_no_focus_window and meta_display_set_input_focus_window, with added checking for display->last_user_time in addition to display->last_focus_time, (meta_display_set_input_focus_window): replace some of the code with a call to timestamp_too_old(), (meta_display_focus_the_no_focus_window): replace some of th ecode with a call to timestamp_too_old() * src/window.h: (meta_window_set_user_time): new function to abstract the many things that need to be done when updating the net_wm_user_time of any window * src/window.c: (meta_window_activate): add debugging spew, make sure the comparison is made with last_user_time NOT last_focus_time, use meta_window_set_user_time() function in order to correct problems, (meta_window_client_message): add a newline to a debugging message to make them easier to read, (meta_window_set_user_time): new function * src/window-props.c (reload_net_wm_user_time): use the new meta_window_ste_user_time() function in order to correct problems --- ChangeLog | 50 +++++++++++++++++ src/display.c | 136 ++++++++++++++++++++++++++++++++++++--------- src/display.h | 31 ++++++++--- src/window-props.c | 7 +-- src/window.c | 46 +++++++++++++-- src/window.h | 3 + 6 files changed, 229 insertions(+), 44 deletions(-) diff --git a/ChangeLog b/ChangeLog index f129c5797..e152a4f03 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,53 @@ +2005-02-20 Elijah Newren + + Big patch to cover about 6 different issues in order to correct + rare problems with timestamps (make sure window selected in + tasklist actually gets focus, sanity check timestamps to avoid + rogue apps hosing the system, correct the updating of + net_wm_user_time, correctly handle timestamps of 0 when comparing + xserver timestamps for those who have had their systems up for + over 25 days or so, add some debugging information to verbose + logs, some code cleanups). Fixes all issues listed in #167358. + + * src/display.h: (struct _MetaDisplay): clarify comment on + last_focus_time, introduce a new variable--last_user_time, + (XSERVER_TIME_IS_BEFORE macro): put this functionality into a + separate macro and then introduce a new macro with this name that + uses the old one but adds additional special-case checks for + timestamps that are 0, (comment to + meta_display_set_input_focus_window): add information about how + last_user_time should be used in this function + + * src/display.c (santiy_check_timestamps): new function, + (meta_display_open): intialize display->last_user_time, + (meta_display_get_current_time_roundtrip): use the timestamp, + which is known to be good, in order to sanity_check_timestamps, + (event_callback): use the new meta_window_ste_user_time() function + in order to correct problems, use the timestamp of KeyPress and + ButtonPress events, which are known to be good, in order to + sanity_check_timestamps, (timestamp_too_old): new function for + common behavior of meta_display_focus_the_no_focus_window and + meta_display_set_input_focus_window, with added checking for + display->last_user_time in addition to display->last_focus_time, + (meta_display_set_input_focus_window): replace some of the code + with a call to timestamp_too_old(), + (meta_display_focus_the_no_focus_window): replace some of th ecode + with a call to timestamp_too_old() + + * src/window.h: (meta_window_set_user_time): new function to + abstract the many things that need to be done when updating the + net_wm_user_time of any window + + * src/window.c: (meta_window_activate): add debugging spew, make + sure the comparison is made with last_user_time NOT + last_focus_time, use meta_window_set_user_time() function in order + to correct problems, (meta_window_client_message): add a newline + to a debugging message to make them easier to read, + (meta_window_set_user_time): new function + + * src/window-props.c (reload_net_wm_user_time): use the new + meta_window_ste_user_time() function in order to correct problems + 2005-02-16 Elijah Newren * src/display.c: (event_callback): trivial fix to a log message: diff --git a/src/display.c b/src/display.c index 8b0c8c98f..33c0d8f14 100644 --- a/src/display.c +++ b/src/display.c @@ -115,6 +115,9 @@ static void update_window_grab_modifiers (MetaDisplay *display); static void prefs_changed_callback (MetaPreference pref, void *data); +static void sanity_check_timestamps (MetaDisplay *display, + Time known_good_timestamp); + static void set_utf8_string_hint (MetaDisplay *display, Window xwindow, @@ -645,6 +648,7 @@ meta_display_open (const char *name) } display->last_focus_time = timestamp; + display->last_user_time = timestamp; display->compositor = meta_compositor_new (display); screens = NULL; @@ -1146,6 +1150,8 @@ meta_display_get_current_time_roundtrip (MetaDisplay *display) timestamp = property_event.xproperty.time; } + sanity_check_timestamps (display, timestamp); + return timestamp; } @@ -1563,11 +1569,8 @@ event_callback (XEvent *event, if (window && ((event->type == KeyPress) || (event->type == ButtonPress))) { g_assert (CurrentTime != display->current_time); - meta_topic (META_DEBUG_WINDOW_STATE, - "Metacity set %s's net_wm_user_time to %lu.\n", - window->desc, display->current_time); - window->net_wm_user_time_set = TRUE; - window->net_wm_user_time = display->current_time; + meta_window_set_user_time (window, display->current_time); + sanity_check_timestamps (display, display->current_time); } switch (event->type) @@ -4648,21 +4651,114 @@ meta_display_focus_sentinel_clear (MetaDisplay *display) return (display->sentinel_counter == 0); } +static void +sanity_check_timestamps (MetaDisplay *display, + Time timestamp) +{ + if (XSERVER_TIME_IS_BEFORE (timestamp, display->last_focus_time)) + { + meta_warning ("last_focus_time (%lu) is greater than comparison " + "timestamp (%lu). This most likely represents a buggy " + "client sending inaccurate timestamps in messages such as " + "_NET_ACTIVE_WINDOW. Trying to work around...\n", + display->last_focus_time, (unsigned long)timestamp); + display->last_focus_time = timestamp; + } + if (XSERVER_TIME_IS_BEFORE (timestamp, display->last_user_time)) + { + GSList *windows; + GSList *tmp; + + meta_warning ("last_user_time (%lu) is greater than comparison " + "timestamp (%lu). This most likely represents a buggy " + "client sending inaccurate timestamps in messages such as " + "_NET_ACTIVE_WINDOW. Trying to work around...\n", + display->last_user_time, (unsigned long)timestamp); + display->last_user_time = timestamp; + + windows = meta_display_list_windows (display); + tmp = windows; + while (tmp != NULL) + { + MetaWindow *window = tmp->data; + + if (XSERVER_TIME_IS_BEFORE (timestamp, window->net_wm_user_time)) + { + meta_warning ("%s appears to be one of the offending windows " + "with a timestamp of %lu. Working around...\n", + window->desc, window->net_wm_user_time); + window->net_wm_user_time = timestamp; + } + + tmp = tmp->next; + } + + g_slist_free (windows); + } +} + +static gboolean +timestamp_too_old (MetaDisplay *display, + MetaWindow *window, + Time *timestamp) +{ + /* FIXME: If Soeren's suggestion in bug 151984 is implemented, it will allow + * us to sanity check the timestamp here and ensure it doesn't correspond to + * a future time (though we would want to rename to + * timestamp_too_old_or_in_future). + */ + + MetaWindow *focus_window; + focus_window = display->focus_window; + + if (*timestamp == CurrentTime) + { + meta_warning ("Got a request to focus %s with a timestamp of 0. This " + "shouldn't happen!\n", + window ? window->desc : "the no_focus_window"); + meta_print_backtrace (); + *timestamp = meta_display_get_current_time_roundtrip (display); + return FALSE; + } + else if (XSERVER_TIME_IS_BEFORE (*timestamp, display->last_focus_time)) + { + if (XSERVER_TIME_IS_BEFORE (*timestamp, display->last_user_time)) + { + meta_topic (META_DEBUG_FOCUS, + "Ignoring focus request for %s since %lu " + "is less than %lu and %lu.\n", + window ? window->desc : "the no_focus_window", + *timestamp, + (unsigned long) display->last_user_time, + (unsigned long) display->last_focus_time); + return TRUE; + } + else + { + meta_topic (META_DEBUG_FOCUS, + "Received focus request for %s which is newer than most " + "recent user_time, but less recent than " + "last_focus_time (%lu < %lu < %lu); adjusting " + "accordingly. (See bug 167358)\n", + window ? window->desc : "the no_focus_window", + display->last_user_time, + *timestamp, + display->last_focus_time); + *timestamp = display->last_focus_time; + return FALSE; + } + } + + return FALSE; +} + void meta_display_set_input_focus_window (MetaDisplay *display, MetaWindow *window, gboolean focus_frame, Time timestamp) { - if (timestamp == CurrentTime) - { - meta_warning ("meta_display_set_input_focus_window called with a " - "timestamp of 0 for window %s. This shouldn't happen!\n", - window->desc); - meta_print_backtrace (); - timestamp = meta_display_get_current_time_roundtrip (display); - } - else if (XSERVER_TIME_IS_BEFORE (timestamp, display->last_focus_time)) + if (timestamp_too_old (display, window, ×tamp)) return; XSetInputFocus (display->xdisplay, @@ -4680,18 +4776,8 @@ void meta_display_focus_the_no_focus_window (MetaDisplay *display, Time timestamp) { - if (timestamp == CurrentTime) - { - meta_warning ("meta_display_focus_the_no_focus_window called with a " - "timestamp of 0. This shouldn't happen!\n"); - meta_print_backtrace (); - timestamp = meta_display_get_current_time_roundtrip (display); - } - else if (XSERVER_TIME_IS_BEFORE (timestamp, display->last_focus_time)) - { - meta_warning ("Ignoring focus request for no_focus_window since %lu is less than %lu.\n", timestamp, display->last_focus_time); + if (timestamp_too_old (display, NULL, ×tamp)) return; - } XSetInputFocus (display->xdisplay, display->no_focus_window, diff --git a/src/display.h b/src/display.h index 07070342c..df309efb3 100644 --- a/src/display.h +++ b/src/display.h @@ -198,9 +198,12 @@ struct _MetaDisplay */ MetaWindow *expected_focus_window; - /* last timestamp that a window was focused */ + /* last timestamp passed to XSetInputFocus */ Time last_focus_time; + /* last user interaction time in any app */ + Time last_user_time; + guint static_gravity_works : 1; /*< private-ish >*/ @@ -361,11 +364,21 @@ struct _MetaDisplay * has occurred, this is equivalent to * time1 < time2 * Of course, the rest of the ugliness of this macro comes from accounting - * for the fact that wraparound can occur. + * for the fact that wraparound can occur and the fact that a timestamp of + * 0 must be special-cased since it means older than anything else. + * + * Note that this is NOT an equivalent for time1 <= time2; if that's what + * you need then you'll need to swap the order of the arguments and negate + * the result. */ -#define XSERVER_TIME_IS_BEFORE(time1, time2) \ - ( (( time1 < time2 ) && ( time2 - time1 < ((guint32)-1)/2 )) || \ - (( time1 > time2 ) && ( time1 - time2 > ((guint32)-1)/2 )) \ +#define XSERVER_TIME_IS_BEFORE_ASSUMING_REAL_TIMESTAMPS(time1, time2) \ + ( (( time1 < time2 ) && ( time2 - time1 < ((guint32)-1)/2 )) || \ + (( time1 > time2 ) && ( time1 - time2 > ((guint32)-1)/2 )) \ + ) +#define XSERVER_TIME_IS_BEFORE(time1, time2) \ + ( time1 == 0 || \ + (XSERVER_TIME_IS_BEFORE_ASSUMING_REAL_TIMESTAMPS(time1, time2) && \ + time2 != 0) \ ) gboolean meta_display_open (const char *name); @@ -507,9 +520,11 @@ gboolean meta_display_focus_sentinel_clear (MetaDisplay *display); /* meta_display_set_input_focus_window is like XSetInputFocus, except * that (a) it can't detect timestamps later than the current time, * since Metacity isn't part of the XServer, and thus gives erroneous - * behavior in this circumstance (so don't do it), and (b) it uses - * display->last_focus_time and display->expected_focus_window since - * we don't have access to the true Xserver ones. + * behavior in this circumstance (so don't do it), (b) it uses + * display->last_focus_time since we don't have access to the true + * Xserver one, (c) it makes use of display->user_time since checking + * whether a window should be allowed to be focused should depend + * on user_time events (see bug 167358, comment 15 in particular) */ void meta_display_set_input_focus_window (MetaDisplay *display, MetaWindow *window, diff --git a/src/window-props.c b/src/window-props.c index 2bde03b58..34881ff19 100644 --- a/src/window-props.c +++ b/src/window-props.c @@ -186,12 +186,7 @@ reload_net_wm_user_time (MetaWindow *window, if (value->type != META_PROP_VALUE_INVALID) { gulong cardinal = value->v.cardinal; - - window->net_wm_user_time_set = TRUE; - window->net_wm_user_time = cardinal; - meta_topic (META_DEBUG_STARTUP, - "Window %s has _NET_WM_USER_TIME of %lu\n", - window->desc, window->net_wm_user_time); + meta_window_set_user_time (window, cardinal); } } diff --git a/src/window.c b/src/window.c index 8470e259d..c281b87f9 100644 --- a/src/window.c +++ b/src/window.c @@ -2206,15 +2206,20 @@ void meta_window_activate (MetaWindow *window, guint32 timestamp) { + meta_topic (META_DEBUG_FOCUS, + "_NET_ACTIVE_WINDOW message sent for %s at time %lu.\n", + window->desc, (unsigned long)timestamp); + /* Older EWMH spec didn't specify a timestamp, so it can be 0 and we * have to treat that as a new request. */ - if (XSERVER_TIME_IS_BEFORE (timestamp, window->display->last_focus_time) && + if (XSERVER_TIME_IS_BEFORE (timestamp, window->display->last_user_time) && timestamp != 0) { meta_topic (META_DEBUG_FOCUS, - "_NET_ACTIVE_WINDOW message sent but with a timestamp that" - "requests the window not be active, so we're ignoring it.\n"); + "last_user_time (%lu) is more recent; ignoring " + " _NET_ACTIVE_WINDOW message.\n", + window->display->last_user_time); window->wm_state_demands_attention = TRUE; set_net_wm_state (window); return; @@ -2223,7 +2228,7 @@ meta_window_activate (MetaWindow *window, if (timestamp == 0) timestamp = meta_display_get_current_time_roundtrip (window->display); - window->net_wm_user_time = timestamp; + meta_window_set_user_time (window, timestamp); /* disable show desktop mode unless we're a desktop component */ maybe_leave_show_desktop_mode (window); @@ -4305,7 +4310,7 @@ meta_window_client_message (MetaWindow *window, else if (event->xclient.message_type == display->atom_net_active_window) { - meta_verbose ("_NET_ACTIVE_WINDOW request for window '%s', activating", + meta_verbose ("_NET_ACTIVE_WINDOW request for window '%s', activating\n", window->desc); if (event->xclient.data.l[0] != 0) @@ -7378,3 +7383,34 @@ meta_window_stack_just_below (MetaWindow *window, window->desc, below_this_one->desc); } } + +void +meta_window_set_user_time (MetaWindow *window, + Time timestamp) +{ + /* FIXME: If Soeren's suggestion in bug 151984 is implemented, it will allow + * us to sanity check the timestamp here and ensure it doesn't correspond to + * a future time. + */ + + /* Only update the time if this timestamp is newer... */ + if (window->net_wm_user_time_set && + XSERVER_TIME_IS_BEFORE (timestamp, window->net_wm_user_time)) + { + meta_topic (META_DEBUG_STARTUP, + "Window %s _NET_WM_USER_TIME not updated to %lu, because it " + "is less than %lu\n", + window->desc, timestamp, window->net_wm_user_time); + + } + else + { + meta_topic (META_DEBUG_STARTUP, + "Window %s has _NET_WM_USER_TIME of %lu\n", + window->desc, timestamp); + window->net_wm_user_time_set = TRUE; + window->net_wm_user_time = timestamp; + if (XSERVER_TIME_IS_BEFORE (window->display->last_user_time, timestamp)) + window->display->last_user_time = timestamp; + } +} diff --git a/src/window.h b/src/window.h index 960f364f5..e9efa0c4e 100644 --- a/src/window.h +++ b/src/window.h @@ -532,4 +532,7 @@ void meta_window_queue_update_icon (MetaWindow *window); void meta_window_stack_just_below (MetaWindow *window, MetaWindow *below_this_one); +void meta_window_set_user_time (MetaWindow *window, + Time timestamp); + #endif