Merge lp:~compiz-team/compiz/compiz.fix_1085591 into lp:compiz/0.9.9

[Sam Spilsbury]

Fix race condition causing --replace to fail occasionally.

Clear XErrors right before taking SubstructureRedirectMask.

This changes the startup order a fair bit. The new order is now:

1. Do non-critical initialization that does not require a server grab
2. Do the ICCCM check to shut down the other window manager
3. Take server grab
4. Create edge windows
5. Clear error buffer
6. Attempt to take SubstructureRedirectMask
7. Clear error buffer -> if fail, exit
8. Create handles based on XQueryTree
9. Release server grab

(LP: #1085591)

System test added to demonstrate that compiz should exit when another client has SubstructureRedirectMask and attempting to take it results in an X error. It wasn't doing that before, and instead checked for X errors much earlier on (in error, because an X error up there wouldn't indicate that another WM was running at all).

In order to make this work properly, some changes had to be made to compiz. This adds a new runtime switch --send-startup-message which simply posts a message to all clients saying that the startup procedure has finished (and either succeeded or failed). Its needed because the location of when we take the server grab was changed to a much smaller critical section of the code, and that doesn't include the property change on the selection window. This is a far more accurate way of knowing that we've started, because we send it /after/ the startup procedure is done (rather than relying on the server to do the right thing.

Also fixed race condition caused by destroying the WMSnSelectionWindow before shutdown.

 Because we haven't changed our active event mask
 to remove SubstructureRedirectMask, other ICCCM
 compliant window managers may receive a DestroyNotify
 (eg, because we're blocked on XSync) before we get
 a chance to close our display connection and remove
 our SubstructureRedirectMask. That will cause them
 to fail to start.

 The selection window is destroyed anyways when we
 close our connection, and that is a very accurate
 indicator to other WM's that we are well and truly
 gone because the protocol requires the implementation
 to remove all client event masks before destroying
 windows.

Added acceptance-tests for testing that behaviour. They are not added to the default test target for obvious reasons.

Fix race condition causing --replace to fail occasionally.

Clear XErrors right before taking SubstructureRedirectMask.

This changes the startup order a fair bit. The new order is now:

1. Do non-critical initialization that does not require a server grab
2. Do the ICCCM check to shut down the other window manager
3. Take server grab
4. Create edge windows
5. Clear error buffer
6. Attempt to take SubstructureRedirectMask
7. Clear error buffer -> if fail, exit
8. Create handles based on XQueryTree
9. Release server grab

System test added to demonstrate that compiz should exit when another client has SubstructureRedirectMask and attempting to take it results in an X error. It wasn't doing that before, and instead checked for X errors much earlier on (in error, because an X error up there wouldn't indicate that another WM was running at all).

In order to make this work properly, some changes had to be made to compiz. This adds a new runtime switch --send-startup-message which simply posts a message to all clients saying that the startup procedure has finished (and either succeeded or failed). Its needed because the location of when we take the server grab was changed to a much smaller critical section of the code, and that doesn't include the property change on the selection window. This is a far more accurate way of knowing that we've started, because we send it /after/ the startup procedure is done (rather than relying on the server to do the right thing.

(LP: #1085591)

Also fixed race condition caused by destroying the WMSnSelectionWindow before shutdown.

 Because we haven't changed our active event mask
 to remove SubstructureRedirectMask, other ICCCM
 compliant window managers may receive a DestroyNotify
 (eg, because we're blocked on XSync) before we get
 a chance to close our display connection and remove
 our SubstructureRedirectMask. That will cause them
 to fail to start.

 The selection window is destroyed anyways when we
 close our connection, and that is a very accurate
 indicator to other WM's that we are well and truly
 gone because the protocol requires the implementation
 to remove all client event masks before destroying
 windows.

Added acceptance-tests for testing that behaviour. They are not added to the default test target for obvious reasons.

Please throw this against the wall to see if any oddities have been introduced - they sometimes happen when you change the startup order as there is coupling between side effects.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3501
http://jenkins.qa.ubuntu.com/job/compiz-ci/287/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/287/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/287//rebuild/?

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3502
http://jenkins.qa.ubuntu.com/job/compiz-ci/290/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/290/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/290//rebuild/?

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

I still get bug 1085591 even with this branch. Seems like nothing is fixed.

1. Start compiz
2. Start another compiz --replace
Expected: Compiz is replaced
Observed: The old compiz is killed but the new one fails to start with:
compiz (core) - Error: Another window manager is already running on screen: 0

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

Hmm.

Well in any case this does fix the problematic nature of the code. We are
meant to check for errors upon taking SubstructureRedirectMask, and we
don't do that properly.

I'm suprised its still not working for you, it definitely fixed the problem
here.
On Dec 4, 2012 11:44 AM, "Daniel van Vugt" <email address hidden>
wrote:

> Review: Needs Fixing
>
> I still get bug 1085591 even with this branch. Seems like nothing is fixed.
>
> 1. Start compiz
> 2. Start another compiz --replace
> Expected: Compiz is replaced
> Observed: The old compiz is killed but the new one fails to start with:
> compiz (core) - Error: Another window manager is already running on
> screen: 0
> --
>
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1085591/+merge/137524
> Your team Compiz Maintainers is subscribed to branch lp:compiz.
>

[Sam Spilsbury (smspillaz)]

Oh, could you get some more output for me?

Change the DEBUG #define to be 1 and run compiz --debug.

That will print out X errors as they occurr.
On Dec 4, 2012 12:32 PM, "Sam Spilsbury" <email address hidden> wrote:

> Hmm.
>
> Well in any case this does fix the problematic nature of the code. We are
> meant to check for errors upon taking SubstructureRedirectMask, and we
> don't do that properly.
>
> I'm suprised its still not working for you, it definitely fixed the
> problem here.
> On Dec 4, 2012 11:44 AM, "Daniel van Vugt" <email address hidden>
> wrote:
>
>> Review: Needs Fixing
>>
>> I still get bug 1085591 even with this branch. Seems like nothing is
>> fixed.
>>
>> 1. Start compiz
>> 2. Start another compiz --replace
>> Expected: Compiz is replaced
>> Observed: The old compiz is killed but the new one fails to start with:
>> compiz (core) - Error: Another window manager is already running on
>> screen: 0
>> --
>>
>> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1085591/+merge/137524
>> Your team Compiz Maintainers is subscribed to branch lp:compiz.
>>
>

[Daniel van Vugt (vanvugt)]

Debug mode says...

./bin/compiz (core) - Info: Loading plugin: core
./bin/compiz (core) - Debug: Trying to load core from: /home/dan/.compiz-1/plugins/libcore.so
./bin/compiz (core) - Debug: dlopen failed: /home/dan/.compiz-1/plugins/libcore.so: cannot open shared object file: No such file or directory
./bin/compiz (core) - Debug: Trying to load core from: /home/dan/tmp.591/lib/compiz/libcore.so
./bin/compiz (core) - Debug: dlopen failed: /home/dan/tmp.591/lib/compiz/libcore.so: cannot open shared object file: No such file or directory
./bin/compiz (core) - Info: Starting plugin: core
./bin/compiz (core) - Debug: Started plugin: core
X Error of failed request: BadWindow (invalid Window parameter)
  Major opcode of failed request: 34 (X_UngrabKey)
  Minor opcode of failed request: 0
  Resource id in failed request: 0x0
X Error of failed request: BadAccess (attempt to access private resource denied)
  Major opcode of failed request: 2 (X_ChangeWindowAttributes)
  Minor opcode of failed request: 0
  Resource id in failed request: 0xe1
X Error of failed request: BadAccess (attempt to access private resource denied)
  Major opcode of failed request: 33 (X_GrabKey)
  Minor opcode of failed request: 0
  Resource id in failed request: 0xe1
... above error repeats several times ...
X Error of failed request: BadAccess (attempt to access private resource denied)
  Major opcode of failed request: 2 (X_ChangeWindowAttributes)
  Minor opcode of failed request: 0
  Resource id in failed request: 0xe1
./bin/compiz (core) - Error: Another window manager is already running on screen: 0
./bin/compiz (core) - Info: Stopping plugin: core
./bin/compiz (core) - Debug: Stopped plugin: core
./bin/compiz (core) - Info: Unloading plugin: core

[Sam Spilsbury (smspillaz)]

OK, thanks for the output.

I've fixed another race condition and added some acceptance tests to verify it (they are quite flakey due to being time-based so they aren't added to the test target by default)

[Daniel van Vugt (vanvugt)]

Seems to work now. Bug fixed.

Just a note on pthread creation in AsyncTask::AsyncTask... It's usually a bad idea to create or touch anything outside of your own class in a constructor. Say for example, someone uses your class and decides they need a vector<> of them. Every time the vector reallocates, every element gets re-copy-constructed and destroyed. In this case, that would mean frequent and unexpected thread creates and joins (and hangs if the thread is not ready to be joined). So it's dangerous touching external things in a destructor or constructor.

It's maybe not a realistic example but there are other similar reasons why touching things outside your class should be left till after construction.

review: Approve

[Sam Spilsbury (smspillaz)]

> Seems to work now. Bug fixed.
>
> Just a note on pthread creation in AsyncTask::AsyncTask... It's usually a bad
> idea to create or touch anything outside of your own class in a constructor.
> Say for example, someone uses your class and decides they need a vector<> of
> them. Every time the vector reallocates, every element gets re-copy-
> constructed and destroyed. In this case, that would mean frequent and
> unexpected thread creates and joins (and hangs if the thread is not ready to
> be joined). So it's dangerous touching external things in a destructor or
> constructor.
>
> It's maybe not a realistic example but there are other similar reasons why
> touching things outside your class should be left till after construction.

You are correct.

A good solution in this case would be to make AsyncTask noncopyable.

Generally speaking I don't mess with external state in constructors and destructors unless its the purpose of the class to do that, and I only do it through well defined interfaces (eg, the class takes an interface to another class which is used to set up and tear down state, and those functions are guarunteed to be called through exceptions)

[Daniel van Vugt (vanvugt)]

Still, side-effects are bad. If constructing a variable (object) does more than allocate memory then you can cause all sorts of bugs. This is where procedural languages excel in long-term maintainability, because you are forced to explicitly state, and make it obvious, what function is being called and when. Nothing is ever hidden in construction and destruction.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3513
http://jenkins.qa.ubuntu.com/job/compiz-ci/295/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/295/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/295//rebuild/?

review: Approve (continuous-integration)

=== modified file 'compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp'
--- compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp	2012-11-20 14:51:27 +0000
+++ compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp	2012-12-05 05:49:21 +0000
@@ -25,6 +25,7 @@
 
 #include <gtest/gtest.h>
 #include <gtest_shared_characterwrapper.h>
+#include <gtest_shared_tmpenv.h>
 
 #include <boost/noncopyable.hpp>
 
@@ -257,35 +258,6 @@
 static const std::string four ("4");
 static const std::string trueStr ("true");
 
-class TmpEnv :
-    boost::noncopyable
-{
-    public:
-
-	explicit TmpEnv (const char *env, const char *val) :
-	    envRestore (env),
-	    envRestoreVal (getenv (env))
-	{
-	    if (!val)
-		unsetenv (env);
-	    else
-		setenv (env, val, TRUE);
-	}
-
-	~TmpEnv ()
-	{
-	    if (envRestoreVal)
-		setenv (envRestore, envRestoreVal, TRUE);
-	    else
-		unsetenv (envRestore);
-	}
-
-    private:
-
-	const char *envRestore;
-	const char *envRestoreVal;
-};
-
 const char * getConstCharFromCharacterWrapper (const CharacterWrapper &wrap)
 {
     return wrap;

=== modified file 'include/core/screen.h'
--- include/core/screen.h	2012-12-02 17:37:35 +0000
+++ include/core/screen.h	2012-12-05 05:49:21 +0000
@@ -52,6 +52,7 @@
 
 extern bool       replaceCurrentWm;
 extern bool       debugOutput;
+extern bool       sendStartupMessage;
 
 extern CompScreen   *screen;
 

=== modified file 'src/global.cpp'
--- src/global.cpp	2012-04-30 16:10:35 +0000
+++ src/global.cpp	2012-12-05 05:49:21 +0000
@@ -36,6 +36,7 @@
 bool restartSignal = false;
 
 bool replaceCurrentWm = false;
+bool sendStartupMessage = false;
 bool useDesktopHints = false;
 bool debugOutput = false;
 bool synchronousX = false;

=== modified file 'src/main.cpp'
--- src/main.cpp	2012-06-29 03:39:30 +0000
+++ src/main.cpp	2012-12-05 05:49:21 +0000
@@ -103,6 +103,10 @@
 	{
 	    replaceCurrentWm = true;
 	}
+	else if (!strcmp (argv[i], "--send-startup-message"))
+	{
+	    sendStartupMessage = true;
+	}
 	else if (!strcmp (argv[i], "--sm-disable"))
 	{
 	    disableSm = true;

=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-12-04 16:09:08 +0000
+++ src/screen.cpp	2012-12-05 05:49:21 +0000
@@ -4675,6 +4675,40 @@
     return privateScreen.orphanData.nextActiveWindow;
 }
 
+namespace
+{
+void sendStartupMessageToClients (Display *dpy, bool success)
+{
+    /* Send a client message indicating that our startup is complete if
+     * we were asked to do so */
+    if (sendStartupMessage)
+    {
+	Atom   startupMessageAtom = XInternAtom (dpy,
+						 "_COMPIZ_TESTING_STARTUP",
+						 FALSE);
+	XEvent startupMessageEvent;
+	Window root = DefaultRootWindow (dpy);
+
+	startupMessageEvent.xclient.type         = ClientMessage;
+	startupMessageEvent.xclient.window       = root;
+	startupMessageEvent.xclient.message_type = startupMessageAtom;
+	startupMessageEvent.xclient.format       = 32;
+	startupMessageEvent.xclient.data.l[0]    = success ? 1 : 0;
+	startupMessageEvent.xclient.data.l[1]    = 0;
+	startupMessageEvent.xclient.data.l[2]    = 0;
+	startupMessageEvent.xclient.data.l[3]    = 0;
+	startupMessageEvent.xclient.data.l[4]    = 0;
+
+	XSendEvent (dpy,
+		    root,
+		    FALSE,
+		    StructureNotifyMask,
+		    &startupMessageEvent);
+	XFlush (dpy);
+    }
+}
+}
+
 
 bool
 PrivateScreen::initDisplay (const char *name, cps::History& history, unsigned int showingDesktopMask)
@@ -4706,6 +4740,7 @@
     {
 	compLogMessage ("core", CompLogLevelFatal,
 		       "No sync extension");
+	sendStartupMessageToClients (dpy, false);
 	return false;
     }
 
@@ -4736,91 +4771,10 @@
     escapeKeyCode = XKeysymToKeycode (dpy, XStringToKeysym ("Escape"));
     returnKeyCode = XKeysymToKeycode (dpy, XStringToKeysym ("Return"));
 
-    char                 buf[128];
-    sprintf (buf, "WM_S%d", DefaultScreen (dpy));
-    wmSnAtom = XInternAtom (dpy, buf, 0);
-
-    Window currentWmSnOwner = XGetSelectionOwner (dpy, wmSnAtom);
-
-    if (currentWmSnOwner != None)
-    {
-	if (!replaceCurrentWm)
-	{
-	    compLogMessage ("core", CompLogLevelError,
-			    "Screen %d on display \"%s\" already "
-			    "has a window manager; try using the "
-			    "--replace option to replace the current "
-			    "window manager.",
-			    DefaultScreen (dpy), DisplayString (dpy));
-
-	    return false;
-	}
-
-	XSelectInput (dpy, currentWmSnOwner, StructureNotifyMask);
-    }
+    modHandler->updateModifierMappings ();
 
     Window root_tmp = XRootWindow (dpy, DefaultScreen (dpy));
 
-    XSetWindowAttributes attr;
-    attr.override_redirect = true;
-    attr.event_mask        = PropertyChangeMask;
-
-    Window newWmSnOwner =
-	XCreateWindow (dpy, root_tmp, -100, -100, 1, 1, 0,
-		       CopyFromParent, CopyFromParent,
-		       CopyFromParent,
-		       CWOverrideRedirect | CWEventMask,
-		       &attr);
-
-    XChangeProperty (dpy, newWmSnOwner, Atoms::wmName, Atoms::utf8String, 8,
-		     PropModeReplace, (unsigned char *) PACKAGE,
-		     strlen (PACKAGE));
-
-    XEvent event;
-    XWindowEvent (dpy, newWmSnOwner, PropertyChangeMask, &event);
-
-    Time wmSnTimestamp = event.xproperty.time;
-
-    XSetSelectionOwner (dpy, wmSnAtom, newWmSnOwner, wmSnTimestamp);
-
-    if (XGetSelectionOwner (dpy, wmSnAtom) != newWmSnOwner)
-    {
-	compLogMessage ("core", CompLogLevelError,
-			"Could not acquire window manager "
-			"selection on screen %d display \"%s\"",
-			DefaultScreen (dpy), DisplayString (dpy));
-
-	XDestroyWindow (dpy, newWmSnOwner);
-
-	return false;
-    }
-
-    /* Send client message indicating that we are now the window manager */
-    event.xclient.type         = ClientMessage;
-    event.xclient.window       = root_tmp;
-    event.xclient.message_type = Atoms::manager;
-    event.xclient.format       = 32;
-    event.xclient.data.l[0]    = wmSnTimestamp;
-    event.xclient.data.l[1]    = wmSnAtom;
-    event.xclient.data.l[2]    = 0;
-    event.xclient.data.l[3]    = 0;
-    event.xclient.data.l[4]    = 0;
-
-    XSendEvent (dpy, root_tmp, FALSE, StructureNotifyMask, &event);
-
-    /* Wait for old window manager to go away */
-    if (currentWmSnOwner != None)
-    {
-	do {
-	    XWindowEvent (dpy, currentWmSnOwner, StructureNotifyMask, &event);
-	} while (event.type != DestroyNotify);
-    }
-
-    modHandler->updateModifierMappings ();
-
-    CompScreenImpl::checkForError (dpy);
-
-    XGrabServer (dpy);
 
     /* Don't select for SubstructureRedirectMask or
      * SubstructureNotifyMask yet since we need to
@@ -4859,17 +4813,6 @@
 		      SubstructureNotifyMask);
     }
 
-    if (CompScreenImpl::checkForError (dpy))
-    {
-	compLogMessage ("core", CompLogLevelError,
-			"Another window manager is "
-			"already running on screen: %d", DefaultScreen (dpy));
-
-	XUngrabServer (dpy);
-	XSync (dpy, false);
-	return false;
-    }
-
     for (int i = 0; i < SCREEN_EDGE_NUM; i++)
     {
 	screenEdge[i].id    = None;
@@ -4883,9 +4826,6 @@
     snContext = sn_monitor_context_new (snDisplay, screenNum,
 					      compScreenSnEvent, this, NULL);
 
-    wmSnSelectionWindow = newWmSnOwner;
-    this->wmSnTimestamp       = wmSnTimestamp;
-
     if (!XGetWindowAttributes (dpy, rootWindow(), &attrib))
 	return false;
 
@@ -4901,6 +4841,7 @@
     {
 	compLogMessage ("core", CompLogLevelFatal,
 			"Couldn't get visual info for default visual");
+	sendStartupMessageToClients (dpy, false);
 	return false;
     }
 
@@ -4911,6 +4852,7 @@
     {
 	compLogMessage ("core", CompLogLevelFatal,
 			"Couldn't allocate color");
+	sendStartupMessageToClients (dpy, false);
 	XFree (visinfo);
 	return false;
     }
@@ -4921,6 +4863,7 @@
     {
 	compLogMessage ("core", CompLogLevelFatal,
 			"Couldn't create bitmap");
+	sendStartupMessageToClients (dpy, false);
 	XFree (visinfo);
 	return false;
     }
@@ -4931,6 +4874,7 @@
     {
 	compLogMessage ("core", CompLogLevelFatal,
 			"Couldn't create invisible cursor");
+	sendStartupMessageToClients (dpy, false);
 	XFree (visinfo);
 	return false;
     }
@@ -4949,6 +4893,93 @@
 
     getDesktopHints (showingDesktopMask);
 
+    /* Check for other window managers */
+
+    char                 buf[128];
+    sprintf (buf, "WM_S%d", DefaultScreen (dpy));
+    wmSnAtom = XInternAtom (dpy, buf, 0);
+
+    Window currentWmSnOwner = XGetSelectionOwner (dpy, wmSnAtom);
+
+    if (currentWmSnOwner != None)
+    {
+	if (!replaceCurrentWm)
+	{
+	    compLogMessage ("core", CompLogLevelError,
+			    "Screen %d on display \"%s\" already "
+			    "has a window manager; try using the "
+			    "--replace option to replace the current "
+			    "window manager.",
+			    DefaultScreen (dpy), DisplayString (dpy));
+	    sendStartupMessageToClients (dpy, false);
+
+	    return false;
+	}
+
+	XSelectInput (dpy, currentWmSnOwner, StructureNotifyMask);
+    }
+
+    XSetWindowAttributes attr;
+    attr.override_redirect = true;
+    attr.event_mask        = PropertyChangeMask;
+
+    Window newWmSnOwner =
+	XCreateWindow (dpy, root_tmp, -100, -100, 1, 1, 0,
+		       CopyFromParent, CopyFromParent,
+		       CopyFromParent,
+		       CWOverrideRedirect | CWEventMask,
+		       &attr);
+
+    XChangeProperty (dpy, newWmSnOwner, Atoms::wmName, Atoms::utf8String, 8,
+		     PropModeReplace, (unsigned char *) PACKAGE,
+		     strlen (PACKAGE));
+
+    XEvent event;
+    XWindowEvent (dpy, newWmSnOwner, PropertyChangeMask, &event);
+
+    Time wmSnTimestamp = event.xproperty.time;
+
+    XSetSelectionOwner (dpy, wmSnAtom, newWmSnOwner, wmSnTimestamp);
+
+    if (XGetSelectionOwner (dpy, wmSnAtom) != newWmSnOwner)
+    {
+	compLogMessage ("core", CompLogLevelError,
+			"Could not acquire window manager "
+			"selection on screen %d display \"%s\"",
+			DefaultScreen (dpy), DisplayString (dpy));
+
+	XDestroyWindow (dpy, newWmSnOwner);
+	sendStartupMessageToClients (dpy, false);
+
+	return false;
+    }
+
+    /* Send client message indicating that we are now the window manager */
+    event.xclient.type         = ClientMessage;
+    event.xclient.window       = root_tmp;
+    event.xclient.message_type = Atoms::manager;
+    event.xclient.format       = 32;
+    event.xclient.data.l[0]    = wmSnTimestamp;
+    event.xclient.data.l[1]    = wmSnAtom;
+    event.xclient.data.l[2]    = 0;
+    event.xclient.data.l[3]    = 0;
+    event.xclient.data.l[4]    = 0;
+
+    XSendEvent (dpy, root_tmp, FALSE, StructureNotifyMask, &event);
+
+    /* Wait for old window manager to go away */
+    if (currentWmSnOwner != None)
+    {
+	do {
+	    XWindowEvent (dpy, currentWmSnOwner, StructureNotifyMask, &event);
+	} while (event.type != DestroyNotify);
+    }
+
+
+    /* Server grab from here, we are creating windows */
+
+    XGrabServer (dpy);
+
     {
 	XSetWindowAttributes attrib;
 	attrib.override_redirect = 1;
@@ -4994,11 +5025,30 @@
 
     XDefineCursor (dpy, rootWindow(), normalCursor);
 
+    /* Attempt to gain SubstructureRedirectMask */
+    CompScreenImpl::checkForError (dpy);
+
     /* We should get DestroyNotify events for any windows that were
      * destroyed while initializing windows here now */
     XSelectInput (dpy, rootWindow(), attrib.your_event_mask |
 		  SubstructureRedirectMask | SubstructureNotifyMask);
 
+    if (CompScreenImpl::checkForError (dpy))
+    {
+	compLogMessage ("core", CompLogLevelError,
+			"Another window manager is "
+			"already running on screen: %d", DefaultScreen (dpy));
+
+	XUngrabServer (dpy);
+	XSync (dpy, FALSE);
+
+	sendStartupMessageToClients (dpy, false);
+	return false;
+    }
+
+    wmSnSelectionWindow = newWmSnOwner;
+    this->wmSnTimestamp       = wmSnTimestamp;
+
     Window rootReturn, parentReturn;
     Window               *children;
     unsigned int         nchildren;
@@ -5096,6 +5146,8 @@
 
     pingTimer.start ();
 
+    sendStartupMessageToClients (dpy, true);
+
     return true;
 }
 
@@ -5278,8 +5330,23 @@
 	if (invisibleCursor != None)
 	    XFreeCursor (dpy, invisibleCursor);
 
-	if (wmSnSelectionWindow != None)
-	    XDestroyWindow (dpy, wmSnSelectionWindow);
+	/* Do not destroy wmSnSelectionWindow here.
+	 *
+	 * Because we haven't changed our active event mask
+	 * to remove SubstructureRedirectMask, other ICCCM
+	 * compliant window managers may receive a DestroyNotify
+	 * (eg, because we're blocked on XSync) before we get
+	 * a chance to close our display connection and remove
+	 * our SubstructureRedirectMask. That will cause them
+	 * to fail to start.
+	 *
+	 * The selection window is destroyed anyways when we
+	 * close our connection, and that is a very accurate
+	 * indicator to other WM's that we are well and truly
+	 * gone because the protocol requires the implementation
+	 * to remove all client event masks before destroying
+	 * windows
+	 */
 
 	XSync (dpy, False);  // Redundant?
 	XCloseDisplay (dpy);

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2012-09-18 10:45:43 +0000
+++ tests/CMakeLists.txt	2012-12-05 05:49:21 +0000
@@ -1,4 +1,6 @@
 if (COMPIZ_BUILD_TESTING)
     add_subdirectory (integration)
     add_subdirectory (system)
+    add_subdirectory (shared)
+    add_subdirectory (acceptance-tests)
 endif (COMPIZ_BUILD_TESTING)

=== added directory 'tests/acceptance-tests'
=== added file 'tests/acceptance-tests/CMakeLists.txt'
--- tests/acceptance-tests/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/CMakeLists.txt	2012-12-05 05:49:21 +0000
@@ -0,0 +1,1 @@
+add_subdirectory (xorg-gtest)

=== added directory 'tests/acceptance-tests/xorg-gtest'
=== added file 'tests/acceptance-tests/xorg-gtest/CMakeLists.txt'
--- tests/acceptance-tests/xorg-gtest/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/xorg-gtest/CMakeLists.txt	2012-12-05 05:49:21 +0000
@@ -0,0 +1,11 @@
+if (BUILD_XORG_GTEST)
+
+    include_directories (${compiz_SOURCE_DIR}/tests/shared
+                         ${COMPIZ_XORG_SYSTEM_TEST_INCLUDE_DIR}
+			 ${X11_INCLUDE_DIRS}
+			 ${XORG_SERVER_INCLUDE_XORG_GTEST}
+			 ${XORG_SERVER_GTEST_SRC}
+			 ${GTEST_INCLUDE_DIRS})
+    add_subdirectory (tests)
+
+endif (BUILD_XORG_GTEST)

=== added directory 'tests/acceptance-tests/xorg-gtest/tests'
=== added file 'tests/acceptance-tests/xorg-gtest/tests/CMakeLists.txt'
--- tests/acceptance-tests/xorg-gtest/tests/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/xorg-gtest/tests/CMakeLists.txt	2012-12-05 05:49:21 +0000
@@ -0,0 +1,21 @@
+link_directories (${X11_XI_LIBRARY_DIRS}
+                  ${compiz_BINARY_DIR}/tests/shared/src)
+
+add_executable (compiz_acceptance_test_xorg_replace_current_wm
+                ${CMAKE_CURRENT_SOURCE_DIR}/compiz_acceptance_replace_current_wm.cpp)
+
+set (COMPIZ_XORG_ACCEPTANCE_TEST_LIBRARIES
+     compiz_xorg_gtest_system_test
+     xorg_gtest_all
+     xorg_gtest_main
+     ${GTEST_BOTH_LIBRARIES}
+     ${CMAKE_THREAD_LIBS_INIT}
+     ${XORG_SERVER_LIBRARIES}
+     ${X11_XI_LIBRARIES})
+
+target_link_libraries (compiz_acceptance_test_xorg_replace_current_wm
+                       ${COMPIZ_XORG_ACCEPTANCE_TEST_LIBRARIES}
+                       compiz_gtest_shared_async_task)
+
+# Not autodiscovering tests here, these tests are
+# inherently flakey. See README

=== added file 'tests/acceptance-tests/xorg-gtest/tests/README'
--- tests/acceptance-tests/xorg-gtest/tests/README	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/xorg-gtest/tests/README	2012-12-05 05:49:21 +0000
@@ -0,0 +1,3 @@
+These tests are acceptance tests and may take a while to run. As such
+please do not add them to the compiz_discover_tests list. Use them to
+check that startup and shutdown work correctly

=== added file 'tests/acceptance-tests/xorg-gtest/tests/compiz_acceptance_replace_current_wm.cpp'
--- tests/acceptance-tests/xorg-gtest/tests/compiz_acceptance_replace_current_wm.cpp	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/xorg-gtest/tests/compiz_acceptance_replace_current_wm.cpp	2012-12-05 05:49:21 +0000
@@ -0,0 +1,255 @@
+/*
+ * Compiz XOrg GTest Acceptance Tests
+ *
+ * Copyright (C) 2012 Canonical Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Authored By:
+ * Sam Spilsbury <[e-mail]>
+ */
+#include <sys/types.h>
+#include <signal.h>
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include <boost/function.hpp>
+#include <boost/bind.hpp>
+
+#include <boost/shared_ptr.hpp>
+#include <boost/make_shared.hpp>
+
+#include <xorg/gtest/xorg-gtest.h>
+#include <compiz-xorg-gtest.h>
+
+#include <gtest_shared_tmpenv.h>
+#include <gtest_shared_characterwrapper.h>
+#include <gtest_shared_asynctask.h>
+
+#include <X11/Xlib.h>
+#include <X11/Xatom.h>
+
+namespace ct = compiz::testing;
+
+class XorgAcceptance :
+    public xorg::testing::Test
+{
+    public:
+
+	typedef boost::shared_ptr <ct::CompizProcess> ProcessPtr;
+};
+
+namespace
+{
+
+char TEST_FAILED_MSG = 's';
+char PROCESS_EXITED_MSG = 'd';
+
+}
+
+class WaitForSuccessDeathTask :
+    public AsyncTask
+{
+    public:
+
+	typedef boost::function <xorg::testing::Process::State ()> GetProcessState;
+
+	WaitForSuccessDeathTask (const GetProcessState &procState) :
+	    mProcessState (procState)
+	{
+	}
+
+    private:
+
+	GetProcessState mProcessState;
+
+	void Task ();
+};
+
+void
+WaitForSuccessDeathTask::Task ()
+{
+    do
+    {
+	if (ReadMsgFromTest (TEST_FAILED_MSG, 1))
+	    return;
+    } while (mProcessState () != xorg::testing::Process::FINISHED_SUCCESS);
+
+    /* The process died, send a message back saying that it did */
+    SendMsgToTest (PROCESS_EXITED_MSG);
+}
+
+TEST_F (XorgAcceptance, SIGINTClosesDown)
+{
+    /* XXX: This is a bit stupid, but we have to do it.
+     * It seems as though closing the child stdout or
+     * stderr will cause the client to hang indefinitely
+     * when the child calls XSync (and that can happen
+     * implicitly, eg XCloseDisplay) */
+    TmpEnv env ("XORG_GTEST_CHILD_STDOUT", "1");
+    ProcessPtr compiz (boost::make_shared <ct::CompizProcess> (Display (),
+							       ct::CompizProcess::WaitForStartupMessage,
+							       3000));
+
+    pid_t firstProcessPid = compiz->Pid ();
+
+    WaitForSuccessDeathTask::GetProcessState procState (boost::bind (&ct::CompizProcess::State,
+								     compiz.get ()));
+    AsyncTask::Ptr task (boost::make_shared <WaitForSuccessDeathTask> (procState));
+
+    kill (firstProcessPid, SIGINT);
+
+    const unsigned int maximumWaitTime = 10 * 1000; // Ten seconds
+
+    if (!task->ReadMsgFromTask (PROCESS_EXITED_MSG, maximumWaitTime))
+    {
+	task->SendMsgToTask (TEST_FAILED_MSG);
+	throw std::runtime_error ("compiz process did not exit with success status");
+    }
+}
+
+TEST_F (XorgAcceptance, ReplaceOtherWMFast)
+{
+    /* XXX: This is a bit stupid, but we have to do it.
+     * It seems as though closing the child stdout or
+     * stderr will cause the client to hang indefinitely
+     * when the child calls XSync (and that can happen
+     * implicitly, eg XCloseDisplay) */
+    TmpEnv env ("XORG_GTEST_CHILD_STDOUT", "1");
+    ProcessPtr firstCompiz (boost::make_shared <ct::CompizProcess> (Display (),
+								    ct::CompizProcess::WaitForStartupMessage,
+								    3000));
+
+    /* Expect it to exit */
+    WaitForSuccessDeathTask::GetProcessState procState (boost::bind (&ct::CompizProcess::State,
+								     firstCompiz.get ()));
+    AsyncTask::Ptr task (boost::make_shared <WaitForSuccessDeathTask> (procState));
+    const unsigned int maximumWaitTime = 10 * 1000; // Ten seconds
+
+    ProcessPtr secondCompiz (boost::make_shared <ct::CompizProcess> (Display (),
+								     static_cast <ct::CompizProcess::StartupFlags> (
+									ct::CompizProcess::WaitForStartupMessage |
+									ct::CompizProcess::ReplaceCurrentWM),
+								     maximumWaitTime));
+
+    if (!task->ReadMsgFromTask (PROCESS_EXITED_MSG, maximumWaitTime))
+    {
+	task->SendMsgToTask (TEST_FAILED_MSG);
+	throw std::runtime_error ("compiz process did not exit with success status");
+    }
+}
+
+namespace
+{
+char TEST_FINISHED_MSG = 's';
+}
+
+class SlowDownTask :
+    public AsyncTask
+{
+    public:
+
+	typedef boost::function <xorg::testing::Process::State ()> GetProcessState;
+	typedef boost::function <pid_t ()> GetPid;
+
+	SlowDownTask (const GetProcessState &procState,
+		      const GetPid          &pid) :
+	    mProcessState (procState),
+	    mPid (pid),
+	    mIsRunning (true)
+	{
+	}
+
+    private:
+
+	GetProcessState mProcessState;
+	GetPid          mPid;
+	bool            mIsRunning;
+
+	void Task ();
+};
+
+void
+SlowDownTask::Task ()
+{
+    do
+    {
+	if (ReadMsgFromTest (TEST_FINISHED_MSG, mIsRunning ? 1 : 400))
+	    return;
+
+	pid_t pid = mPid ();
+
+	if (pid)
+	{
+	    if (mIsRunning)
+	    {
+		kill (pid, SIGSTOP);
+		mIsRunning = false;
+	    }
+	    else
+	    {
+		kill (pid, SIGCONT);
+		mIsRunning = true;
+	    }
+	}
+
+    } while (mProcessState () != xorg::testing::Process::FINISHED_SUCCESS);
+
+    /* The process died, send a message back saying that it did */
+    SendMsgToTest (PROCESS_EXITED_MSG);
+}
+
+TEST_F (XorgAcceptance, ReplaceOtherWMSlow)
+{
+    ::Display *dpy = Display ();
+
+    /* XXX: This is a bit stupid, but we have to do it.
+     * It seems as though closing the child stdout or
+     * stderr will cause the client to hang indefinitely
+     * when the child calls XSync (and that can happen
+     * implicitly, eg XCloseDisplay) */
+    TmpEnv env ("XORG_GTEST_CHILD_STDOUT", "1");
+    ProcessPtr firstCompiz (boost::make_shared <ct::CompizProcess> (dpy,
+								    ct::CompizProcess::WaitForStartupMessage,
+								    3000));
+
+    SlowDownTask::GetProcessState procState (boost::bind (&ct::CompizProcess::State,
+							  firstCompiz.get ()));
+    SlowDownTask::GetPid getPid (boost::bind (&ct::CompizProcess::Pid,
+					      firstCompiz.get ()));
+
+    /* Slow down the first compiz */
+    AsyncTask::Ptr task (boost::make_shared <SlowDownTask> (procState, getPid));
+
+    /* Select for StructureNotifyMask */
+    XSelectInput (dpy,
+		  DefaultRootWindow (dpy),
+		  StructureNotifyMask);
+
+    const unsigned int maximumWaitTime = 20 * 1000; // Twenty seconds
+
+    ProcessPtr secondCompiz (boost::make_shared <ct::CompizProcess> (Display (),
+								     static_cast <ct::CompizProcess::StartupFlags> (
+									ct::CompizProcess::ReplaceCurrentWM |
+									ct::CompizProcess::WaitForStartupMessage),
+								     maximumWaitTime));
+
+    /* Wait until the first one goes away */
+    if (!task->ReadMsgFromTask (PROCESS_EXITED_MSG, maximumWaitTime))
+    {
+	task->SendMsgToTask (TEST_FINISHED_MSG);
+	throw std::runtime_error ("compiz process did not exit with success status");
+    }
+}

=== added file 'tests/shared/CMakeLists.txt'
--- tests/shared/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/shared/CMakeLists.txt	2012-12-05 05:49:21 +0000
@@ -0,0 +1,2 @@
+set (GTEST_SHARED_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+add_subdirectory (src)

=== added file 'tests/shared/gtest_shared_asynctask.h'
--- tests/shared/gtest_shared_asynctask.h	1970-01-01 00:00:00 +0000
+++ tests/shared/gtest_shared_asynctask.h	2012-12-05 05:49:21 +0000
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2012 Canonical Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Authored By:
+ * Sam Spilsbury <[e-mail]>
+ */
+#ifndef _COMPIZ_GTEST_SHARED_ASYNCTASK_H
+#define _COMPIZ_GTEST_SHARED_ASYNCTASK_H
+
+#include <memory>
+#include <boost/shared_ptr.hpp>
+
+class PrivateAsyncTask;
+class AsyncTask
+{
+    public:
+
+	typedef boost::shared_ptr <AsyncTask> Ptr;
+
+	AsyncTask ();
+	~AsyncTask ();
+
+	void SendMsgToTask (char msg);
+	bool ReadMsgFromTask (char msg, int maximumWait);
+
+    protected:
+
+	void SendMsgToTest (char msg);
+	bool ReadMsgFromTest (char msg, int maximumWait);
+
+    private:
+
+	static void * ThreadEntry (void *data);
+	virtual void Task () = 0;
+
+	std::auto_ptr <PrivateAsyncTask> priv;
+};
+
+#endif

=== added file 'tests/shared/gtest_shared_tmpenv.h'
--- tests/shared/gtest_shared_tmpenv.h	1970-01-01 00:00:00 +0000
+++ tests/shared/gtest_shared_tmpenv.h	2012-12-05 05:49:21 +0000
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2012 Canonical Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Authored By:
+ * Sam Spilsbury <[e-mail]>
+ */
+#ifndef _COMPIZ_GTEST_SHARED_TMPENV
+#define _COMPIZ_GTEST_SHARED_TMPENV
+
+#include <cstdlib>
+#include <boost/noncopyable.hpp>
+
+class TmpEnv :
+    boost::noncopyable
+{
+    public:
+
+	explicit TmpEnv (const char *env, const char *val) :
+	    envRestore (env),
+	    envRestoreVal (getenv (env))
+	{
+	    if (!val)
+		unsetenv (env);
+	    else
+		setenv (env, val, 1);
+	}
+
+	~TmpEnv ()
+	{
+	    if (envRestoreVal)
+		setenv (envRestore, envRestoreVal, 1);
+	    else
+		unsetenv (envRestore);
+	}
+
+    private:
+
+	const char *envRestore;
+	const char *envRestoreVal;
+};
+
+#endif

=== added directory 'tests/shared/src'
=== added file 'tests/shared/src/CMakeLists.txt'
--- tests/shared/src/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/shared/src/CMakeLists.txt	2012-12-05 05:49:21 +0000
@@ -0,0 +1,7 @@
+include_directories (${GTEST_SHARED_INCLUDE_DIR})
+
+add_library (compiz_gtest_shared_async_task STATIC
+             gtest_shared_asynctask.cpp)
+
+target_link_libraries (compiz_gtest_shared_async_task
+		       ${CMAKE_THREAD_LIBS_INIT})

=== added file 'tests/shared/src/gtest_shared_asynctask.cpp'
--- tests/shared/src/gtest_shared_asynctask.cpp	1970-01-01 00:00:00 +0000
+++ tests/shared/src/gtest_shared_asynctask.cpp	2012-12-05 05:49:21 +0000
@@ -0,0 +1,138 @@
+/*
+ * Copyright (C) 2012 Canonical Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Authored By:
+ * Sam Spilsbury <[e-mail]>
+ */
+#include <stdexcept>
+#include <string.h>
+#include <errno.h>
+#include <poll.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include "gtest_shared_asynctask.h"
+#include "gtest_shared_characterwrapper.h"
+
+namespace
+{
+void
+runtimeErrorWithErrno ()
+{
+    CharacterWrapper allocatedError (strerror (errno));
+    throw std::runtime_error (std::string (allocatedError));
+}
+
+bool
+checkForMessageOnFd (int fd, int timeout, char expected)
+{
+    struct pollfd pfd;
+    pfd.fd = fd;
+    pfd.events = POLLIN;
+    pfd.revents = 0;
+
+    /* Wait 1ms */
+    poll (&pfd, 1, timeout);
+
+    if (pfd.revents == POLLIN)
+    {
+	char msg[1];
+	size_t sz = read (pfd.fd, reinterpret_cast <void *> (msg), 1);
+	if (sz == 1 &&
+	    msg[0] == expected)
+	    return true;
+    }
+
+    return false;
+}
+}
+
+class PrivateAsyncTask
+{
+    public:
+
+	int testToTaskFd[2];
+	int taskToTestFd[2];
+	pthread_t mThread;
+};
+
+AsyncTask::AsyncTask () :
+    priv (new PrivateAsyncTask)
+{
+    if (pipe2 (priv->testToTaskFd, O_NONBLOCK | O_CLOEXEC) == -1)
+	runtimeErrorWithErrno ();
+
+    if (pipe2 (priv->taskToTestFd, O_NONBLOCK | O_CLOEXEC) == -1)
+	runtimeErrorWithErrno ();
+
+    if (pthread_create (&priv->mThread,
+			NULL,
+			AsyncTask::ThreadEntry,
+			reinterpret_cast<void *> (this)) != 0)
+	runtimeErrorWithErrno ();
+}
+
+AsyncTask::~AsyncTask ()
+{
+    void *ret;
+    if (pthread_join (priv->mThread, &ret) != 0)
+    {
+	runtimeErrorWithErrno ();
+    }
+
+    close (priv->testToTaskFd[0]);
+    close (priv->testToTaskFd[1]);
+    close (priv->taskToTestFd[0]);
+    close (priv->taskToTestFd[0]);
+}
+
+bool
+AsyncTask::ReadMsgFromTest (char msg, int maximumWait)
+{
+    return checkForMessageOnFd (priv->testToTaskFd[0], maximumWait, msg);
+}
+
+bool
+AsyncTask::ReadMsgFromTask (char msg, int maximumWait)
+{
+    return checkForMessageOnFd (priv->taskToTestFd[0], maximumWait, msg);
+}
+
+void
+AsyncTask::SendMsgToTest (char msg)
+{
+    char buf[1] = { msg };
+    if (write (priv->taskToTestFd[1], reinterpret_cast <void *> (buf), 1) == -1)
+	runtimeErrorWithErrno ();
+}
+
+void
+AsyncTask::SendMsgToTask (char msg)
+{
+    char buf[1] = { msg };
+    if (write (priv->testToTaskFd[1], reinterpret_cast <void *> (buf), 1) == -1)
+	runtimeErrorWithErrno ();
+}
+
+void *
+AsyncTask::ThreadEntry (void *data)
+{
+    AsyncTask *task = reinterpret_cast <AsyncTask *> (data);
+    task->Task ();
+    return NULL;
+}

=== modified file 'tests/system/xorg-gtest/include/compiz-xorg-gtest.h'
--- tests/system/xorg-gtest/include/compiz-xorg-gtest.h	2012-09-18 15:48:30 +0000
+++ tests/system/xorg-gtest/include/compiz-xorg-gtest.h	2012-12-05 05:49:21 +0000
@@ -22,6 +22,8 @@
  */
 #ifndef _COMPIZ_XORG_GTEST_H
 #define _COMPIZ_XORG_GTEST_H
+#include <memory>
+#include <list>
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 #include <xorg/gtest/xorg-gtest.h>
@@ -35,6 +37,8 @@
 	typedef  ::testing::MatcherInterface <const XEvent &> XEventMatcher;
 
 	std::list <Window> NET_CLIENT_LIST_STACKING (Display *);
+	bool AdvanceToNextEventOnSuccess (Display *dpy,
+					  bool waitResult);
 	bool WaitForEventOfTypeOnWindow (Display *dpy,
 					 Window  w,
 					 int     type,
@@ -49,17 +53,50 @@
 						 const XEventMatcher &matcher,
 						 int                 timeout = 1000);
 
-	class XorgSystemTest :
+	class PrivateCompizProcess;
+	class CompizProcess
+	{
+	    public:
+		typedef enum _StartupFlags
+		{
+		    ReplaceCurrentWM = (1 << 0),
+		    WaitForStartupMessage = (1 << 1),
+		    ExpectStartupFailure = (1 << 2)
+		} StartupFlags;
+
+		CompizProcess (Display *dpy, StartupFlags, unsigned int waitTimeout);
+		~CompizProcess ();
+		xorg::testing::Process::State State ();
+		pid_t Pid ();
+
+	    private:
+		std::auto_ptr <PrivateCompizProcess> priv;
+	};
+
+	class PrivateCompizXorgSystemTest;
+	class CompizXorgSystemTest :
 	    public xorg::testing::Test
 	{
 	    public:
 
+		CompizXorgSystemTest ();
+
 		virtual void SetUp ();
 		virtual void TearDown ();
 
+		xorg::testing::Process::State CompizProcessState ();
+		void StartCompiz (CompizProcess::StartupFlags flags);
+
 	    private:
-
-		xorg::testing::Process mCompizProcess;
+		std::auto_ptr <PrivateCompizXorgSystemTest> priv;
+	};
+
+	class AutostartCompizXorgSystemTest :
+	    public CompizXorgSystemTest
+	{
+	    public:
+
+		virtual void SetUp ();
 	};
     }
 }

=== modified file 'tests/system/xorg-gtest/src/compiz-xorg-gtest.cpp'
--- tests/system/xorg-gtest/src/compiz-xorg-gtest.cpp	2012-09-18 16:14:34 +0000
+++ tests/system/xorg-gtest/src/compiz-xorg-gtest.cpp	2012-12-05 05:49:21 +0000
@@ -24,6 +24,7 @@
 #include <stdexcept>
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
+#include <boost/shared_ptr.hpp>
 #include <xorg/gtest/xorg-gtest.h>
 #include <compiz-xorg-gtest.h>
 #include <X11/Xlib.h>
@@ -31,6 +32,9 @@
 
 #include "compiz-xorg-gtest-config.h"
 
+using ::testing::MatchResultListener;
+using ::testing::MatcherInterface;
+
 namespace ct = compiz::testing;
 namespace
 {
@@ -44,6 +48,16 @@
 }
 
 bool
+ct::AdvanceToNextEventOnSuccess (Display *dpy,
+				 bool waitResult)
+{
+    if (waitResult)
+	RemoveEventFromQueue (dpy);
+
+    return waitResult;
+}
+
+bool
 ct::WaitForEventOfTypeOnWindow (Display *dpy,
 				Window  w,
 				int     type,
@@ -57,10 +71,11 @@
 	if (!XPeekEvent (dpy, &event))
 	    throw std::runtime_error ("Failed to peek event");
 
-	RemoveEventFromQueue (dpy);
-
 	if (event.xany.window != w)
+	{
+	    RemoveEventFromQueue (dpy);
 	    continue;
+	}
 
 	return true;
     }
@@ -84,9 +99,10 @@
 	    throw std::runtime_error ("Failed to peek event");
 
 	if (!matcher.MatchAndExplain (event, NULL))
+	{
+	    RemoveEventFromQueue (dpy);
 	    continue;
-
-	RemoveEventFromQueue (dpy);
+	}
 
 	return true;
     }
@@ -124,27 +140,190 @@
     return stackingOrder;
 }
 
-void
-ct::XorgSystemTest::SetUp ()
+Display *d;
+
+namespace
+{
+class StartupClientMessageMatcher :
+    public ct::XEventMatcher
+{
+    public:
+
+	StartupClientMessageMatcher (Atom                             startup,
+				     Window                           root,
+				     ct::CompizProcess::StartupFlags state) :
+	    mStartup (startup),
+	    mRoot (root),
+	    mFlags (state)
+	{
+	}
+
+	virtual bool MatchAndExplain (const XEvent &event, MatchResultListener *listener) const
+	{
+	    int state = mFlags & ct::CompizProcess::ExpectStartupFailure ? 0 : 1;
+
+	    if (event.xclient.window == mRoot &&
+		event.xclient.message_type == mStartup &&
+		event.xclient.data.l[0] == state)
+		return true;
+
+	    return false;
+	}
+
+	virtual void DescribeTo (std::ostream *os) const
+	{
+	    *os << "is startup message";
+	}
+
+	virtual void DescribeNegationTo (std::ostream *os) const
+	{
+	    *os << "is not startup message";
+	}
+
+    private:
+
+	Atom mStartup;
+	Window mRoot;
+	ct::CompizProcess::StartupFlags mFlags;
+};
+}
+
+class ct::PrivateCompizProcess
+{
+    public:
+	PrivateCompizProcess (ct::CompizProcess::StartupFlags flags) :
+	    mFlags (flags),
+	    mIsRunning (true)
+	{
+	}
+
+	void WaitForStartupMessage (Display                         *dpy,
+				    ct::CompizProcess::StartupFlags flags,
+				    unsigned int                    waitTimeout);
+
+	typedef boost::shared_ptr <xorg::testing::Process> ProcessPtr;
+
+	ct::CompizProcess::StartupFlags mFlags;
+	bool                            mIsRunning;
+	xorg::testing::Process          mProcess;
+};
+
+void
+ct::PrivateCompizProcess::WaitForStartupMessage (Display                         *dpy,
+						 ct::CompizProcess::StartupFlags flags,
+						 unsigned int                    waitTimeout)
+{
+    XWindowAttributes attrib;
+    Window    root = DefaultRootWindow (dpy);
+
+    Atom    startup = XInternAtom (dpy,
+				   "_COMPIZ_TESTING_STARTUP",
+				   false);
+
+    StartupClientMessageMatcher matcher (startup, root, flags);
+
+    d = dpy;
+    /* Save the current event mask and subscribe to StructureNotifyMask only */
+    ASSERT_TRUE (XGetWindowAttributes (dpy, root, &attrib));
+    XSelectInput (dpy, root, StructureNotifyMask |
+			     attrib.your_event_mask);
+
+    ASSERT_TRUE (ct::AdvanceToNextEventOnSuccess (
+		     dpy,
+		     ct::WaitForEventOfTypeOnWindowMatching (dpy,
+							     root,
+							     ClientMessage,
+							     -1,
+							     -1,
+							     matcher,
+							     waitTimeout)));
+
+    XSelectInput (dpy, root, attrib.your_event_mask);
+}
+
+ct::CompizProcess::CompizProcess (::Display                       *dpy,
+				  ct::CompizProcess::StartupFlags flags,
+				  unsigned int                    waitTimeout) :
+    priv (new PrivateCompizProcess (flags))
+{
+    xorg::testing::Process::SetEnv ("LD_LIBRARY_PATH", compizLDLibraryPath, true);
+
+    std::vector <std::string> args;
+
+    if (flags & ct::CompizProcess::ReplaceCurrentWM)
+	args.push_back ("--replace");
+
+    args.push_back ("--send-startup-message");
+
+    priv->mProcess.Start (compizBinaryPath, args);
+    EXPECT_EQ (priv->mProcess.GetState (), xorg::testing::Process::RUNNING);
+
+    if (flags & ct::CompizProcess::WaitForStartupMessage)
+	priv->WaitForStartupMessage (dpy, flags, waitTimeout);
+}
+
+ct::CompizProcess::~CompizProcess ()
+{
+    if (priv->mProcess.GetState () == xorg::testing::Process::RUNNING)
+	priv->mProcess.Kill ();
+}
+
+xorg::testing::Process::State
+ct::CompizProcess::State ()
+{
+    return priv->mProcess.GetState ();
+}
+
+pid_t
+ct::CompizProcess::Pid ()
+{
+    return priv->mProcess.Pid ();
+}
+
+class ct::PrivateCompizXorgSystemTest
+{
+    public:
+
+	boost::shared_ptr <ct::CompizProcess> mProcess;
+};
+
+ct::CompizXorgSystemTest::CompizXorgSystemTest () :
+    priv (new PrivateCompizXorgSystemTest)
+{
+}
+
+void
+ct::CompizXorgSystemTest::SetUp ()
 {
     xorg::testing::Test::SetUp ();
-
-    ::Display *dpy = Display ();
-    XSelectInput (dpy, DefaultRootWindow (dpy), SubstructureNotifyMask | PropertyChangeMask);
-
-    xorg::testing::Process::SetEnv ("LD_LIBRARY_PATH", compizLDLibraryPath, true);
-    mCompizProcess.Start (compizBinaryPath, "--replace", NULL);
-
-    /* Output buffer is flushed by this point, compiz has a server grab,
-     * so we will only get this once the relevant initialization is complete */
-    ASSERT_TRUE (xorg::testing::XServer::WaitForEventOfType (Display (), PropertyNotify, -1, -1, 3000));
-
-    ASSERT_EQ (mCompizProcess.GetState (), xorg::testing::Process::RUNNING);
 }
 
 void
-ct::XorgSystemTest::TearDown ()
+ct::CompizXorgSystemTest::TearDown ()
 {
-    mCompizProcess.Kill ();
+    priv->mProcess.reset ();
+
     xorg::testing::Test::TearDown ();
 }
+
+xorg::testing::Process::State
+ct::CompizXorgSystemTest::CompizProcessState ()
+{
+    if (priv->mProcess)
+	return priv->mProcess->State ();
+    return xorg::testing::Process::NONE;
+}
+
+void
+ct::CompizXorgSystemTest::StartCompiz (ct::CompizProcess::StartupFlags flags)
+{
+    priv->mProcess.reset (new ct::CompizProcess (Display (), flags, 3000));
+}
+
+void
+ct::AutostartCompizXorgSystemTest::SetUp ()
+{
+    StartCompiz (static_cast <ct::CompizProcess::StartupFlags> (
+		     ct::CompizProcess::ReplaceCurrentWM |
+		     ct::CompizProcess::WaitForStartupMessage));
+}

=== modified file 'tests/system/xorg-gtest/tests/CMakeLists.txt'
--- tests/system/xorg-gtest/tests/CMakeLists.txt	2012-09-18 13:17:09 +0000
+++ tests/system/xorg-gtest/tests/CMakeLists.txt	2012-12-05 05:49:21 +0000
@@ -2,26 +2,40 @@
 
 if (BUILD_XORG_GTEST AND X11_XI_FOUND)
 
-    include_directories (${COMPIZ_XORG_SYSTEM_TEST_INCLUDE_DIR}
+    include_directories (${compiz_SOURCE_DIR}/tests/shared
+                         ${COMPIZ_XORG_SYSTEM_TEST_INCLUDE_DIR}
 			 ${X11_INCLUDE_DIRS}
 			 ${XORG_SERVER_INCLUDE_XORG_GTEST}
 			 ${XORG_SERVER_GTEST_SRC}
 			 ${GTEST_INCLUDE_DIRS})
 
-    link_directories (${X11_XI_LIBRARY_DIRS})
+    link_directories (${X11_XI_LIBRARY_DIRS}
+                      ${compiz_BINARY_DIR}/tests/shared/src)
 
     add_executable (compiz_xorg_gtest_test_window_stacking
 		    ${CMAKE_CURRENT_SOURCE_DIR}/compiz_xorg_gtest_test_window_stacking.cpp)
 
+    add_executable (compiz_xorg_gtest_test_icccm
+		    ${CMAKE_CURRENT_SOURCE_DIR}/compiz_xorg_gtest_icccm.cpp)
+
+    set (COMPIZ_XORG_GTEST_LIBRARIES
+	 compiz_xorg_gtest_system_test
+	 xorg_gtest_all
+	 xorg_gtest_main
+         ${GTEST_BOTH_LIBRARIES}
+	 ${XORG_SERVER_LIBRARIES}
+	 ${X11_XI_LIBRARIES}
+	 ${CMAKE_THREAD_LIBS_INIT})
+
     target_link_libraries (compiz_xorg_gtest_test_window_stacking
-			   compiz_xorg_gtest_system_test
-			   xorg_gtest_all
-			   xorg_gtest_main
-			   ${GTEST_BOTH_LIBRARIES}
-			   ${CMAKE_THREAD_LIBS_INIT}
-			   ${XORG_SERVER_LIBRARIES}
-			   ${X11_XI_LIBRARIES})
+			   ${COMPIZ_XORG_GTEST_LIBRARIES})
 
     compiz_discover_tests (compiz_xorg_gtest_test_window_stacking)
 
+    target_link_libraries (compiz_xorg_gtest_test_icccm
+                           compiz_gtest_shared_async_task
+			   ${COMPIZ_XORG_GTEST_LIBRARIES})
+
+    compiz_discover_tests (compiz_xorg_gtest_test_icccm)
+
 endif (BUILD_XORG_GTEST AND X11_XI_FOUND)

=== added file 'tests/system/xorg-gtest/tests/compiz_xorg_gtest_icccm.cpp'
--- tests/system/xorg-gtest/tests/compiz_xorg_gtest_icccm.cpp	1970-01-01 00:00:00 +0000
+++ tests/system/xorg-gtest/tests/compiz_xorg_gtest_icccm.cpp	2012-12-05 05:49:21 +0000
@@ -0,0 +1,136 @@
+/*
+ * Compiz XOrg GTest, ICCCM compliance
+ *
+ * Copyright (C) 2012 Canonical Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Authored By:
+ * Sam Spilsbury <[e-mail]>
+ */
+#include <list>
+#include <string>
+#include <stdexcept>
+#include <boost/function.hpp>
+#include <boost/bind.hpp>
+#include <boost/shared_ptr.hpp>
+#include <boost/make_shared.hpp>
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+#include <xorg/gtest/xorg-gtest.h>
+#include <compiz-xorg-gtest.h>
+
+#include <gtest_shared_tmpenv.h>
+#include <gtest_shared_characterwrapper.h>
+#include <gtest_shared_asynctask.h>
+
+#include <X11/Xlib.h>
+#include <X11/Xatom.h>
+
+using ::testing::MatchResultListener;
+using ::testing::MakeMatcher;
+using ::testing::Matcher;
+
+namespace ct = compiz::testing;
+
+namespace
+{
+
+char TEST_FAILED_MSG = 's';
+char PROCESS_EXITED_MSG = 'd';
+
+}
+
+class WaitForSuccessDeathTask :
+    public AsyncTask
+{
+    public:
+
+	typedef boost::function <xorg::testing::Process::State ()> GetProcessState;
+
+	WaitForSuccessDeathTask (const GetProcessState &procState) :
+	    mProcessState (procState)
+	{
+	}
+
+    private:
+
+	GetProcessState mProcessState;
+
+	void Task ();
+};
+
+void
+WaitForSuccessDeathTask::Task ()
+{
+    do
+    {
+	if (ReadMsgFromTest (TEST_FAILED_MSG, 1))
+	    return;
+    } while (mProcessState () != xorg::testing::Process::FINISHED_FAILURE);
+
+    /* The process died, send a message back saying that it did */
+    SendMsgToTest (PROCESS_EXITED_MSG);
+}
+
+class CompizXorgSystemICCCM :
+    public ct::CompizXorgSystemTest
+{
+    public:
+
+	virtual void SetUp ()
+	{
+	    ct::CompizXorgSystemTest::SetUp ();
+
+	    ::Display *dpy = Display ();
+
+	    XSelectInput (dpy, DefaultRootWindow (dpy),
+			  StructureNotifyMask |
+			  FocusChangeMask |
+			  PropertyChangeMask |
+			  SubstructureRedirectMask);
+	}
+
+    private:
+};
+
+TEST_F (CompizXorgSystemICCCM, SomeoneElseHasSubstructureRedirectMask)
+{
+    /* XXX: This is a bit stupid, but we have to do it.
+     * It seems as though closing the child stdout or
+     * stderr will cause the client to hang indefinitely
+     * when the child calls XSync (and that can happen
+     * implicitly, eg XCloseDisplay) */
+    TmpEnv env ("XORG_GTEST_CHILD_STDOUT", "1");
+
+    StartCompiz (static_cast <ct::CompizProcess::StartupFlags> (
+		     ct::CompizProcess::ExpectStartupFailure |
+		     ct::CompizProcess::ReplaceCurrentWM |
+		     ct::CompizProcess::WaitForStartupMessage));
+
+    WaitForSuccessDeathTask::GetProcessState processState (boost::bind (&CompizXorgSystemICCCM::CompizProcessState,
+								 this));
+    AsyncTask::Ptr task (boost::make_shared <WaitForSuccessDeathTask> (processState));
+
+    /* Now wait for the thread to tell us the news -
+     * this will block for up to ten seconds */
+    const int maximumWaitTime = 1000 * 10; // 10 seconds
+
+    if (!task->ReadMsgFromTask (PROCESS_EXITED_MSG, maximumWaitTime))
+    {
+	task->SendMsgToTask (TEST_FAILED_MSG);
+	throw std::runtime_error ("compiz process did not exit with failure status");
+    }
+}

=== modified file 'tests/system/xorg-gtest/tests/compiz_xorg_gtest_test_window_stacking.cpp'
--- tests/system/xorg-gtest/tests/compiz_xorg_gtest_test_window_stacking.cpp	2012-11-28 18:14:56 +0000
+++ tests/system/xorg-gtest/tests/compiz_xorg_gtest_test_window_stacking.cpp	2012-12-05 05:49:21 +0000
@@ -38,8 +38,17 @@
 namespace ct = compiz::testing;
 
 class CompizXorgSystemStackingTest :
-    public compiz::testing::XorgSystemTest
+    public ct::AutostartCompizXorgSystemTest
 {
+    public:
+
+	virtual void SetUp ()
+	{
+	    ct::CompizXorgSystemTest::SetUp ();
+
+	    ::Display *dpy = Display ();
+	    XSelectInput (dpy, DefaultRootWindow (dpy), SubstructureNotifyMask | PropertyChangeMask);
+	}
 };
 
 namespace
@@ -56,6 +65,12 @@
 
     const long                 WINDOW_ATTRIB_VALUE_MASK = 0;
 
+    bool Advance (Display *dpy,
+		  bool waitResult)
+    {
+	return ct::AdvanceToNextEventOnSuccess (dpy, waitResult);
+    }
+
     void MakeDock (Display *dpy, Window w)
     {
 	Atom _NET_WM_WINDOW_TYPE = XInternAtom (dpy, "_NET_WM_WINDOW_TYPE", false);
@@ -166,16 +181,26 @@
     XMapRaised (dpy, w2);
 
     /* Both reparented and both mapped */
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindow (dpy, w1, ReparentNotify, -1, -1));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindow (dpy, w1, MapNotify, -1, -1));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindow (dpy, w2, ReparentNotify, -1, -1));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindow (dpy, w2, MapNotify, -1, -1));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindow (dpy,w1, ReparentNotify, -1, -1)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindow (dpy, w1, MapNotify, -1, -1)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindow (dpy, w2, ReparentNotify, -1, -1)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindow (dpy, w2, MapNotify, -1, -1)));
 
     PropertyNotifyXEventMatcher matcher (dpy, "_NET_CLIENT_LIST_STACKING");
 
     /* Wait for property change notify on the root window to happen twice */
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       matcher)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       matcher)));
 
     /* Check the client list to see that w2 > w1 */
     std::list <Window> clientList = ct::NET_CLIENT_LIST_STACKING (dpy);
@@ -198,12 +223,16 @@
     /* Immediately map the dock window and clear the event queue for it */
     XMapRaised (dpy, dock);
 
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindow (dpy, dock, ReparentNotify, -1, -1));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindow (dpy, dock, MapNotify, -1, -1));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindow (dpy, dock, ReparentNotify, -1, -1)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindow (dpy, dock, MapNotify, -1, -1)));
 
     /* Dock window needs to be in the client list */
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
-
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       matcher)));
     std::list <Window> clientList = ct::NET_CLIENT_LIST_STACKING (dpy);
     ASSERT_EQ (clientList.size (), 1);
 
@@ -216,15 +245,24 @@
     XMapRaised (dpy, w2);
 
     /* Both reparented and both mapped */
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindow (dpy, w1, ReparentNotify, -1, -1));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindow (dpy, w1, MapNotify, -1, -1));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindow (dpy, w2, ReparentNotify, -1, -1));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindow (dpy, w2, MapNotify, -1, -1));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindow (dpy, w1, ReparentNotify, -1, -1)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindow (dpy, w1, MapNotify, -1, -1)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindow (dpy, w2, ReparentNotify, -1, -1)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindow (dpy, w2, MapNotify, -1, -1)));
 
     /* Wait for property change notify on the root window to happen twice */
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
-
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       matcher)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       matcher)));
     clientList = ct::NET_CLIENT_LIST_STACKING (dpy);
 
     /* Check the client list to see that dock > w2 > w1 */
@@ -250,8 +288,18 @@
     XMapRaised (dpy, w2);
 
     /* Wait for property change notify on the root window to happen twice */
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       matcher)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       matcher)));
 
     SetUserTime (dpy, w2, 200);
     SetClientLeader (dpy, w2, w2);
@@ -259,7 +307,7 @@
     SetClientLeader (dpy, w3, w3);
 
     XMapRaised (dpy, w3);
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher)));
 
     /* Check the client list to see that w2 > w3 > w1 */
     std::list <Window> clientList = ct::NET_CLIENT_LIST_STACKING (dpy);
@@ -284,15 +332,28 @@
     XMapRaised (dpy, w2);
 
     /* Wait for property change notify on the root window to happen twice */
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
-
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       matcher)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       matcher)));
     SetUserTime (dpy, w3, 0);
     SetClientLeader (dpy, w3, w3);
 
     XMapRaised (dpy, w3);
-    ASSERT_TRUE (ct::WaitForEventOfTypeOnWindowMatching (dpy, DefaultRootWindow (dpy), PropertyNotify, -1, -1, matcher));
-
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       matcher)));
     /* Check the client list to see that w2 > w3 > w1 */
     std::list <Window> clientList = ct::NET_CLIENT_LIST_STACKING (dpy);
     ASSERT_EQ (clientList.size (), 3);