Merge lp:~smspillaz/compiz-core/compiz-core.more-sound-reparenting-behaviour into lp:compiz-core

[Sam Spilsbury]

Stab in the dark.

I think whats going on for bug 940603 is that windows aren't being removed from the window list if they are unreparented by the application itself in very special circumstances (not sure what because I can't reproduce the bug).

This code adds some more sound checks to see where the window is going and destroys it based on that, rather than using the same logic for both reparented and unreparented windows.

Also removed the whole destroyedFrameWindows tracking code which was more or ineffectual and could potentially result in a frame window leak.

[Daniel van Vugt (vanvugt)]

In case you're wondering, I just corrected the bug number in the description.

[Sam Spilsbury (smspillaz)]

thanks

[Daniel van Vugt (vanvugt)]

I was optimistically hoping this branch would fix bug 865672. But it doesn't.

[Daniel van Vugt (vanvugt)]

Seems to work perfectly without any visible regressions.

I cannot confirm it fixes any particular bug at all. And I don't claim to fully understand the code. Based on recent history, that would lead me to abstain. However I strongly support any changes that make the code significantly smaller and easier to maintain. And if there is a chance this fixes bug 940603 then we really want to commit it for the sake of testing at least.

Less importantly, I think it's bad practice to assume None == 0 even though it is true. So this:
  if (priv->serverFrame)
should be written as:
  if (priv->serverFrame != None)

review: Approve

=== modified file 'src/event.cpp'
--- src/event.cpp	2012-03-24 09:49:30 +0000
+++ src/event.cpp	2012-04-04 03:23:19 +0000
@@ -1355,7 +1355,8 @@
 
         if (w)
 	{
-	    if (event->xreparent.parent != w->priv->wrapper)
+	    if ((w->priv->wrapper && event->xreparent.parent != w->priv->wrapper) ||
+		(!w->priv->wrapper && event->xreparent.parent != priv->root))
 	    {
 		w->moveInputFocusToOtherWindow ();
 		w->destroy ();
 
=== modified file 'src/privatescreen.h'
--- src/privatescreen.h	2012-03-29 16:54:46 +0000
+++ src/privatescreen.h	2012-04-04 03:23:19 +0000
@@ -523,8 +523,6 @@
 	CompWindow::Map windowsMap;
 	std::list<CompGroup *> groups;
 
-	std::map <CompWindow *, CompWindow *> detachedFrameWindows;
-
 	CompWindowVector clientList;            /* clients in mapping order */
 	CompWindowVector clientListStacking;    /* clients in stacking order */
 
 
=== modified file 'src/window.cpp'
--- src/window.cpp	2012-03-30 11:43:20 +0000
+++ src/window.cpp	2012-04-04 03:23:19 +0000
@@ -1441,8 +1441,11 @@
 	windowNotify (CompWindowNotifyBeforeDestroy);
 
 	/* Don't allow frame windows to block input */
-	XUnmapWindow (screen->dpy (), priv->serverFrame);
-	XUnmapWindow (screen->dpy (), priv->wrapper);
+	if (priv->serverFrame)
+	    XUnmapWindow (screen->dpy (), priv->serverFrame);
+
+	if (priv->wrapper)
+	    XUnmapWindow (screen->dpy (), priv->wrapper);
 
 	oldServerNext = serverNext;
 	oldServerPrev = serverPrev;
@@ -6461,21 +6464,6 @@
 	    dw->serverPrev = this->serverPrev;
     }
 
-    /* If this window has a detached frame, destroy it, but only
-     * using XDestroyWindow since there may be pending restack
-     * requests relative to it */
-
-    std::map <CompWindow *, CompWindow *>::iterator it =
-	    screen->priv->detachedFrameWindows.find (this);
-
-    if (it != screen->priv->detachedFrameWindows.end ())
-    {
-	CompWindow *fw = (it->second);
-
-	XDestroyWindow (screen->dpy (), fw->id ());
-	screen->priv->detachedFrameWindows.erase (it);
-    }
-
     if (!priv->destroyed)
     {
 	StackDebugger *dbg = StackDebugger::Default ();
@@ -6934,22 +6922,6 @@
     attr.colormap          = cmap;
     attr.override_redirect = true;
 
-    /* Look for existing detached frame windows and reattach them
-     * in case this window as reparented again after being withdrawn */
-    std::map <CompWindow *, CompWindow *>::iterator it =
-	    screen->priv->detachedFrameWindows.find (window);
-
-    if (it != screen->priv->detachedFrameWindows.end ())
-    {
-	/* Trash the old frame window
-	 * TODO: It would be nicer if we could just
-	 * reparent back into it, but there are some
-	 * problems with that */
-
-	XDestroyWindow (dpy, (it->second)->id ());
-	screen->priv->detachedFrameWindows.erase (it);
-    }
-
     /* We need to know when the frame window is created
      * but that's all */
     XSelectInput (dpy, screen->root (), SubstructureNotifyMask);
@@ -7192,17 +7164,11 @@
 
 	/* Put the frame window "above" the client window
 	 * in the stack */
-	CompWindow *fw = PrivateWindow::createCompWindow (id, attrib, serverFrame);
-
-	/* Put this window in the list of "detached frame windows"
-	 * so that we can reattach it or destroy it when we are
-	 * done with it */
-
-	screen->priv->detachedFrameWindows[window] = fw;
+	PrivateWindow::createCompWindow (id, attrib, serverFrame);
     }
 
-    /* Safe to destroy the wrapper but not the frame */
-    XUnmapWindow (screen->dpy (), serverFrame);
+    /* Issue a DestroyNotify */
+    XDestroyWindow (screen->dpy (), serverFrame);
     XDestroyWindow (screen->dpy (), wrapper);
 
     window->windowNotify (CompWindowNotifyUnreparent);