Activation request for the focused window is dropped while a focus transfer to another window is pending

Description

clientSetFocus() ignores a request for the client that is client_focus ("already focused, ignoring request"). client_focus is only updated on FocusIn, so it can still point to the previous client while a focus transfer to another client is pending (pending_focus). For a client using the globally active input model (input hint False and WM_TAKE_FOCUS), the transfer completes only when the client handles WM_TAKE_FOCUS. A busy client may take a while to do that.

Steps:

  1. Client windows A and C, A is focused.
  2. The client sends _NET_ACTIVE_WINDOW for C. xfwm4 sends WM_TAKE_FOCUS to C and sets pending_focus to C; client_focus is still A.
  3. Before the client handles that message, a pager or task bar (another X client) sends _NET_ACTIVE_WINDOW for A with a newer timestamp.
  4. clientSetFocus() sees c == client_focus and returns, no WM_TAKE_FOCUS is sent to A.
  5. The client handles WM_TAKE_FOCUS for C, C becomes focused and active.

The newer request is lost, and C ends up active instead of A. Wine applications hit this, since Wine uses this input model and an application may not process messages for a while right after activating a window.

Reproducer

xfwm4-pending-noop-repro.c (Xlib only):

cc -o xfwm4-pending-noop-repro xfwm4-pending-noop-repro.c -lX11
xfwm4-pending-noop-repro   # in a session running xfwm4

Output with xfwm4 4.20.0 on Xvfb:

setup: A=0x400001 C=0x400002 active=0x400001 focus=0x400001
requested C time=955507165, busy for 500 ms
other client requested A time=955507265 after 100 ms
WM_TAKE_FOCUS window=0x400002 time=955507165
result: active=0x400002 focus=0x400002 -> FAIL

With the change below, xfwm4 also sends WM_TAKE_FOCUS for A, and A ends up active and focused (PASS in 3 of 3 runs). Openbox passes too.

Possible fix

Treat the request as a no-op only when no transfer to another client is pending:

--- a/src/focus.c
+++ b/src/focus.c
@@ -569,7 +569,8 @@
             FLAG_UNSET (c->flags, CLIENT_FLAG_DEMANDS_ATTENTION);
             clientSetNetState (c);
         }
-        if ((c == client_focus) && !(flags & FOCUS_FORCE))
+        /* a focus transfer to another client may still be pending, then the request isn't a no-op */
+        if ((c == client_focus) && (!pending_focus || pending_focus == c) && !(flags & FOCUS_FORCE))
         {
             TRACE ("client \"%s\" (0x%lx) is already focused, ignoring request", c->name, c->window);
             return;

pending_focus is reset in clientUpdateFocus() on FocusIn, so the behaviour doesn't change once the transfer completes. The same check is in the current master.