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

[Sam Spilsbury]

Send cached geometry and "none" as the stack-sibling in synthetic configure notify effects.

We don't need to grab the server and query for the current geometry and stack position, upon reading section 4.1.5 of the ICCCM, it seems that clients don't need to know about the stack sibling in synthetic ConfigureNotify events (and other window managers don't do this either).

This really helps with performance, since we're not constantly blocking on the server to process our geometry updates.

(First commit of many, required to fix LP: #1027211)

Ditch the logic that queries the server for the current stack position and
attributes of the window on sendConfigureNotify.

It seems like that's not a part of the spec upon reading s4.1.5

I haven't marked this as "fixing" bug 1027211 because its very difficult to classify what "fixes" a performance bug (you can always do better). However, here are my test results (dragging around glxgears):

GPU: NVIDIA 8600GTM
Resolution: 3360x1050
NVIDIA Sync to VBlank: Off
Driver Version: 304.41

Startup command: compiz --replace composite opengl move resize decor bench

With this branch, and with the XSync commented out handling damage repair: ~40fps dragging fast
Just with this branch: ~30fps dragging fast
Without this branch: ~20fps dragging fast

So this branch gains us about 10fps in dragging performance.

We should look into whether or not that XSync in the damage repair is really necessary. If it isn't, we get another 10fps, and as far as I can tell, won't be repeatedly calling synchronous protocol functions at all when doing things like window dragging.

Added appropriate tests to confirm the behaviour of synthetic ConfigureNotify events, as usual.

I note that with NVIDIA's sync to vblank its still really slow, even if I force the usage of the old copy-back-to-front-buffers path, so I suspect there's a problem with the driver there.

Mutter seems to get it right. See bug 1027211 for details and my thoughts on that.

[MC Return (mc-return)]

LGTM. Compiles and seems to work. Hard to tell the exact speedup here though, because I'm having some troubles using the same CCSM configuration I usually use with the freshly compiled Compiz running from ~/staging.

review: Approve

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

It's an improvement but definitely not a fix for bug 1027211. The bug is still quite noticeable. And if you try resizing another window while glxgears is running then you still get a complete freeze.

I know Sam said he had more ideas to try and will propose them tomorrow...

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

On Thu, Dec 6, 2012 at 2:44 PM, Daniel van Vugt
<email address hidden> wrote:
> Review: Needs Fixing
>
> It's an improvement but definitely not a fix for bug 1027211. The bug is still quite noticeable. And if you try resizing another window while glxgears is running then you still get a complete freeze.
>

Its not a fix, no, but it is an incremental improvement.

I don't think it should be marked needs fixing for that reason.

> I know Sam said he had more ideas to try and will propose them tomorrow...
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.performance_1027211/+merge/138255
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

--
Sam Spilsbury

[Sam Spilsbury (smspillaz)]

On Thu, Dec 6, 2012 at 2:46 PM, Sam Spilsbury <email address hidden> wrote:
> On Thu, Dec 6, 2012 at 2:44 PM, Daniel van Vugt
> <email address hidden> wrote:
>> Review: Needs Fixing
>>
>> It's an improvement but definitely not a fix for bug 1027211. The bug is still quite noticeable. And if you try resizing another window while glxgears is running then you still get a complete freeze.
>>
>
> Its not a fix, no, but it is an incremental improvement.

Also, we should have this anyways.

The way the code currently works doesn't make any sense in the broad
scheme of things and is just highly inefficient protocol requests for
reasons that no application would need to us.

The other fix I'm thinking of for slow opengl movement peformance is
quite separate to this, so it doesn't make sense to put them together.

>
> I don't think it should be marked needs fixing for that reason.
>
>> I know Sam said he had more ideas to try and will propose them tomorrow...
>> --
>> https://code.launchpad.net/~compiz-team/compiz/compiz.performance_1027211/+merge/138255
>> Your team Compiz Maintainers is subscribed to branch lp:compiz.
>
>
>
> --
> Sam Spilsbury

--
Sam Spilsbury

[Daniel van Vugt (vanvugt)]

I'd rather have a single proposal that actually fixes the bug. It creates confusion for people looking at the history later if you have proposals linked to a bug that don't fix it.

[Daniel van Vugt (vanvugt)]

Well, maybe. Wait and see how well the proposals work together and individually.

[Sam Spilsbury (smspillaz)]

Shall I create a new bug and resubmit then ?

On Thu, Dec 6, 2012 at 3:00 PM, Daniel van Vugt
<email address hidden> wrote:
> I'd rather have a single proposal that actually fixes the bug. It creates confusion for people looking at the history later if you have proposals linked to a bug that don't fix it.
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.performance_1027211/+merge/138255
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

--
Sam Spilsbury

[Daniel van Vugt (vanvugt)]

Also, I'm only testing right now. I won't do any code review till there's a patch or set of patches that completely fixes the bug.

[Daniel van Vugt (vanvugt)]

I just got around to analyzing some snapshots of the original hang. This proposal definitely fixes the primary problem. Weirdly though, after applying this fix the remaining hangs seem to be XSync calls from compiz_core too but gdb is not getting any symbol information to tell me which XSync is the primary offender now. I get function "??" at an address inside compiz_core.

[Daniel van Vugt (vanvugt)]

OK, I've tracked down the main offenders that remain _after_ this fix.

1. XSync() inside PrivateWindow::updateRegion()
2. XShapeGetRectangles() inside PrivateWindow::updateRegion()
3. XShapeGetRectangles() called from decor.

If you remove those from the equation then there is no longer a complete freeze. But it still stutters. Further analysis then shows compiz spending its time in one of:

(a) XSync in PixmapBinding::bind
(b) bindTexImageGLX
(c) PrivateVertexBuffer::render

I should add that reverting to the old asynchronous damage handling does not help at all with stutters/freezing. The main issues are those listed above.

I now agree that fixing bug 1027211 will be a long-term effort of many different proposals after this one. :(

[Daniel van Vugt (vanvugt)]

review: Approve

[Daniel van Vugt (vanvugt)]

Also, making Compiz work without XShape should be possible right? It seems quite broken if I force the extension disabled in core.

[Daniel van Vugt (vanvugt)]

OK, found and fixed the primary bug that happens when XShape is disabled: bug 1087193.
It's not the only one but probably the most severe issue.

=== modified file 'src/window.cpp'
--- src/window.cpp	2012-12-05 09:38:59 +0000
+++ src/window.cpp	2012-12-05 17:16:31 +0000
@@ -1268,66 +1268,50 @@
 CompWindow::sendConfigureNotify ()
 {
     XConfigureEvent xev;
-    XWindowAttributes attrib;
-    unsigned int      nchildren;
-    Window            rootRet, parentRet = 0;
-    Window            *children;
 
     xev.type   = ConfigureNotify;
     xev.event  = priv->id;
     xev.window = priv->id;
 
-    /* in order to avoid race conditions we must use the current
-     * server configuration */
-
-    XGrabServer (screen->dpy ());
-    XSync (screen->dpy (), false);
-
-    if (XGetWindowAttributes (screen->dpy (), priv->id, &attrib))
-    {
-	xev.x	     = attrib.x;
-	xev.y	     = attrib.y;
-	xev.width	     = attrib.width;
-	xev.height	     = attrib.height;
-	xev.border_width = attrib.border_width;
-	xev.above = None;
-
-	/* Translate co-ordinates to root space */
-	XTranslateCoordinates (screen->dpy (), priv->id, screen->root (), 0, 0,
-			       &xev.x, &xev.y, &parentRet);
-
-	/* We need to ensure that the stacking order is
-	 * based on the current server stacking order so
-	 * find the sibling to this window's frame in the
-	 * server side stack and stack above that */
-	XQueryTree (screen->dpy (), screen->root (), &rootRet, &parentRet, &children, &nchildren);
-
-	if (nchildren)
-	{
-	    for (unsigned int i = 0; i < nchildren; i++)
-	    {
-		if (i + 1 == nchildren ||
-		    children[i + 1] == ROOTPARENT (this))
-		{
-		    xev.above = children[i];
-		    break;
-		}
-	    }
-	}
-
-	if (children)
-	    XFree (children);
-
-	if (!xev.above)
-	    xev.above		  = (serverPrev) ? ROOTPARENT (serverPrev) : None;
-	xev.override_redirect = priv->attrib.override_redirect;
-
-	XSendEvent (screen->dpy (), priv->id, false,
-		    StructureNotifyMask, (XEvent *) &xev);
-    }
-
-    XUngrabServer (screen->dpy ());
-    XSync (screen->dpy (), false);
+    xev.x	     = priv->serverGeometry.x ();
+    xev.y	     = priv->serverGeometry.y ();
+    xev.width	     = priv->serverGeometry.width ();
+    xev.height	     = priv->serverGeometry.height ();
+    xev.border_width = priv->serverGeometry.border ();
+    xev.override_redirect = priv->attrib.override_redirect;
+
+    /* These used to be based on the actual sibling of the window
+     * (eg, obtained using XQueryTree), but they are now zeroed out.
+     *
+     * The ICCCM is a big unclear on what these should be - it
+     * requires that after the client attempts to configure a window
+     * we should issue a synthetic ConfigureNotify to work around
+     * the change of co-ordinates due to reparenting:
+     *
+     * "A client will receive a synthetic ConfigureNotify event
+     *  following the change that describes the new geometry of the window"
+     *
+     * However there is an acknowledgement on stacking order:
+     *
+     * "Not changing the size, location, border width,
+     *  or stacking order of the window at all."
+     *
+     * Since there isn't any advice as to what to set the above and
+     * detail members, the only choices are to either grab the server
+     * and query it for the sibling to the window's parent, or to just
+     * set them to zero.
+     *
+     * An evaluation of other window managers showed that they just set
+     * these fields to zero. This is probably a safe option and justifies
+     * the potential performance tradeoff that we get out of not having
+     * to grab the server, query window attributes and children and
+     * translate co-ordinates every time a window is moved
+     */
+
+    xev.above = None;
+
+    XSendEvent (screen->dpy (), priv->id, false,
+		StructureNotifyMask, (XEvent *) &xev);
 }
 
 void
@@ -3226,13 +3210,17 @@
 	    xwc->x = 0;
 	    xwc->y = 0;
 	}
-
-	window->sendConfigureNotify ();
     }
 
     if (valueMask)
 	XConfigureWindow (screen->dpy (), id, valueMask, xwc);
 
+    /* Send the synthetic configure notify
+     * after the real configure notify arrives
+     * (ICCCM s4.1.5) */
+    if (serverFrame)
+	window->sendConfigureNotify ();
+
     /* When updating plugins we care about
      * the absolute position */
     if (dx)

=== modified file 'tests/system/xorg-gtest/include/compiz-xorg-gtest.h'
--- tests/system/xorg-gtest/include/compiz-xorg-gtest.h	2012-12-05 05:32:45 +0000
+++ tests/system/xorg-gtest/include/compiz-xorg-gtest.h	2012-12-05 17:16:31 +0000
@@ -29,6 +29,7 @@
 #include <xorg/gtest/xorg-gtest.h>
 
 using ::testing::MatcherInterface;
+using ::testing::MatchResultListener;
 
 namespace compiz
 {
@@ -36,6 +37,46 @@
     {
 	typedef  ::testing::MatcherInterface <const XEvent &> XEventMatcher;
 
+	class PrivatePropertyNotifyXEventMatcher;
+	class PropertyNotifyXEventMatcher :
+	    public compiz::testing::XEventMatcher
+	{
+	    public:
+
+		PropertyNotifyXEventMatcher (Display *dpy,
+					     const std::string &propertyName);
+
+		virtual bool MatchAndExplain (const XEvent &event, MatchResultListener *listener) const;
+		virtual void DescribeTo (std::ostream *os) const;
+
+	    private:
+
+		std::auto_ptr <PrivatePropertyNotifyXEventMatcher> priv;
+	};
+
+	class PrivateConfigureNotifyXEventMatcher;
+	class ConfigureNotifyXEventMatcher :
+	    public compiz::testing::XEventMatcher
+	{
+	    public:
+
+		ConfigureNotifyXEventMatcher (Window       above,
+					      unsigned int border,
+					      int          x,
+					      int          y,
+					      unsigned int width,
+					      unsigned int height);
+
+		virtual bool MatchAndExplain (const XEvent &event, MatchResultListener *listener) const;
+		virtual void DescribeTo (std::ostream *os) const;
+
+	    private:
+
+		std::auto_ptr <PrivateConfigureNotifyXEventMatcher> priv;
+	};
+
+	Window CreateNormalWindow (Display *dpy);
+
 	std::list <Window> NET_CLIENT_LIST_STACKING (Display *);
 	bool AdvanceToNextEventOnSuccess (Display *dpy,
 					  bool waitResult);

=== modified file 'tests/system/xorg-gtest/src/compiz-xorg-gtest.cpp'
--- tests/system/xorg-gtest/src/compiz-xorg-gtest.cpp	2012-12-05 05:32:45 +0000
+++ tests/system/xorg-gtest/src/compiz-xorg-gtest.cpp	2012-12-05 17:16:31 +0000
@@ -22,6 +22,7 @@
  */
 #include <list>
 #include <stdexcept>
+#include <iomanip>
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 #include <boost/shared_ptr.hpp>
@@ -38,13 +39,46 @@
 namespace ct = compiz::testing;
 namespace
 {
-    void RemoveEventFromQueue (Display *dpy)
-    {
-	XEvent event;
-
-	if (XNextEvent (dpy, &event) != Success)
-	    throw std::runtime_error("Failed to remove X event");
-    }
+const int          WINDOW_X = 0;
+const int          WINDOW_Y = 0;
+const unsigned int WINDOW_WIDTH = 640;
+const unsigned int WINDOW_HEIGHT = 480;
+const unsigned int WINDOW_BORDER = 0;
+const unsigned int WINDOW_DEPTH = CopyFromParent;
+const unsigned int WINDOW_CLASS = InputOutput;
+Visual             *WINDOW_VISUAL = CopyFromParent;
+
+
+const long                 WINDOW_ATTRIB_VALUE_MASK = 0;
+
+void RemoveEventFromQueue (Display *dpy)
+{
+    XEvent event;
+
+    if (XNextEvent (dpy, &event) != Success)
+	throw std::runtime_error("Failed to remove X event");
+}
+}
+
+Window
+ct::CreateNormalWindow (Display *dpy)
+{
+    XSetWindowAttributes WINDOW_ATTRIB;
+    Window w = XCreateWindow (dpy,
+			      DefaultRootWindow (dpy),
+			      WINDOW_X,
+			      WINDOW_Y,
+			      WINDOW_WIDTH,
+			      WINDOW_HEIGHT,
+			      WINDOW_BORDER,
+			      WINDOW_DEPTH,
+			      WINDOW_CLASS,
+			      WINDOW_VISUAL,
+			      WINDOW_ATTRIB_VALUE_MASK,
+			      &WINDOW_ATTRIB);
+
+    XSelectInput (dpy, w, StructureNotifyMask);
+    return w;
 }
 
 bool
@@ -140,8 +174,6 @@
     return stackingOrder;
 }
 
-Display *d;
-
 namespace
 {
 class StartupClientMessageMatcher :
@@ -188,6 +220,111 @@
 };
 }
 
+class ct::PrivatePropertyNotifyXEventMatcher
+{
+    public:
+
+	PrivatePropertyNotifyXEventMatcher (Display           *dpy,
+					    const std::string &propertyName) :
+	    mPropertyName (propertyName),
+	    mProperty (XInternAtom (dpy, propertyName.c_str (), false))
+	{
+	}
+
+	std::string mPropertyName;
+	Atom	mProperty;
+};
+
+ct::PropertyNotifyXEventMatcher::PropertyNotifyXEventMatcher (Display           *dpy,
+							      const std::string &propertyName) :
+    priv (new ct::PrivatePropertyNotifyXEventMatcher (dpy, propertyName))
+{
+}
+
+bool
+ct::PropertyNotifyXEventMatcher::MatchAndExplain (const XEvent &event, MatchResultListener *listener) const
+{
+    const XPropertyEvent *propertyEvent = reinterpret_cast <const XPropertyEvent *> (&event);
+
+    if (priv->mProperty == propertyEvent->atom)
+	return true;
+    else
+	return false;
+}
+
+void
+ct::PropertyNotifyXEventMatcher::DescribeTo (std::ostream *os) const
+{
+    *os << "Is property identified by " << priv->mPropertyName;
+}
+
+class ct::PrivateConfigureNotifyXEventMatcher
+{
+    public:
+
+	PrivateConfigureNotifyXEventMatcher (Window       above,
+					     unsigned int border,
+					     int          x,
+					     int          y,
+					     unsigned int width,
+					     unsigned int height) :
+	    mAbove (above),
+	    mBorder (border),
+	    mX (x),
+	    mY (y),
+	    mWidth (width),
+	    mHeight (height)
+	{
+	}
+
+	Window       mAbove;
+	int          mBorder;
+	int          mX;
+	int          mY;
+	int          mWidth;
+	int          mHeight;
+};
+
+ct::ConfigureNotifyXEventMatcher::ConfigureNotifyXEventMatcher (Window       above,
+								unsigned int border,
+								int          x,
+								int          y,
+								unsigned int width,
+								unsigned int height) :
+    priv (new ct::PrivateConfigureNotifyXEventMatcher (above,
+						       border,
+						       x,
+						       y,
+						       width,
+						       height))
+{
+}
+
+bool
+ct::ConfigureNotifyXEventMatcher::MatchAndExplain (const XEvent &event, MatchResultListener *listener) const
+{
+    const XConfigureEvent *ce = reinterpret_cast <const XConfigureEvent *> (&event);
+
+    return ce->above == priv->mAbove &&
+	   ce->border_width == priv->mBorder &&
+	   ce->x == priv->mX &&
+	   ce->y == priv->mY &&
+	   ce->width == priv->mWidth &&
+	   ce->height == priv->mHeight;
+}
+
+void
+ct::ConfigureNotifyXEventMatcher::DescribeTo (std::ostream *os) const
+{
+    *os << "Matches ConfigureNotify with parameters : " << std::endl <<
+	   " x: " << priv->mX <<
+	   " y: " << priv->mY <<
+	   " width: " << priv->mWidth <<
+	   " height: " << priv->mHeight <<
+	   " border: " << priv->mBorder <<
+	   " above: " << std::hex << priv->mAbove << std::dec;
+}
+
 class ct::PrivateCompizProcess
 {
     public:
@@ -222,7 +359,6 @@
 
     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 |
@@ -323,6 +459,7 @@
 void
 ct::AutostartCompizXorgSystemTest::SetUp ()
 {
+    ct::CompizXorgSystemTest::SetUp ();
     StartCompiz (static_cast <ct::CompizProcess::StartupFlags> (
 		     ct::CompizProcess::ReplaceCurrentWM |
 		     ct::CompizProcess::WaitForStartupMessage));

=== modified file 'tests/system/xorg-gtest/tests/compiz_xorg_gtest_icccm.cpp'
--- tests/system/xorg-gtest/tests/compiz_xorg_gtest_icccm.cpp	2012-12-05 03:35:37 +0000
+++ tests/system/xorg-gtest/tests/compiz_xorg_gtest_icccm.cpp	2012-12-05 17:16:31 +0000
@@ -51,6 +51,11 @@
 char TEST_FAILED_MSG = 's';
 char PROCESS_EXITED_MSG = 'd';
 
+bool Advance (Display *d, bool r)
+{
+    return ct::AdvanceToNextEventOnSuccess (d, r);
+}
+
 }
 
 class WaitForSuccessDeathTask :
@@ -134,3 +139,129 @@
 	throw std::runtime_error ("compiz process did not exit with failure status");
     }
 }
+
+class AutostartCompizXorgSystemICCCM :
+    public ct::CompizXorgSystemTest
+{
+    public:
+
+	virtual void SetUp ()
+	{
+	    ct::CompizXorgSystemTest::SetUp ();
+
+	    ::Display *dpy = Display ();
+
+	    XSelectInput (dpy, DefaultRootWindow (dpy),
+			  StructureNotifyMask |
+			  SubstructureNotifyMask |
+			  FocusChangeMask |
+			  PropertyChangeMask);
+
+	    StartCompiz (static_cast <ct::CompizProcess::StartupFlags> (
+			     ct::CompizProcess::ReplaceCurrentWM |
+			     ct::CompizProcess::WaitForStartupMessage));
+	}
+};
+
+TEST_F (AutostartCompizXorgSystemICCCM, ConfigureRequestSendsBackAppropriateConfigureNotify)
+{
+    ::Display *dpy = Display ();
+    Window    root = DefaultRootWindow (dpy);
+
+    Window w1 = ct::CreateNormalWindow (dpy);
+    Window w2 = ct::CreateNormalWindow (dpy);
+
+    XMapRaised (dpy, w1);
+    XMapRaised (dpy, w2);
+
+    ct::PropertyNotifyXEventMatcher propertyMatcher (dpy, "_NET_CLIENT_LIST_STACKING");
+
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       propertyMatcher)));
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       propertyMatcher)));
+
+    /* Select for StructureNotify on w1 */
+    XSelectInput (dpy, w1, StructureNotifyMask);
+
+    /* Send a ConfigureRequest to the server asking for
+     * a particular geometry and stack position relative
+     * to w2 */
+
+    XEvent                 event;
+    XConfigureRequestEvent *configureRequestEvent = &event.xconfigurerequest;
+
+    event.type = ConfigureRequest;
+
+    const Window REQUEST_ABOVE = w2;
+    const unsigned int REQUEST_BORDER_WIDTH = 0;
+    const unsigned int REQUEST_X = 110;
+    const unsigned int REQUEST_Y = 120;
+    const unsigned int REQUEST_WIDTH = 200;
+    const unsigned int REQUEST_HEIGHT = 200;
+
+    configureRequestEvent->window = w1;
+
+    configureRequestEvent->above = REQUEST_ABOVE;
+    configureRequestEvent->detail = Above;
+    configureRequestEvent->border_width = REQUEST_BORDER_WIDTH;
+    configureRequestEvent->x = REQUEST_X;
+    configureRequestEvent->y = REQUEST_Y;
+    configureRequestEvent->width = REQUEST_WIDTH;
+    configureRequestEvent->height = REQUEST_HEIGHT;
+
+    configureRequestEvent->value_mask = CWX |
+					CWY |
+					CWWidth |
+					CWHeight |
+					CWBorderWidth |
+					CWSibling |
+					CWStackMode;
+
+    XSendEvent (dpy,
+		root,
+		0,
+		SubstructureRedirectMask | SubstructureNotifyMask,
+		&event);
+
+    XFlush (dpy);
+
+    /* Now wait for a ConfigureNotify to be sent back */
+    ct::ConfigureNotifyXEventMatcher configureMatcher (0, // Always zero
+						       REQUEST_BORDER_WIDTH,
+						       REQUEST_X,
+						       REQUEST_Y,
+						       REQUEST_WIDTH,
+						       REQUEST_HEIGHT);
+
+    /* Should get a ConfigureNotify with the right parameters */
+    EXPECT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       w1,
+								       ConfigureNotify,
+								       -1,
+								       -1,
+								       configureMatcher)));
+
+    /* Check if the window is actually above */
+    ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
+								       DefaultRootWindow (dpy),
+								       PropertyNotify,
+								       -1,
+								       -1,
+								       propertyMatcher)));
+
+    /* Check the client list to see that w1 > w2 */
+    std::list <Window> clientList = ct::NET_CLIENT_LIST_STACKING (dpy);
+
+    ASSERT_EQ (2, clientList.size ());
+    EXPECT_EQ (w2, clientList.front ());
+    EXPECT_EQ (w1, clientList.back ());
+}

=== 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-12-04 15:21:08 +0000
+++ tests/system/xorg-gtest/tests/compiz_xorg_gtest_test_window_stacking.cpp	2012-12-05 17:16:31 +0000
@@ -44,7 +44,7 @@
 
 	virtual void SetUp ()
 	{
-	    ct::CompizXorgSystemTest::SetUp ();
+	    ct::AutostartCompizXorgSystemTest::SetUp ();
 
 	    ::Display *dpy = Display ();
 	    XSelectInput (dpy, DefaultRootWindow (dpy), SubstructureNotifyMask | PropertyChangeMask);
@@ -53,18 +53,6 @@
 
 namespace
 {
-    const int          WINDOW_X = 0;
-    const int          WINDOW_Y = 0;
-    const unsigned int WINDOW_WIDTH = 640;
-    const unsigned int WINDOW_HEIGHT = 480;
-    const unsigned int WINDOW_BORDER = 0;
-    const unsigned int WINDOW_DEPTH = CopyFromParent;
-    const unsigned int WINDOW_CLASS = InputOutput;
-    Visual             *WINDOW_VISUAL = CopyFromParent;
-
-
-    const long                 WINDOW_ATTRIB_VALUE_MASK = 0;
-
     bool Advance (Display *dpy,
 		  bool waitResult)
     {
@@ -86,26 +74,6 @@
 			 1);
     }
 
-    Window CreateNormalWindow (Display *dpy)
-    {
-	XSetWindowAttributes WINDOW_ATTRIB;
-	Window w = XCreateWindow (dpy,
-				  DefaultRootWindow (dpy),
-				  WINDOW_X,
-				  WINDOW_Y,
-				  WINDOW_WIDTH,
-				  WINDOW_HEIGHT,
-				  WINDOW_BORDER,
-				  WINDOW_DEPTH,
-				  WINDOW_CLASS,
-				  WINDOW_VISUAL,
-				  WINDOW_ATTRIB_VALUE_MASK,
-				  &WINDOW_ATTRIB);
-
-	XSelectInput (dpy, w, StructureNotifyMask);
-	return w;
-    }
-
     void SetUserTime (Display *dpy, Window w, Time time)
     {
 	Atom _NET_WM_USER_TIME = XInternAtom (dpy, "_NET_WM_USER_TIME", false);
@@ -126,44 +94,6 @@
 			 XA_WINDOW, 32, PropModeReplace,
 			 (unsigned char *) &leader, 1);
     }
-
-    class PropertyNotifyXEventMatcher :
-	public ct::XEventMatcher
-    {
-	public:
-
-	    PropertyNotifyXEventMatcher (Display *dpy,
-					 const std::string &propertyName) :
-		mPropertyName (propertyName),
-		mProperty (XInternAtom (dpy, propertyName.c_str (), false))
-	    {
-	    }
-
-	    virtual bool MatchAndExplain (const XEvent &event, MatchResultListener *listener) const
-	    {
-		const XPropertyEvent *propertyEvent = reinterpret_cast <const XPropertyEvent *> (&event);
-
-		if (mProperty == propertyEvent->atom)
-		    return true;
-		else
-		    return false;
-	    }
-
-	    virtual void DescribeTo (std::ostream *os) const
-	    {
-		*os << "Is property identified by " << mPropertyName;
-	    }
-
-	    virtual void DescribeNegationTo (std::ostream *os) const
-	    {
-		*os << "Is not a property identified by" << mPropertyName;
-	    }
-
-	private:
-
-	    std::string mPropertyName;
-	    Atom	mProperty;
-    };
 }
 
 TEST_F (CompizXorgSystemStackingTest, TestSetup)
@@ -174,8 +104,8 @@
 {
     ::Display *dpy = Display ();
 
-    Window w1 = CreateNormalWindow (dpy);
-    Window w2 = CreateNormalWindow (dpy);
+    Window w1 = ct::CreateNormalWindow (dpy);
+    Window w2 = ct::CreateNormalWindow (dpy);
 
     XMapRaised (dpy, w1);
     XMapRaised (dpy, w2);
@@ -186,7 +116,7 @@
     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");
+    ct::PropertyNotifyXEventMatcher matcher (dpy, "_NET_CLIENT_LIST_STACKING");
 
     /* Wait for property change notify on the root window to happen twice */
     ASSERT_TRUE (Advance (dpy, ct::WaitForEventOfTypeOnWindowMatching (dpy,
@@ -213,9 +143,9 @@
 TEST_F (CompizXorgSystemStackingTest, TestCreateWindowsAndRestackRelativeToEachOtherDockAlwaysOnTop)
 {
     ::Display *dpy = Display ();
-    PropertyNotifyXEventMatcher matcher (dpy, "_NET_CLIENT_LIST_STACKING");
+    ct::PropertyNotifyXEventMatcher matcher (dpy, "_NET_CLIENT_LIST_STACKING");
 
-    Window dock = CreateNormalWindow (dpy);
+    Window dock = ct::CreateNormalWindow (dpy);
 
     /* Make it a dock */
     MakeDock (dpy, dock);
@@ -236,8 +166,8 @@
     std::list <Window> clientList = ct::NET_CLIENT_LIST_STACKING (dpy);
     ASSERT_EQ (clientList.size (), 1);
 
-    Window w1 = CreateNormalWindow (dpy);
-    Window w2 = CreateNormalWindow (dpy);
+    Window w1 = ct::CreateNormalWindow (dpy);
+    Window w2 = ct::CreateNormalWindow (dpy);
 
     XSelectInput (dpy, w2, StructureNotifyMask);
 
@@ -278,11 +208,11 @@
 TEST_F (CompizXorgSystemStackingTest, TestMapWindowWithOldUserTime)
 {
     ::Display *dpy = Display ();
-    PropertyNotifyXEventMatcher matcher (dpy, "_NET_CLIENT_LIST_STACKING");
+    ct::PropertyNotifyXEventMatcher matcher (dpy, "_NET_CLIENT_LIST_STACKING");
 
-    Window w1 = CreateNormalWindow (dpy);
-    Window w2 = CreateNormalWindow (dpy);
-    Window w3 = CreateNormalWindow (dpy);
+    Window w1 = ct::CreateNormalWindow (dpy);
+    Window w2 = ct::CreateNormalWindow (dpy);
+    Window w3 = ct::CreateNormalWindow (dpy);
 
     XMapRaised (dpy, w1);
     XMapRaised (dpy, w2);
@@ -322,11 +252,11 @@
 TEST_F (CompizXorgSystemStackingTest, TestMapWindowAndDenyFocus)
 {
     ::Display *dpy = Display ();
-    PropertyNotifyXEventMatcher matcher (dpy, "_NET_CLIENT_LIST_STACKING");
+    ct::PropertyNotifyXEventMatcher matcher (dpy, "_NET_CLIENT_LIST_STACKING");
 
-    Window w1 = CreateNormalWindow (dpy);
-    Window w2 = CreateNormalWindow (dpy);
-    Window w3 = CreateNormalWindow (dpy);
+    Window w1 = ct::CreateNormalWindow (dpy);
+    Window w2 = ct::CreateNormalWindow (dpy);
+    Window w3 = ct::CreateNormalWindow (dpy);
 
     XMapRaised (dpy, w1);
     XMapRaised (dpy, w2);