Merge lp:~compiz-team/compiz/compiz.fix_1002602_tests into lp:compiz/0.9.8

[Sam Spilsbury]

Adds some tests for the pixmap rebinding code ... which also means some refactoring was done.

At the moment, I'm not too much of a fan that we need a method to change the strategy for notifying renderers that a pixmap rebind is required - it really should be injected on the constructor ... but the only way that we'd be able to do that is to actually inject the PixmapRebinder object into CompositeWindow from the renderer, which isn't very good either.

[Daniel van Vugt (vanvugt)]

Fails to cmake before I can even build it (no Makefile generated):

CMake Error at src/CMakeLists.txt:12 (add_subdirectory):
  add_subdirectory given source "servergrab" which is not an existing
  directory.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Again, I would like to put a stop to using so many verbs in class names ("Rebind"-er). It usually indicates bad design, but sometimes can be solved just by thinking of a better class name.

Perhaps the solution here is to just rename "PixmapRebinder" to "PixmapBinding".

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

The file path (pixmapbinder) no longer matches the class name (PixmapBinding).

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Valgrind: Good
Callgrind: Good
Code review: Needs fixing (minor issues):

1. Copyrights look a bit wrong. New files that are entirely your own work (like test cases) should be copyright to Canonical (and mention yourself as author). Please don't copyright them to other people unless portions of the code is by them.

2. mPixmapRebinder should be called mPixmapBinding

review: Needs Fixing

[lp:~compiz-team/compiz/compiz.fix_1002602_tests]

3238. By Daniel van Vugt on 2012-06-05

Remove a pointless line of code that was also causing a minor conflict.

[Daniel van Vugt (vanvugt)]

A couple of minor complaints, which I'm sure I've mentioned before:

1. X functions use Bool, not bool. So should be passed False, not false:
   XSync (privateScreen.dpy, false);

2. For correctness and readability, don't assume None == 0. So:
     if (mPixmap)
   should be:
     if (mPixmap != None)

review: Approve

[Sam Spilsbury (smspillaz)]

Okay, I'll quickly fix those so we can get this merged in

[Daniel van Vugt (vanvugt)]

It's approved, so you don't need to fix them.

=== modified file 'include/core/abiversion.h'
--- include/core/abiversion.h	2012-05-14 15:45:24 +0000
+++ include/core/abiversion.h	2012-06-05 09:04:24 +0000
@@ -5,6 +5,6 @@
 #  error Conflicting definitions of CORE_ABIVERSION
 #endif
 
-#define CORE_ABIVERSION 20120514
+#define CORE_ABIVERSION 20120526
 
 #endif // COMPIZ_ABIVERSION_H

=== modified file 'include/core/screen.h'
--- include/core/screen.h	2012-05-27 04:32:55 +0000
+++ include/core/screen.h	2012-06-05 09:04:24 +0000
@@ -45,6 +45,7 @@
 class CompManager;
 class CoreWindow;
 class CoreOptions;
+class ServerGrabInterface;
 
 typedef std::list<CompWindow *> CompWindowList;
 typedef std::vector<CompWindow *> CompWindowVector;
@@ -391,6 +392,8 @@
     virtual void processEvents () = 0;
     virtual void alwaysHandleEvent (XEvent *event) = 0;
 
+    virtual ServerGrabInterface * serverGrabInterface () = 0;
+
     // Replacements for friends accessing priv. They are declared virtual to
     // ensure the ABI is stable if/when they are moved to CompScreenImpl.
     // They are only intended for use within compiz-core

=== modified file 'plugins/CMakeLists.txt'
--- plugins/CMakeLists.txt	2012-05-21 06:43:20 +0000
+++ plugins/CMakeLists.txt	2012-06-05 09:04:24 +0000
@@ -16,6 +16,7 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/../src/pluginclasshandler/include
   ${CMAKE_CURRENT_SOURCE_DIR}/../src/point/include
   ${CMAKE_CURRENT_SOURCE_DIR}/../src/rect/include
+  ${CMAKE_CURRENT_SOURCE_DIR}/../src/servergrab/include
   ${CMAKE_CURRENT_SOURCE_DIR}/../src/region/include
   ${CMAKE_CURRENT_SOURCE_DIR}/../src/window/geometry/include
   ${CMAKE_CURRENT_SOURCE_DIR}/../src/window/geometry-saver/include

=== modified file 'plugins/composite/CMakeLists.txt'
--- plugins/composite/CMakeLists.txt	2012-02-08 10:54:35 +0000
+++ plugins/composite/CMakeLists.txt	2012-06-05 09:04:24 +0000
@@ -2,5 +2,9 @@
 
 include (CompizPlugin)
 
-compiz_plugin (composite)
-
+include_directories (${CMAKE_CURRENT_SOURCE_DIR}/src/pixmapbinding/include)
+link_directories (${CMAKE_CURRENT_BINARY_DIR}/src/pixmapbinding)
+
+compiz_plugin (composite LIBRARIES compiz_composite_pixmapbinding)
+
+add_subdirectory (src/pixmapbinding)

=== modified file 'plugins/composite/include/composite/composite.h'
--- plugins/composite/include/composite/composite.h	2012-06-05 07:40:03 +0000
+++ plugins/composite/include/composite/composite.h	2012-06-05 09:04:24 +0000
@@ -171,7 +171,6 @@
 	virtual void damageRegion (const CompRegion &r);
 };
 
-
 class CompositeScreen :
     public WrapableHandler<CompositeScreenInterface, 7>,
     public PluginClassHandler<CompositeScreen, CompScreen, COMPIZ_COMPOSITE_ABI>,
@@ -327,8 +326,6 @@
 {
     public:
 
-	typedef boost::function <void ()> NewPixmapReadyCallback;
-
 	CompositeWindow (CompWindow *w);
 	~CompositeWindow ();
 
@@ -430,7 +427,7 @@
 	 * A function to call when a new pixmap is ready to
 	 * be bound just before the old one is released
 	 */
-	void setNewPixmapReadyCallback (const NewPixmapReadyCallback &cb);
+	void setNewPixmapReadyCallback (const boost::function <void ()> &cb);
 
 	WRAPABLE_HND (0, CompositeWindowInterface, bool, damageRect,
 		      bool, const CompRect &);

=== added directory 'plugins/composite/src/pixmapbinding'
=== added file 'plugins/composite/src/pixmapbinding/CMakeLists.txt'
--- plugins/composite/src/pixmapbinding/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ plugins/composite/src/pixmapbinding/CMakeLists.txt	2012-06-05 09:04:24 +0000
@@ -0,0 +1,43 @@
+include (FindPkgConfig)
+
+PKG_CHECK_MODULES (X11 x11)
+
+INCLUDE_DIRECTORIES (  
+  ${CMAKE_CURRENT_SOURCE_DIR}/include
+  ${CMAKE_CURRENT_SOURCE_DIR}/src
+    
+  ${Boost_INCLUDE_DIRS}
+  
+  ${X11_INCLUDE_DIRS}
+)
+
+LINK_DIRECTORIES (${GLIBMM_LIBRARY_DIRS} ${COMPIZ_LIBRARY_DIRS}) 
+
+SET ( 
+  PRIVATE_HEADERS
+  ${CMAKE_CURRENT_SOURCE_DIR}/include/pixmapbinding.h
+)
+
+SET( 
+  SRCS 
+  ${CMAKE_CURRENT_SOURCE_DIR}/src/pixmapbinding.cpp
+  ${compiz_SOURCE_DIR}/src/size.cpp # XXX: Remove once we've merged this into a general geometry lib
+)
+
+ADD_LIBRARY( 
+  compiz_composite_pixmapbinding STATIC
+  
+  ${SRCS}
+  
+  ${PRIVATE_HEADERS}
+)
+
+if (COMPIZ_BUILD_TESTING)
+ADD_SUBDIRECTORY( ${CMAKE_CURRENT_SOURCE_DIR}/tests )
+endif (COMPIZ_BUILD_TESTING)
+
+TARGET_LINK_LIBRARIES(
+  compiz_composite_pixmapbinding
+  compiz_servergrab
+  ${X11_LIBRARIES}
+)

=== added directory 'plugins/composite/src/pixmapbinding/include'
=== added file 'plugins/composite/src/pixmapbinding/include/pixmapbinding.h'
--- plugins/composite/src/pixmapbinding/include/pixmapbinding.h	1970-01-01 00:00:00 +0000
+++ plugins/composite/src/pixmapbinding/include/pixmapbinding.h	2012-06-05 09:04:24 +0000
@@ -0,0 +1,191 @@
+/*
+ * Copyright © 2012 Canonical Ltd.
+ * Copyright © 2008 Dennis Kasprzyk
+ * Copyright © 2007 Novell, Inc.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Dennis Kasprzyk not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Dennis Kasprzyk makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * DENNIS KASPRZYK DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL DENNIS KASPRZYK BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Authors: Dennis Kasprzyk <[e-mail]>
+ *          David Reveman <[e-mail]>
+ *          Sam Spilsbury <[e-mail]>
+ */
+
+#ifndef _COMPOSITE_PIXMAP_REBIND_H
+#define _COMPOSITE_PIXMAP_REBIND_H
+
+#include <memory>
+#include <boost/shared_ptr.hpp>
+#include <boost/function.hpp>
+#include <boost/bind.hpp>
+
+#include <core/size.h>
+
+#include <X11/Xlib.h>
+
+class ServerGrabInterface;
+
+class CompositePixmapRebindInterface
+{
+    public:
+
+	virtual ~CompositePixmapRebindInterface () {}
+
+	virtual Pixmap pixmap () const = 0;
+	virtual bool bind () = 0;
+	virtual const CompSize & size () const = 0;
+	virtual void release () = 0;
+
+	/* This isn't great API, but probably necessary
+	 * unless we make it a requirement that the
+	 * renderer sets the strategy for the rebinder */
+	virtual void setNewPixmapReadyCallback (const boost::function <void ()> &) = 0;
+
+	/* Also don't like this either */
+	virtual void allowFurtherRebindAttempts () = 0;
+};
+
+class WindowAttributesGetInterface
+{
+    public:
+
+	virtual ~WindowAttributesGetInterface () {}
+
+	virtual bool getAttributes (XWindowAttributes &) = 0;
+};
+
+class WindowPixmapInterface
+{
+    public:
+
+	virtual ~WindowPixmapInterface () {}
+
+	typedef boost::shared_ptr <WindowPixmapInterface> Ptr;
+
+	virtual Pixmap pixmap () const = 0;
+	virtual void releasePixmap ()  = 0;
+};
+
+class X11WindowPixmap :
+    public WindowPixmapInterface
+{
+    public:
+
+	X11WindowPixmap (Display *d, Pixmap p) :
+	    mDisplay (d),
+	    mPixmap (p)
+	{
+	}
+
+	Pixmap pixmap () const
+	{
+	    return mPixmap;
+	}
+
+	void releasePixmap ()
+	{
+	    if (mPixmap)
+		XFreePixmap (mDisplay, mPixmap);
+
+	    mPixmap = None;
+	}
+
+    private:
+
+	Display *mDisplay;
+	Pixmap  mPixmap;
+};
+
+class WindowPixmap
+{
+    public:
+
+	WindowPixmap () :
+	    mPixmap ()
+	{
+	}
+
+	WindowPixmap (WindowPixmapInterface::Ptr &pm) :
+	    mPixmap (pm)
+	{
+	}
+
+	Pixmap pixmap () const
+	{
+	    if (mPixmap)
+		return mPixmap->pixmap ();
+
+	    return None;
+	}
+
+	~WindowPixmap ()
+	{
+	    if (mPixmap)
+		mPixmap->releasePixmap ();
+	}
+    private:
+
+	WindowPixmapInterface::Ptr mPixmap;
+};
+
+class WindowPixmapGetInterface
+{
+    public:
+
+	virtual ~WindowPixmapGetInterface () {}
+
+	virtual WindowPixmapInterface::Ptr getPixmap () = 0;
+};
+
+class PixmapBinding :
+    public CompositePixmapRebindInterface
+{
+    public:
+
+	typedef boost::function <void ()> NewPixmapReadyCallback;
+
+	PixmapBinding (const NewPixmapReadyCallback &,
+			WindowPixmapGetInterface *,
+			WindowAttributesGetInterface *,
+			ServerGrabInterface *);
+
+	~PixmapBinding ();
+
+	Pixmap pixmap () const;
+	bool bind ();
+	const CompSize & size () const;
+	void release ();
+	void setNewPixmapReadyCallback (const boost::function <void ()> &);
+	void allowFurtherRebindAttempts ();
+
+    private:
+
+	std::auto_ptr <WindowPixmap>  mPixmap;
+	CompSize      mSize;
+	bool	      needsRebind;
+	bool          bindFailed;
+	NewPixmapReadyCallback newPixmapReadyCallback;
+
+	WindowPixmapGetInterface *windowPixmapRetreiver;
+	WindowAttributesGetInterface *windowAttributesRetreiver;
+	ServerGrabInterface *serverGrab;
+
+};
+
+#endif

=== added directory 'plugins/composite/src/pixmapbinding/src'
=== added file 'plugins/composite/src/pixmapbinding/src/pixmapbinding.cpp'
--- plugins/composite/src/pixmapbinding/src/pixmapbinding.cpp	1970-01-01 00:00:00 +0000
+++ plugins/composite/src/pixmapbinding/src/pixmapbinding.cpp	2012-06-05 09:04:24 +0000
@@ -0,0 +1,141 @@
+/*
+ * Copyright © 2012 Canonical Ltd.
+ * Copyright © 2008 Dennis Kasprzyk
+ * Copyright © 2007 Novell, Inc.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Dennis Kasprzyk not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Dennis Kasprzyk makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * DENNIS KASPRZYK DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL DENNIS KASPRZYK BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Authors: Dennis Kasprzyk <[e-mail]>
+ *          David Reveman <[e-mail]>
+ *          Sam Spilsbury <[e-mail]>
+ */
+
+#include <core/servergrab.h>
+#include "pixmapbinding.h"
+
+PixmapBinding::PixmapBinding (const NewPixmapReadyCallback &cb,
+				WindowPixmapGetInterface *pmg,
+				WindowAttributesGetInterface *wag,
+				ServerGrabInterface *sg) :
+    mPixmap (),
+    mSize (),
+    needsRebind (true),
+    bindFailed (false),
+    newPixmapReadyCallback (cb),
+    windowPixmapRetreiver (pmg),
+    windowAttributesRetreiver (wag),
+    serverGrab (sg)
+{
+}
+
+PixmapBinding::~PixmapBinding ()
+{
+    needsRebind = false;
+}
+
+const CompSize &
+PixmapBinding::size () const
+{
+    return mSize;
+}
+
+Pixmap
+PixmapBinding::pixmap () const
+{
+    static Pixmap nPixmap = None;
+
+    if (needsRebind ||
+	!mPixmap.get ())
+	return nPixmap;
+
+    return mPixmap->pixmap ();
+}
+
+bool
+PixmapBinding::bind ()
+{
+    /* don't try to bind window again if it failed previously */
+    if (bindFailed)
+	return false;
+
+    if (needsRebind)
+    {
+	XWindowAttributes attr;
+
+	/* We have to grab the server here to make sure that window
+	   is mapped when getting the window pixmap */
+	ServerLock mLock (serverGrab);
+
+	windowAttributesRetreiver->getAttributes (attr);
+	if (attr.map_state != IsViewable ||
+	    (attr.width == 0 && attr.border_width == 0) ||
+	    (attr.height == 0 && attr.border_width == 0))
+	{
+	    bindFailed = true;
+	    needsRebind = false;
+	    return false;
+	}
+
+	WindowPixmapInterface::Ptr newPixmap = windowPixmapRetreiver->getPixmap ();
+	CompSize newSize = CompSize (attr.border_width * 2 + attr.width,
+				     attr.border_width * 2 + attr.height);
+
+	if (newPixmap->pixmap () && newSize.width () && newSize.height ())
+	{
+	    /* Notify renderer that a new pixmap is about to
+	     * be bound */
+	    if (newPixmapReadyCallback)
+		newPixmapReadyCallback ();
+
+	    /* Assign new pixmap */
+	    std::auto_ptr <WindowPixmap> newPixmapWrapper (new WindowPixmap (newPixmap));
+	    mPixmap = newPixmapWrapper;
+	    mSize = newSize;
+
+	    needsRebind = false;
+	}
+	else
+	{
+	    bindFailed = true;
+	    needsRebind = false;
+	    return false;
+	}
+    }
+    return true;
+}
+
+void
+PixmapBinding::release ()
+{
+    needsRebind = true;
+}
+
+void
+PixmapBinding::setNewPixmapReadyCallback (const NewPixmapReadyCallback &cb)
+{
+    newPixmapReadyCallback = cb;
+}
+
+void
+PixmapBinding::allowFurtherRebindAttempts ()
+{
+    bindFailed = false;
+    needsRebind = true;
+}

=== added directory 'plugins/composite/src/pixmapbinding/tests'
=== added file 'plugins/composite/src/pixmapbinding/tests/CMakeLists.txt'
--- plugins/composite/src/pixmapbinding/tests/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ plugins/composite/src/pixmapbinding/tests/CMakeLists.txt	2012-06-05 09:04:24 +0000
@@ -0,0 +1,24 @@
+find_library (GMOCK_LIBRARY gmock)
+find_library (GMOCK_MAIN_LIBRARY gmock_main)
+
+if (NOT GMOCK_LIBRARY OR NOT GMOCK_MAIN_LIBRARY OR NOT GTEST_FOUND)
+  message ("Google Mock and Google Test not found - cannot build tests!")
+  set (COMPIZ_BUILD_TESTING OFF)
+endif (NOT GMOCK_LIBRARY OR NOT GMOCK_MAIN_LIBRARY OR NOT GTEST_FOUND)
+
+include_directories (${GTEST_INCLUDE_DIRS})
+
+link_directories (${COMPIZ_LIBRARY_DIRS})
+
+add_executable (compiz_test_composite_pixmapbinding
+                ${CMAKE_CURRENT_SOURCE_DIR}/test-composite-pixmapbinding.cpp)
+
+target_link_libraries (compiz_test_composite_pixmapbinding
+                       compiz_composite_pixmapbinding
+                       ${GTEST_BOTH_LIBRARIES}
+		       ${GMOCK_LIBRARY}
+		       ${GMOCK_MAIN_LIBRARY}
+		       ${CMAKE_THREAD_LIBS_INIT} # Link in pthread.
+                       )
+
+gtest_add_tests (compiz_test_composite_pixmapbinding "" ${CMAKE_CURRENT_SOURCE_DIR}/test-composite-pixmapbinding.cpp)

=== added file 'plugins/composite/src/pixmapbinding/tests/test-composite-pixmapbinding.cpp'
--- plugins/composite/src/pixmapbinding/tests/test-composite-pixmapbinding.cpp	1970-01-01 00:00:00 +0000
+++ plugins/composite/src/pixmapbinding/tests/test-composite-pixmapbinding.cpp	2012-06-05 09:04:24 +0000
@@ -0,0 +1,444 @@
+/*
+ * Copyright © 2012 Canonical Ltd.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Dennis Kasprzyk not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Dennis Kasprzyk makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * DENNIS KASPRZYK DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL DENNIS KASPRZYK BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Authors: Sam Spilsbury <[e-mail]>
+ */
+
+#include <boost/shared_ptr.hpp>
+#include <boost/make_shared.hpp>
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include <core/servergrab.h>
+#include "pixmapbinding.h"
+
+using ::testing::Invoke;
+using ::testing::Return;
+using ::testing::_;
+
+class CompositePixmapBinderTest :
+    public ::testing::Test
+{
+};
+
+class MockWindowPixmapGet :
+    public WindowPixmapGetInterface
+{
+    public:
+
+	MOCK_METHOD0 (getPixmap, WindowPixmapInterface::Ptr ());
+};
+
+class FakeWindowAttributesGet :
+    public WindowAttributesGetInterface
+{
+    public:
+
+	FakeWindowAttributesGet (XWindowAttributes &wa) :
+	    mAttributes (wa)
+	{
+	}
+
+	bool getAttributes (XWindowAttributes &wa)
+	{
+	    wa = mAttributes;
+	    return true;
+	}
+
+    private:
+
+	XWindowAttributes mAttributes;
+};
+
+class MockWindowAttributesGet :
+    public WindowAttributesGetInterface
+{
+    public:
+
+	MOCK_METHOD1 (getAttributes, bool (XWindowAttributes &));
+};
+
+class MockPixmap :
+    public WindowPixmapInterface
+{
+    public:
+
+	typedef boost::shared_ptr <MockPixmap> Ptr;
+
+	MOCK_CONST_METHOD0 (pixmap, Pixmap ());
+	MOCK_METHOD0 (releasePixmap, void ());
+};
+
+class MockServerGrab :
+    public ServerGrabInterface
+{
+    public:
+
+	MOCK_METHOD0 (grabServer, void ());
+	MOCK_METHOD0 (ungrabServer, void ());
+	MOCK_METHOD0 (syncServer, void ());
+};
+
+class PixmapReadyInterface
+{
+    public:
+
+	virtual ~PixmapReadyInterface () {}
+
+	virtual void ready () = 0;
+};
+
+class MockPixmapReady :
+    public PixmapReadyInterface
+{
+    public:
+
+	MOCK_METHOD0 (ready, void ());
+};
+
+TEST(CompositePixmapBinderTest, TestInitialBindSuccess)
+{
+    /* Leave this here
+     * There's a bug in Google Mock at the moment where
+     * if a function with an expectation which returns
+     * Mock B through a container is itself contained in
+     * Mock A, then when Mock A is destroyed, it will first
+     * lock a mutex, and then it will invoke
+     * the destructor the container containing Mock B which
+     * in turn might invoke Mock B's destructor and thus
+     * lock the same mutex in the same thread (deadlock).
+     *
+     * Keeping this reference here ensures that MockPixmap
+     * stays alive while MockWindowPixmapGet is being destroyed
+     */
+    MockPixmap::Ptr wp (boost::make_shared <MockPixmap> ());
+
+    MockWindowPixmapGet mwpg;
+    MockWindowAttributesGet mwag;
+    MockServerGrab          msg;
+    XWindowAttributes xwa;
+
+    xwa.width = 100;
+    xwa.height = 200;
+    xwa.map_state = IsViewable;
+    xwa.border_width = 1;
+
+    FakeWindowAttributesGet fwag (xwa);
+
+    MockPixmapReady ready;
+
+    boost::function <void ()> readyCb (boost::bind (&PixmapReadyInterface::ready, &ready));
+
+    PixmapBinding pr (readyCb,
+			&mwpg,
+			&mwag,
+			&msg);
+
+    EXPECT_CALL (msg, grabServer ());
+    EXPECT_CALL (msg, syncServer ()).Times (2);
+    EXPECT_CALL (mwag, getAttributes (_)).WillOnce (Invoke (&fwag, &FakeWindowAttributesGet::getAttributes));
+    EXPECT_CALL (mwpg, getPixmap ()).WillOnce (Return (boost::shared_static_cast <WindowPixmapInterface> (wp)));
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (1));
+
+    EXPECT_CALL (ready, ready ());
+    EXPECT_CALL (msg, ungrabServer ());
+
+    pr.bind ();
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (1));
+
+    EXPECT_EQ (pr.pixmap (), 1);
+    EXPECT_EQ (pr.size (), CompSize (102, 202));
+
+    EXPECT_CALL (*wp, releasePixmap ());
+}
+
+TEST(CompositePixmapBinderTest, TestInitialBindSuccessNoRebind)
+{
+    MockPixmap::Ptr wp (boost::make_shared <MockPixmap> ());
+
+    MockWindowPixmapGet mwpg;
+    MockWindowAttributesGet mwag;
+    MockServerGrab          msg;
+    XWindowAttributes xwa;
+
+    xwa.width = 100;
+    xwa.height = 200;
+    xwa.map_state = IsViewable;
+    xwa.border_width = 1;
+
+    FakeWindowAttributesGet fwag (xwa);
+
+    MockPixmapReady ready;
+
+    boost::function <void ()> readyCb (boost::bind (&PixmapReadyInterface::ready, &ready));
+
+    PixmapBinding pr (readyCb,
+			&mwpg,
+			&mwag,
+			&msg);
+
+    EXPECT_CALL (msg, grabServer ());
+    EXPECT_CALL (msg, syncServer ()).Times (2);
+    EXPECT_CALL (mwag, getAttributes (_)).WillOnce (Invoke (&fwag, &FakeWindowAttributesGet::getAttributes));
+    EXPECT_CALL (mwpg, getPixmap ()).WillOnce (Return (boost::shared_static_cast <WindowPixmapInterface> (wp)));
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (1));
+
+    EXPECT_CALL (ready, ready ());
+    EXPECT_CALL (msg, ungrabServer ());
+
+    EXPECT_TRUE (pr.bind ());
+    EXPECT_TRUE (pr.bind ());
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (1));
+
+    EXPECT_EQ (pr.pixmap (), 1);
+    EXPECT_EQ (pr.size (), CompSize (102, 202));
+
+    EXPECT_CALL (*wp, releasePixmap ());
+}
+
+TEST(CompositePixmapBinderTest, TestRebindAfterRelease)
+{
+    MockPixmap::Ptr wp (boost::make_shared <MockPixmap> ());
+
+    MockWindowPixmapGet mwpg;
+    MockWindowAttributesGet mwag;
+    MockServerGrab          msg;
+    XWindowAttributes xwa;
+
+    xwa.width = 100;
+    xwa.height = 200;
+    xwa.map_state = IsViewable;
+    xwa.border_width = 1;
+
+    FakeWindowAttributesGet fwag (xwa);
+
+    MockPixmapReady ready;
+
+    boost::function <void ()> readyCb (boost::bind (&PixmapReadyInterface::ready, &ready));
+
+    PixmapBinding pr (readyCb,
+			&mwpg,
+			&mwag,
+			&msg);
+
+    EXPECT_CALL (msg, grabServer ());
+    EXPECT_CALL (msg, syncServer ()).Times (2);
+    EXPECT_CALL (mwag, getAttributes (_)).WillOnce (Invoke (&fwag, &FakeWindowAttributesGet::getAttributes));
+    EXPECT_CALL (mwpg, getPixmap ()).WillOnce (Return (boost::shared_static_cast <WindowPixmapInterface> (wp)));
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (1));
+
+    EXPECT_CALL (ready, ready ());
+    EXPECT_CALL (msg, ungrabServer ());
+
+    EXPECT_TRUE (pr.bind ());
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (1));
+
+    EXPECT_EQ (pr.pixmap (), 1);
+    EXPECT_EQ (pr.size (), CompSize (102, 202));
+
+    EXPECT_CALL (*wp, releasePixmap ());
+
+    pr.release ();
+
+    EXPECT_CALL (msg, grabServer ());
+    EXPECT_CALL (msg, syncServer ()).Times (2);
+    EXPECT_CALL (mwag, getAttributes (_)).WillOnce (Invoke (&fwag, &FakeWindowAttributesGet::getAttributes));
+    EXPECT_CALL (mwpg, getPixmap ()).WillOnce (Return (boost::shared_static_cast <WindowPixmapInterface> (wp)));
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (1));
+
+    EXPECT_CALL (ready, ready ());
+    EXPECT_CALL (msg, ungrabServer ());
+
+    EXPECT_TRUE (pr.bind ());
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (1));
+
+    EXPECT_EQ (pr.pixmap (), 1);
+    EXPECT_EQ (pr.size (), CompSize (102, 202));
+
+    EXPECT_CALL (*wp, releasePixmap ());
+}
+
+TEST(CompositePixmapBinderTest, TestInitialBindFailureWindowUnmapped)
+{
+    MockWindowPixmapGet mwpg;
+    MockWindowAttributesGet mwag;
+    MockServerGrab          msg;
+    XWindowAttributes xwa;
+
+    xwa.width = 100;
+    xwa.height = 200;
+    xwa.map_state = IsUnmapped;
+    xwa.border_width = 1;
+
+    FakeWindowAttributesGet fwag (xwa);
+
+    PixmapBinding pr (boost::function <void ()> (),
+			&mwpg,
+			&mwag,
+			&msg);
+
+    EXPECT_CALL (msg, grabServer ());
+    EXPECT_CALL (msg, syncServer ()).Times (2);
+    EXPECT_CALL (mwag, getAttributes (_)).WillOnce (Invoke (&fwag, &FakeWindowAttributesGet::getAttributes));
+
+    EXPECT_CALL (msg, ungrabServer ());
+
+    EXPECT_FALSE (pr.bind ());
+
+    EXPECT_EQ (pr.pixmap (), 0);
+    EXPECT_EQ (pr.size (), CompSize (0, 0));
+}
+
+TEST(CompositePixmapBinderTest, TestInitialBindFailureWindowZeroSize)
+{
+    MockWindowPixmapGet mwpg;
+    MockWindowAttributesGet mwag;
+    MockServerGrab          msg;
+    XWindowAttributes xwa;
+
+    xwa.width = 0;
+    xwa.height = 0;
+    xwa.map_state = IsViewable;
+    xwa.border_width = 0;
+
+    FakeWindowAttributesGet fwag (xwa);
+
+    PixmapBinding pr (boost::function <void ()> (),
+			&mwpg,
+			&mwag,
+			&msg);
+
+    EXPECT_CALL (msg, grabServer ());
+    EXPECT_CALL (msg, syncServer ()).Times (2);
+    EXPECT_CALL (mwag, getAttributes (_)).WillOnce (Invoke (&fwag, &FakeWindowAttributesGet::getAttributes));
+
+    EXPECT_CALL (msg, ungrabServer ());
+
+    EXPECT_FALSE (pr.bind ());
+
+    EXPECT_EQ (pr.pixmap (), 0);
+    EXPECT_EQ (pr.size (), CompSize (0, 0));
+}
+
+TEST(CompositePixmapBinderTest, TestInitialBindFailureNilPixmapReturned)
+{
+    MockPixmap::Ptr wp (boost::make_shared <MockPixmap> ());
+
+    MockWindowPixmapGet mwpg;
+    MockWindowAttributesGet mwag;
+    MockServerGrab          msg;
+    XWindowAttributes xwa;
+
+    xwa.width = 100;
+    xwa.height =  200;
+    xwa.map_state = IsViewable;
+    xwa.border_width = 0;
+
+    FakeWindowAttributesGet fwag (xwa);
+
+    PixmapBinding pr (boost::function <void ()> (),
+			&mwpg,
+			&mwag,
+			&msg);
+
+    EXPECT_CALL (msg, grabServer ());
+    EXPECT_CALL (msg, syncServer ()).Times (2);
+    EXPECT_CALL (mwag, getAttributes (_)).WillOnce (Invoke (&fwag, &FakeWindowAttributesGet::getAttributes));
+    EXPECT_CALL (mwpg, getPixmap ()).WillOnce (Return (boost::shared_static_cast <WindowPixmapInterface> (wp)));
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (0));
+
+    EXPECT_CALL (msg, ungrabServer ());
+
+    EXPECT_FALSE (pr.bind ());
+
+    EXPECT_EQ (pr.pixmap (), 0);
+    EXPECT_EQ (pr.size (), CompSize (0, 0));
+}
+
+TEST(CompositePixmapBinderTest, TestInitialBindFailureWindowUnmappedSuccessOnRemap)
+{
+    MockPixmap::Ptr wp (boost::make_shared <MockPixmap> ());
+
+    MockWindowPixmapGet mwpg;
+    MockWindowAttributesGet mwag;
+    MockServerGrab          msg;
+    XWindowAttributes xwa;
+
+    xwa.width = 100;
+    xwa.height = 200;
+    xwa.map_state = IsUnmapped;
+    xwa.border_width = 1;
+
+    FakeWindowAttributesGet fwag (xwa);
+
+    PixmapBinding pr (boost::function <void ()> (),
+			&mwpg,
+			&mwag,
+			&msg);
+
+    EXPECT_CALL (msg, grabServer ());
+    EXPECT_CALL (msg, syncServer ()).Times (2);
+    EXPECT_CALL (mwag, getAttributes (_)).WillOnce (Invoke (&fwag, &FakeWindowAttributesGet::getAttributes));
+
+    EXPECT_CALL (msg, ungrabServer ());
+
+    EXPECT_FALSE (pr.bind ());
+
+    EXPECT_EQ (pr.pixmap (), 0);
+    EXPECT_EQ (pr.size (), CompSize (0, 0));
+
+    EXPECT_FALSE (pr.bind ());
+
+    pr.allowFurtherRebindAttempts ();
+
+    xwa.width = 100;
+    xwa.height = 200;
+    xwa.map_state = IsViewable;
+    xwa.border_width = 1;
+
+    FakeWindowAttributesGet fwag2 (xwa);
+
+    EXPECT_CALL (msg, grabServer ());
+    EXPECT_CALL (msg, syncServer ()).Times (2);
+    EXPECT_CALL (mwag, getAttributes (_)).WillOnce (Invoke (&fwag2, &FakeWindowAttributesGet::getAttributes));
+    EXPECT_CALL (mwpg, getPixmap ()).WillOnce (Return (boost::shared_static_cast <WindowPixmapInterface> (wp)));
+
+    EXPECT_CALL (msg, ungrabServer ());
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (1));
+
+    EXPECT_TRUE (pr.bind ());
+
+    EXPECT_CALL (*wp, pixmap ()).WillOnce (Return (1));
+    EXPECT_EQ (pr.pixmap (), 1);
+    EXPECT_EQ (pr.size (), CompSize (102, 202));
+}

=== modified file 'plugins/composite/src/privates.h'
--- plugins/composite/src/privates.h	2012-06-05 07:40:03 +0000
+++ plugins/composite/src/privates.h	2012-06-05 09:04:24 +0000
@@ -28,10 +28,14 @@
 #ifndef _COMPOSITE_PRIVATES_H
 #define _COMPOSITE_PRIVATES_H
 
+#include <memory>
+#include <boost/shared_ptr.hpp>
+
 #include <composite/composite.h>
 #include <core/atoms.h>
 #include <set>
 
+#include "pixmapbinding.h"
 #include "composite_options.h"
 
 extern CompPlugin::VTable *compositeVTable;
@@ -111,7 +115,11 @@
 	std::set<Damage> damages;
 };
 
-class PrivateCompositeWindow : WindowInterface
+class PrivateCompositeWindow :
+    public WindowInterface,
+    public CompositePixmapRebindInterface,
+    public WindowPixmapGetInterface,
+    public WindowAttributesGetInterface
 {
     public:
 	PrivateCompositeWindow (CompWindow *w, CompositeWindow *cw);
@@ -121,6 +129,13 @@
 	void resizeNotify (int dx, int dy, int dwidth, int dheight);
 	void moveNotify (int dx, int dy, bool now);
 
+	Pixmap pixmap () const;
+	bool   bind ();
+	const CompSize & size () const;
+	void release ();
+	void setNewPixmapReadyCallback (const boost::function <void ()> &);
+	void allowFurtherRebindAttempts ();
+
 	static void handleDamageRect (CompositeWindow *w,
 				      int             x,
 				      int             y,
@@ -132,17 +147,13 @@
 	CompositeWindow *cWindow;
 	CompositeScreen *cScreen;
 
-	Pixmap	      pixmap;
-	CompSize      size;
-	bool	      needsRebind;
-	CompositeWindow::NewPixmapReadyCallback newPixmapReadyCallback;
+	PixmapBinding mPixmapBinding;
 
 	Damage	      damage;
 
 	bool	      damaged;
 	bool	      redirected;
 	bool          overlayWindow;
-	bool          bindFailed;
 
 	unsigned short opacity;
 	unsigned short brightness;
@@ -151,6 +162,10 @@
 	XRectangle *damageRects;
 	int        sizeDamage;
 	int        nDamage;
+    private:
+
+	bool getAttributes (XWindowAttributes &);
+	WindowPixmapInterface::Ptr getPixmap ();
 };
 
 #endif

=== modified file 'plugins/composite/src/screen.cpp'
--- plugins/composite/src/screen.cpp	2012-06-05 07:40:03 +0000
+++ plugins/composite/src/screen.cpp	2012-06-05 09:04:24 +0000
@@ -840,6 +840,8 @@
     return false;
 }
 
+
+
 void
 CompositeScreen::preparePaint (int msSinceLastPaint)
     WRAPABLE_HND_FUNCTN (preparePaint, msSinceLastPaint)

=== modified file 'plugins/composite/src/window.cpp'
--- plugins/composite/src/window.cpp	2012-06-05 07:40:03 +0000
+++ plugins/composite/src/window.cpp	2012-06-05 09:04:24 +0000
@@ -83,9 +83,21 @@
 }
 
 void
-CompositeWindow::setNewPixmapReadyCallback (const NewPixmapReadyCallback &cb)
-{
-    priv->newPixmapReadyCallback = cb;
+PrivateCompositeWindow::setNewPixmapReadyCallback (const PixmapBinding::NewPixmapReadyCallback &cb)
+{
+    mPixmapBinding.setNewPixmapReadyCallback (cb);
+}
+
+void
+CompositeWindow::setNewPixmapReadyCallback (const PixmapBinding::NewPixmapReadyCallback &cb)
+{
+    priv->setNewPixmapReadyCallback (cb);
+}
+
+void
+PrivateCompositeWindow::allowFurtherRebindAttempts ()
+{
+    mPixmapBinding.allowFurtherRebindAttempts ();
 }
 
 PrivateCompositeWindow::PrivateCompositeWindow (CompWindow      *w,
@@ -93,14 +105,14 @@
     window (w),
     cWindow (cw),
     cScreen (CompositeScreen::get (screen)),
-    pixmap (None),
-    needsRebind (true),
-    newPixmapReadyCallback (),
+    mPixmapBinding (boost::function <void ()> (),
+		     this,
+		     this,
+		     screen->serverGrabInterface ()),
     damage (None),
     damaged (false),
     redirected (cScreen->compositingActive ()),
     overlayWindow (false),
-    bindFailed (false),
     opacity (OPAQUE),
     brightness (BRIGHT),
     saturation (COLOR),
@@ -118,6 +130,14 @@
 	free (damageRects);
 }
 
+
+
+bool
+PrivateCompositeWindow::bind ()
+{
+    return mPixmapBinding.bind ();
+}
+
 bool
 CompositeWindow::bind ()
 {
@@ -125,85 +145,61 @@
 	return false;
 
     redirect ();
-    if (priv->needsRebind)
-    {
-	XWindowAttributes attr;
-
-	/* don't try to bind window again if it failed previously */
-	if (priv->bindFailed)
-	    return false;
-
-	/* We have to grab the server here to make sure that window
-	   is mapped when getting the window pixmap */
-	XGrabServer (screen->dpy ());
-
-	/* Flush changes to the server and wait for it to process them */
-	XSync (screen->dpy (), false);
-	XGetWindowAttributes (screen->dpy (),
-			      ROOTPARENT (priv->window), &attr);
-	if (attr.map_state != IsViewable)
-	{
-	    XUngrabServer (screen->dpy ());
-	    XSync (screen->dpy (), false);
-	    priv->bindFailed = true;
-	    return false;
-	}
-
-	Pixmap newPixmap = XCompositeNameWindowPixmap
-	    (screen->dpy (), ROOTPARENT (priv->window));
-	CompSize newSize = CompSize (attr.border_width * 2 + attr.width,
-			             attr.border_width * 2 + attr.height);
-	XUngrabServer (screen->dpy ());
-	XSync (screen->dpy (), false);
-
-	if (newPixmap && newSize.width () && newSize.height ())
-	{
-	    /* Notify renderer that a new pixmap is about to
-	     * be bound */
-	    if (priv->newPixmapReadyCallback)
-		priv->newPixmapReadyCallback ();
-
-	    /* Release old pixmap */
-	    if (priv->pixmap)
-		XFreePixmap (screen->dpy (), priv->pixmap);
-
-	    /* Assign new pixmap */
-	    priv->pixmap = newPixmap;
-	    priv->size = newSize;
-
-	    priv->needsRebind = false;
-	}
-	else
-	{
-	    priv->bindFailed = true;
-	    priv->needsRebind = false;
-	    return false;
-	}
-    }
-    return true;
+    return priv->bind ();
+}
+
+void
+PrivateCompositeWindow::release ()
+{
+    mPixmapBinding.release ();
 }
 
 void
 CompositeWindow::release ()
 {
-    priv->needsRebind = true;
+    return priv->release ();
+}
+
+Pixmap
+PrivateCompositeWindow::pixmap () const
+{
+    return mPixmapBinding.pixmap ();
+}
+
+WindowPixmapInterface::Ptr
+PrivateCompositeWindow::getPixmap ()
+{
+    Pixmap pixmap = XCompositeNameWindowPixmap (screen->dpy (), ROOTPARENT (window));
+    WindowPixmapInterface::Ptr p (new X11WindowPixmap (screen->dpy (), pixmap));
+    return p;
+}
+
+bool
+PrivateCompositeWindow::getAttributes (XWindowAttributes &attr)
+{
+    if (XGetWindowAttributes (screen->dpy (),
+			      ROOTPARENT (window), &attr))
+	return true;
+
+    return false;
 }
 
 Pixmap
 CompositeWindow::pixmap ()
 {
-    static Pixmap nPixmap = None;
-
-    if (priv->needsRebind)
-	return nPixmap;
-
-    return priv->pixmap;
+    return priv->pixmap ();
+}
+
+const CompSize &
+PrivateCompositeWindow::size () const
+{
+    return mPixmapBinding.size ();
 }
 
 const CompSize &
 CompositeWindow::size ()
 {
-    return priv->size;
+    return priv->size ();
 }
 
 void
@@ -537,7 +533,7 @@
     switch (n)
     {
 	case CompWindowNotifyMap:
-	    bindFailed = false;
+	    allowFurtherRebindAttempts ();
 	    damaged = false;
 	    break;
 	case CompWindowNotifyUnmap:

=== modified file 'plugins/opengl/src/window.cpp'
--- plugins/opengl/src/window.cpp	2012-05-25 03:54:23 +0000
+++ plugins/opengl/src/window.cpp	2012-06-05 09:04:24 +0000
@@ -112,6 +112,8 @@
 	    priv->needsRebind = false;
 	    return true;
 	}
+	else
+	    return false;
     }
 
     GLTexture::List textures =

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2012-05-24 00:55:17 +0000
+++ src/CMakeLists.txt	2012-06-05 09:04:24 +0000
@@ -9,6 +9,7 @@
 add_subdirectory( rect )
 add_subdirectory( region )
 add_subdirectory( window )
+add_subdirectory( servergrab )
 
 IF (COMPIZ_BUILD_TESTING)
 add_subdirectory( privatescreen/tests )
@@ -57,6 +58,9 @@
     ${CMAKE_CURRENT_SOURCE_DIR}/rect/include
     ${CMAKE_CURRENT_SOURCE_DIR}/rect/src
 
+    ${CMAKE_CURRENT_SOURCE_DIR}/servergrab/include
+    ${CMAKE_CURRENT_SOURCE_DIR}/servergrab/src
+
     ${CMAKE_CURRENT_SOURCE_DIR}/region/include
     ${CMAKE_CURRENT_SOURCE_DIR}/region/src
 
@@ -162,6 +166,7 @@
     compiz_window_geometry_saver
     compiz_window_extents
     compiz_window_constrainment
+    compiz_servergrab
     -Wl,-no-whole-archive
 #    ${CORE_MOD_LIBRARIES}
 )

=== modified file 'src/event.cpp'
--- src/event.cpp	2012-06-01 03:47:24 +0000
+++ src/event.cpp	2012-06-05 09:04:24 +0000
@@ -37,6 +37,7 @@
 #include <X11/extensions/Xfixes.h>
 
 #include <core/atoms.h>
+#include <core/servergrab.h>
 #include "privatescreen.h"
 #include "privatewindow.h"
 #include "privatestackdebugger.h"
@@ -1079,6 +1080,30 @@
     }
 }
 
+ServerGrabInterface *
+CompScreenImpl::serverGrabInterface ()
+{
+    return static_cast <ServerGrabInterface *> (this);
+}
+
+void
+CompScreenImpl::grabServer ()
+{
+    XGrabServer (privateScreen.dpy);
+}
+
+void
+CompScreenImpl::syncServer ()
+{
+    XSync (privateScreen.dpy, false);
+}
+
+void
+CompScreenImpl::ungrabServer ()
+{
+    XUngrabServer (privateScreen.dpy);
+}
+
 void
 CompScreenImpl::_handleEvent (XEvent *event)
 {

=== modified file 'src/plugin.cpp'
--- src/plugin.cpp	2012-05-15 15:01:16 +0000
+++ src/plugin.cpp	2012-06-05 09:04:24 +0000
@@ -24,6 +24,8 @@
  */
 
 #include "core/plugin.h"
+
+/* XXX: This dependency needs to go away */
 #include "privatescreen.h"
 
 #include <boost/scoped_array.hpp>

=== modified file 'src/plugin/CMakeLists.txt'
--- src/plugin/CMakeLists.txt	2012-05-21 06:43:20 +0000
+++ src/plugin/CMakeLists.txt	2012-06-05 09:04:24 +0000
@@ -14,6 +14,7 @@
   ${compiz_SOURCE_DIR}/src/window/extents/include
 
   ${compiz_SOURCE_DIR}/src/pluginclasshandler/include
+  ${compiz_SOURCE_DIR}/src/servergrab/include
 
   ${COMPIZ_INCLUDE_DIRS}
 

=== modified file 'src/privatescreen.h'
--- src/privatescreen.h	2012-05-23 07:37:37 +0000
+++ src/privatescreen.h	2012-06-05 09:04:24 +0000
@@ -33,6 +33,7 @@
 #include <core/point.h>
 #include <core/timer.h>
 #include <core/plugin.h>
+#include <core/servergrab.h>
 #include <time.h>
 #include <boost/shared_ptr.hpp>
 
@@ -826,6 +827,7 @@
  * X server.
  */
 class CompScreenImpl : public CompScreen,
+    public ServerGrabInterface,
     ::compiz::private_screen::DesktopWindowCount,
     ::compiz::private_screen::MapNum,
     ::compiz::private_screen::Ping,
@@ -1045,6 +1047,8 @@
 	virtual void processEvents ();
 	virtual void alwaysHandleEvent (XEvent *event);
 
+	virtual ServerGrabInterface * serverGrabInterface ();
+
 	virtual void updatePassiveKeyGrabs () const;
 	virtual void updatePassiveButtonGrabs(Window serverFrame);
 
@@ -1145,6 +1149,10 @@
         virtual void _matchPropertyChanged(CompWindow *);
         virtual void _outputChangeNotify();
 
+	void grabServer ();
+	void ungrabServer ();
+	void syncServer ();
+
         bool handlePingTimeout();
 
         Window below;

=== modified file 'src/privatescreen/tests/CMakeLists.txt'
--- src/privatescreen/tests/CMakeLists.txt	2012-05-21 06:43:20 +0000
+++ src/privatescreen/tests/CMakeLists.txt	2012-06-05 09:04:24 +0000
@@ -17,6 +17,7 @@
   ${compiz_SOURCE_DIR}/src/window/geometry/include
   ${compiz_SOURCE_DIR}/src/window/extents/include
   ${compiz_SOURCE_DIR}/src/screen/extents/include
+  ${compiz_SOURCE_DIR}/src/servergrab/include
 
   ${compiz_SOURCE_DIR}/src/pluginclasshandler/include
 

=== modified file 'src/privatescreen/tests/test-privatescreen.cpp'
--- src/privatescreen/tests/test-privatescreen.cpp	2012-05-23 07:37:37 +0000
+++ src/privatescreen/tests/test-privatescreen.cpp	2012-06-05 09:04:24 +0000
@@ -204,6 +204,11 @@
     MOCK_METHOD1(getProtocols, unsigned int (Window id));
     MOCK_METHOD1(getWindowType, unsigned int (Window id));
     MOCK_METHOD1(getWindowState, unsigned int (Window id));
+
+    MOCK_METHOD0(grabServer, void ());
+    MOCK_METHOD0(ungrabServer, void ());
+    MOCK_METHOD0(syncServer, void ());
+    MOCK_METHOD0(serverGrabInterface, ServerGrabInterface * ());
 };
 
 class MockViewportRetreival :

=== added directory 'src/servergrab'
=== added file 'src/servergrab/CMakeLists.txt'
--- src/servergrab/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ src/servergrab/CMakeLists.txt	2012-06-05 09:04:24 +0000
@@ -0,0 +1,42 @@
+INCLUDE_DIRECTORIES (  
+  ${CMAKE_CURRENT_SOURCE_DIR}/include
+  ${CMAKE_CURRENT_SOURCE_DIR}/src
+
+  ${Boost_INCLUDE_DIRS}
+)
+
+LINK_DIRECTORIES (${GLIBMM_LIBRARY_DIRS}) 
+
+SET ( 
+  PUBLIC_HEADERS 
+  ${CMAKE_CURRENT_SOURCE_DIR}/include/core/servergrab.h
+)
+
+SET ( 
+  PRIVATE_HEADERS 
+)
+
+SET( 
+  SRCS 
+  ${CMAKE_CURRENT_SOURCE_DIR}/src/servergrab.cpp
+)
+
+ADD_LIBRARY( 
+  compiz_servergrab STATIC
+  
+  ${SRCS}
+  
+  ${PUBLIC_HEADERS}
+  ${PRIVATE_HEADERS}
+)
+
+IF (COMPIZ_BUILD_TESTING)
+ADD_SUBDIRECTORY( ${CMAKE_CURRENT_SOURCE_DIR}/tests )
+ENDIF (COMPIZ_BUILD_TESTING)
+
+SET_TARGET_PROPERTIES(
+  compiz_servergrab PROPERTIES
+  PUBLIC_HEADER "${PUBLIC_HEADERS}"
+)
+
+install (FILES ${PUBLIC_HEADERS} DESTINATION ${COMPIZ_CORE_INCLUDE_DIR})

=== added directory 'src/servergrab/include'
=== added directory 'src/servergrab/include/core'
=== added file 'src/servergrab/include/core/servergrab.h'
--- src/servergrab/include/core/servergrab.h	1970-01-01 00:00:00 +0000
+++ src/servergrab/include/core/servergrab.h	2012-06-05 09:04:24 +0000
@@ -0,0 +1,51 @@
+/*
+ * Copyright © 2012 Canonical Ltd.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Canonical Ltd. not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Canonical Ltd. makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * CANONICAL, LTD. DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL CANONICAL, LTD. BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Authored by: Sam Spilsbury <[e-mail]>
+ */
+
+#ifndef _COMP_SERVER_GRAB_H
+#define _COMP_SERVER_GRAB_H
+
+class ServerGrabInterface
+{
+    public:
+	virtual ~ServerGrabInterface () {}
+
+	virtual void grabServer () = 0;
+	virtual void ungrabServer () = 0;
+	virtual void syncServer () = 0;
+};
+
+class ServerLock
+{
+    public:
+
+	ServerLock (ServerGrabInterface *i);
+	~ServerLock ();
+
+    private:
+
+	ServerGrabInterface *mServerGrab;
+};
+
+#endif

=== added directory 'src/servergrab/src'
=== added file 'src/servergrab/src/servergrab.cpp'
--- src/servergrab/src/servergrab.cpp	1970-01-01 00:00:00 +0000
+++ src/servergrab/src/servergrab.cpp	2012-06-05 09:04:24 +0000
@@ -0,0 +1,39 @@
+/*
+ * Copyright © 2012 Canonical Ltd.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Canonical Ltd. not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Canonical Ltd. makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * CANONICAL, LTD. DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL CANONICAL, LTD. BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Authored by: Sam Spilsbury <[e-mail]>
+ */
+
+#include <core/servergrab.h>
+
+ServerLock::ServerLock (ServerGrabInterface *i) :
+    mServerGrab (i)
+{
+    mServerGrab->grabServer ();
+    mServerGrab->syncServer ();
+}
+
+ServerLock::~ServerLock ()
+{
+    mServerGrab->ungrabServer ();
+    mServerGrab->syncServer ();
+}

=== added directory 'src/servergrab/tests'
=== added file 'src/servergrab/tests/CMakeLists.txt'
--- src/servergrab/tests/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ src/servergrab/tests/CMakeLists.txt	2012-06-05 09:04:24 +0000
@@ -0,0 +1,14 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR})
+
+add_executable (compiz_test_servergrab
+                ${CMAKE_CURRENT_SOURCE_DIR}/test-servergrab.cpp)
+
+target_link_libraries (compiz_test_servergrab
+                       compiz_servergrab
+                       ${GTEST_BOTH_LIBRARIES}
+		       ${GMOCK_LIBRARY}
+		       ${GMOCK_MAIN_LIBRARY}
+		       ${CMAKE_THREAD_LIBS_INIT} # Link in pthread. 
+		      )
+
+gtest_add_tests (compiz_test_servergrab "" ${CMAKE_CURRENT_SOURCE_DIR}/test-servergrab.cpp)

=== added file 'src/servergrab/tests/test-servergrab.cpp'
--- src/servergrab/tests/test-servergrab.cpp	1970-01-01 00:00:00 +0000
+++ src/servergrab/tests/test-servergrab.cpp	2012-06-05 09:04:24 +0000
@@ -0,0 +1,55 @@
+/*
+ * Copyright © 2012 Canonical Ltd.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Canonical Ltd. not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Canonical Ltd. makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * CANONICAL, LTD. DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL CANONICAL, LTD. BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Authored by: Sam Spilsbury <[e-mail]>
+ */
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+#include <core/servergrab.h>
+
+class MockServerGrab :
+    public ServerGrabInterface
+{
+    public:
+
+	MOCK_METHOD0 (grabServer, void ());
+	MOCK_METHOD0 (ungrabServer, void ());
+	MOCK_METHOD0 (syncServer, void ());
+};
+
+class ServerGrabTest :
+    public ::testing::Test
+{
+};
+
+TEST(ServerGrabTest, TestServerGrab)
+{
+    MockServerGrab msg;
+
+    EXPECT_CALL (msg, grabServer ());
+    EXPECT_CALL (msg, syncServer ()).Times (2);
+
+    ServerLock l (&msg);
+
+    EXPECT_CALL (msg, ungrabServer ());
+}