Merge lp:~alan-griffiths/compiz-core/private_screen-GrabList into lp:compiz-core

[Alan Griffiths]

Encapsulate grabs - the list of Grab*

[Daniel van Vugt (vanvugt)]

review: Approve

=== modified file 'include/core/screen.h'
--- include/core/screen.h	2012-03-05 09:04:16 +0000
+++ include/core/screen.h	2012-03-08 17:50:24 +0000
@@ -141,6 +141,10 @@
 
 };
 
+namespace compiz { namespace private_screen {
+    class Grab;
+}}
+
 class CompScreen :
     public WrapableHandler<ScreenInterface, 18>,
     public PluginClassStorage, // TODO should be an interface here
@@ -148,7 +152,7 @@
     public CompOption::Class   // TODO should be an interface here
 {
 public:
-    typedef void* GrabHandle;
+    typedef compiz::private_screen::Grab* GrabHandle;
 
     WRAPABLE_HND (0, ScreenInterface, void, fileWatchAdded, CompFileWatch *)
     WRAPABLE_HND (1, ScreenInterface, void, fileWatchRemoved, CompFileWatch *)
 
=== modified file 'src/event.cpp'
--- src/event.cpp	2012-03-08 08:06:02 +0000
+++ src/event.cpp	2012-03-08 17:50:24 +0000
@@ -141,7 +141,7 @@
 {
     bool actionEventHandled = false;
 
-    if (state == CompAction::StateInitKey && grabs.empty ())
+    if (state == CompAction::StateInitKey && grabsEmpty ())
     {
         if (grabbed)
         {
@@ -219,7 +219,7 @@
 
 	if (event->window != edgeWindow)
 	{
-	    if (grabs.empty () || event->window != root)
+	    if (grabsEmpty () || event->window != root)
 		return false;
 	}
 
@@ -1079,7 +1079,7 @@
 	XAllowEvents (priv->dpy, mode, event->xkey.time);
     }
 
-    if (priv->grabs.empty () && event->type == KeyPress)
+    if (priv->grabsEmpty () && event->type == KeyPress)
     {
 	XUngrabKeyboard (priv->dpy, event->xkey.time);
     }
@@ -1120,7 +1120,7 @@
     priv->eventHandled = priv->handleActionEvent (event);
     if (priv->eventHandled)
     {
-	if (priv->grabs.empty ())
+	if (priv->grabsEmpty ())
 	    XAllowEvents (priv->dpy, AsyncPointer, event->xbutton.time);
 	return;
     }
@@ -1407,7 +1407,7 @@
 	    }
 	}
 
-	if (priv->grabs.empty ())
+	if (priv->grabsEmpty ())
 	    XAllowEvents (priv->dpy, ReplayPointer, event->xbutton.time);
 
 	break;
@@ -2102,7 +2102,7 @@
 	    priv->below = w->id ();
 
 	    if (!priv->optionGetClickToFocus () &&
-		priv->grabs.empty ()                                 &&
+		priv->grabsEmpty ()                                 &&
 		event->xcrossing.mode   != NotifyGrab                &&
 		event->xcrossing.detail != NotifyInferior)
 	    {
 
=== modified file 'src/privatescreen.h'
--- src/privatescreen.h	2012-03-07 15:25:23 +0000
+++ src/privatescreen.h	2012-03-08 17:50:24 +0000
@@ -577,9 +577,32 @@
 	Time  tapStart;
 };
 
+class GrabList
+{
+    // TODO: std::list<Grab *> is almost certainly the wrong data
+    // structure. Probably better as std::vector<Grab> - many fewer
+    // memory allocations and releases.
+    typedef std::list<Grab *> GrabPtrList;
+
+public:
+    typedef GrabPtrList::iterator GrabIterator;
+
+    bool grabsEmpty() const { return grabs.empty(); }
+    void grabsPush(Grab* grab) { grabs.push_back (grab); }
+    GrabIterator grabsBegin() { return grabs.begin(); }
+    GrabIterator grabsEnd() { return grabs.end(); }
+    void grabsRemove(Grab* grab);
+    bool grabExist (const char *grab);
+    Grab* grabsBack() { return grabs.back (); }
+
+private:
+    GrabPtrList grabs;
+};
+
 class EventManager :
     public PluginManager,
     public ValueHolder,
+    public GrabList,
     public virtual ScreenUser
 {
 public:
@@ -641,11 +664,8 @@
 	int          count;
 };
 
-class Grab {
-    public:
-
-	friend class ::CompScreenImpl;
-    private:
+struct Grab {
+	Grab(Cursor cursor, const char *name) : cursor(cursor), name(name) {}
 	Cursor     cursor;
 	const char *name;
 };
@@ -659,7 +679,6 @@
     Window	edgeWindow;
     Window	xdndWindow;
     bool 	eventHandled;
-    std::list<Grab *> grabs;
     bool	grabbed;   /* true once we recieve a GrabNotify
 			      on FocusOut and false on
 			      UngrabNotify from FocusIn */
 
=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-03-08 10:37:58 +0000
+++ src/screen.cpp	2012-03-08 17:50:24 +0000
@@ -2490,7 +2490,7 @@
 
 bool CompScreenImpl::grabsEmpty() const
 {
-    return priv->grabs.empty();
+    return priv->grabsEmpty();
 }
 
 void
@@ -2929,7 +2929,7 @@
 CompScreenImpl::GrabHandle
 CompScreenImpl::pushGrab (Cursor cursor, const char *name)
 {
-    if (priv->grabs.empty ())
+    if (priv->grabsEmpty ())
     {
 	int status;
 
@@ -2960,11 +2960,8 @@
 				  cursor, CurrentTime);
     }
 
-    cps::Grab *grab = new cps::Grab ();
-    grab->cursor = cursor;
-    grab->name   = name;
-
-    priv->grabs.push_back (grab);
+    cps::Grab *grab = new cps::Grab (cursor, name);
+    priv->grabsPush (grab);
 
     return grab;
 }
@@ -2988,20 +2985,13 @@
     if (!handle)
 	return;
 
-    std::list<cps::Grab *>::iterator it;
-
-    it = std::find (priv->grabs.begin (), priv->grabs.end (), handle);
-
-    if (it != priv->grabs.end ())
-    {
-	priv->grabs.erase (it);
-	delete (static_cast<cps::Grab *> (handle));
-    }
-    if (!priv->grabs.empty ())
+    priv-> grabsRemove(handle);
+
+    if (!priv->grabsEmpty ())
     {
 	XChangeActivePointerGrab (priv->dpy,
 				  POINTER_GRAB_MASK,
-				  priv->grabs.back ()->cursor,
+				  priv->grabsBack ()->cursor,
 				  CurrentTime);
     }
     else
@@ -3015,6 +3005,20 @@
     }
 }
 
+void
+cps::GrabList::grabsRemove(Grab* handle)
+{
+    PrivateScreen::GrabIterator it;
+
+    it = std::find (grabsBegin (), grabsEnd (), handle);
+
+    if (it != grabsEnd ())
+    {
+	grabs.erase (it);
+	delete (handle);
+    }
+}
+
 /* otherScreenGrabExist takes a series of strings terminated by a NULL.
    It returns true if a grab exists but it is NOT held by one of the
    plugins listed, returns false otherwise. */
@@ -3027,7 +3031,7 @@
 
     std::list<cps::Grab *>::iterator it;
 
-    for (it = priv->grabs.begin (); it != priv->grabs.end (); it++)
+    for (it = priv->grabsBegin (); it != priv->grabsEnd (); it++)
     {
 	va_start (ap, first);
 
@@ -3052,7 +3056,13 @@
 bool
 CompScreenImpl::grabExist (const char *grab)
 {
-    foreach (cps::Grab* g, priv->grabs)
+    return priv->grabExist (grab);
+}
+
+bool
+cps::GrabList::grabExist (const char *grab)
+{
+    foreach (cps::Grab* g, grabs)
     {
 	if (strcmp (g->name, grab) == 0)
 	    return true;
@@ -5060,7 +5070,6 @@
     edgeWindow (None),
     xdndWindow (None),
     eventHandled (false),
-    grabs (),
     grabbed (false)
 {
 }