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