From c884a8ede0189efae770501fa9abbc837f6d6464 Mon Sep 17 00:00:00 2001
From: oga
Date: Sat, 17 Jan 2009 18:41:50 +0000
Subject: Finally fix the really annoying race where if you rapidly switch
 groups several times you'd end up losing clients (thinking they had gone
 away).

From the ICCCM (which should not be read without a stiff drink in hand,
I made this mistake so you don't have to): to request a window to be
withdrawn one should send a synthetic UnmapRequest event when iconified.
To request iconification one should just unmap the window. The ICCM
further recommends that the synthetic event should just be taken as a
cue to withdraw, to deal with legacy clients. Taking a hint from this,
rework xev_handle_unmaprequest to correctly detect these situations.  A
Withdrawn window may come back anywhere, even as a subwindow of
something else, so the correct way to handle this state is to forget it
ever existed.

While i'm here, kill a dumb attempt to notice this in client_delete, and
nuke the very unnecessary arguments.

Todd confirmed this fixes the `race'.

ok todd@, ok ok okan@
---
 xevents.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

(limited to 'xevents.c')

diff --git a/xevents.c b/xevents.c
index 28e88fd..cc322ad 100644
--- a/xevents.c
+++ b/xevents.c
@@ -15,7 +15,7 @@
  * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  *
- * $Id: xevents.c,v 1.34 2009/01/16 16:49:17 okan Exp $
+ * $Id: xevents.c,v 1.35 2009/01/17 18:41:50 oga Exp $
  */
 
 /*
@@ -36,12 +36,8 @@ void
 xev_handle_maprequest(struct xevent *xev, XEvent *ee)
 {
 	XMapRequestEvent	*e = &ee->xmaprequest;
-	XWindowAttributes	 xattr;
 	struct client_ctx	*cc = NULL, *old_cc;
-
-#ifdef notyet
-	int state;
-#endif
+	XWindowAttributes	 xattr;
 
 	if ((old_cc = client_current()) != NULL)
 		client_ptrsave(old_cc);
@@ -51,12 +47,6 @@ xev_handle_maprequest(struct xevent *xev, XEvent *ee)
 		cc = client_new(e->window, screen_fromroot(xattr.root), 1);
 	}
 
-#ifdef notyet			/* XXX - possibly, we shouldn't map if
-				 * the window is withdrawn. */
-	if (xu_getstate(cc, &state) == 0 && state == WithdrawnState)
-		warnx("WITHDRAWNSTATE for %s", cc->name);
-#endif
-
 	client_ptrwarp(cc);
 	xev_register(xev);
 }
@@ -65,10 +55,27 @@ void
 xev_handle_unmapnotify(struct xevent *xev, XEvent *ee)
 {
 	XUnmapEvent		*e = &ee->xunmap;
+	XEvent			ev;
 	struct client_ctx	*cc;
 
-	if ((cc = client_find(e->window)) != NULL)
-		client_delete(cc, e->send_event, 0);
+	/* XXX, we need a recursive locking wrapper around grab server */
+	XGrabServer(X_Dpy);
+	if ((cc = client_find(e->window)) != NULL) {
+		/*
+		 * If it's going to die anyway, nuke it.
+		 *
+		 * Else, if it's a synthetic event delete state, since they
+		 * want it to be withdrawn. ICCM recommends you withdraw on
+		 * this even if we haven't alredy been told to iconify, to
+		 * deal with legacy clients.
+		 */
+		if (XCheckTypedWindowEvent(X_Dpy, cc->win,
+		    DestroyNotify, &ev) || e->send_event != 0) {
+			client_delete(cc);
+		} else
+			client_hide(cc);
+	}
+	XUngrabServer(X_Dpy);
 
 	xev_register(xev);
 }
@@ -80,7 +87,7 @@ xev_handle_destroynotify(struct xevent *xev, XEvent *ee)
 	struct client_ctx	*cc;
 
 	if ((cc = client_find(e->window)) != NULL)
-		client_delete(cc, 1, 1);
+		client_delete(cc);
 
 	xev_register(xev);
 }
-- 
cgit v1.2.3-2-gb3c3