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

[Sam Spilsbury]

Don't trigger actions unless they are actually marked active. It is possible that we may have a passive button grab for other reasons than actions being bound, so actions which haven't been added should be ignored.

Don't trigger actions unless they are actually marked active. It is possible that we may have a passive button grab for other reasons than actions being bound, so actions which haven't been added should be ignored.

Tests added for refactored code:

 Start 107: privatescreen_ButtonPressEdgeEventManagementTest.IgnoreWhenEventAndRootWindowMismatch
107/548 Test #107: privatescreen_ButtonPressEdgeEventManagementTest.IgnoreWhenEventAndRootWindowMismatch ....................................................................... Passed 0.01 sec
        Start 108: privatescreen_ButtonPressEdgeEventManagementTest.IgnoreWhenEventMismatchAndNoGrabs
108/548 Test #108: privatescreen_ButtonPressEdgeEventManagementTest.IgnoreWhenEventMismatchAndNoGrabs .......................................................................... Passed 0.01 sec
        Start 109: privatescreen_ButtonPressEdgeEventManagementTest.AllowWhenEventButNotRootWindowMismatchWhileGrabbed
109/548 Test #109: privatescreen_ButtonPressEdgeEventManagementTest.AllowWhenEventButNotRootWindowMismatchWhileGrabbed ......................................................... Passed 0.01 sec
        Start 110: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnInvalidEdge
110/548 Test #110: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnInvalidEdge ..................................................................................... Passed 0.01 sec
        Start 111: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnUnboundAction
111/548 Test #111: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnUnboundAction ................................................................................... Passed 0.01 sec
        Start 112: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedEdgeMask
112/548 Test #112: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedEdgeMask .............................................................................. Passed 0.01 sec
        Start 113: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedButtonNumber
113/548 Test #113: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedButtonNumber .......................................................................... Passed 0.01 sec
        Start 114: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedButtonState
114/548 Test #114: privatescreen_ButtonPressEdgeEventManagementTest.NoTriggerOnMismatchedButtonState ........................................................................... Passed 0.01 sec
        Start 115: privatescreen_ButtonPressEdgeEventManagementTest.TriggerWhenStateButtonAndEdgeMaskMatch
115/548 Test #115: privatescreen_ButtonPressEdgeEventManagementTest.TriggerWhenStateButtonAndEdgeMaskMatch ..................................................................... Passed 0.01 sec
        Start 116: privatescreen_ButtonPressEventManagementTest.SetEventWindowArgument
116/548 Test #116: privatescreen_ButtonPressEventManagementTest.SetEventWindowArgument ......................................................................................... Passed 0.01 sec
        Start 117: privatescreen_ButtonPressEventManagementTest.NoTriggerOnUnboundAction
117/548 Test #117: privatescreen_ButtonPressEventManagementTest.NoTriggerOnUnboundAction ....................................................................................... Passed 0.01 sec
        Start 118: privatescreen_ButtonPressEventManagementTest.NoTriggerOnUnboundInactiveAction
118/548 Test #118: privatescreen_ButtonPressEventManagementTest.NoTriggerOnUnboundInactiveAction ............................................................................... Passed 0.01 sec
        Start 119: privatescreen_ButtonPressEventManagementTest.NoTriggerOnMismatchedButtonNumber
119/548 Test #119: privatescreen_ButtonPressEventManagementTest.NoTriggerOnMismatchedButtonNumber .............................................................................. Passed 0.01 sec
        Start 120: privatescreen_ButtonPressEventManagementTest.NoTriggerOnMismatchedButtonState
120/548 Test #120: privatescreen_ButtonPressEventManagementTest.NoTriggerOnMismatchedButtonState ............................................................................... Passed 0.01 sec
        Start 121: privatescreen_ButtonPressEventManagementTest.TriggerWhenStateAndButtonMatch
121/548 Test #121: privatescreen_ButtonPressEventManagementTest.TriggerWhenStateAndButtonMatch ................................................................................. Passed 0.01 sec

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:3390
http://jenkins.qa.ubuntu.com/job/compiz-ci/95/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/95/console

review: Needs Fixing (continuous-integration)

[Martin Mrazik (mrazik)]

This is the jenkins error:
[ 14%] Building CXX object src/CMakeFiles/compiz_core.dir/event.cpp.o
cd /tmp/buildd/compiz-0.9.8.2+bzr3377/obj-x86_64-linux-gnu/src && /usr/lib/ccache/c++ -Dcompiz_core_EXPORTS -DHAVE_SCANDIR_POSIX -DHAVE_SCANDIR_POSIX -DHAVE_SCANDIR_POSIX -DHAVE_CONFIG_H -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -fPIC -Wall -Wl,-zdefs -Wno-deprecated-declarations -Werror -fPIC -Wall -Wl,-zdefs -Wno-deprecated-declarations -Werror -fPIC -Wall -Wl,-zdefs -Wno-deprecated-declarations -Werror -O2 -g -fPIC -I/tmp/buildd/compiz-0.9.8.2+bzr3377/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/obj-x86_64-linux-gnu -I/tmp/buildd/compiz-0.9.8.2+bzr3377/obj-x86_64-linux-gnu/generated -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/glibmm-2.4 -I/usr/lib/x86_64-linux-gnu/glibmm-2.4/include -I/usr/include/sigc++-2.0 -I/usr/lib/x86_64-linux-gnu/sigc++-2.0/include -I/usr/include/startup-notification-1.0 -I/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/string/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/string/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/logmessage/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/logmessage/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/timer/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/timer/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/pluginclasshandler/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/pluginclasshandler/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/point/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/point/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/rect/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/rect/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/servergrab/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/servergrab/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/region/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/region/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/geometry/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/geometry/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/geometry-saver/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/geometry-saver/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/extents/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/extents/src -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/constrainment/include -I/tmp/buildd/compiz-0.9.8.2+bzr3377/src/window/constrainment/src -DPLUGINDIR=\"/usr/lib/compiz\" -DSHAREDIR=\"/usr/share/compiz/\" -DMETADATADIR=\"/usr/share/compiz\" -o CMakeFiles/compiz_core.dir/event.cpp.o -c /tmp/buildd/compiz-0.9.8.2+bzr3377/src/event.cpp
/tmp/buildd/compiz-0.9.8.2+bzr3377/src/event.cpp:44:29: fatal error: eventmanagement.h: No such file or directory
compilation terminated.
make[3]: *** [src/CMakeFiles/compiz_core.dir/event.cpp.o] Error 1
make[3]: Leaving directory `/tmp/buildd/compiz-0.9.8.2+bzr3377/obj-x86_64-linux-gnu'

[Daniel van Vugt (vanvugt)]

What's the real bug ID? I don't think it's bug 1053820.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

And after that, please set to Resubmit. I think this is too large and dangerous to squeeze in 0.9.8.

[Daniel van Vugt (vanvugt)]

Resubmit per the prereq branch.

review: Needs Resubmitting

[Daniel van Vugt (vanvugt)]

Sorry, that comment doesn't belong here. Please fix as requested above.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Confirmed the same build failure as jenkins:
/home/dan/bzr/compiz/tmp.142/src/event.cpp:44:29: fatal error: eventmanagement.h: No such file or directory

Please also correct the bug links and branch name. Then leave it in progress or Resubmit state.

review: Needs Fixing

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

3391. By Sam Spilsbury on 2012-09-24

Add missing file

3392. By Sam Spilsbury on 2012-09-24

Merge lp:compiz

3393. By Sam Spilsbury on 2012-09-24

Move check to the right place

3394. By Sam Spilsbury on 2012-10-02

Merge lp:compiz and handle some BCI stuff

3395. By Sam Spilsbury on 2012-10-02

Constify the API a bit

3396. By Sam Spilsbury on 2012-10-02

Cleanup.

Unindent namespaces, simplify interface, add namespace alias where
appropriate.

Unmerged revisions

3396. By Sam Spilsbury on 2012-10-02

Cleanup.

Unindent namespaces, simplify interface, add namespace alias where
appropriate.

3395. By Sam Spilsbury on 2012-10-02

Constify the API a bit

3394. By Sam Spilsbury on 2012-10-02

Merge lp:compiz and handle some BCI stuff

3393. By Sam Spilsbury on 2012-09-24

Move check to the right place

3392. By Sam Spilsbury on 2012-09-24

Merge lp:compiz

3391. By Sam Spilsbury on 2012-09-24

Add missing file

3390. By Sam Spilsbury on 2012-09-21

Don't trigger actions unless they're actually added

3389. By Sam Spilsbury on 2012-09-21

Restrict access of private data in CompAction to a separate nonmember
function, add a test for its usage

3388. By Sam Spilsbury on 2012-09-21

Refactored out the rest of that function and tested it

3387. By Sam Spilsbury on 2012-09-21

Reworked the way actions are set up to be more in line with how
compiz expects it. Now the tests pass

=== modified file 'include/core/action.h'
--- include/core/action.h	2012-09-07 22:37:20 +0000
+++ include/core/action.h	2012-09-22 03:07:19 +0000
@@ -34,6 +34,14 @@
 
 #include <X11/Xlib-xcb.h>
 
+namespace compiz
+{
+    namespace actions
+    {
+	void setActionActiveState (const CompAction  &action,
+				   bool              active);
+    }
+}
 
 class PrivateAction;
 
@@ -171,7 +179,10 @@
 
 	bool active ();
 
-	friend class CompScreenImpl;
+	/* BCI: Not optimal, CompAction should implement
+	 * BoundAction instead or be a PV class */
+	friend void compiz::actions::setActionActiveState (const CompAction  &action,
+							   bool              active);
 
     private:
 	PrivateAction *priv;
 
=== modified file 'include/core/option.h'
--- include/core/option.h	2012-05-30 09:24:15 +0000
+++ include/core/option.h	2012-09-22 03:07:19 +0000
@@ -244,8 +244,10 @@
 
 	CompString name ();
 
-	Type type ();
-	Value & value ();
+	Type type () const;
+	Type type (); // BCI: drop this in 0.9.9
+	Value & value () const;
+	Value & value (); // BCI: drop this in 0.9.9
 	Restriction & rest ();
 
 	bool set (Value &val);
 
=== modified file 'src/action.cpp'
--- src/action.cpp	2012-05-10 12:58:01 +0000
+++ src/action.cpp	2012-09-22 03:07:19 +0000
@@ -33,6 +33,8 @@
 #include <core/screen.h>
 #include "privateaction.h"
 
+namespace ca = compiz::actions;
+
 struct _Modifier {
     const char *name;
     int        modifier;
@@ -626,6 +628,28 @@
     return priv->active;
 }
 
+void
+PrivateAction::makeActive ()
+{
+    active = true;
+}
+
+void
+PrivateAction::makeInactive ()
+{
+    active = false;
+}
+
+void
+ca::setActionActiveState (const CompAction &action,
+			  bool                  active)
+{
+    if (active)
+	action.priv->makeActive ();
+    else
+	action.priv->makeInactive ();
+}
+
 PrivateAction::PrivateAction () :
     initiate (),
     terminate (),
 
=== modified file 'src/event.cpp'
--- src/event.cpp	2012-09-05 16:42:03 +0000
+++ src/event.cpp	2012-09-22 03:07:19 +0000
@@ -41,8 +41,10 @@
 #include "privatescreen.h"
 #include "privatewindow.h"
 #include "privatestackdebugger.h"
+#include "eventmanagement.h"
 
 namespace cps = compiz::private_screen;
+namespace ce = compiz::events;
 
 namespace
 {
@@ -124,6 +126,9 @@
     if (option.value ().action ().initiate ().empty ())
 	return false;
 
+    if (!option.value ().action ().active ())
+	return false;
+
     *action = &option.value ().action ();
 
     return true;
@@ -189,78 +194,160 @@
     return false;
 }
 
+int
+ce::processButtonPressOnEdgeWindow (Window               edgeWindow,
+				    Window               root,
+				    Window               eventWindow,
+				    Window               eventRoot,
+				    cps::GrabList        &grabList,
+				    const CompScreenEdge *screenEdge)
+{
+    int edge = -1;
+
+    if (eventRoot != root)
+	return edge;
+
+    if (eventWindow != edgeWindow)
+    {
+	if (grabList.grabsEmpty () ||
+	    eventRoot != root)
+	    return edge;
+    }
+
+    for (unsigned int i = 0; i < SCREEN_EDGE_NUM; i++)
+    {
+	if (edgeWindow == screenEdge[i].id)
+	{
+	    edge = 1 << i;
+	    break;
+	}
+    }
+
+    return edge;
+}
+
+void
+ce::setEventWindowInButtonPressArguments (ce::EventArguments &arguments,
+					  Window             eventWindow)
+{
+    arguments[1].value ().set ((int) eventWindow);
+}
+
+namespace
+{
+    bool buttonActionModifiersMatchEventState (unsigned int actionModifiers,
+					       unsigned int eventState)
+    {
+	const unsigned int ignored = modHandler->ignoredModMask ();
+	const unsigned int modMask = REAL_MOD_MASK & ~ignored;
+	const unsigned int bindMods = modHandler->virtualToRealModMask (actionModifiers);
+
+	return (bindMods & modMask) == (eventState & modMask);
+    }
+}
+
+bool
+ce::activateButtonPressOnWindowBindingOption (CompOption                            &option,
+					      unsigned int                          eventButton,
+					      unsigned int                          eventState,
+					      cps::EventManager                     &eventManager,
+					      const ActionModsMatchesEventStateFunc &matchEventState,
+					      ce::EventArguments                    &arguments)
+{
+    CompAction              *action;
+    const CompAction::State state = CompAction::StateInitButton;
+
+    if (isBound (option, CompAction::BindingTypeButton, state, &action))
+    {
+	if (action->button ().button () == (int) eventButton)
+	{
+	    if (matchEventState (action->button ().modifiers (),
+				 eventState))
+	    {
+		if (eventManager.triggerPress (action, state, arguments))
+		    return true;
+	    }
+	}
+    }
+
+    return false;
+}
+
+bool
+ce::activateButtonPressOnEdgeBindingOption (CompOption                            &option,
+					    unsigned int                          eventButton,
+					    unsigned int                          eventState,
+					    int                                   edge,
+					    cps::EventManager                     &eventManager,
+					    const ActionModsMatchesEventStateFunc &matchEventState,
+					    ce::EventArguments                    &arguments)
+{
+    CompAction              *action;
+    const CompAction::State state = CompAction::StateInitButton |
+				    CompAction::StateInitEdge;
+
+    if (edge != -1)
+    {
+	if (isInitiateBinding (option, CompAction::BindingTypeEdgeButton,
+			       state, &action))
+	{
+	    if ((action->button ().button () == (int) eventButton) &&
+		(action->edgeMask () & edge))
+	    {
+		if (matchEventState (action->button ().modifiers (),
+				     eventState))
+		    if (action->initiate () (action, state,
+					     arguments))
+			return true;
+	    }
+	}
+    }
+
+    return false;
+}
+
 bool
 PrivateScreen::triggerButtonPressBindings (CompOption::Vector &options,
 					   XButtonEvent       *event,
 					   CompOption::Vector &arguments)
 {
-    CompAction::State state = CompAction::StateInitButton;
-    CompAction        *action;
-    unsigned int      ignored = modHandler->ignoredModMask ();
-    unsigned int      modMask = REAL_MOD_MASK & ~ignored;
-    unsigned int      bindMods;
-    unsigned int      edge = 0;
+    int               edge = -1;
+
+    static const ce::ActionModsMatchesEventStateFunc matchEventState (
+		boost::bind (buttonActionModifiersMatchEventState,
+			     _1, _2));
 
     if (edgeWindow)
-    {
-	unsigned int i;
-
-	if (event->root != screen->root())
-	    return false;
-
-	if (event->window != edgeWindow)
-	{
-	    if (eventManager.grabsEmpty () || event->window != screen->root())
-		return false;
-	}
-
-	for (i = 0; i < SCREEN_EDGE_NUM; i++)
-	{
-	    if (edgeWindow == screenEdge[i].id)
-	    {
-		edge = 1 << i;
-		arguments[1].value ().set ((int) orphanData.activeWindow);
-		break;
-	    }
-	}
-    }
+	edge = ce::processButtonPressOnEdgeWindow (edgeWindow,
+						   screen->root (),
+						   event->window,
+						   event->root,
+						   eventManager,
+						   screenEdge);
+
+    if (edge != -1)
+	ce::setEventWindowInButtonPressArguments (arguments,
+						  orphanData.activeWindow);
 
     foreach (CompOption &option, options)
     {
-	if (isBound (option, CompAction::BindingTypeButton, state, &action))
-	{
-	    if (action->button ().button () == (int) event->button)
-	    {
-		bindMods = modHandler->virtualToRealModMask (
-		    action->button ().modifiers ());
-
-		if ((bindMods & modMask) == (event->state & modMask))
-		{
-		    if (eventManager.triggerPress (action, state, arguments))
-			return true;
-		}
-	    }
-	}
-
-	if (edge)
-	{
-	    if (isInitiateBinding (option, CompAction::BindingTypeEdgeButton,
-				   state | CompAction::StateInitEdge, &action))
-	    {
-		if ((action->button ().button () == (int) event->button) &&
-		    (action->edgeMask () & edge))
-		{
-		    bindMods = modHandler->virtualToRealModMask (
-			action->button ().modifiers ());
-
-		    if ((bindMods & modMask) == (event->state & modMask))
-			if (action->initiate () (action, state |
-						 CompAction::StateInitEdge,
-						 arguments))
-			    return true;
-		}
-	    }
-	}
+	if (ce::activateButtonPressOnWindowBindingOption (option,
+							  event->button,
+							  event->state,
+							  eventManager,
+							  matchEventState,
+							  arguments))
+	    return true;
+
+	if (ce::activateButtonPressOnEdgeBindingOption (option,
+							event->button,
+							event->state,
+							edge,
+							eventManager,
+							matchEventState,
+							arguments))
+	    return true;
+
     }
 
     return false;
 
=== modified file 'src/option.cpp'
--- src/option.cpp	2012-06-24 09:00:27 +0000
+++ src/option.cpp	2012-09-22 03:07:19 +0000
@@ -462,12 +462,24 @@
     return priv->type;
 }
 
+CompOption::Type
+CompOption::type () const
+{
+    return priv->type;
+}
+
 CompOption::Value &
 CompOption::value ()
 {
     return priv->value;
 }
 
+CompOption::Value &
+CompOption::value () const
+{
+    return priv->value;
+}
+
 CompOption::Restriction &
 CompOption::rest ()
 {
@@ -477,6 +489,12 @@
 bool
 CompOption::set (CompOption::Value &val)
 {
+    /* XXX: It is uncertain as to why this is done. The only
+     * logical reason would be that actions are stateful and
+     * we don't want to care about the old state from the
+     * action that we're setting this value to, so we're just
+     * clearing that state and starting over, however copyState
+     * does a lot more than that */
     if (isAction () && priv->type != CompOption::TypeAction)
 	val.action ().copyState (priv->value.action ());
 
 
=== modified file 'src/option/tests/option.cpp'
--- src/option/tests/option.cpp	2012-05-27 03:27:57 +0000
+++ src/option/tests/option.cpp	2012-09-22 03:07:19 +0000
@@ -206,3 +206,34 @@
 	ASSERT_EQ(expectTwo, as_const.f());
     }
 }
+
+TEST (CompOption, AssignDefaultActionValueToUnsetTypeClearsOldStateKeepsInfo)
+{
+    /* Value is unset at this point */
+    CompOption        option ("testing", CompOption::TypeKey);
+    CompAction        action;
+
+    /* We need to set up the state here as
+     * the CompOption::Value constructor makes
+     * a copy of the action */
+    action.setState (CompAction::StateInitKey);
+    action.setButton (CompAction::ButtonBinding (1,
+						 1 << 1));
+
+    CompOption::Value value (action);
+
+    ASSERT_EQ (value.action ().state (), CompAction::StateInitKey);
+
+    /* Actually set the action value, this will
+     * overwrite the internal value */
+    option.set (value);
+
+    /* We don't care about the old action's state, so get
+     * rid of it */
+    ASSERT_EQ (option.value ().action ().state (), 0);
+
+    /* We do want to keep the non-stateful data which is
+     * pure info */
+    ASSERT_EQ (option.value ().action ().button ().button (), 1);
+    ASSERT_EQ (option.value ().action ().button ().modifiers (), 1 << 1);
+}
 
=== modified file 'src/privateaction.h'
--- src/privateaction.h	2012-02-14 11:39:59 +0000
+++ src/privateaction.h	2012-09-22 03:07:19 +0000
@@ -28,11 +28,35 @@
 #ifndef _PRIVATEACTION_H
 #define _PRIVATEACTION_H
 
-class PrivateAction {
+namespace compiz
+{
+    namespace actions
+    {
+	class BoundAction
+	{
+	    public:
+
+		virtual ~BoundAction () {}
+
+		virtual void makeActive () = 0;
+		virtual void makeInactive () = 0;
+	};
+
+	void setActionActiveState (const CompAction  &action,
+				   bool              active);
+    }
+}
+
+class PrivateAction :
+    public compiz::actions::BoundAction
+{
     public:
 	PrivateAction ();
 	PrivateAction (const PrivateAction&);
 
+	void makeActive ();
+	void makeInactive ();
+
 	CompAction::CallBack initiate;
 	CompAction::CallBack terminate;
 
 
=== modified file 'src/privatescreen/tests/test-privatescreen.cpp'
--- src/privatescreen/tests/test-privatescreen.cpp	2012-06-24 09:00:27 +0000
+++ src/privatescreen/tests/test-privatescreen.cpp	2012-09-22 03:07:19 +0000
@@ -1,5 +1,6 @@
 #include "privatescreen.h"
-
+#include "privateaction.h"
+#include "eventmanagement.h"
 
 // Get rid of stupid macro from X.h
 // Why, oh why, are we including X.h?
@@ -16,6 +17,8 @@
 
 namespace {
 
+const unsigned long None = 0;
+
 class MockCompScreen : public CompScreen
 {
 public:
@@ -355,6 +358,7 @@
 } // (abstract) namespace
 
 namespace cps = compiz::private_screen;
+namespace ce = compiz::events;
 
 TEST(privatescreen_PluginManagerTest, create_and_destroy)
 {
@@ -909,3 +913,511 @@
 
     EXPECT_EQ (vp, CompPoint (0, 1));
 }
+
+namespace
+{
+    const Window topLeftScreenEdge = 1;
+    const Window topScreenEdge = 2;
+    const Window topRightScreenEdge = 3;
+    const Window rightScreenEdge = 4;
+    const Window bottomRightScreenEdge = 5;
+    const Window bottomScreenEdge = 6;
+    const Window bottomLeftScreenEdge = 7;
+    const Window leftScreenEdge = 8;
+
+    const CompScreenEdge screenEdges[SCREEN_EDGE_NUM] =
+    {
+	{ leftScreenEdge, SCREEN_EDGE_LEFT },
+	{ rightScreenEdge, SCREEN_EDGE_RIGHT },
+	{ topScreenEdge, SCREEN_EDGE_TOP },
+	{ bottomScreenEdge , SCREEN_EDGE_BOTTOM },
+	{ topLeftScreenEdge, SCREEN_EDGE_TOPLEFT },
+	{ topRightScreenEdge, SCREEN_EDGE_TOPRIGHT },
+	{ bottomLeftScreenEdge, SCREEN_EDGE_BOTTOMLEFT },
+	{ bottomRightScreenEdge, SCREEN_EDGE_BOTTOMRIGHT}
+    };
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, IgnoreWhenEventAndRootWindowMismatch)
+{
+    const Window rootWindow = 1;
+    const Window edgeWindow = topScreenEdge;
+
+    cps::OrphanData orphanData;
+    cps::GrabList grabList;
+
+    EXPECT_EQ (ce::processButtonPressOnEdgeWindow (edgeWindow,
+						   rootWindow,
+						   0,
+						   0,
+						   grabList,
+						   screenEdges), -1);
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, IgnoreWhenEventMismatchAndNoGrabs)
+{
+    const Window rootWindow = 1;
+    const Window edgeWindow = topScreenEdge;
+
+    cps::OrphanData orphanData;
+    cps::GrabList grabList;
+
+    EXPECT_EQ (ce::processButtonPressOnEdgeWindow (edgeWindow,
+						   rootWindow,
+						   0,
+						   rootWindow,
+						   grabList,
+						   screenEdges), -1);
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, AllowWhenEventButNotRootWindowMismatchWhileGrabbed)
+{
+    const Window rootWindow = 1;
+    const Window edgeWindow = topScreenEdge;
+    unsigned int topEdgeMask = 1 << SCREEN_EDGE_TOP;
+
+    cps::OrphanData orphanData;
+    cps::GrabList grabList;
+
+    grabList.grabsPush (new cps::Grab (None, "Nil"));
+
+    EXPECT_EQ (ce::processButtonPressOnEdgeWindow (edgeWindow,
+						   rootWindow,
+						   0,
+						   rootWindow,
+						   grabList,
+						   screenEdges), topEdgeMask);
+
+    grabList.grabsRemove (grabList.grabsBack ());
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, SetEventWindowArgument)
+{
+    const Window activeWindow = 1;
+
+    ce::EventArguments arguments (2);
+    ce::setEventWindowInButtonPressArguments (arguments, activeWindow);
+    EXPECT_EQ (arguments[1].value ().i (), activeWindow);
+}
+
+namespace
+{
+    class MockTriggerableAction
+    {
+	public:
+
+	    MOCK_METHOD2 (matchEventState, bool (unsigned int,
+						 unsigned int));
+	    MOCK_METHOD3 (initiate, bool (CompAction         *,
+					  CompAction::State   ,
+					  CompOption::Vector &));
+	    MOCK_METHOD3 (terminate, bool (CompAction         *,
+					   CompAction::State   ,
+					   CompOption::Vector &));
+    };
+
+    const unsigned int testingButtonNumber = 1;
+    const unsigned int testingButtonState = (1 << 1);
+
+    ce::ActionModsMatchesEventStateFunc
+    GetMatchEventStateFuncForMock (MockTriggerableAction &triggerableAction)
+    {
+	return boost::bind (&MockTriggerableAction::matchEventState,
+			    &triggerableAction,
+			    _1, _2);
+    }
+
+    CompAction::CallBack
+    GetInitiateForMock (MockTriggerableAction &triggerableAction)
+    {
+	return boost::bind (&MockTriggerableAction::initiate,
+			    &triggerableAction,
+			    _1, _2, _3);
+    }
+
+    CompAction::CallBack
+    GetTerminateForMock (MockTriggerableAction &triggerableAction)
+    {
+	return boost::bind (&MockTriggerableAction::terminate,
+			    &triggerableAction,
+			    _1, _2, _3);
+    }
+}
+
+bool operator== (const CompOption &lhs,
+		 const CompOption &rhs)
+{
+    if (lhs.type () != rhs.type ())
+	return false;
+
+    return lhs.value () == rhs.value ();
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, NoTriggerOnUnboundAction)
+{
+    CompAction            action;
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    MockTriggerableAction triggerableAction;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+
+    EXPECT_FALSE (ce::activateButtonPressOnWindowBindingOption (option,
+								testingButtonNumber,
+								testingButtonState,
+								eventManager,
+								matchEventState,
+								arguments));
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, NoTriggerOnUnboundInactiveAction)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+    action.setEdgeMask (edgeMask);
+
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), false);
+
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      testingButtonNumber,
+							      testingButtonState,
+							      edgeMask,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, NoTriggerOnMismatchedButtonNumber)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, initiate (_, _, _)).Times (0);
+    EXPECT_FALSE (ce::activateButtonPressOnWindowBindingOption (option,
+								0,
+								testingButtonState,
+								eventManager,
+								matchEventState,
+								arguments));
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, NoTriggerOnMismatchedButtonState)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, matchEventState (testingButtonState, 0))
+	    .WillOnce (Return (false));
+    EXPECT_CALL (triggerableAction, initiate (_, _, _)).Times (0);
+    EXPECT_FALSE (ce::activateButtonPressOnWindowBindingOption (option,
+								testingButtonNumber,
+								0,
+								eventManager,
+								matchEventState,
+								arguments));
+}
+
+TEST (privatescreen_ButtonPressEventManagementTest, TriggerWhenStateAndButtonMatch)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, matchEventState (testingButtonState, testingButtonState))
+	    .WillOnce (Return (true));
+    EXPECT_CALL (triggerableAction, initiate (&option.value ().action (),
+					      CompAction::StateInitButton,
+					      arguments)).WillOnce (Return (true));
+    EXPECT_TRUE (ce::activateButtonPressOnWindowBindingOption (option,
+							       testingButtonNumber,
+							       testingButtonState,
+							       eventManager,
+							       matchEventState,
+							       arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, NoTriggerOnInvalidEdge)
+{
+    CompAction            action;
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    MockTriggerableAction triggerableAction;
+    ce::EventArguments    arguments;
+    int                   edgeMask = -1;
+
+    option.set (value);
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      testingButtonNumber,
+							      testingButtonState,
+							      edgeMask,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, NoTriggerOnUnboundAction)
+{
+    CompAction            action;
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    MockTriggerableAction triggerableAction;
+    ce::EventArguments    arguments;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    option.set (value);
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      testingButtonNumber,
+							      testingButtonState,
+							      edgeMask,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, NoTriggerOnMismatchedEdgeMask)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+    action.setEdgeMask (edgeMask);
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, initiate (_, _, _)).Times (0);
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      testingButtonNumber,
+							      testingButtonState,
+							      0,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, NoTriggerOnMismatchedButtonNumber)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+    action.setEdgeMask (edgeMask);
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, initiate (_, _, _)).Times (0);
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      0,
+							      testingButtonState,
+							      edgeMask,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, NoTriggerOnMismatchedButtonState)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+    action.setEdgeMask (edgeMask);
+
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, matchEventState (testingButtonState, 0))
+	    .WillOnce (Return (false));
+    EXPECT_CALL (triggerableAction, initiate (_, _, _)).Times (0);
+    EXPECT_FALSE (ce::activateButtonPressOnEdgeBindingOption (option,
+							      testingButtonNumber,
+							      0,
+							      edgeMask,
+							      eventManager,
+							      matchEventState,
+							      arguments));
+}
+
+TEST (privatescreen_ButtonPressEdgeEventManagementTest, TriggerWhenStateButtonAndEdgeMaskMatch)
+{
+    CompAction            action;
+    MockTriggerableAction triggerableAction;
+    int                   edgeMask = 1 << SCREEN_EDGE_TOP;
+
+    const ce::ActionModsMatchesEventStateFunc &matchEventState =
+	    GetMatchEventStateFuncForMock (triggerableAction);
+    const CompAction::CallBack                &initiate =
+	    GetInitiateForMock (triggerableAction);
+
+    action.setButton (CompAction::ButtonBinding (testingButtonNumber,
+						 testingButtonState));
+    action.setEdgeMask (edgeMask);
+
+    CompOption            option ("button", CompOption::TypeButton);
+    CompOption::Value     value (action);
+    cps::EventManager     eventManager;
+    ce::EventArguments    arguments;
+
+    option.set (value);
+    option.value ().action ().setInitiate (initiate);
+    option.value ().action ().setState (CompAction::StateInitButton |
+					CompAction::StateInitEdge);
+    compiz::actions::setActionActiveState (option.value ().action (), true);
+
+    EXPECT_CALL (triggerableAction, matchEventState (testingButtonState, testingButtonState))
+	    .WillOnce (Return (true));
+    EXPECT_CALL (triggerableAction, initiate (&option.value ().action (),
+					      CompAction::StateInitButton |
+					      CompAction::StateInitEdge,
+					      arguments)).WillOnce (Return (true));
+    EXPECT_TRUE (ce::activateButtonPressOnEdgeBindingOption (option,
+							     testingButtonNumber,
+							     testingButtonState,
+							     edgeMask,
+							     eventManager,
+							     matchEventState,
+							     arguments));
+}
+
+TEST (privatescreen_ActionActiveTest, TestMakeActionActive)
+{
+    CompAction action;
+
+    ASSERT_EQ (action.active (), false);
+    compiz::actions::setActionActiveState (action, true);
+    ASSERT_EQ (action.active (), true);
+    compiz::actions::setActionActiveState (action, false);
+    ASSERT_EQ (action.active (), false);
+}
 
=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-09-18 01:44:23 +0000
+++ src/screen.cpp	2012-09-22 03:07:19 +0000
@@ -89,6 +89,7 @@
 } MwmHints;
 
 namespace cps = compiz::private_screen;
+namespace ca = compiz::actions;
 
 
 
@@ -3565,7 +3566,7 @@
 		privateScreen.enableEdge (i);
     }
 
-    action->priv->active = true;
+    ca::setActionActiveState (*action, true);
 
     return true;
 }
@@ -3594,7 +3595,7 @@
 		privateScreen.disableEdge (i);
     }
 
-    action->priv->active = false;
+    ca::setActionActiveState (*action, false);
 }
 
 CompRect