Merge lp:~compiz-team/compiz/compiz.fix_1101026.1 into lp:compiz/0.9.10

[Sam Spilsbury]

Added some new hooks to PluginClassHandler to allow a VTable to specify if loaded.

PluginClassHandler::get () was designed to simply instantiate an instance of that
class for the core structure, but it did this without checking if the plugin was
loaded.

Added some new methods to PluginClassHandler exposed by LoadedPluginClassBridge
and only accessible by those who implement PluginKey to specify globally
whether or not a plugin is actually loaded, so that PluginClassHandler can
return accordingly.

Integration and unit tests added as appropriate

(LP: #1169620)
(LP: #1101026)

Added some new hooks to PluginClassHandler to allow a VTable to specify if loaded.

PluginClassHandler::get () was designed to simply instantiate an instance of that
class for the core structure, but it did this without checking if the plugin was
loaded.

Added some new methods to PluginClassHandler exposed by LoadedPluginClassBridge
and only accessible by those who implement PluginKey to specify globally
whether or not a plugin is actually loaded, so that PluginClassHandler can
return accordingly.

I note that the LoadedPluginBridge / PluginKey abstraction is a little weird. I'd like to make it so that CompPlugin::VTable had access to a specific interface on PluginClassHandler to mark certain classes as loaded and unloaded. Unfortunately, in order to do that it would require substantial changes of the API - and I don't really want to do that.

Integration and unit tests added as appropriate

(LP: #1169620)
(LP: #1101026)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3658
http://jenkins.qa.ubuntu.com/job/compiz-ci/135/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/172/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/524/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/135/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3658
http://jenkins.qa.ubuntu.com/job/compiz-ci/138/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/176
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/527

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/138/rebuild

review: Approve (continuous-integration)

[MC Return (mc-return)]

This MP sounds very promising.
I guess this: 9 +#define CORE_ABIVERSION 20130415 means all plugins will have to be recompiled ?

Maybe adding 2 newlines here, after public and protected ?:

520 + public:
521 + virtual bool
522 + LoadPlugin(CompPlugin *p, const char *path, const char *name) const = 0;
523 +
524 + virtual void
525 + UnloadPlugin(CompPlugin *p) const = 0;
526 +
527 + static PluginFilesystem const* instance;
528 +
529 + protected:
530 + PluginFilesystem();
531 + virtual ~PluginFilesystem() {}

One whitespace too much here:

1124 + * declare PluginKey, which users should inherit from and take

Other than that, I still need to test this, "Abstain" for now.

review: Abstain

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3659
http://jenkins.qa.ubuntu.com/job/compiz-ci/144/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/182
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/533

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/144/rebuild

review: Approve (continuous-integration)

[MC Return (mc-return)]

I am on the one hand very excited about this one, but on the other hand this means for me personally, that my self-compiled non-trunk plugins will probably all need fixing, then each branch will have to merge with trunk, then I will
have to recompile all those branches again, then copy the compiled binaries, before I can start to use them again ->

I would really like to see screenshot merged to trunk at least, before this one gets merged...

Abstain for now...

review: Abstain

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3726
http://jenkins.qa.ubuntu.com/job/compiz-ci/186/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/227
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-ci/43
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-ci/43
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-ci/41

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/186/rebuild

review: Approve (continuous-integration)

[MC Return (mc-return)]

Sam, I have linked the 2 corresponding bug reports to this MP.

[MC Return (mc-return)]

A huge change. :)

Forces me to finally merge trunk with my working branch and fix all the conflicts.
Forces me to recompile all plugins not yet merged with trunk.
Forces me to do a lot of things.

Could not find anything in the code looking bad though and it is great you fixed those nasty bugs with this MP, so this forces me to "Approve" this thingy... ;)

review: Approve

[Brandon Schaefer (brandontschaefer)]

Hmm strange:

/home/bschaefer/src/compiz.fix_1101026.1/src/plugin/tests/test-plugin.cpp: In member function ‘virtual void PluginClassIntegrationTest_get_plugin_structure_null_on_not_loaded_Test<gtest_TypeParam_>::TestBody()’:
/home/bschaefer/src/compiz.fix_1101026.1/src/plugin/tests/test-plugin.cpp:588:52: error: typedef ‘TestParam’ locally defined but not used [-Werror=unused-local-typedefs]
     typedef PluginClassIntegrationTest <TypeParam> TestParam;
                                                    ^
At global scope:
cc1plus: error: unrecognized command line option "-Wno-unused-private-field" [-Werror]
cc1plus: error: unrecognized command line option "-Wno-unused-private-field" [-Werror]
cc1plus: error: unrecognized command line option "-Wno-unused-private-field" [-Werror]
cc1plus: error: unrecognized command line option "-Wno-unused-private-field" [-Werror]
cc1plus: all warnings being treated as errors

I've not had a huge amount of time to look into why this fails (or if I cmaked it wrong :)

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

Saucy's compiler added a new warning. CI isn't yet running on saucy. All fixed, tests passing.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3727
http://jenkins.qa.ubuntu.com/job/compiz-ci/187/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/228
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-ci/44
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-ci/44
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-ci/42

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/187/rebuild

review: Approve (continuous-integration)

[Brandon Schaefer (brandontschaefer)]

538 + PluginFilesystem();

You shouldn't need this, the default ctor should be work...(though ive started getting to use to c++0x11), but it really doesn't matter.

Though annoyingly I keep getting this test failing (on trunk compiz):
The following tests FAILED:
  15 - GWDMockSettingsTest.TestMock (Failed)
Errors while running CTest

Still not this branches problem :).

Overall looks good to me.

review: Approve

[Brandon Schaefer (brandontschaefer)]

On a side note theres still some crashes when enabling/disabling plugins :(, though im guessing its an unloading/loading problem which is different then this branch!

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

[MC Return (mc-return)]

On a side note, this breaks Unity, see bug #1185778.

7 === modified file 'include/core/abiversion.h'
8 --- include/core/abiversion.h 2013-01-28 17:54:50 +0000
9 +++ include/core/abiversion.h 2013-05-28 23:39:25 +0000
10 @@ -5,6 +5,6 @@
11 # error Conflicting definitions of CORE_ABIVERSION
12 #endif
13
14 -#define CORE_ABIVERSION 20130125
15 +#define CORE_ABIVERSION 20130415

=== added file '.bzrignore'
--- .bzrignore	1970-01-01 00:00:00 +0000
+++ .bzrignore	2013-05-28 23:39:25 +0000
@@ -0,0 +1,1 @@
+.bzr-repo

=== modified file 'include/core/abiversion.h'
--- include/core/abiversion.h	2013-01-28 17:54:50 +0000
+++ include/core/abiversion.h	2013-05-28 23:39:25 +0000
@@ -5,6 +5,6 @@
 #  error Conflicting definitions of CORE_ABIVERSION
 #endif
 
-#define CORE_ABIVERSION 20130125
+#define CORE_ABIVERSION 20130415
 
 #endif // COMPIZ_ABIVERSION_H

=== modified file 'include/core/plugin.h'
--- include/core/plugin.h	2013-01-24 14:18:03 +0000
+++ include/core/plugin.h	2013-05-28 23:39:25 +0000
@@ -29,6 +29,7 @@
 #include <core/string.h>
 #include <core/option.h>
 #include <core/privateunion.h>
+#include <core/pluginclasshandler.h>
 
 #include <string.h>
 
@@ -70,6 +71,37 @@
 extern UnloadPluginProc loaderUnloadPlugin;
 extern ListPluginsProc  loaderListPlugins;
 
+namespace compiz
+{
+namespace plugin
+{
+namespace internal
+{
+class VTableBase;
+
+/**
+ * Provide a definition for PluginKey, with a protected
+ * constructor which is a friend of VTableBase */
+class PluginKey
+{
+    protected:
+
+	PluginKey () {}
+
+	friend class VTableBase;
+};
+
+class VTableBase :
+    public PluginKey
+{
+    public:
+
+	virtual ~VTableBase () {};
+};
+}
+}
+}
+
 /**
  * Base plug-in interface for Compiz. All plugins must implement this
  * interface, which provides basics for loading, unloading, options,
@@ -77,8 +109,11 @@
  */
 class CompPlugin {
     public:
-	class VTable {
+	class VTable :
+	    public compiz::plugin::internal::VTableBase
+	{
 	    public:
+
 		VTable ();
 		virtual ~VTable ();
 
@@ -91,9 +126,14 @@
 		const CompString &name () const;
 
 		virtual bool init () = 0;
-
 		virtual void fini ();
 
+		/* Mark the plugin as ready to be instantiated */
+		virtual void markReadyToInstantiate () = 0;
+
+		/* Mark the plugin as disallowing further instantiation */
+		virtual void markNoFurtherInstantiation () = 0;
+
 		virtual bool initScreen (CompScreen *s);
 
 		virtual void finiScreen (CompScreen *s);
@@ -111,36 +151,53 @@
 		VTable       **mSelf;
         };
 
-    /**
-     * TODO (or not?)
-     */
-	template <typename T, typename T2>
-	class VTableForScreenAndWindow : public VTable {
-	    bool initScreen (CompScreen *s);
-
-	    void finiScreen (CompScreen *s);
-
-	    bool initWindow (CompWindow *w);
-
-	    void finiWindow (CompWindow *w);
-
-	    CompOption::Vector & getOptions ();
-
-	    bool setOption (const CompString &name, CompOption::Value &value);
+	/**
+	 * TODO (or not?)
+	 */
+	template <typename T, typename T2, int ABI = 0>
+	class VTableForScreenAndWindow :
+	    public VTable
+	{
+	    public:
+
+		bool initScreen (CompScreen *s);
+		void finiScreen (CompScreen *s);
+		bool initWindow (CompWindow *w);
+		void finiWindow (CompWindow *w);
+		CompOption::Vector & getOptions ();
+		bool setOption (const CompString &name,
+				CompOption::Value &value);
+
+	    private:
+
+		/* Mark the plugin as ready to be instantiated */
+		void markReadyToInstantiate ();
+
+		/* Mark the plugin as disallowing further instantiation */
+		void markNoFurtherInstantiation ();
 	};
 
-    /**
-     * TODO (or not?)
-     */
-	template <typename T>
-	class VTableForScreen : public VTable {
-	    bool initScreen (CompScreen *s);
-
-	    void finiScreen (CompScreen *s);
-
-	    CompOption::Vector & getOptions ();
-
-	    bool setOption (const CompString &name, CompOption::Value &value);
+	/**
+	 * TODO (or not?)
+	 */
+	template <typename T, int ABI = 0>
+	class VTableForScreen :
+	    public VTable
+	{
+	    public:
+
+		bool initScreen (CompScreen *s);
+		void finiScreen (CompScreen *s);
+		CompOption::Vector & getOptions ();
+		bool setOption (const CompString &name, CompOption::Value &value);
+
+	    private:
+
+		/* Mark the plugin as ready to be instantiated */
+		void markReadyToInstantiate ();
+
+		/* Mark the plugin as disallowing further instantiation */
+		void markNoFurtherInstantiation ();
 	};
 	
 	typedef std::map<CompString, CompPlugin *> Map;
@@ -211,9 +268,47 @@
 
 };
 
-
-template <typename T, typename T2>
-bool CompPlugin::VTableForScreenAndWindow<T,T2>::initScreen (CompScreen *s)
+/**
+ * Mark the plugin class handlers as ready to be initialized
+ */
+template <typename T, typename T2, int ABI>
+void
+CompPlugin::VTableForScreenAndWindow<T, T2, ABI>::markReadyToInstantiate ()
+{
+    namespace cpi = compiz::plugin::internal;
+
+    typedef typename T::ClassPluginBaseType ScreenBase;
+    typedef typename T2::ClassPluginBaseType WindowBase;
+
+    typedef cpi::LoadedPluginClassBridge <T, ScreenBase, ABI> ScreenBridge;
+    typedef cpi::LoadedPluginClassBridge <T2, WindowBase, ABI> WindowBridge;
+
+    ScreenBridge::allowInstantiations (*this);
+    WindowBridge::allowInstantiations (*this);
+}
+
+/**
+ * Allow no further class handler initialization
+ */
+template <typename T, typename T2, int ABI>
+void
+CompPlugin::VTableForScreenAndWindow<T, T2, ABI>::markNoFurtherInstantiation ()
+{
+    namespace cpi = compiz::plugin::internal;
+
+    typedef typename T::ClassPluginBaseType ScreenBase;
+    typedef typename T2::ClassPluginBaseType WindowBase;
+
+    typedef cpi::LoadedPluginClassBridge <T, ScreenBase, ABI> ScreenBridge;
+    typedef cpi::LoadedPluginClassBridge <T2, WindowBase, ABI> WindowBridge;
+
+    ScreenBridge::disallowInstantiations (*this);
+    WindowBridge::disallowInstantiations (*this);
+}
+
+template <typename T, typename T2, int ABI>
+bool
+CompPlugin::VTableForScreenAndWindow<T, T2, ABI>::initScreen (CompScreen *s)
 {
     T * ps = T::get (s);
     if (!ps)
@@ -222,15 +317,17 @@
     return true;
 }
 
-template <typename T, typename T2>
-void CompPlugin::VTableForScreenAndWindow<T,T2>::finiScreen (CompScreen *s)
+template <typename T, typename T2, int ABI>
+void
+CompPlugin::VTableForScreenAndWindow<T, T2, ABI>::finiScreen (CompScreen *s)
 {
     T * ps = T::get (s);
     delete ps;
 }
 
-template <typename T, typename T2>
-bool CompPlugin::VTableForScreenAndWindow<T,T2>::initWindow (CompWindow *w)
+template <typename T, typename T2, int ABI>
+bool
+CompPlugin::VTableForScreenAndWindow<T, T2, ABI>::initWindow (CompWindow *w)
 {
     T2 * pw = T2::get (w);
     if (!pw)
@@ -239,15 +336,16 @@
     return true;
 }
 
-template <typename T, typename T2>
-void CompPlugin::VTableForScreenAndWindow<T,T2>::finiWindow (CompWindow *w)
+template <typename T, typename T2, int ABI>
+void
+CompPlugin::VTableForScreenAndWindow<T, T2, ABI>::finiWindow (CompWindow *w)
 {
     T2 * pw = T2::get (w);
     delete pw;
 }
 
-template <typename T, typename T2>
-CompOption::Vector & CompPlugin::VTableForScreenAndWindow<T,T2>::getOptions ()
+template <typename T, typename T2, int ABI>
+CompOption::Vector & CompPlugin::VTableForScreenAndWindow<T, T2, ABI>::getOptions ()
 {
     CompOption::Class *oc = dynamic_cast<CompOption::Class *> (T::get (screen));
     if (!oc)
@@ -255,9 +353,10 @@
     return oc->getOptions ();
 }
 
-template <typename T, typename T2>
-bool CompPlugin::VTableForScreenAndWindow<T,T2>::setOption (const CompString  &name,
-							    CompOption::Value &value)
+template <typename T, typename T2, int ABI>
+bool
+CompPlugin::VTableForScreenAndWindow<T, T2, ABI>::setOption (const CompString  &name,
+							     CompOption::Value &value)
 {
     CompOption::Class *oc = dynamic_cast<CompOption::Class *> (T::get (screen));
     if (!oc)
@@ -265,8 +364,39 @@
     return oc->setOption (name, value);
 }
 
-template <typename T>
-bool CompPlugin::VTableForScreen<T>::initScreen (CompScreen *s)
+/**
+ * Mark the plugin class handlers as ready to be initialized
+ */
+template <typename T, int ABI>
+void
+CompPlugin::VTableForScreen<T, ABI>::markReadyToInstantiate ()
+{
+    namespace cpi = compiz::plugin::internal;
+
+    typedef typename T::ClassPluginBaseType ScreenBase;
+    typedef cpi::LoadedPluginClassBridge <T, ScreenBase, ABI> ScreenBridge;
+
+    ScreenBridge::allowInstantiations (*this);
+}
+
+/**
+ * Allow no further class handler initialization
+ */
+template <typename T, int ABI>
+void
+CompPlugin::VTableForScreen<T, ABI>::markNoFurtherInstantiation ()
+{
+    namespace cpi = compiz::plugin::internal;
+
+    typedef typename T::ClassPluginBaseType ScreenBase;
+    typedef cpi::LoadedPluginClassBridge <T, ScreenBase, ABI> ScreenBridge;
+
+    ScreenBridge::disallowInstantiations (*this);
+}
+
+template <typename T, int ABI>
+bool
+CompPlugin::VTableForScreen<T, ABI>::initScreen (CompScreen *s)
 {
     T * ps = new T (s);
     if (ps->loadFailed ())
@@ -277,15 +407,16 @@
     return true;
 }
 
-template <typename T>
-void CompPlugin::VTableForScreen<T>::finiScreen (CompScreen *s)
+template <typename T, int ABI>
+void CompPlugin::VTableForScreen<T, ABI>::finiScreen (CompScreen *s)
 {
     T * ps = T::get (s);
     delete ps;
 }
 
-template <typename T>
-CompOption::Vector & CompPlugin::VTableForScreen<T>::getOptions ()
+template <typename T, int ABI>
+CompOption::Vector &
+CompPlugin::VTableForScreen<T, ABI>::getOptions ()
 {
     CompOption::Class *oc = dynamic_cast<CompOption::Class *> (T::get (screen));
     if (!oc)
@@ -293,8 +424,9 @@
     return oc->getOptions ();
 }
 
-template <typename T>
-bool CompPlugin::VTableForScreen<T>::setOption (const CompString  &name,
+template <typename T, int ABI>
+bool
+CompPlugin::VTableForScreen<T, ABI>::setOption (const CompString  &name,
 						CompOption::Value &value)
 {
     CompOption::Class *oc = dynamic_cast<CompOption::Class *> (T::get (screen));

=== modified file 'plugins/animation/src/animation.cpp'
--- plugins/animation/src/animation.cpp	2013-05-09 13:43:07 +0000
+++ plugins/animation/src/animation.cpp	2013-05-28 23:39:25 +0000
@@ -94,7 +94,7 @@
 using namespace compiz::core;
 
 class AnimPluginVTable :
-    public CompPlugin::VTableForScreenAndWindow<AnimScreen, AnimWindow>
+    public CompPlugin::VTableForScreenAndWindow<AnimScreen, AnimWindow, ANIMATION_ABI>
 {
 public:
     bool init ();

=== modified file 'plugins/composite/src/composite.cpp'
--- plugins/composite/src/composite.cpp	2013-05-09 13:43:07 +0000
+++ plugins/composite/src/composite.cpp	2013-05-28 23:39:25 +0000
@@ -33,7 +33,7 @@
 
 
 class CompositePluginVTable :
-    public CompPlugin::VTableForScreenAndWindow<CompositeScreen, CompositeWindow>
+    public CompPlugin::VTableForScreenAndWindow<CompositeScreen, CompositeWindow, COMPIZ_COMPOSITE_ABI>
 {
     public:
 

=== modified file 'plugins/cube/src/cube.cpp'
--- plugins/cube/src/cube.cpp	2013-05-19 11:50:08 +0000
+++ plugins/cube/src/cube.cpp	2013-05-28 23:39:25 +0000
@@ -33,7 +33,7 @@
 #include <privates.h>
 
 class CubePluginVTable :
-    public CompPlugin::VTableForScreenAndWindow<CubeScreen, PrivateCubeWindow>
+    public CompPlugin::VTableForScreenAndWindow<CubeScreen, PrivateCubeWindow, COMPIZ_CUBE_ABI>
 {
     public:
 

=== modified file 'plugins/opengl/src/opengl.cpp'
--- plugins/opengl/src/opengl.cpp	2013-05-09 13:43:07 +0000
+++ plugins/opengl/src/opengl.cpp	2013-05-28 23:39:25 +0000
@@ -79,7 +79,7 @@
 }
 
 class OpenglPluginVTable :
-    public CompPlugin::VTableForScreenAndWindow<GLScreen, GLWindow>
+    public CompPlugin::VTableForScreenAndWindow<GLScreen, GLWindow, COMPIZ_OPENGL_ABI>
 {
     public:
 

=== modified file 'plugins/scale/src/scale.cpp'
--- plugins/scale/src/scale.cpp	2013-05-11 09:30:49 +0000
+++ plugins/scale/src/scale.cpp	2013-05-28 23:39:25 +0000
@@ -39,7 +39,7 @@
 #define EDGE_STATE (CompAction::StateInitEdge)
 
 class ScalePluginVTable :
-    public CompPlugin::VTableForScreenAndWindow<ScaleScreen, ScaleWindow>
+    public CompPlugin::VTableForScreenAndWindow<ScaleScreen, ScaleWindow, COMPIZ_SCALE_ABI>
 {
     public:
 

=== modified file 'src/plugin.cpp'
--- src/plugin.cpp	2013-02-14 02:43:51 +0000
+++ src/plugin.cpp	2013-05-28 23:39:25 +0000
@@ -54,6 +54,9 @@
 
 	bool init ();
 
+	void markReadyToInstantiate ();
+	void markNoFurtherInstantiation ();
+
 	CompOption::Vector & getOptions ();
 
 	bool setOption (const CompString  &name,
@@ -78,6 +81,16 @@
     return true;
 }
 
+void
+CorePluginVTable::markReadyToInstantiate ()
+{
+}
+
+void
+CorePluginVTable::markNoFurtherInstantiation ()
+{
+}
+
 CompOption::Vector &
 CorePluginVTable::getOptions ()
 {
@@ -228,6 +241,8 @@
 	return false;
     }
 
+    p->vTable->markReadyToInstantiate ();
+
     if (screen && screen->displayInitialised())
     {
 	if (!p->vTable->initScreen (screen))
@@ -252,6 +267,7 @@
 void
 CompManager::finiPlugin (CompPlugin *p)
 {
+    p->vTable->markNoFurtherInstantiation ();
 
     if (screen)
     {

=== modified file 'src/plugin/tests/test-plugin.cpp'
--- src/plugin/tests/test-plugin.cpp	2013-02-14 02:43:51 +0000
+++ src/plugin/tests/test-plugin.cpp	2013-05-28 23:39:25 +0000
@@ -1,8 +1,9 @@
+#include <boost/shared_ptr.hpp>
+#include <boost/foreach.hpp>
+#define foreach BOOST_FOREACH
+
 #include "core/plugin.h"
 
-// This prevents an instantiation error - not sure why ATM
-#include "core/screen.h"
-
 // Get rid of stupid macro from X.h
 // Why, oh why, are we including X.h?
 #undef None
@@ -10,37 +11,54 @@
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 #include <gtest_shared_tmpenv.h>
-
-namespace {
+#include <gtest_shared_autodestroy.h>
+
+/* This is a link-seam so that we don't have to include screen.h */
+class CompAction
+{
+};
+
+class CompMatch
+{
+};
+
+namespace
+{
 
 class PluginFilesystem
 {
-public:
-    virtual bool
-    LoadPlugin(CompPlugin *p, const char *path, const char *name) const = 0;
-
-    virtual void
-    UnloadPlugin(CompPlugin *p) const = 0;
-
-    static PluginFilesystem const* instance;
-
-protected:
-    PluginFilesystem();
-    virtual ~PluginFilesystem() {}
+    public:
+
+	virtual bool
+	LoadPlugin(CompPlugin *p, const char *path, const char *name) const = 0;
+
+	virtual void
+	UnloadPlugin(CompPlugin *p) const = 0;
+
+	static PluginFilesystem const* instance;
+
+    protected:
+
+	PluginFilesystem();
+	virtual ~PluginFilesystem() {}
 };
 
 class MockPluginFilesystem : public PluginFilesystem
 {
-public:
-    MOCK_CONST_METHOD3(LoadPlugin, bool (CompPlugin *, const char *, const char *));
-
-    MOCK_CONST_METHOD1(UnloadPlugin, void (CompPlugin *p));
+    public:
+
+	MOCK_CONST_METHOD3(LoadPlugin, bool (CompPlugin *, const char *, const char *));
+
+	MOCK_CONST_METHOD1(UnloadPlugin, void (CompPlugin *p));
 };
 
-class MockVTable : public CompPlugin::VTable
+class MockVTable :
+    public CompPlugin::VTable
 {
-public:
-    MOCK_METHOD0(init, bool ());
+    public:
+	MOCK_METHOD0(init, bool ());
+	MOCK_METHOD0(markReadyToInstantiate, void ());
+	MOCK_METHOD0(markNoFurtherInstantiation, void ());
 };
 
 
@@ -59,22 +77,26 @@
 
 PluginFilesystem::PluginFilesystem()
 {
-	::loaderLoadPlugin = ::ThunkLoadPluginProc;
-	::loaderUnloadPlugin = ::ThunkUnloadPluginProc;
+    ::loaderLoadPlugin = ::ThunkLoadPluginProc;
+    ::loaderUnloadPlugin = ::ThunkUnloadPluginProc;
 
-	instance = this;
+    instance = this;
 }
 
 PluginFilesystem const* PluginFilesystem::instance = 0;
 
 } // (abstract) namespace
 
-
-
-TEST(PluginTest, load_non_existant_plugin_must_fail)
-{
-    MockPluginFilesystem mockfs;
-
+class PluginTest :
+    public ::testing::Test
+{
+    public:
+
+	MockPluginFilesystem mockfs;
+};
+
+TEST_F (PluginTest, load_non_existant_plugin_must_fail)
+{
     using namespace testing;
 
     EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("dummy"))).
@@ -91,10 +113,8 @@
     ASSERT_EQ(0, CompPlugin::load("dummy"));
 }
 
-TEST(PluginTest, load_plugin_from_HOME_PLUGINDIR_succeeds)
+TEST_F (PluginTest, load_plugin_from_HOME_PLUGINDIR_succeeds)
 {
-    MockPluginFilesystem mockfs;
-
     using namespace testing;
 
     EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("dummy"))).
@@ -114,10 +134,8 @@
     CompPlugin::unload(cp);
 }
 
-TEST(PluginTest, load_plugin_from_PLUGINDIR_succeeds)
+TEST_F (PluginTest, load_plugin_from_PLUGINDIR_succeeds)
 {
-    MockPluginFilesystem mockfs;
-
     using namespace testing;
 
     EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("dummy"))).
@@ -137,11 +155,10 @@
     CompPlugin::unload(cp);
 }
 
-TEST(PluginTest, load_plugin_from_COMPIZ_PLUGIN_DIR_env_succeeds)
+TEST_F (PluginTest, load_plugin_from_COMPIZ_PLUGIN_DIR_env_succeeds)
 {
     const char *COMPIZ_PLUGIN_DIR_VALUE = "/path/to/plugin/dir";
     TmpEnv env ("COMPIZ_PLUGIN_DIR", COMPIZ_PLUGIN_DIR_VALUE);
-    MockPluginFilesystem mockfs;
 
     using namespace testing;
 
@@ -165,10 +182,8 @@
     CompPlugin::unload(cp);
 }
 
-TEST(PluginTest, load_plugin_from_void_succeeds)
+TEST_F (PluginTest, load_plugin_from_void_succeeds)
 {
-    MockPluginFilesystem mockfs;
-
     using namespace testing;
 
     EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("dummy"))).
@@ -188,31 +203,429 @@
     CompPlugin::unload(cp);
 }
 
-TEST(PluginTest, when_we_push_plugin_init_is_called)
-{
-    MockPluginFilesystem mockfs;
-
-    using namespace testing;
-
-    EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(HOME_PLUGINDIR), StrEq("dummy"))).
-	WillOnce(Return(true));
-
-    EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), EndsWith(PLUGINDIR), StrEq("dummy"))).
-	Times(AtMost(0));
-
-    EXPECT_CALL(mockfs, LoadPlugin(Ne((void*)0), Eq((void*)0), StrEq("dummy"))).
-	Times(AtMost(0));
-
-    EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(1);
-
-    MockVTable mockVtable;
-    EXPECT_CALL(mockVtable, init()).WillOnce(Return(true));
-
-    CompPlugin* cp = CompPlugin::load("dummy");
-
-    cp->vTable = &mockVtable;
-
-    CompPlugin::push(cp);
-    ASSERT_EQ(cp, CompPlugin::pop());
-    CompPlugin::unload(cp);
+TEST_F (PluginTest, when_we_load_plugin_init_load_is_called_without_null_pointer)
+{
+    using namespace testing;
+
+    EXPECT_CALL (mockfs, LoadPlugin (NotNull (),
+				     EndsWith(HOME_PLUGINDIR),
+				     StrEq("dummy")))
+	.Times (1)
+	.WillOnce (Return (true));
+
+    EXPECT_CALL(mockfs, UnloadPlugin(_)).Times(AtLeast (0));
+
+    CompPlugin* cp = CompPlugin::load("dummy");
+    CompPlugin::unload(cp);
+}
+
+TEST_F (PluginTest, when_we_push_plugin_init_is_called)
+{
+    using namespace testing;
+
+    EXPECT_CALL (mockfs, LoadPlugin (_, _, _)).Times (AtLeast (0));
+    EXPECT_CALL (mockfs, UnloadPlugin(_)).Times (AtLeast (0));
+
+    ON_CALL(mockfs, LoadPlugin (_, _, _)).
+	WillByDefault (Return (true));
+
+    MockVTable mockVtable;
+    EXPECT_CALL (mockVtable, markReadyToInstantiate ()).Times (AtLeast (0));
+    EXPECT_CALL (mockVtable, markNoFurtherInstantiation ())
+	.Times (AtLeast (0));
+    EXPECT_CALL (mockVtable, init ()).WillOnce (Return (true));
+
+    CompPlugin* cp = CompPlugin::load("dummy");
+
+    cp->vTable = &mockVtable;
+
+    CompPlugin::push(cp);
+    CompPlugin::pop ();
+    CompPlugin::unload(cp);
+}
+
+TEST_F (PluginTest, when_we_push_plugin_mark_ready_to_instantiate_is_called)
+{
+    using namespace testing;
+
+    EXPECT_CALL (mockfs, LoadPlugin (_, _, _)).Times (AtLeast (0));
+    EXPECT_CALL (mockfs, UnloadPlugin(_)).Times (AtLeast (0));
+
+    ON_CALL(mockfs, LoadPlugin (_, _, _)).
+	WillByDefault (Return (true));
+
+    MockVTable mockVtable;
+    EXPECT_CALL (mockVtable, init ())
+	.Times (AtLeast (0))
+	.WillOnce (Return (true));
+    EXPECT_CALL (mockVtable, markReadyToInstantiate ()).Times (1);
+    EXPECT_CALL (mockVtable, markNoFurtherInstantiation ())
+	.Times (AtLeast (0));
+
+    CompPlugin* cp = CompPlugin::load("dummy");
+
+    cp->vTable = &mockVtable;
+
+    CompPlugin::push(cp);
+    CompPlugin::pop ();
+    CompPlugin::unload(cp);
+}
+
+TEST_F (PluginTest, when_we_pop_plugin_mark_no_further_instantiation_is_called)
+{
+    using namespace testing;
+
+    EXPECT_CALL (mockfs, LoadPlugin (_, _, _)).Times (AtLeast (0));
+    EXPECT_CALL (mockfs, UnloadPlugin(_)).Times (AtLeast (0));
+
+    ON_CALL(mockfs, LoadPlugin (_, _, _)).
+	WillByDefault (Return (true));
+
+    MockVTable mockVtable;
+    EXPECT_CALL (mockVtable, init ())
+	.Times (AtLeast (0))
+	.WillOnce (Return (true));
+    EXPECT_CALL (mockVtable, markReadyToInstantiate ()).Times (AtLeast (0));
+
+    CompPlugin* cp = CompPlugin::load("dummy");
+
+    cp->vTable = &mockVtable;
+
+    CompPlugin::push(cp);
+
+    EXPECT_CALL (mockVtable, markNoFurtherInstantiation ())
+	.Times (1);
+
+    CompPlugin::pop ();
+    CompPlugin::unload(cp);
+}
+
+TEST_F (PluginTest, pop_returns_plugin_pointer)
+{
+    using namespace testing;
+
+    EXPECT_CALL (mockfs, LoadPlugin (_, _, _)).Times (AtLeast (0));
+    EXPECT_CALL (mockfs, UnloadPlugin(_)).Times (AtLeast (0));
+
+    ON_CALL(mockfs, LoadPlugin (_, _, _)).
+	WillByDefault (Return (true));
+
+    MockVTable mockVtable;
+    EXPECT_CALL (mockVtable, init ())
+	.Times (AtLeast (0))
+	.WillOnce (Return (true));
+    EXPECT_CALL (mockVtable, markReadyToInstantiate ())
+	.Times (AtLeast (0));
+    EXPECT_CALL (mockVtable, markNoFurtherInstantiation ())
+	.Times (AtLeast (0));
+
+    CompPlugin* cp = CompPlugin::load("dummy");
+
+    cp->vTable = &mockVtable;
+
+    CompPlugin::push(cp);
+    EXPECT_EQ (cp, CompPlugin::pop ());
+    CompPlugin::unload(cp);
+}
+
+TEST_F (PluginTest, unload_calls_unload_on_fs)
+{
+    using namespace testing;
+
+    EXPECT_CALL (mockfs, LoadPlugin (_, _, _)).Times (AtLeast (0));
+
+    ON_CALL(mockfs, LoadPlugin (_, _, _)).
+	WillByDefault (Return (true));
+
+    CompPlugin* cp = CompPlugin::load("dummy");
+
+    EXPECT_CALL (mockfs, UnloadPlugin (cp)).Times (1);
+
+    CompPlugin::unload(cp);
+}
+
+namespace
+{
+
+/* TODO (cleanup): Extract this into a separate library because it
+ * duplicates what's in test-pluginclasshandler */
+template <typename BaseType>
+class Base :
+    public PluginClassStorage
+{
+    public:
+
+	typedef BaseType Type;
+
+	Base ();
+	~Base ();
+
+	static unsigned int allocPluginClassIndex ();
+	static void freePluginClassIndex (unsigned int index);
+
+    private:
+
+	static PluginClassStorage::Indices indices;
+	static std::list <Base *>        bases;
+};
+
+template <typename BaseType>
+PluginClassStorage::Indices Base <BaseType>::indices;
+
+template <typename BaseType>
+std::list <Base <BaseType> * > Base <BaseType>::bases;
+
+template <typename BaseType>
+Base <BaseType>::Base () :
+    PluginClassStorage (indices)
+{
+    bases.push_back (this);
+}
+
+template <typename BaseType>
+Base <BaseType>::~Base ()
+{
+    bases.remove (this);
+}
+
+template <typename BaseType>
+unsigned int
+Base <BaseType>::allocPluginClassIndex ()
+{
+    unsigned int i = PluginClassStorage::allocatePluginClassIndex (indices);
+
+    foreach (Base *b, bases)
+    {
+	if (indices.size () != b->pluginClasses.size ())
+	    b->pluginClasses.resize (indices.size ());
+    }
+
+    return i;
+}
+
+template <typename BaseType>
+void
+Base <BaseType>::freePluginClassIndex (unsigned int index)
+{
+    PluginClassStorage::freePluginClassIndex (indices, index);
+
+    foreach (Base *b, bases)
+    {
+	if (indices.size () != b->pluginClasses.size ())
+	    b->pluginClasses.resize (indices.size ());
+    }
+}
+
+class BaseScreen :
+    public Base <BaseScreen>
+{
+};
+
+class BaseWindow :
+    public Base <BaseWindow>
+{
+};
+
+class DestructionVerifier
+{
+    public:
+
+	DestructionVerifier ();
+
+	virtual ~DestructionVerifier ();
+	MOCK_METHOD0 (destroy, void ());
+};
+
+DestructionVerifier::DestructionVerifier ()
+{
+    using namespace testing;
+
+    /* By default we don't care */
+    EXPECT_CALL (*this, destroy ()).Times (AtLeast (0));
+}
+
+DestructionVerifier::~DestructionVerifier ()
+{
+    destroy ();
+}
+
+class PluginScreen :
+    public PluginClassHandler <PluginScreen, BaseScreen>,
+    public DestructionVerifier
+{
+    public:
+
+	PluginScreen (BaseScreen *s);
+};
+
+class PluginWindow :
+    public PluginClassHandler <PluginWindow, BaseWindow>,
+    public DestructionVerifier
+{
+    public:
+
+	virtual ~PluginWindow () {}
+	PluginWindow (BaseWindow *s);
+};
+
+PluginScreen::PluginScreen (BaseScreen *s) :
+    PluginClassHandler (s)
+{
+}
+
+PluginWindow::PluginWindow (BaseWindow *w) :
+    PluginClassHandler (w)
+{
+}
+
+class MockScreenAndWindowVTable :
+    public CompPlugin::VTableForScreenAndWindow <PluginScreen, PluginWindow>
+{
+    public:
+
+	MOCK_METHOD0 (init, bool ());
+};
+
+template <class BasePluginType>
+class PluginClassIntegrationTest :
+    public PluginTest
+{
+    public:
+
+	PluginClassIntegrationTest ();
+	~PluginClassIntegrationTest ();
+
+	boost::shared_ptr <CompPlugin>
+	LoadPlugin (MockScreenAndWindowVTable &);
+
+	MockPluginFilesystem      mockfs;
+	ValueHolder               *valueHolder;
+};
+
+void
+PopAndUnloadPlugin (CompPlugin *p)
+{
+    ASSERT_EQ (p, CompPlugin::pop ());
+    CompPlugin::unload (p);
+}
+
+template <class BasePluginType>
+boost::shared_ptr <CompPlugin>
+PluginClassIntegrationTest <BasePluginType>::LoadPlugin (MockScreenAndWindowVTable &v)
+{
+    typedef PluginClassIntegrationTest <BasePluginType> TestParam;
+
+    using namespace testing;
+
+    EXPECT_CALL (TestParam::mockfs, LoadPlugin (_, _, _))
+	.Times (AtLeast (0))
+	.WillOnce (Return (true));
+    EXPECT_CALL (TestParam::mockfs, UnloadPlugin(_))
+	.Times (AtLeast (0));
+
+    /* Load a plugin, we'll assign the vtable later */
+    CompPlugin *cp = CompPlugin::load("dummy");
+    cp->vTable = &v;
+
+    EXPECT_CALL (v, init ())
+	.Times (AtLeast (0))
+	.WillOnce (Return (true));
+
+    CompPlugin::push(cp);
+
+    return AutoDestroy (cp,
+			PopAndUnloadPlugin);
+}
+
+template <class BasePluginType>
+PluginClassIntegrationTest <BasePluginType>::PluginClassIntegrationTest ()
+{
+    valueHolder = new ValueHolder ();
+    ValueHolder::SetDefault (valueHolder);
+}
+
+template <class BasePluginType>
+PluginClassIntegrationTest <BasePluginType>::~PluginClassIntegrationTest ()
+{
+    delete valueHolder;
+    ValueHolder::SetDefault (NULL);
+}
+
+template <typename BaseType, typename PluginType>
+class BasePluginTemplate
+{
+    public:
+
+	typedef BaseType Base;
+	typedef PluginType Plugin;
+};
+
+typedef BasePluginTemplate <BaseScreen, PluginScreen> BasePluginScreen;
+typedef BasePluginTemplate <BaseWindow, PluginWindow> BasePluginWindow;
+
+}
+
+/* TODO: Extract actual interfaces out of CompScreen
+ * and CompWindow that can be passed to the vTables without
+ * using a link-seam like this one */
+class CompScreen :
+    public BaseScreen
+{
+};
+
+class CompWindow :
+    public BaseWindow
+{
+};
+
+typedef ::testing::Types <BasePluginScreen, BasePluginWindow> PluginTypes;
+TYPED_TEST_CASE (PluginClassIntegrationTest, PluginTypes);
+
+TYPED_TEST (PluginClassIntegrationTest, get_plugin_structure_null_on_not_loaded)
+{
+    using namespace testing;
+
+    /* Can't figure out how to get this out of TestParam::base at the moment */
+    typename TypeParam::Base base;
+    typename TypeParam::Plugin *p = TypeParam::Plugin::get (&base);
+
+    EXPECT_THAT (p, IsNull ());
+}
+
+TYPED_TEST (PluginClassIntegrationTest, get_plugin_structure_nonnull_on_loaded)
+{
+    using namespace testing;
+
+    typedef PluginClassIntegrationTest <TypeParam> TestParam;
+
+    MockScreenAndWindowVTable vTable;
+    boost::shared_ptr <CompPlugin> cp (TestParam::LoadPlugin (vTable));
+
+    typename TypeParam::Base base;
+    typedef boost::shared_ptr <typename TypeParam::Plugin> PluginPtr;
+
+    /* Because CompScreen is not available, we just need to delete
+     * the plugin structure ourselves */
+    PluginPtr p (TypeParam::Plugin::get (&base));
+
+    EXPECT_THAT (p.get (), NotNull ());
+}
+
+TYPED_TEST (PluginClassIntegrationTest, plugin_class_destroyed_when_vtable_is)
+{
+    using namespace testing;
+
+    typedef PluginClassIntegrationTest <TypeParam> TestParam;
+
+    MockScreenAndWindowVTable vTable;
+    boost::shared_ptr <CompPlugin> cp (TestParam::LoadPlugin (vTable));
+
+    typename TypeParam::Base base;
+    typedef boost::shared_ptr <typename TypeParam::Plugin> PluginPtr;
+
+    /* Because CompScreen is not available, we just need to delete
+     * the plugin structure ourselves */
+    PluginPtr p (TypeParam::Plugin::get (&base));
+
+    EXPECT_CALL (*p, destroy ()).Times (1);
 }

=== modified file 'src/pluginclasshandler/include/core/pluginclasshandler.h'
--- src/pluginclasshandler/include/core/pluginclasshandler.h	2013-02-26 11:56:10 +0000
+++ src/pluginclasshandler/include/core/pluginclasshandler.h	2013-05-28 23:39:25 +0000
@@ -45,9 +45,50 @@
  */
 extern unsigned int pluginClassHandlerIndex;
 
+namespace compiz
+{
+namespace plugin
+{
+namespace internal
+{
+class PluginKey;
+/**
+ * LoadedPluginClassBridge
+ *
+ * This template essentially exists so that we can reduce the
+ * scope of functions which are allowed to mark plugin classes
+ * as instantiatable as we can't really inject the interface
+ * from PluginClassHandler to do that anywhere. We also can't
+ * forward declare a nested class declaration, but we can forward
+ * declare PluginKey, which users should inherit from and take
+ * it by reference. If the class we're depending on can only be
+ * defined in one other place along with a private constructor
+ * accessible only to a friend it means that we've effectively
+ * limited the scope of users of this class.
+ */
+template <class Tp, class Tb, int ABI = 0>
+class LoadedPluginClassBridge
+{
+    public:
+
+	static void
+	allowInstantiations (const PluginKey &);
+
+	static void
+	disallowInstantiations (const PluginKey &);
+};
+}
+}
+}
+
 template<class Tp, class Tb, int ABI = 0>
-class PluginClassHandler {
+class PluginClassHandler
+{
     public:
+
+	typedef Tp ClassPluginType;
+	typedef Tb ClassPluginBaseType;
+
 	PluginClassHandler (Tb *);
 	~PluginClassHandler ();
 
@@ -90,16 +131,49 @@
 	static bool initializeIndex (Tb *base);
 	static inline Tp * getInstance (Tb *base);
 
+	static void allowInstantiations ();
+	static void disallowInstantiations ();
+
+	template <class Plugin, class Base, int PluginABI>
+	friend class compiz::plugin::internal::LoadedPluginClassBridge;
+
     private:
 	bool mFailed;
 	Tb   *mBase;
 
 	static PluginClassIndex mIndex;
+	static bool             mPluginLoaded;
 };
 
+namespace compiz
+{
+namespace plugin
+{
+namespace internal
+{
+template <class Tp, class Tb, int ABI>
+void
+LoadedPluginClassBridge <Tp, Tb, ABI>::allowInstantiations (const PluginKey &)
+{
+    PluginClassHandler <Tp, Tb, ABI>::allowInstantiations ();
+}
+
+template <class Tp, class Tb, int ABI>
+void
+LoadedPluginClassBridge <Tp, Tb, ABI>::disallowInstantiations (const PluginKey &)
+{
+    PluginClassHandler <Tp, Tb, ABI>::disallowInstantiations ();
+}
+}
+}
+}
+
 template<class Tp, class Tb, int ABI>
 PluginClassIndex PluginClassHandler<Tp,Tb,ABI>::mIndex;
 
+template<class Tp, class Tb, int ABI>
+bool PluginClassHandler<Tp,Tb,ABI>::mPluginLoaded = false;
+
 /**
  * Attaches a unique instance of the specified plugin class to a
  * unique instance of a specified base class
@@ -253,15 +327,23 @@
 Tp *
 PluginClassHandler<Tp,Tb,ABI>::get (Tb *base)
 {
+    /* Never instantiate an instance of this class
+     * if the relevant plugin has not been loaded
+     */
+    if (!mPluginLoaded)
+	return NULL;
+
     /* Always ensure that the index is initialized before
      * calls to ::get */
     if (!mIndex.initiated)
 	initializeIndex (base);
+
     /* If pluginClassHandlerIndex == mIndex.pcIndex it means that our
      * mIndex.index is fresh and can be used directly without needing
      * to fetch it from ValueHolder */
     if (mIndex.initiated && pluginClassHandlerIndex == mIndex.pcIndex)
 	return getInstance (base);
+
     /* If allocating or getting the updated index failed at any point
      * then just return NULL we don't know where our private data is stored */
     if (mIndex.failed && pluginClassHandlerIndex == mIndex.pcIndex)
@@ -285,4 +367,18 @@
     }
 }
 
+template<class Tp, class Tb, int ABI>
+void
+PluginClassHandler<Tp, Tb, ABI>::allowInstantiations ()
+{
+    mPluginLoaded = true;
+}
+
+template<class Tp, class Tb, int ABI>
+void
+PluginClassHandler<Tp, Tb, ABI>::disallowInstantiations ()
+{
+    mPluginLoaded = false;
+}
+
 #endif

=== modified file 'src/pluginclasshandler/tests/construct/src/test-pch-construct.cpp'
--- src/pluginclasshandler/tests/construct/src/test-pch-construct.cpp	2012-01-12 06:48:58 +0000
+++ src/pluginclasshandler/tests/construct/src/test-pch-construct.cpp	2013-05-28 23:39:25 +0000
@@ -1,23 +1,46 @@
 #include <test-pluginclasshandler.h>
 
-class ConstructPlugin: public Plugin, public PluginClassHandler<ConstructPlugin,
-	Base>
-{
-public:
-    ConstructPlugin (Base * base) :
-	    Plugin(base), PluginClassHandler<ConstructPlugin, Base>(base)
-    {
-    }
-};
-
-TEST_F( CompizPCHTest, TestConstruct )
+class BuildPlugin:
+    public Plugin,
+    public PluginClassHandler <BuildPlugin, Base>
+{
+    public:
+	BuildPlugin (Base *base) :
+	    Plugin(base),
+	    PluginClassHandler <BuildPlugin, Base> (base)
+	{
+	}
+};
+
+class PluginClassHandlerConstruction :
+    public CompizPCHTest
+{
+    public:
+
+	PluginClassHandlerConstruction ();
+	~PluginClassHandlerConstruction ();
+};
+
+PluginClassHandlerConstruction::PluginClassHandlerConstruction ()
+{
+    namespace cpi = compiz::plugin::internal;
+    cpi::LoadedPluginClassBridge <BuildPlugin, Base>::allowInstantiations (key);
+}
+
+PluginClassHandlerConstruction::~PluginClassHandlerConstruction ()
+{
+    namespace cpi = compiz::plugin::internal;
+    cpi::LoadedPluginClassBridge <BuildPlugin, Base>::disallowInstantiations (key);
+}
+
+TEST_F (PluginClassHandlerConstruction, TestConstruction)
 {
     Plugin *p;
 
     bases.push_back(new Base());
-    plugins.push_back(static_cast<Plugin *>(new ConstructPlugin(bases.back())));
+    plugins.push_back(static_cast<Plugin *>(new BuildPlugin(bases.back())));
     bases.push_back(new Base());
-    plugins.push_back(static_cast<Plugin *>(new ConstructPlugin(bases.back())));
+    plugins.push_back(static_cast<Plugin *>(new BuildPlugin(bases.back())));
 
     if (bases.front()->pluginClasses.size() != globalPluginClassIndices.size())
     {
@@ -26,20 +49,20 @@
     }
 
     if (!ValueHolder::Default()->hasValue(
-	    compPrintf("%s_index_%lu", typeid(ConstructPlugin).name(), 0)))
+	    compPrintf("%s_index_%lu", typeid(BuildPlugin).name(), 0)))
     {
 	FAIL() << "ValueHolder does not have value "
-		<< compPrintf("%s_index_%lu", typeid(ConstructPlugin).name(), 0);
+		<< compPrintf("%s_index_%lu", typeid(BuildPlugin).name(), 0);
     }
 
-    p = ConstructPlugin::get(bases.front());
+    p = BuildPlugin::get(bases.front());
 
     if (p != plugins.front())
     {
 	FAIL() << "Returned Plugin * is not plugins.front ()";
     }
 
-    p = ConstructPlugin::get(bases.back());
+    p = BuildPlugin::get(bases.back());
 
     if (p != plugins.back())
     {

=== modified file 'src/pluginclasshandler/tests/get/src/test-pch-get.cpp'
--- src/pluginclasshandler/tests/get/src/test-pch-get.cpp	2012-01-12 06:48:58 +0000
+++ src/pluginclasshandler/tests/get/src/test-pch-get.cpp	2013-05-28 23:39:25 +0000
@@ -1,5 +1,9 @@
 #include <test-pluginclasshandler.h>
 
+namespace cpi = compiz::plugin::internal;
+
+using ::testing::IsNull;
+
 class GetPlugin :
     public Plugin,
     public PluginClassHandler <GetPlugin, Base>
@@ -14,7 +18,26 @@
 {
 }
 
-TEST_F( CompizPCHTest, TestGet)
+class PluginClassHandlerGet :
+    public CompizPCHTest
+{
+    public:
+
+	PluginClassHandlerGet ();
+	~PluginClassHandlerGet ();
+};
+
+PluginClassHandlerGet::PluginClassHandlerGet ()
+{
+    cpi::LoadedPluginClassBridge <GetPlugin, Base>::allowInstantiations (key);
+}
+
+PluginClassHandlerGet::~PluginClassHandlerGet ()
+{
+    cpi::LoadedPluginClassBridge <GetPlugin, Base>::disallowInstantiations (key);
+}
+
+TEST_F (PluginClassHandlerGet, TestGet)
 {
     Plugin *p;
 
@@ -49,3 +72,17 @@
 	FAIL() << "Returned Plugin * is not the plugin for bases.back ()";
     }
 }
+
+TEST_F (PluginClassHandlerGet, TestGetNoInstantiationsAllowed)
+{
+    cpi::LoadedPluginClassBridge <GetPlugin, Base>::disallowInstantiations (key);
+
+    Plugin *p;
+
+    bases.push_back (new Base ());
+    plugins.push_back (new GetPlugin (bases.back ()));
+
+    p = GetPlugin::get (bases.front ());
+
+    EXPECT_THAT (p, IsNull ());
+}

=== modified file 'src/pluginclasshandler/tests/test-pluginclasshandler.h'
--- src/pluginclasshandler/tests/test-pluginclasshandler.h	2012-01-19 18:12:31 +0000
+++ src/pluginclasshandler/tests/test-pluginclasshandler.h	2013-05-28 23:39:25 +0000
@@ -2,6 +2,7 @@
 #include <core/pluginclasses.h>
 
 #include <gtest/gtest.h>
+#include <gmock/gmock.h>
 
 #include <list>
 
@@ -44,6 +45,21 @@
 	Base *b;
 };
 
+namespace compiz
+{
+namespace plugin
+{
+namespace internal
+{
+/* The version available in the tests is
+ * readily constructed */
+class PluginKey
+{
+};
+}
+}
+}
+
 class CompizPCHTest : public ::testing::Test
 {
 public:
@@ -54,4 +70,6 @@
      Global *global;
      std::list <Base *> bases;
      std::list <Plugin *> plugins;
+
+     compiz::plugin::internal::PluginKey key;
 };

=== modified file 'src/privatescreen/tests/test-privatescreen.cpp'
--- src/privatescreen/tests/test-privatescreen.cpp	2012-12-30 11:13:36 +0000
+++ src/privatescreen/tests/test-privatescreen.cpp	2013-05-28 23:39:25 +0000
@@ -249,23 +249,23 @@
 
 namespace {
 
-class MockVTable: public CompPlugin::VTable {
+class MockVTable:
+    public CompPlugin::VTable
+{
 public:
     MockVTable (CompString const& name) { initVTable (name); }
 
     MOCK_METHOD0(init, bool ());
     MOCK_METHOD0(fini, void ());
 
+    MOCK_METHOD0(markReadyToInstantiate, void ());
+    MOCK_METHOD0(markNoFurtherInstantiation, void ());
+
     MOCK_METHOD1(initScreen, bool (CompScreen *s));
-
     MOCK_METHOD1(finiScreen, void (CompScreen *s));
-
     MOCK_METHOD1(initWindow, bool (CompWindow *w));
-
     MOCK_METHOD1(finiWindow, void (CompWindow *w));
-
     MOCK_METHOD0(getOptions, CompOption::Vector & ());
-
     MOCK_METHOD2(setOption, bool (const CompString  &name, CompOption::Value &value));
 };
 
@@ -285,7 +285,8 @@
     virtual ~PluginFilesystem() {}
 };
 
-class MockPluginFilesystem : public PluginFilesystem
+class MockPluginFilesystem :
+    public PluginFilesystem
 {
 public:
     MockVTable mockVtableOne;