Wrap XSetInputFocus, making display->expected_focus_window a little more

2004-12-22  Elijah Newren  <newren@gmail.com>

	Wrap XSetInputFocus, making display->expected_focus_window a
	little more reliable (see #154598)

	* src/display.h: (struct _MetaDisplay): add a large comment about
	the expected_focus_window, add a last_focus_time field,
	(XSERVER_TIME_IS_BEFORE): new macro moved from window.c but fixed
	for 64-bit systems, (meta_display_set_input_focus_window): new
	function

	* src/display.c (meta_display_open): initialize last_focus_time,
	add a comment about brokenness of trying to set intial focus
	window, (meta_display_set_input_focus_window): new function that
	wraps XSetInputFocus,
	(meta_display_focus_the_no_focus_window): make this function
	closer to a wrapping of XSetInputFocus for the no_focus_window.

	* src/window.c (XSERVER_TIME_IS_LATER): remove this macro in favor
	of the improved one added to display.h

	* src/display.c (meta_display_open):
	* src/window.c (meta_window_focus):
	use meta_display_focus_the_no_focus_window and
	meta_display_set_input_focus instead of XSetInputFocus

ChangeLog

@@ -1,3 +1,29 @@
+2004-12-22  Elijah Newren  <newren@gmail.com>
+
+	Wrap XSetInputFocus, making display->expected_focus_window a
+	little more reliable (see #154598)
+
+	* src/display.h: (struct _MetaDisplay): add a large comment about
+	the expected_focus_window, add a last_focus_time field,
+	(XSERVER_TIME_IS_BEFORE): new macro moved from window.c but fixed
+	for 64-bit systems, (meta_display_set_input_focus_window): new
+	function
+
+	* src/display.c (meta_display_open): initialize last_focus_time,
+	add a comment about brokenness of trying to set intial focus
+	window, (meta_display_set_input_focus_window): new function that
+	wraps XSetInputFocus,
+	(meta_display_focus_the_no_focus_window): make this function
+	closer to a wrapping of XSetInputFocus for the no_focus_window.
+
+	* src/window.c (XSERVER_TIME_IS_LATER): remove this macro in favor
+	of the improved one added to display.h
+
+	* src/display.c (meta_display_open):
+	* src/window.c (meta_window_focus):
+	use meta_display_focus_the_no_focus_window and
+	meta_display_set_input_focus instead of XSetInputFocus
+
 2004-12-22  Elijah Newren  <newren@gmail.com>
 
 	* src/core.c (meta_core_user_lower_and_unfocus):

src/display.c

@@ -632,6 +632,7 @@ meta_display_open (const char *name)
     timestamp = event.xproperty.time;
   }
 
+  display->last_focus_time = timestamp;
   display->compositor = meta_compositor_new (display);
   
   screens = NULL;
@@ -676,19 +677,27 @@ meta_display_open (const char *name)
     /* kinda bogus because GetInputFocus has no possible errors */
     meta_error_trap_push (display);
 
+    /* FIXME: This is totally broken; see comment 9 of bug 88194 about this */
     focus = None;
     ret_to = RevertToPointerRoot;
     XGetInputFocus (display->xdisplay, &focus, &ret_to);
 
     /* Force a new FocusIn (does this work?) */
-    if (focus == None || focus == PointerRoot)
-      focus = display->no_focus_window;
 
     /* Use the same timestamp that was passed to meta_screen_new(),
      * as it is the most recent timestamp.
      */
-    XSetInputFocus (display->xdisplay, focus, RevertToPointerRoot,
+    if (focus == None || focus == PointerRoot)
-                    timestamp);
+      meta_display_focus_the_no_focus_window (display, timestamp);
+    else
+      {
+        MetaWindow * window;
+        window  = meta_display_lookup_x_window (display, focus);
+        if (window)
+          meta_display_set_input_focus_window (display, window, FALSE, timestamp);
+        else
+          meta_display_focus_the_no_focus_window (display, timestamp);
+      }
 
     meta_error_trap_pop (display, FALSE);
   }
@@ -4596,15 +4605,40 @@ meta_display_focus_sentinel_clear (MetaDisplay *display)
   return (display->sentinel_counter == 0);
 }
 
+void
+meta_display_set_input_focus_window (MetaDisplay *display, 
+                                     MetaWindow  *window,
+                                     gboolean     focus_frame,
+                                     Time         timestamp)
+{
+  if (XSERVER_TIME_IS_BEFORE (timestamp, display->last_focus_time))
+    return;
+
+  XSetInputFocus (display->xdisplay,
+                  focus_frame ? window->frame->xwindow : window->xwindow,
+                  RevertToPointerRoot,
+                  timestamp);
+  display->expected_focus_window = window;
+  display->last_focus_time = timestamp;
+
+  if (window != display->autoraise_window)
+    meta_display_remove_autoraise_callback (window->display);
+}
+
 void
 meta_display_focus_the_no_focus_window (MetaDisplay *display, 
                                         Time         timestamp)
 {
+  if (XSERVER_TIME_IS_BEFORE (timestamp, display->last_focus_time))
+    return;
+
   XSetInputFocus (display->xdisplay,
                   display->no_focus_window,
                   RevertToPointerRoot,
                   timestamp);
   display->expected_focus_window = NULL;
+  display->last_focus_time = timestamp;
+
   meta_display_remove_autoraise_callback (display);
 }
 

src/display.h

@@ -192,10 +192,18 @@ struct _MetaDisplay
   */
   MetaWindow *previously_focused_window;
 
-  /* window we are expecting a FocusIn event for
+  /* window we are expecting a FocusIn event for or the current focus
+   * window if we are not expecting any FocusIn/FocusOut events; not
+   * perfect because applications can call XSetInputFocus directly.
+   * (It could also be messed up if a timestamp later than current
+   * time is sent to meta_display_set_input_focus_window, though that
+   * would be a programming error).  See bug 154598 for more info.
    */
   MetaWindow *expected_focus_window;
 
+  /* last timestamp that a window was focused */
+  Time last_focus_time;
+
   guint static_gravity_works : 1;
   
   /*< private-ish >*/
@@ -350,6 +358,18 @@ struct _MetaDisplay
 #endif
 };
 
+/* Xserver time can wraparound, thus comparing two timestamps needs to take
+ * this into account.  Here's a little macro to help out.  If no wraparound
+ * 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.
+ */
+#define XSERVER_TIME_IS_BEFORE(time1, time2)                       \
+  ( (( time1 < time2 ) && ( time2 - time1 < ((guint32)-1)/2 )) ||  \
+    (( time1 > time2 ) && ( time1 - time2 > ((guint32)-1)/2 ))     \
+  )
+
 gboolean      meta_display_open                (const char  *name);
 void          meta_display_close               (MetaDisplay *display);
 MetaScreen*   meta_display_screen_for_root     (MetaDisplay *display,
@@ -486,8 +506,25 @@ void meta_display_increment_focus_sentinel (MetaDisplay *display);
 void meta_display_decrement_focus_sentinel (MetaDisplay *display);
 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.
+ */
+void meta_display_set_input_focus_window   (MetaDisplay *display, 
+                                            MetaWindow  *window,
+                                            gboolean     focus_frame,
+                                            Time         timestamp);
+
+/* meta_display_focus_the_no_focus_window is called when the
+ * designated no_focus_window should be focused, but is otherwise the
+ * same as meta_display_set_input_focus_window
+ */
 void meta_display_focus_the_no_focus_window (MetaDisplay *display, 
                                              Time         timestamp);
+
 void meta_display_queue_autoraise_callback  (MetaDisplay *display,
                                              MetaWindow  *window);
 void meta_display_remove_autoraise_callback (MetaDisplay *display);

src/window.c

@@ -47,14 +47,6 @@
 #include <X11/extensions/shape.h>
 #endif
 
-/* Xserver time can wraparound, thus comparing two timestamps needs to take
- * this into account.  Here's a little macro to help out.
- */
-#define XSERVER_TIME_IS_LATER(time1, time2)                    \
-  ( ((time1 >= time2) && (time1 - time2 < G_MAXULONG / 2)) ||  \
-    ((time1 <  time2) && (time2 - time1 > G_MAXULONG / 2))     \
-  )
-
 typedef enum
 {
   META_IS_CONFIGURE_REQUEST = 1 << 0,
@@ -1633,7 +1625,7 @@ window_takes_focus_on_map (MetaWindow *window)
       compare = window->net_wm_user_time_set  ? window->net_wm_user_time  : compare;
 
       if ((window->display->focus_window == NULL) ||
-          (XSERVER_TIME_IS_LATER (compare, window->display->focus_window->net_wm_user_time)))
+          XSERVER_TIME_IS_BEFORE (window->display->focus_window->net_wm_user_time, compare))
         {
           meta_topic (META_DEBUG_STARTUP,
                       "new window %s with no intervening events\n",
@@ -3284,11 +3276,10 @@ meta_window_focus (MetaWindow  *window,
         {
           meta_topic (META_DEBUG_FOCUS,
                       "Focusing frame of %s\n", window->desc);
-          XSetInputFocus (window->display->xdisplay,
+          meta_display_set_input_focus_window (window->display,
-                          window->frame->xwindow,
+                                               window,
-                          RevertToPointerRoot,
+                                               TRUE,
                                                timestamp);
-          window->display->expected_focus_window = window;
         }
     }
   else
@@ -3298,13 +3289,12 @@ meta_window_focus (MetaWindow  *window,
       if (window->input)
         {
           meta_topic (META_DEBUG_FOCUS,
-                      "Calling XSetInputFocus() on client window %s since input = true\n",
+                      "Setting input focus on %s since input = true\n",
                       window->desc);
-          XSetInputFocus (window->display->xdisplay,
+          meta_display_set_input_focus_window (window->display,
-                          window->xwindow,
+                                               window,
-                          RevertToPointerRoot,
+                                               FALSE,
                                                timestamp);
-          window->display->expected_focus_window = window;
         }
       
       if (window->take_focus)
@@ -3326,10 +3316,6 @@ meta_window_focus (MetaWindow  *window,
       window->wm_state_demands_attention = FALSE;
       set_net_wm_state (window);
     }  
-
-  /* Check if there's an autoraise timeout for a different window */
-  if (window != window->display->autoraise_window)
-    meta_display_remove_autoraise_callback (window->display);
 }
 
 static void