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

[Sam Spilsbury]

Don't trigger the grid plugin during expo mode. Remove redundant checks in Expo which could cause this to happen.

Don't trigger the grid plugin during expo mode. Remove redundant checks in Expo which could cause this to happen.

Tests added for the first part. The second part is better tested by the branch this one depends on.

[jenkins (martin-mrazik+qa)]

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

review: Needs Fixing (continuous-integration)

[Sam Spilsbury (smspillaz)]

CI appears to be 404ing

[Martin Mrazik (mrazik)]

> CI appears to be 404ing

Sorry. That is a but that needs to be workarounded every now and then. Should work now. This is the relevant error:

patching file plugins/expo/src/windows_on_viewport/src/windows-on-viewport.cpp
patching file plugins/expo/src/windows_on_viewport/tests/CMakeLists.txt
patching file plugins/expo/src/windows_on_viewport/tests/test-windows-on-viewport.cpp
patching file plugins/expo/src/client-list-generator.h
patching file plugins/expo/src/viewport-member-window.h
Patch 100_expo_layout.patch does not apply (enforce with -f)
dh_quilt_patch: quilt --quiltrc /dev/null push -a || test $? = 2 returned exit code 1
make[1]: *** [override_dh_quilt_patch] Error 1
make[1]: Leaving directory `/tmp/buildd/compiz-0.9.8.2+bzr3377'
make: *** [build] Error 2
dpkg-buildpackage: error: debian/rules build gave error exit status 2
E: Failed autobuilding of package

[Daniel van Vugt (vanvugt)]

Resubmit per the prereq branch.

review: Needs Resubmitting

Unmerged revisions

3391. By Sam Spilsbury on 2012-09-22

Don't trigger the grid grab handler when expo is active, don't trigger
inactive actions and remove redundant checks for the root window match
because we don't support the multi-screen case and haven't since
0.9.0

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

3386. By Sam Spilsbury on 2012-09-21

Split out some of triggerButtonPressBindings and get it under test, currently
some tests fail due to incorrect behaviour in CompOption::set, a failing
test added to show this

3385. By Sam Spilsbury on 2012-09-21

Use boost::bind

3384. By Sam Spilsbury on 2012-09-21

Move the check for the button action modifiers & event state to a separate
function

3383. By Sam Spilsbury on 2012-09-21

Refactored out the edge detection code from triggerButtonPressBindings
into a separate util function under test

=== modified file 'plugins/expo/src/expo.cpp'
--- plugins/expo/src/expo.cpp	2012-09-05 17:15:39 +0000
+++ plugins/expo/src/expo.cpp	2012-09-22 03:08:44 +0000
@@ -46,10 +46,6 @@
 		     CompAction::State   state,
 		     CompOption::Vector& options)
 {
-    Window xid = CompOption::getIntOptionNamed (options, "root", 0);
-    if (xid != screen->root ())
-	return false;
-
     if (expoMode)
     {
 	dndState = DnDStart;
@@ -67,10 +63,6 @@
 		     CompAction::State   state,
 		     CompOption::Vector& options)
 {
-    Window xid = CompOption::getIntOptionNamed (options, "root", 0);
-    if (xid != screen->root ())
-	return false;
-
     if (dndState == DnDDuring || dndState == DnDStart)
     {
 	if (dndWindow)
@@ -93,10 +85,6 @@
 		    CompAction::State   state,
 		    CompOption::Vector& options)
 {
-    Window xid = CompOption::getIntOptionNamed (options, "root", 0);
-    if (xid != screen->root ())
-	return false;
-
     if (screen->otherGrabExist ("expo", NULL))
 	return false;
 
@@ -139,10 +127,6 @@
 		      CompAction::State   state,
 		      CompOption::Vector& options)
 {
-    Window xid = CompOption::getIntOptionNamed (options, "root", 0);
-    if (xid && xid != screen->root ())
-	return false;
-
     if (!expoMode)
 	return true;
 
@@ -175,10 +159,6 @@
 		      CompAction::State   state,
 		      CompOption::Vector& options)
 {
-    Window xid = CompOption::getIntOptionNamed (options, "root", 0);
-    if (xid != screen->root ())
-	return false;
-
     if (!expoMode)
 	return false;
 
@@ -195,9 +175,6 @@
 		    CompOption::Vector& options)
 {
     unsigned int newX, newY;
-    Window       xid = CompOption::getIntOptionNamed (options, "root", 0);
-    if (xid != screen->root ())
-	return false;
 
     if (!expoMode)
 	return false;
@@ -226,9 +203,6 @@
 		    CompOption::Vector& options)
 {
     int newX, newY;
-    Window       xid = CompOption::getIntOptionNamed (options, "root", 0);
-    if (xid != screen->root ())
-	return false;
 
     if (!expoMode)
 	return false;

=== modified file 'plugins/grid/src/grabhandler/include/grabhandler.h'
--- plugins/grid/src/grabhandler/include/grabhandler.h	2012-05-26 23:31:12 +0000
+++ plugins/grid/src/grabhandler/include/grabhandler.h	2012-09-22 03:08:44 +0000
@@ -1,3 +1,33 @@
+/*
+ * Compiz Fusion Grid plugin, GrabHandler class
+ *
+ * Copyright (c) 2011 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Description:
+ *
+ * Plugin to act like winsplit revolution (http://www.winsplit-revolution.com/)
+ * use <Control><Alt>NUMPAD_KEY to move and tile your windows.
+ *
+ * Press the tiling keys several times to cycle through some tiling options.
+ *
+ * Authored By:
+ * Sam Spilsbury <[e-mail]>
+ */
+#ifndef _COMPIZ_GRID_GRABHANDLER_H
+#define _COMPIZ_GRID_GRABHANDLER_H
+
+#include <boost/function.hpp>
+
 #define CompWindowGrabKeyMask         (1 << 0)
 #define CompWindowGrabButtonMask      (1 << 1)
 #define CompWindowGrabMoveMask        (1 << 2)
@@ -11,11 +41,14 @@
 namespace window
 {
 
+typedef boost::function <bool (const char *)> GrabActiveFunc;
+
 class GrabWindowHandler
 {
     public:
 
-	GrabWindowHandler (unsigned int mask);
+	GrabWindowHandler (unsigned int         mask,
+			   const GrabActiveFunc &grabActive);
 	~GrabWindowHandler ();
 
     	bool track ();
@@ -23,9 +56,12 @@
 
     private:
 
-	unsigned int mMask;
+	unsigned int         mMask;
+	const GrabActiveFunc &mGrabActive;
 
 };
 }
 }
 }
+
+#endif

=== modified file 'plugins/grid/src/grabhandler/src/grabhandler.cpp'
--- plugins/grid/src/grabhandler/src/grabhandler.cpp	2012-05-26 23:31:12 +0000
+++ plugins/grid/src/grabhandler/src/grabhandler.cpp	2012-09-22 03:08:44 +0000
@@ -24,8 +24,10 @@
 
 #include "grabhandler.h"
 
-compiz::grid::window::GrabWindowHandler::GrabWindowHandler (unsigned int mask) :
-    mMask (mask)
+compiz::grid::window::GrabWindowHandler::GrabWindowHandler (unsigned int         mask,
+							    const GrabActiveFunc &grabActive) :
+    mMask (mask),
+    mGrabActive (grabActive)
 {
 }
 
@@ -36,6 +38,9 @@
 bool
 compiz::grid::window::GrabWindowHandler::track ()
 {
+    if (mGrabActive ("expo"))
+	return false;
+
     return ((mMask & (CompWindowGrabMoveMask | CompWindowGrabButtonMask)) &&
 	    !(mMask & CompWindowGrabResizeMask));
 }

=== modified file 'plugins/grid/src/grabhandler/tests/test-grid-grab-handler.cpp'
--- plugins/grid/src/grabhandler/tests/test-grid-grab-handler.cpp	2012-05-26 23:31:12 +0000
+++ plugins/grid/src/grabhandler/tests/test-grid-grab-handler.cpp	2012-09-22 03:08:44 +0000
@@ -1,4 +1,32 @@
+/*
+ * Compiz Fusion Grid plugin, GrabHandler class
+ *
+ * Copyright (c) 2011 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Description:
+ *
+ * Plugin to act like winsplit revolution (http://www.winsplit-revolution.com/)
+ * use <Control><Alt>NUMPAD_KEY to move and tile your windows.
+ *
+ * Press the tiling keys several times to cycle through some tiling options.
+ *
+ * Authored By:
+ * Sam Spilsbury <[e-mail]>
+ */
+
 #include <gtest/gtest.h>
+#include <gmock/gmock.h>
+#include <boost/bind.hpp>
 
 #include <grabhandler.h>
 
@@ -12,15 +40,26 @@
 #define CompWindowGrabResizeMask      (1 << 3)
 #define CompWindowGrabExternalAppMask (1 << 4)
 
-class GridGrabHandlerTest :
-    public ::testing::Test
+using testing::Eq;
+using testing::Return;
+
+namespace
 {
-};
+    bool returnFalse () { return false; }
+
+    class MockGrabExist
+    {
+	public:
+
+	    MOCK_METHOD1 (grabExist, bool (const char *));
+    };
+}
 
 TEST(GridGrabHandlerTest, TestMoveHandler)
 {
     compiz::grid::window::GrabWindowHandler moveHandler (CompWindowGrabMoveMask |
-							 CompWindowGrabButtonMask);
+							 CompWindowGrabButtonMask,
+							 boost::bind (returnFalse));
 
     EXPECT_TRUE (moveHandler.track ());
     EXPECT_FALSE (moveHandler.resetResize ());
@@ -29,8 +68,22 @@
 TEST(GridGrabHandlerTest, TestResizeHandler)
 {
     compiz::grid::window::GrabWindowHandler resizeHandler (CompWindowGrabButtonMask |
-						    	   CompWindowGrabResizeMask);
+						    	   CompWindowGrabResizeMask,
+							   boost::bind (returnFalse));
 
     EXPECT_FALSE (resizeHandler.track ());
     EXPECT_TRUE (resizeHandler.resetResize ());
 }
+
+TEST(GridGrabHandlerTest, TestNoTrackOnExpoGrab)
+{
+    MockGrabExist                           mge;
+    compiz::grid::window::GrabActiveFunc    grabExist (boost::bind (&MockGrabExist::grabExist,
+								    &mge, _1));
+    compiz::grid::window::GrabWindowHandler moveHandler (CompWindowGrabMoveMask |
+							 CompWindowGrabButtonMask,
+							 grabExist);
+
+    EXPECT_CALL (mge, grabExist (Eq ("expo"))).WillOnce (Return (true));
+    EXPECT_FALSE (moveHandler.track ());
+}

=== modified file 'plugins/grid/src/grid.cpp'
--- plugins/grid/src/grid.cpp	2012-08-06 01:36:00 +0000
+++ plugins/grid/src/grid.cpp	2012-09-22 03:08:44 +0000
@@ -22,10 +22,12 @@
  * Press the tiling keys several times to cycle through some tiling options.
  */
 
+#include <boost/bind.hpp>
 #include "grid.h"
 #include "grabhandler.h"
 
 using namespace GridWindowType;
+namespace cgw = compiz::grid::window;
 
 static std::map <unsigned int, GridProps> gridProps;
 
@@ -834,7 +836,9 @@
 			unsigned int state,
 			unsigned int mask)
 {
-    compiz::grid::window::GrabWindowHandler gwHandler (mask);
+    static cgw::GrabActiveFunc grabActive (boost::bind (&CompScreen::grabExist,
+							screen, _1));
+    cgw::GrabWindowHandler gwHandler (mask, grabActive);
 
     if (gwHandler.track ())
     {

=== modified file 'src/event.cpp'
--- src/event.cpp	2012-09-22 03:08:44 +0000
+++ src/event.cpp	2012-09-22 03:08:44 +0000
@@ -111,6 +111,9 @@
     if (!(option.value ().action ().state () & state))
 	return false;
 
+    if (!option.value ().action ().active ())
+	return false;
+
     return true;
 }
 
@@ -126,9 +129,6 @@
     if (option.value ().action ().initiate ().empty ())
 	return false;
 
-    if (!option.value ().action ().active ())
-	return false;
-
     *action = &option.value ().action ();
 
     return true;