Merge lp:~chasedouglas/unity/fix-EndDrag into lp:unity

[Chase Douglas]

Fix potential pointer lockup on initial drag gesture.

GestureEngine: Fix potential pointer lockup on initial drag gesture.

[Daniel d'Andrada (dandrader)]

On tests/test-gesture-engine/test_gesture_engine.cpp:
230| drag_data.id = 0;

Might be worth adding a side comment there stating that it's important that the ID is specifically zero.
And/Or commenting on top of that test that it's a regression test for lp bug #979418

[Alex Launi (alexlauni)]

Tests looks good.

review: Approve (quality)

[Marco Trevisan (Treviño) (3v1n0)]

LGTM

review: Approve

=== modified file 'plugins/unityshell/src/GestureEngine.cpp'
--- plugins/unityshell/src/GestureEngine.cpp	2012-04-10 01:41:16 +0000
+++ plugins/unityshell/src/GestureEngine.cpp	2012-04-12 14:08:19 +0000
@@ -32,6 +32,7 @@
   _screen = screen;
 
   _drag_id = 0;
+  _drag_window = 0;
   _pinch_id = 0;
   _touch_id = 0;
   _drag_grab = 0;
@@ -174,7 +175,7 @@
 void
 GestureEngine::EndDrag()
 {
-  if (_drag_id && _drag_window)
+  if (_drag_window)
   {
     _screen->removeGrab(_drag_grab, NULL);
     _drag_grab = 0;

=== modified file 'tests/test-gesture-engine/compiz_mock/core/screen.h'
--- tests/test-gesture-engine/compiz_mock/core/screen.h	2012-04-04 14:33:07 +0000
+++ tests/test-gesture-engine/compiz_mock/core/screen.h	2012-04-12 14:08:19 +0000
@@ -31,6 +31,8 @@
 
 class CompScreenMock {
 public:
+  CompScreenMock() : _grab_count(0) {}
+
   typedef int GrabHandle;
 
   int width() const {return _width;}
@@ -47,8 +49,13 @@
 
   Window root() {return _root;}
 
-  GrabHandle pushGrab(Cursor cursor, const char *name) {return 0;}
-  void removeGrab(GrabHandle handle, CompPoint *restorePointer) {}
+  GrabHandle pushGrab(Cursor cursor, const char *name) {
+    _grab_count++;
+    return 0;
+  }
+  void removeGrab(GrabHandle handle, CompPoint *restorePointer) {
+    _grab_count--;
+  }
 
   Cursor invisibleCursor() {return 1;}
 
@@ -58,6 +65,7 @@
   CompWindowMockVector _client_list;
   CompWindowMockVector _client_list_stacking;
   Window _root;
+  int _grab_count;
 };
 
 extern CompScreenMock *screen_mock;

=== modified file 'tests/test-gesture-engine/test_gesture_engine.cpp'
--- tests/test-gesture-engine/test_gesture_engine.cpp	2012-04-04 16:47:51 +0000
+++ tests/test-gesture-engine/test_gesture_engine.cpp	2012-04-12 14:08:19 +0000
@@ -220,6 +220,27 @@
   ASSERT_EQ(0, middle_window->_maximize_count);
 }
 
+/* Regression test for lp:979418, where the grab is not removed if the gesture
+ * id is 0. */
+TEST_F(GestureEngineTest, DragGrabCheck)
+{
+  screen_mock->_grab_count = 0;
+
+  GestureEngine gesture_engine(screen_mock);
+
+  GeisAdapterMock::GeisDragData drag_data;
+  drag_data.id = 0;
+  drag_data.touches = 3;
+  drag_data.window = 123;
+  drag_data.focus_x = 100; /* hits the middle window */
+  drag_data.focus_y = 100;
+  gesture_engine.OnDragStart(&drag_data);
+
+  gesture_engine.OnDragFinish(&drag_data);
+
+  ASSERT_EQ(0, screen_mock->_grab_count);
+}
+
 int main(int argc, char** argv)
 {
   ::testing::InitGoogleTest(&argc, argv);