Merge lp:~smspillaz/compiz-core/compiz-core.decor_928173 into lp:compiz-core

[Sam Spilsbury]

Be a little smarter about updating decorations

Be a little smarter about updating decorations (bug 928173)

[Sam Spilsbury (smspillaz)]

I can't say for sure whether or not this will fix bug 928173 but it will certainly help

[Alan Griffiths (alan-griffiths)]

There's a lot of shared_(ptr|array) where scoped_(ptr|array) would be more appropriate. E.g. "boost::shared_array <decor_quad_t> quad" - scoped_array would be more appropriate.

"throw std::exception ();" - this doesn't give the client code much chance of identifying the problem.

There's inconsistent use of "Decoration::Ptr (new ...)" and "Decoration::Ptr = Decoration::create (...)" - have you come across make_shared<>()? (Part of boost and C++11)

[Sam Spilsbury (smspillaz)]

> There's a lot of shared_(ptr|array) where scoped_(ptr|array) would be more
> appropriate. E.g. "boost::shared_array <decor_quad_t> quad" - scoped_array
> would be more appropriate.
>

scoped_* is noncopyable, and I need it to be copyable so that I can store it in DecorWindow

> "throw std::exception ();" - this doesn't give the client code much chance of
> identifying the problem.

There are no users except internally and the users of that code only need to know if creating the decoration failed for /some/ reason, eg pixmap binding failed or whatever.

>
> There's inconsistent use of "Decoration::Ptr (new ...)" and "Decoration::Ptr =
> Decoration::create (...)" - have you come across make_shared<>()? (Part of
> boost and C++11)

I agree that the combination of Decoration::Ptr (new) and Decoration::create (...) is awkward - I would have made the constructor protected in favor of the named constructor, however the default "window" type decoration is just initialized from some property data. Maybe a named constructor there (eg defaultWindow () defaultPixmap () would be useful ?)

[Daniel van Vugt (vanvugt)]

Nice. It works and bug 928173 is no more. Two stylistic comments though...

1. Shouldn't these:
    compiz::window::extents::Extents::Extents () {}

    compiz::window::extents::Extents::Extents (int left, int right, int top, int bottom) :
    ...
instead be written as:
    namespace compiz::window::extents {

    Extents::Extents () {}

    Extents::Extents (int left, int right, int top, int bottom) :
    ...
    } // namespace compiz::window::extents

2. Unnecessary namespaces: Shouldn't "compiz::window::extents" simply be "compiz::window::extents"? Or even just "compiz"? Namespaces should really only change across different build targets. So that would mean core uses "compiz" and plugin Foo uses "compiz::Foo". Though, this point is purely opinion.

review: Approve

[Daniel van Vugt (vanvugt)]

If only the prerequisite branch didn't still cause a regression, I would merge this.

[Sam Spilsbury (smspillaz)]

On Thu, 9 Feb 2012, Daniel van Vugt wrote:

> Review: Approve
>
> Nice. It works and bug 928173 is no more. Two stylistic comments though...
>

Good to hear.

> 1. Shouldn't these:
> compiz::window::extents::Extents::Extents () {}
>
> compiz::window::extents::Extents::Extents (int left, int right, int top, int bottom) :
> ...
> instead be written as:
> namespace compiz::window::extents {
>
> Extents::Extents () {}
>
> Extents::Extents (int left, int right, int top, int bottom) :
> ...
> } // namespace compiz::window::extents
>
> 2. Unnecessary namespaces: Shouldn't "compiz::window::extents" simply be "compiz::window::extents"? Or even just "compiz"? Namespaces should really only change across different build targets. So that would mean core uses "compiz" and plugin Foo uses "compiz::Foo". Though, this point is purely opinion.
> --
> https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.decor_928173/+merge/91986
> You are the owner of lp:~smspillaz/compiz-core/compiz-core.decor_928173.
>

[Daniel van Vugt (vanvugt)]

Also, as we have discussed before, memcmp on structures is usually unsafe:

70 + if (memcmp (&cFrame, frame, sizeof (decor_extents_t)) ||
71 + memcmp (&cBorder, border, sizeof (decor_extents_t)) ||
72 + memcmp (&cMax_frame, max_frame, sizeof (decor_extents_t)) ||
73 + memcmp (&cMax_border, max_border, sizeof (decor_extents_t)))
74 + continue;

However in this case, I believe it will be safe with decor_extents_t on any architecture. Because 4 ints should always add up to a multiple of the struct alignment. There *should* be no padding bytes to worry about.

[Daniel van Vugt (vanvugt)]

> If only the prerequisite branch didn't still cause a regression, I would merge
> this.

I found that if I only cherry pick revisions 2984+2985 then the fix for bug 928173 continues to work without the regression introduced by the prerequisite branch. So I'll do that.

=== modified file 'include/decoration.h'
--- include/decoration.h	2011-05-08 13:44:09 +0000
+++ include/decoration.h	2012-02-08 07:32:18 +0000
@@ -267,6 +267,22 @@
 				decor_quad_t    *quad);
 
 int
+decor_match_pixmap (long		 *data,
+		    int		 size,
+		    Pixmap		 *pixmap,
+		    decor_extents_t  *frame,
+		    decor_extents_t  *border,
+		    decor_extents_t  *max_frame,
+		    decor_extents_t  *max_border,
+		    int min_width,
+		    int min_height,
+		    unsigned int frame_type,
+		    unsigned int frame_state,
+		    unsigned int frame_actions,
+		    decor_quad_t     *quad,
+		    unsigned int     n_quad);
+
+int
 decor_window_property (long	       *data,
 		       unsigned int    n,
 		       int	       size,

=== modified file 'libdecoration/decoration.c'
--- libdecoration/decoration.c	2012-01-17 05:19:03 +0000
+++ libdecoration/decoration.c	2012-02-08 07:32:18 +0000
@@ -369,6 +369,66 @@
     return n;
 }
 
+/* Returns n for a match, returns -1 for no match */
+
+int
+decor_match_pixmap (long		 *data,
+		    int		 size,
+		    Pixmap		 *pixmap,
+		    decor_extents_t  *frame,
+		    decor_extents_t  *border,
+		    decor_extents_t  *max_frame,
+		    decor_extents_t  *max_border,
+		    int		 min_width,
+		    int		 min_height,
+		    unsigned int     frame_type,
+		    unsigned int     frame_state,
+		    unsigned int     frame_actions,
+		    decor_quad_t     *quad,
+		    unsigned int     n_quad)
+{
+    int n = decor_property_get_num (data);
+    unsigned int i = 0;
+
+    for (; i < n; i++)
+    {
+	Pixmap cPixmap;
+	decor_extents_t cFrame, cBorder, cMax_frame, cMax_border;
+	int cMin_width, cMin_height;
+	unsigned int cFrame_type, cFrame_state, cFrame_actions, cNQuad;
+	decor_quad_t cQuad[N_QUADS_MAX];
+	cNQuad = decor_pixmap_property_to_quads (data, i, size, &cPixmap, &cFrame, &cBorder, &cMax_frame,
+						 &cMax_border, &cMin_width, &cMin_height, &cFrame_type,
+						 &cFrame_state, &cFrame_actions, cQuad);
+
+	if (cPixmap != *pixmap)
+	    continue;
+
+	if (memcmp (&cFrame, frame, sizeof (decor_extents_t)) ||
+	    memcmp (&cBorder, border, sizeof (decor_extents_t)) ||
+	    memcmp (&cMax_frame, max_frame, sizeof (decor_extents_t)) ||
+	    memcmp (&cMax_border, max_border, sizeof (decor_extents_t)))
+	    continue;
+
+	if (cFrame_type != frame_type ||
+	    cFrame_state != frame_state ||
+	    cFrame_actions != frame_actions ||
+	    cMin_width != min_width ||
+	    cMin_height != min_height)
+	    continue;
+
+	if (cNQuad != n_quad)
+	    continue;
+
+	if (memcmp (cQuad, quad, sizeof (decor_quad_t) * n_quad))
+	    continue;
+
+	return n;
+    }
+
+    return -1;
+}
+
 int
 decor_window_property (long	       *data,
 		       unsigned int    n,

=== modified file 'plugins/decor/src/decor.cpp'
--- plugins/decor/src/decor.cpp	2012-02-08 07:32:18 +0000
+++ plugins/decor/src/decor.cpp	2012-02-08 07:32:18 +0000
@@ -536,63 +536,48 @@
  *
  */
 
-Decoration *
+Decoration::Ptr
 Decoration::create (Window        id,
 		    long          *prop,
 		    unsigned int  size,
 		    unsigned int  type,
 		    unsigned int  nOffset)
 {
-    Decoration	    *decoration;
     unsigned int    frameType, frameState, frameActions;
     Pixmap	    pixmap = None;
     decor_extents_t border;
     decor_extents_t input;
     decor_extents_t maxBorder;
     decor_extents_t maxInput;
-    decor_quad_t    *quad = NULL;
-    int		    nQuad = 0;
     int		    minWidth;
     int		    minHeight;
-    int		    left, right, top, bottom;
-    int		    x1, y1, x2, y2;
+    int             nQuad = N_QUADS_MAX;
+    boost::shared_array <decor_quad_t> quad (new decor_quad_t[nQuad]);
 
     if (type ==  WINDOW_DECORATION_TYPE_PIXMAP &&
 	!DecorScreen::get (screen)->cmActive)
     {
-        compLogMessage ("decor", CompLogLevelWarn, "requested a pixmap type decoration when compositing isn't available");
-	return NULL;
+	compLogMessage ("decor", CompLogLevelWarn, "requested a pixmap type decoration when compositing isn't available");
+	throw std::exception ();
     }
 
     if (type == WINDOW_DECORATION_TYPE_PIXMAP)
     {
-        nQuad = N_QUADS_MAX;
-
-	quad = new decor_quad_t [nQuad];
-	if (!quad)
-	{
-	    compLogMessage ("decor", CompLogLevelWarn, "failed to allocate %i quads\n", nQuad);
-	    return NULL;
-	}
-
 	nQuad = decor_pixmap_property_to_quads (prop, nOffset, size, &pixmap, &input,
 						&border, &maxInput,
 						&maxBorder, &minWidth, &minHeight,
-						&frameType, &frameState, &frameActions, quad);
+						&frameType, &frameState, &frameActions, quad.get ());
 
 	if (!nQuad)
-	{
-	    delete [] quad;
-	    return NULL;
-	}
+	    throw std::exception ();
     }
     else if (type == WINDOW_DECORATION_TYPE_WINDOW)
     {
-        if (!decor_window_property (prop, nOffset, size, &input, &maxInput,
-                                    &minWidth, &minHeight, &frameType, &frameState, &frameActions))
+	if (!decor_window_property (prop, nOffset, size, &input, &maxInput,
+				    &minWidth, &minHeight, &frameType, &frameState, &frameActions))
 	{
 	    compLogMessage ("decor", CompLogLevelWarn, "malformed decoration - not a window");
-	    return NULL;
+	    throw std::exception ();
 	}
 
 	border = input;
@@ -600,35 +585,49 @@
     }
     else
     {
-        compLogMessage ("decor", CompLogLevelWarn, "malformed decoration - undetermined type");
-	return NULL;
-    }
-
-    decoration = new Decoration ();
-    if (!decoration)
-    {
-	delete [] quad;
-	return NULL;
-    }
-
-    if (pixmap)
-	decoration->texture = DecorScreen::get (screen)->getTexture (pixmap);
-    else
-	decoration->texture = NULL;
-
-    if (!decoration->texture && type == WINDOW_DECORATION_TYPE_PIXMAP)
+	compLogMessage ("decor", CompLogLevelWarn, "malformed decoration - undetermined type");
+	throw std::exception ();
+    }
+
+    return Decoration::Ptr (new Decoration (type, border, input, maxBorder, maxInput, frameType, frameState, frameActions, minWidth, minHeight, pixmap, quad, nQuad));
+}
+
+Decoration::Decoration (int   type,
+			const decor_extents_t &border,
+			const decor_extents_t &input,
+			const decor_extents_t &maxBorder,
+			const decor_extents_t &maxInput,
+			unsigned int frameType,
+			unsigned int frameState,
+			unsigned int frameActions,
+			unsigned int minWidth,
+			unsigned int minHeight,
+			Pixmap       pixmap,
+			const boost::shared_array <decor_quad_t> &quad,
+			unsigned int nQuad) :
+    texture (DecorScreen::get (screen)->getTexture (pixmap)),
+    border (border.left, border.right, border.top, border.bottom),
+    input (input.left, input.right, input.top, input.bottom),
+    maxBorder (maxBorder.left, maxBorder.right, maxBorder.top, maxBorder.bottom),
+    maxInput (maxInput.left, maxInput.right, maxInput.top, maxInput.bottom),
+    minWidth (minWidth),
+    minHeight (minHeight),
+    frameType (frameType),
+    frameState (frameState),
+    frameActions (frameActions),
+    quad (quad),
+    nQuad (nQuad),
+    type (type)
+{
+    int		    left, right, top, bottom;
+    int		    x1, y1, x2, y2;
+
+    if (!texture && type == WINDOW_DECORATION_TYPE_PIXMAP)
     {
         compLogMessage ("decor", CompLogLevelWarn, "failed to bind pixmap to texture");
-	delete decoration;
-	delete [] quad;
-	return NULL;
+	throw std::exception ();
     }
 
-    decoration->minWidth  = minWidth;
-    decoration->minHeight = minHeight;
-    decoration->quad	  = quad;
-    decoration->nQuad	  = nQuad;
-
     if (type == WINDOW_DECORATION_TYPE_PIXMAP)
     {
 	left   = 0;
@@ -636,9 +635,9 @@
 	top    = 0;
 	bottom = minHeight;
 
-	while (nQuad--)
+	for (unsigned int i = 0; i  < nQuad; i++)
 	{
-	    computeQuadBox (quad, minWidth, minHeight, &x1, &y1, &x2, &y2,
+	    computeQuadBox (&(quad[i]), minWidth, minHeight, &x1, &y1, &x2, &y2,
 			    NULL, NULL);
 
 	    if (x1 < left)
@@ -649,83 +648,34 @@
 		right = x2;
 	    if (y2 > bottom)
 		bottom = y2;
-
-	    quad++;
 	}
 
-	decoration->output.left   = -left;
-	decoration->output.right  = right - minWidth;
-	decoration->output.top    = -top;
-	decoration->output.bottom = bottom - minHeight;
+	this->output.left   = -left;
+	this->output.right  = right - minWidth;
+	this->output.top    = -top;
+	this->output.bottom = bottom - minHeight;
     }
     else
     {
-	decoration->output.left   = MAX (input.left, maxInput.left);
-	decoration->output.right  = MAX (input.right, maxInput.right);
-	decoration->output.top    = MAX (input.top, maxInput.top);
-	decoration->output.bottom = MAX (input.bottom, maxInput.bottom);
+	this->output.left   = MAX (input.left, maxInput.left);
+	this->output.right  = MAX (input.right, maxInput.right);
+	this->output.top    = MAX (input.top, maxInput.top);
+	this->output.bottom = MAX (input.bottom, maxInput.bottom);
     }
-
-    /* Extents of actual frame window */
-
-    decoration->input.left   = input.left;
-    decoration->input.right  = input.right;
-    decoration->input.top    = input.top;
-    decoration->input.bottom = input.bottom;
-
-    /* Border extents */
-
-    decoration->border.left   = border.left;
-    decoration->border.right  = border.right;
-    decoration->border.top    = border.top;
-    decoration->border.bottom = border.bottom;
-
-    /* Extents of actual frame window */
-
-    decoration->maxInput.left   = maxInput.left;
-    decoration->maxInput.right  = maxInput.right;
-    decoration->maxInput.top    = maxInput.top;
-    decoration->maxInput.bottom = maxInput.bottom;
-
-    /* Border extents */
-
-    decoration->maxBorder.left   = maxBorder.left;
-    decoration->maxBorder.right  = maxBorder.right;
-    decoration->maxBorder.top    = maxBorder.top;
-    decoration->maxBorder.bottom = maxBorder.bottom;
-
-    /* Decoration info */
-
-    decoration->frameType = frameType;
-    decoration->frameState = frameState;
-    decoration->frameActions = frameActions;
-
-    decoration->refCount = 1;
-    decoration->type = type;
-
-    return decoration;
 }
 
 /*
- * Decoration::release
+ * Decoration::~Decoration
  *
  * Unreferences the decoration, also unreferences the texture
  * if this decoration is about to be destroyed
  *
  */
 
-void
-Decoration::release (Decoration *decoration)
+Decoration::~Decoration ()
 {
-    decoration->refCount--;
-    if (decoration->refCount)
-	return;
-
-    if (decoration->texture)
-	DecorScreen::get (screen)->releaseTexture (decoration->texture);
-
-    delete [] decoration->quad;
-    delete decoration;
+    if (texture)
+	DecorScreen::get (screen)->releaseTexture (texture);
 }
 
 /*
@@ -772,11 +722,6 @@
     int		    result, format;
     unsigned int    type;
 
-    /* FIXME: Should really check the property to see
-     * if the decoration changed, and /then/ release
-     * and re-update it, but for now just releasing all */
-    mList.clear ();
-
     result = XGetWindowProperty (screen->dpy (), id,
                                  decorAtom, 0L,
                                  PROP_HEADER_SIZE + 6 * (BASE_PROP_SIZE +
@@ -818,21 +763,121 @@
 
     type = decor_property_get_type (prop);
 
+    std::list <Decoration::Ptr> remove;
+    std::list <int> skip;
+
+    /* only recreate decorations if they need to be updated */
+    foreach (const Decoration::Ptr &d, mList)
+    {
+	decor_extents_t input, border, maxInput, maxBorder;
+
+	input.left = d->input.left;
+	input.right = d->input.right;
+	input.top = d->input.top;
+	input.bottom = d->input.bottom;
+
+	border.left = d->border.left;
+	border.right = d->border.right;
+	border.top = d->border.top;
+	border.bottom = d->border.bottom;
+
+	maxInput.left = d->maxInput.left;
+	maxInput.right = d->maxInput.right;
+	maxInput.top = d->maxInput.top;
+	maxInput.bottom = d->maxInput.bottom;
+
+	maxBorder.left = d->maxBorder.left;
+	maxBorder.right = d->maxBorder.right;
+	maxBorder.top = d->maxBorder.top;
+	maxBorder.bottom = d->maxBorder.bottom;
+
+	int num = decor_match_pixmap (prop, n, &d->texture->pixmap, &input, &border, &maxInput, &maxBorder,
+				     d->minWidth, d->minHeight, d->frameType, d->frameState, d->frameActions,
+				    d->quad.get (), d->nQuad);
+	if (num != -1)
+	    skip.push_back (num);
+	else
+	    remove.push_back (d);
+    }
+
     /* Create a new decoration for each individual item on the property */
     for (int i = 0; i < decor_property_get_num (prop); i++)
     {
-        Decoration *d = Decoration::create (id, prop, n, type, i);
-
-	if (!d)
-        {
-            XFree (data);
-            mList.clear ();
-            return false;
-        }
-        else
-            mList.push_back (d);
+	if (std::find (skip.begin (), skip.end (), i) != skip.end ())
+	    continue;
+
+	try
+	{
+	    std::list <Decoration::Ptr>::iterator it = mList.begin ();
+	    Decoration::Ptr d = Decoration::create (id, prop, n, type, i);
+
+	    /* Try to replace an existing decoration */
+	    for (; it != mList.end (); it++)
+	    {
+		if ((*it)->frameType == d->frameType &&
+		    (*it)->frameState == d->frameState &&
+		    (*it)->frameActions == d->frameActions)
+		{
+		    remove.remove ((*it));
+		    (*it) = d;
+		    break;
+		}
+	    }
+
+	    if (it == mList.end ())
+		mList.push_back (d);
+	}
+	catch (...)
+	{
+	    /* Creating a new decoration failed ... see if we can use
+	     * the old one */
+
+	    unsigned int    frameType, frameState, frameActions;
+	    Pixmap	    pixmap = None;
+	    decor_extents_t border;
+	    decor_extents_t input;
+	    decor_extents_t maxBorder;
+	    decor_extents_t maxInput;
+	    int		    minWidth;
+	    int		    minHeight;
+	    int             ok = 0;
+	    boost::shared_array <decor_quad_t> quad (new decor_quad_t[N_QUADS_MAX]);
+
+	    if (type == WINDOW_DECORATION_TYPE_PIXMAP)
+	    {
+		ok = decor_pixmap_property_to_quads (prop, i, n, &pixmap, &input,
+							&border, &maxInput,
+							&maxBorder, &minWidth, &minHeight,
+							&frameType, &frameState, &frameActions, quad.get ());
+	    }
+	    else if (type == WINDOW_DECORATION_TYPE_WINDOW)
+	    {
+		ok = decor_window_property (prop, i, n, &input, &maxInput,
+					    &minWidth, &minHeight, &frameType, &frameState, &frameActions);
+	    }
+
+	    if (ok)
+	    {
+		std::list <Decoration::Ptr>::iterator it = mList.begin ();
+
+		/* Use an existing decoration */
+		for (; it != mList.end (); it++)
+		{
+		    if ((*it)->frameType == frameType &&
+			(*it)->frameState == frameState &&
+			(*it)->frameActions == frameActions)
+		    {
+			remove.remove ((*it));
+			break;
+		    }
+		}
+	    }
+	}
     }
 
+    foreach (const Decoration::Ptr &d, remove)
+	mList.remove (d);
+
     XFree (data);
 
     return true;
@@ -869,7 +914,7 @@
  *
  */
 WindowDecoration *
-WindowDecoration::create (Decoration *d)
+WindowDecoration::create (const Decoration::Ptr &d)
 {
     WindowDecoration *wd;
 
@@ -907,7 +952,6 @@
 void
 WindowDecoration::destroy (WindowDecoration *wd)
 {
-    Decoration::release (wd->decor);
     delete [] wd->quad;
     delete wd;
 }
@@ -1057,7 +1101,7 @@
  *
  */
 bool
-DecorWindow::checkSize (Decoration *decoration)
+DecorWindow::checkSize (const Decoration::Ptr &decoration)
 {
     return (decoration->minWidth <= (int) window->geometry ().width () &&
 	    decoration->minHeight <= (int) window->geometry ().height ());
@@ -1225,11 +1269,11 @@
  * match the locked property, then it is not matched
  *
  */
-Decoration *
+const Decoration::Ptr &
 DecorationList::findMatchingDecoration (CompWindow *w,
                                         bool       sizeCheck)
 {
-    Decoration *decoration = NULL;
+    std::list <Decoration::Ptr>::iterator cit = mList.end ();
     DECOR_WINDOW (w);
 
     if (mList.size ())
@@ -1240,17 +1284,22 @@
 
         unsigned int currentDecorState = 0;
 
-        decoration = (sizeCheck ? dw->checkSize (mList.front ()) : true) ? mList.front () : NULL;
+	if (sizeCheck)
+	    if (dw->checkSize (mList.front ()))
+		cit = mList.begin ();
 
-	foreach (Decoration *d, mList)
+	for (std::list <Decoration::Ptr>::iterator it = mList.begin ();
+	     it != mList.end (); it++)
 	{
+	    const Decoration::Ptr &d = *it;
+
 	    /* Must always match type */
 	    if (DecorWindow::matchType (w, d->frameType))
 	    {
 		/* Use this decoration if the type matched */
 		if (!(typeMatch & currentDecorState) && (!sizeCheck || dw->checkSize (d)))
 		{
-		    decoration = d;
+		    cit = it;
 		    currentDecorState |= typeMatch;
 		}
 
@@ -1260,7 +1309,7 @@
 		    /* Use this decoration if the type and state match */
 		    if (!(stateMatch & currentDecorState))
 		    {
-			decoration = d;
+			cit = it;
 			currentDecorState |= stateMatch;
 		    }
 
@@ -1270,7 +1319,7 @@
 			/* Use this decoration if the requested actions match */
 			if (!(actionsMatch & currentDecorState))
 			{
-			    decoration = d;
+			    cit = it;
 			    currentDecorState |= actionsMatch;
 
 			    /* Perfect match, no need to continue searching */
@@ -1282,7 +1331,10 @@
 	}
     }
 
-    return decoration;
+    if (cit == mList.end ())
+	throw std::exception ();
+
+    return *cit;
 }
 
 /*
@@ -1324,12 +1376,15 @@
 bool
 DecorWindow::update (bool allowDecoration)
 {
-    Decoration	     *old, *decoration = NULL;
+    Decoration::Ptr  old, decoration;
     bool	     decorate = false;
     bool	     shadowOnly = true;
     CompPoint        oldShift, movement;
 
-    old = (wd) ? wd->decor : NULL;
+    if (wd)
+	old = wd->decor;
+    else
+	old.reset ();
 
     /* Only want to decorate windows which have a frame or are in the process
      * of waiting for an animation to be unmapped (in which case we can give
@@ -1377,9 +1432,11 @@
     if (decorate || frameExtentsRequested)
     {
         /* Attempt to find a matching decoration */
-        decoration = decor.findMatchingDecoration (window, true);
-
-	if (!decoration)
+	try
+	{
+	    decoration = decor.findMatchingDecoration (window, true);
+	}
+	catch (...)
 	{
 	    /* Find an appropriate default decoration to use */
 	    if (dScreen->dmSupports & WINDOW_DECORATION_TYPE_PIXMAP &&
@@ -1387,13 +1444,18 @@
 		!(dScreen->dmSupports & WINDOW_DECORATION_TYPE_WINDOW &&
 		  pixmapFailed))
 	    {
-		decoration = dScreen->decor[DECOR_ACTIVE].findMatchingDecoration (window, false);
-
-		if (!decoration)
+		try
+		{
+		    decoration = dScreen->decor[DECOR_ACTIVE].findMatchingDecoration (window, false);
+		}
+		catch (...)
+		{
 		    compLogMessage ("decor", CompLogLevelWarn, "No default decoration found, placement will not be correct");
+		    decoration.reset ();
+		}
 	    }
 	    else if (dScreen->dmSupports & WINDOW_DECORATION_TYPE_WINDOW)
-		decoration = &dScreen->windowDefault;
+		decoration = dScreen->windowDefault;
 	}
 
 	/* Do not allow windows which are later undecorated
@@ -1413,7 +1475,7 @@
 	    if (decoration)
 	    {
 		if (!checkSize (decoration))
-		    decoration = NULL;
+		    decoration.reset ();
 	    }
 	}
     }
@@ -1423,7 +1485,7 @@
      * (nobody owns the selection window) 
      */
     if (!dScreen->dmWin || !allowDecoration)
-	decoration = NULL;
+	decoration.reset ();
 
     /* Don't bother going any further if
      * this window is going to get the same
@@ -2885,7 +2947,20 @@
     textures (),
     dmWin (None),
     dmSupports (0),
-    cmActive (false)
+    cmActive (false),
+    windowDefault (new Decoration (WINDOW_DECORATION_TYPE_WINDOW,
+				   decor_extents_t (),
+				   decor_extents_t (),
+				   decor_extents_t (),
+				   decor_extents_t (),
+				   0,
+				   0,
+				   0,
+				   0,
+				   0,
+				   None,
+				   boost::shared_array <decor_quad_t> (NULL),
+				   0))
 {
     supportingDmCheckAtom =
 	XInternAtom (s->dpy (), DECOR_SUPPORTING_DM_CHECK_ATOM_NAME, 0);
@@ -2914,24 +2989,6 @@
     shadowInfoAtom =
 	XInternAtom (s->dpy (), "_COMPIZ_NET_CM_SHADOW_PROPERTIES", 0);
 
-    windowDefault.texture   = NULL;
-    windowDefault.minWidth  = 0;
-    windowDefault.minHeight = 0;
-    windowDefault.quad      = NULL;
-    windowDefault.nQuad     = 0;
-    windowDefault.type      = WINDOW_DECORATION_TYPE_WINDOW;
-
-    windowDefault.input.left   = 0;
-    windowDefault.input.right  = 0;
-    windowDefault.input.top    = 1;
-    windowDefault.input.bottom = 0;
-
-    windowDefault.border = windowDefault.maxBorder =
-	windowDefault.maxInput = windowDefault.output =
-	    windowDefault.input;
-
-    windowDefault.refCount = 1;
-
     cmActive = (cScreen) ? cScreen->compositingActive () &&
                GLScreen::get (s) != NULL : false;
 

=== modified file 'plugins/decor/src/decor.h'
--- plugins/decor/src/decor.h	2012-01-21 19:05:23 +0000
+++ plugins/decor/src/decor.h	2012-02-08 07:32:18 +0000
@@ -24,6 +24,7 @@
  */
 
 #include <boost/shared_ptr.hpp>
+#include <boost/shared_array.hpp>
 #include <core/window.h>
 #include <core/pluginclasshandler.h>
 
@@ -68,12 +69,30 @@
 class Decoration {
 
     public:
-        static Decoration * create (Window        id,
-                                    long          *prop,
-                                    unsigned int  size,
-                                    unsigned int  type,
-                                    unsigned int  nOffset);
-	static void release (Decoration *);
+
+	typedef boost::shared_ptr <Decoration> Ptr;
+
+	static Decoration::Ptr create (Window        id,
+				       long          *prop,
+				       unsigned int  size,
+				       unsigned int  type,
+				       unsigned int  nOffset);
+
+	Decoration (int   type,
+		    const decor_extents_t &border,
+		    const decor_extents_t &input,
+		    const decor_extents_t &maxBorder,
+		    const decor_extents_t &maxInput,
+		    unsigned int frameType,
+		    unsigned int frameState,
+		    unsigned int frameActions,
+		    unsigned int minWidth,
+		    unsigned int minHeight,
+		    Pixmap       pixmap,
+		    const boost::shared_array <decor_quad_t> &quad,
+		    unsigned int nQuad);
+
+	~Decoration ();
 
     public:
 	int                       refCount;
@@ -88,7 +107,7 @@
 	unsigned int		  frameType;
 	unsigned int		  frameState;
 	unsigned int		  frameActions;
-	decor_quad_t              *quad;
+	boost::shared_array <decor_quad_t> quad;
 	int                       nQuad;
 	int                       type;
 };
@@ -97,16 +116,15 @@
 {
     public:
         bool updateDecoration  (Window id, Atom decorAtom);
-        Decoration *findMatchingDecoration (CompWindow *w, bool sizeCheck);
+	const Decoration::Ptr & findMatchingDecoration (CompWindow *w, bool sizeCheck);
         void clear ()
         {
-            foreach (Decoration *d, mList)
-                Decoration::release (d);
+	    mList.clear ();
         };
 
         DecorationList ();
 
-        std::vector <Decoration *> mList;
+	std::list <Decoration::Ptr> mList;
 };
 
 struct ScaledQuad {
@@ -118,11 +136,11 @@
 
 class WindowDecoration {
     public:
-	static WindowDecoration * create (Decoration *);
+	static WindowDecoration * create (const Decoration::Ptr &);
 	static void destroy (WindowDecoration *);
 
     public:
-        Decoration *decor;
+	Decoration::Ptr decor;
 	ScaledQuad *quad;
 	int	   nQuad;
 };
@@ -178,10 +196,10 @@
 	Window dmWin;
 	int    dmSupports;
 
+	bool cmActive;
+
 	DecorationList decor[DECOR_NUM];
-	Decoration     windowDefault;
-
-	bool cmActive;
+	Decoration::Ptr     windowDefault;
 
 	std::map<Window, DecorWindow *> frames;
 
@@ -226,7 +244,7 @@
 	void updateOutputFrame ();
 	void updateWindowRegions ();
 
-	bool checkSize (Decoration *decor);
+	bool checkSize (const Decoration::Ptr &decor);
 
 	int shiftX ();
 	int shiftY ();

=== modified file 'plugins/resize/src/resize.cpp'
--- plugins/resize/src/resize.cpp	2012-01-25 09:20:47 +0000
+++ plugins/resize/src/resize.cpp	2012-02-08 07:32:18 +0000
@@ -510,10 +510,7 @@
 		int top = screen->outputDevs ().at (tco).workArea ().top ();
 		int bottom = screen->outputDevs ().at (bco).workArea ().bottom ();
 
-		if (rs->grabWindowWorkArea)
-		    delete rs->grabWindowWorkArea;
-
-		rs->grabWindowWorkArea = new CompRect (0, 0, 0, 0);
+		rs->grabWindowWorkArea.reset (new CompRect (0, 0, 0, 0));
 		rs->grabWindowWorkArea->setLeft (left);
 		rs->grabWindowWorkArea->setRight (right);
 		rs->grabWindowWorkArea->setTop (top);
@@ -1767,7 +1764,7 @@
     releaseButton (0),
     isConstrained (false),
     offWorkAreaConstrained (true),
-    grabWindowWorkArea (NULL)
+    grabWindowWorkArea ()
 {
     CompOption::Vector atomTemplate;
     Display *dpy = s->dpy ();

=== modified file 'plugins/resize/src/resize.h'
--- plugins/resize/src/resize.h	2012-01-18 16:26:45 +0000
+++ plugins/resize/src/resize.h	2012-02-08 07:32:18 +0000
@@ -26,6 +26,7 @@
 #ifndef _RESIZE_H
 #define _RESIZE_H
 
+#include <boost/shared_ptr.hpp>
 #include <core/screen.h>
 #include <core/pluginclasshandler.h>
 #include <core/propertywriter.h>
@@ -148,7 +149,7 @@
 	CompSize   lastGoodSize;
 
 	bool		 offWorkAreaConstrained;
-	CompRect   *grabWindowWorkArea;
+	boost::shared_ptr <CompRect> grabWindowWorkArea;
 };
 
 class ResizeWindow :

=== modified file 'src/window/extents/include/core/windowextents.h'
--- src/window/extents/include/core/windowextents.h	2012-01-19 20:18:20 +0000
+++ src/window/extents/include/core/windowextents.h	2012-02-08 07:32:18 +0000
@@ -41,8 +41,13 @@
  * Specifies the left, right, top and bottom positions of a window's
  * geometry
  */
-struct Extents {
+class Extents
+{
     public:
+
+	Extents ();
+	Extents (int left, int right, int top, int bottom);
+
 	int left;
 	int right;
 	int top;

=== modified file 'src/window/extents/src/windowextents.cpp'
--- src/window/extents/src/windowextents.cpp	2012-01-19 20:18:20 +0000
+++ src/window/extents/src/windowextents.cpp	2012-02-08 07:32:18 +0000
@@ -65,6 +65,16 @@
     return rv;
 }
 
+compiz::window::extents::Extents::Extents () {}
+
+compiz::window::extents::Extents::Extents (int left, int right, int top, int bottom) :
+    left (left),
+    right (right),
+    top (top),
+    bottom (bottom)
+{
+}
+
 bool
 compiz::window::extents::Extents::operator== (const Extents &other)
 {