Merge lp:~dandrader/geis/lp1015775 into lp:geis

[Daniel d'Andrada]

Fixes bug #1015775.

Includes regression test.

[Chase Douglas (chasedouglas)]

Looks right to me.

review: Approve

=== modified file 'libutouch-geis/backend/grail/geis_grail_backend.c'
--- libutouch-geis/backend/grail/geis_grail_backend.c	2012-05-22 12:18:55 +0000
+++ libutouch-geis/backend/grail/geis_grail_backend.c	2012-06-20 20:08:20 +0000
@@ -1610,10 +1610,12 @@
   }
 
   struct _GeisSliceState *slice_state = _grail_be_slice_state_from_id(gbe, id);
-
-  geis_remove_matching_events(gbe->geis,
-                              _grail_be_match_overlapping_gesture_event,
-                              slice_state);
+  /* the corresponding grail gesture might have already ended, in which case we
+     no longer hold information over its state. */
+  if (slice_state)
+    geis_remove_matching_events(gbe->geis,
+                                _grail_be_match_overlapping_gesture_event,
+                                slice_state);
   return status;
 }
 

=== modified file 'testsuite/geis2/Makefile.am'
--- testsuite/geis2/Makefile.am	2012-06-05 20:04:24 +0000
+++ testsuite/geis2/Makefile.am	2012-06-20 20:08:20 +0000
@@ -47,6 +47,7 @@
 
 gtest_geis2_grail_backend_SOURCES = \
 	gtest_grail_backend.h gtest_grail_backend.cpp \
+	gtest_gbe_accept_ended_grail_gesture.cpp \
 	gtest_gbe_direct_touch_coords.cpp \
 	gtest_gbe_construction_finished.cpp \
 	gtest_gbe_outdated_events.cpp

=== added file 'testsuite/geis2/gtest_gbe_accept_ended_grail_gesture.cpp'
--- testsuite/geis2/gtest_gbe_accept_ended_grail_gesture.cpp	1970-01-01 00:00:00 +0000
+++ testsuite/geis2/gtest_gbe_accept_ended_grail_gesture.cpp	2012-06-20 20:08:20 +0000
@@ -0,0 +1,85 @@
+#include "gtest_grail_backend.h"
+#include "x11_mocks.h"
+
+/*
+  Regression test for bug https://bugs.launchpad.net/utouch-geis/+bug/1015775
+
+  Checks that the following situation doesn't lead to a crash:
+    - Geis processed the end event of a grail gesture
+    - Geis has a number of geis events pending delivery to application
+    - Application accepts that ended grail gesture
+
+  Cause:
+    - While checking the queue of pending events for events that should
+      no longer be delivered due to the gesture acceptance, geis would
+      try to use a struct with information from the ended grail gesture
+      that also no longer exists.
+ */
+
+class AcceptEndedGesture : public Geis2GrailBackendBase
+{
+ protected:
+  AcceptEndedGesture() :  _subscription(nullptr) {}
+
+  void SendXInput2Events();
+
+  virtual void OnEventInitComplete(GeisEvent event);
+  virtual void OnEventGestureBegin(GeisEvent event);
+
+  GeisSubscription _subscription;
+};
+
+void AcceptEndedGesture::SendXInput2Events()
+{
+  /* args: event type, touch id, X and Y  */
+  SendTouchEvent(XI_TouchBegin, 1, 10.0f, 10.0f);
+  SendTouchEvent(XI_TouchBegin, 2, 20.0f, 10.0f);
+
+  xmock_server_time += 5;
+
+  /* args: touch id  */
+  SendTouchOwnershipEvent(1);
+  SendTouchOwnershipEvent(2);
+
+  xmock_server_time += 5;
+
+  SendTouchEvent(XI_TouchUpdate, 1, 10.0f, 20.0f);
+  SendTouchEvent(XI_TouchUpdate, 2, 20.0f, 20.0f);
+
+  xmock_server_time += 5;
+
+  SendTouchEvent(XI_TouchEnd, 1, 10.0f, 30.0f);
+  SendTouchEvent(XI_TouchEnd, 2, 20.0f, 30.0f);
+}
+
+void AcceptEndedGesture::OnEventInitComplete(GeisEvent event)
+{
+  _subscription = CreateFilteredSubscription(
+      "My 2-touches Touch", 2, GEIS_GESTURE_TOUCH);
+  ASSERT_NE(nullptr, _subscription);
+
+  SendXInput2Events();
+}
+
+void AcceptEndedGesture::OnEventGestureBegin(GeisEvent event)
+{
+  AcceptGestureInEvent(event);
+}
+
+TEST_F(AcceptEndedGesture, Test)
+{
+  CreateXMockTouchScreenDevice();
+
+  _geis = geis_new(GEIS_INIT_UTOUCH_GRAIL_BACKEND,
+                       GEIS_INIT_NO_ATOMIC_GESTURES,
+                       nullptr);
+  ASSERT_NE(nullptr, _geis);
+
+  Run();
+
+  if (_subscription)
+    geis_subscription_delete(_subscription);
+  geis_delete(_geis);
+
+  DestroyXMockDevices();
+}

=== modified file 'testsuite/geis2/gtest_gbe_construction_finished.cpp'
--- testsuite/geis2/gtest_gbe_construction_finished.cpp	2012-05-16 18:14:14 +0000
+++ testsuite/geis2/gtest_gbe_construction_finished.cpp	2012-06-20 20:08:20 +0000
@@ -307,19 +307,19 @@
 {
   CreateXMockTouchScreenDevice();
 
-  Geis geis = geis_new(GEIS_INIT_UTOUCH_GRAIL_BACKEND,
+  _geis = geis_new(GEIS_INIT_UTOUCH_GRAIL_BACKEND,
                        GEIS_INIT_NO_ATOMIC_GESTURES,
                        nullptr);
-  ASSERT_NE(nullptr, geis);
+  ASSERT_NE(nullptr, _geis);
 
-  Run(geis);
+  Run();
 
   /* there should have been two gesture update events from geis */
   ASSERT_EQ(2, _gesture_update_count);
 
   if (_subscription)
     geis_subscription_delete(_subscription);
-  geis_delete(geis);
+  geis_delete(_geis);
 
   DestroyXMockDevices();
 }

=== modified file 'testsuite/geis2/gtest_gbe_direct_touch_coords.cpp'
--- testsuite/geis2/gtest_gbe_direct_touch_coords.cpp	2012-05-16 18:13:54 +0000
+++ testsuite/geis2/gtest_gbe_direct_touch_coords.cpp	2012-06-20 20:08:20 +0000
@@ -226,15 +226,15 @@
 {
   CreateXMockTouchScreenDevice();
 
-  Geis geis = geis_new(GEIS_INIT_UTOUCH_GRAIL_BACKEND,
+  _geis = geis_new(GEIS_INIT_UTOUCH_GRAIL_BACKEND,
                        nullptr);
-  ASSERT_NE(nullptr, geis);
+  ASSERT_NE(nullptr, _geis);
 
-  Run(geis);
+  Run();
 
   if (_subscription)
     geis_subscription_delete(_subscription);
-  geis_delete(geis);
+  geis_delete(_geis);
 
   DestroyXMockDevices();
 }

=== modified file 'testsuite/geis2/gtest_gbe_outdated_events.cpp'
--- testsuite/geis2/gtest_gbe_outdated_events.cpp	2012-05-18 22:27:59 +0000
+++ testsuite/geis2/gtest_gbe_outdated_events.cpp	2012-06-20 20:08:20 +0000
@@ -22,30 +22,18 @@
 class OutdatedEventsOverlappingGestures : public Geis2GrailBackendBase
 {
  protected:
-  OutdatedEventsOverlappingGestures() : _geis(nullptr),
-                           _subscription(nullptr),
-                           _xevent_serial_number(0) {}
+  OutdatedEventsOverlappingGestures() : _subscription(nullptr) {}
 
   void SendXInput2Events();
-  void SendTouchEvent(int event_type, int touch_id, float x, float y);
-  void SendTouchOwnershipEvent(int touch_id);
 
   virtual void OnEventInitComplete(GeisEvent event);
   virtual void OnEventGestureBegin(GeisEvent event);
   virtual void OnEventGestureUpdate(GeisEvent event);
 
-  void AcceptRejectGestureInEvent(GeisEvent event, bool accept);
-  void AcceptGestureInEvent(GeisEvent event);
-  void RejectGestureInEvent(GeisEvent event);
-
   int GetGestureIdInEvent(GeisEvent event);
 
-  Geis _geis;
   GeisSubscription _subscription;
 
-  /* holds the serial number to be used on the next synthetic XEvent */
-  int _xevent_serial_number;
-
   std::set<int> _accepted_gestures;
 };
 
@@ -73,71 +61,6 @@
   SendTouchEvent(XI_TouchUpdate, 4, 40.0f, 20.0f);
 }
 
-void OutdatedEventsOverlappingGestures::SendTouchEvent(
-    int event_type, int touch_id, float x, float y)
-{
-  XEvent event;
-  XGenericEventCookie *xcookie = 0;
-  XIDeviceEvent *device_event = 0;
-
-  event.type = GenericEvent;
-  device_event = (XIDeviceEvent*)calloc(1, sizeof(XIDeviceEvent));
-  device_event->serial = _xevent_serial_number++;
-  device_event->display = xmock_display;
-  device_event->extension = xmock_xi2_opcode;
-  device_event->evtype = event_type;
-  device_event->time = xmock_server_time;
-  device_event->deviceid = 0;
-  device_event->sourceid = device_event->deviceid;
-  device_event->detail = touch_id;
-  device_event->root = DefaultRootWindow(xmock_display);
-  device_event->event = DefaultRootWindow(xmock_display);
-  device_event->child = 0;
-  device_event->root_x = x;
-  device_event->root_y = y;
-  device_event->event_x = device_event->root_x;
-  device_event->event_y = device_event->root_y;
-  device_event->valuators.mask_len = 2;
-  device_event->valuators.mask = (unsigned char*) malloc(2);
-  XISetMask(device_event->valuators.mask, 0);
-  XISetMask(device_event->valuators.mask, 1);
-  device_event->valuators.values = (double*) malloc(sizeof(double)*2);
-  device_event->valuators.values[0] = 0; /* just change the coordinate system */
-  device_event->valuators.values[1] = 0;
-  xcookie = &event.xcookie;
-  xcookie->extension = xmock_xi2_opcode;
-  xcookie->evtype = event_type;
-  xcookie->data = device_event;
-  xmock_add_to_event_queue(&event);
-}
-
-void OutdatedEventsOverlappingGestures::SendTouchOwnershipEvent(int touch_id)
-{
-  XEvent event;
-  XGenericEventCookie *xcookie = 0;
-  XITouchOwnershipEvent *ownership_event = 0;
-
-  event.type = GenericEvent;
-  ownership_event = (XITouchOwnershipEvent*)calloc(1, sizeof(XITouchOwnershipEvent));
-  ownership_event->type = GenericEvent;
-  ownership_event->serial = _xevent_serial_number++;
-  ownership_event->display = xmock_display;
-  ownership_event->extension = xmock_xi2_opcode;
-  ownership_event->evtype = XI_TouchOwnership;
-  ownership_event->time = xmock_server_time;
-  ownership_event->deviceid = 0;
-  ownership_event->sourceid = ownership_event->deviceid;
-  ownership_event->touchid = touch_id;
-  ownership_event->root = DefaultRootWindow(xmock_display);
-  ownership_event->event = DefaultRootWindow(xmock_display);
-  ownership_event->child = 0;
-  xcookie = &event.xcookie;
-  xcookie->extension = xmock_xi2_opcode;
-  xcookie->evtype = XI_TouchOwnership;
-  xcookie->data = ownership_event;
-  xmock_add_to_event_queue(&event);
-}
-
 void OutdatedEventsOverlappingGestures::OnEventInitComplete(GeisEvent event)
 {
   _subscription = CreateFilteredSubscription(
@@ -147,48 +70,6 @@
   SendXInput2Events();
 }
 
-void OutdatedEventsOverlappingGestures::AcceptRejectGestureInEvent(
-    GeisEvent event, bool accept)
-{
-  /* We expect only one group with one gesture frame.
-     Multiple groups with several gesture frames is just not supported or used.
-     That is, one GeisEvent is sent for each possible gesture instead of a single
-     GeisEvent with all possible gestures (arranged in groups). */
-
-  GeisAttr attr = geis_event_attr_by_name(event, GEIS_EVENT_ATTRIBUTE_GROUPSET);
-  GeisGroupSet group_set =
-    static_cast<GeisGroupSet>(geis_attr_value_to_pointer(attr));
-
-  ASSERT_EQ(1, geis_groupset_group_count(group_set));
-
-  GeisGroup group = geis_groupset_group(group_set, 0);
-
-  ASSERT_EQ(1, geis_group_frame_count(group));
-
-  GeisFrame frame = geis_group_frame(group, 0);
-
-  GeisStatus status;
-  if (accept)
-  {
-    status = geis_gesture_accept(_geis, group, geis_frame_id(frame));
-  }
-  else
-  {
-    status = geis_gesture_reject(_geis, group, geis_frame_id(frame));
-  }
-  ASSERT_EQ(GEIS_STATUS_SUCCESS, status);
-}
-
-void OutdatedEventsOverlappingGestures::AcceptGestureInEvent(GeisEvent event)
-{
-  AcceptRejectGestureInEvent(event, true);
-}
-
-void OutdatedEventsOverlappingGestures::RejectGestureInEvent(GeisEvent event)
-{
-  AcceptRejectGestureInEvent(event, false);
-}
-
 int OutdatedEventsOverlappingGestures::GetGestureIdInEvent(GeisEvent event)
 {
   /* We expect only one group with one gesture frame.
@@ -256,7 +137,7 @@
                        nullptr);
   ASSERT_NE(nullptr, _geis);
 
-  Run(_geis);
+  Run();
 
   ASSERT_EQ(2, _accepted_gestures.size());
 

=== modified file 'testsuite/geis2/gtest_grail_backend.cpp'
--- testsuite/geis2/gtest_grail_backend.cpp	2012-05-18 22:27:20 +0000
+++ testsuite/geis2/gtest_grail_backend.cpp	2012-06-20 20:08:20 +0000
@@ -187,10 +187,8 @@
   return got_events;
 }
 
-void Geis2GrailBackendBase::Run(Geis geis)
+void Geis2GrailBackendBase::Run()
 {
-  _geis = geis;
-
   bool got_events;
   do
   {
@@ -198,6 +196,114 @@
   } while (got_events);
 }
 
+void Geis2GrailBackendBase::SendTouchEvent(
+    int event_type, int touch_id, float x, float y)
+{
+  XEvent event;
+  XGenericEventCookie *xcookie = 0;
+  XIDeviceEvent *device_event = 0;
+
+  event.type = GenericEvent;
+  device_event = (XIDeviceEvent*)calloc(1, sizeof(XIDeviceEvent));
+  device_event->serial = _xevent_serial_number++;
+  device_event->display = xmock_display;
+  device_event->extension = xmock_xi2_opcode;
+  device_event->evtype = event_type;
+  device_event->time = xmock_server_time;
+  device_event->deviceid = 0;
+  device_event->sourceid = device_event->deviceid;
+  device_event->detail = touch_id;
+  device_event->root = DefaultRootWindow(xmock_display);
+  device_event->event = DefaultRootWindow(xmock_display);
+  device_event->child = 0;
+  device_event->root_x = x;
+  device_event->root_y = y;
+  device_event->event_x = device_event->root_x;
+  device_event->event_y = device_event->root_y;
+  device_event->valuators.mask_len = 2;
+  device_event->valuators.mask = (unsigned char*) malloc(2);
+  XISetMask(device_event->valuators.mask, 0);
+  XISetMask(device_event->valuators.mask, 1);
+  device_event->valuators.values = (double*) malloc(sizeof(double)*2);
+  device_event->valuators.values[0] = 0; /* just change the coordinate system */
+  device_event->valuators.values[1] = 0;
+  xcookie = &event.xcookie;
+  xcookie->extension = xmock_xi2_opcode;
+  xcookie->evtype = event_type;
+  xcookie->data = device_event;
+  xmock_add_to_event_queue(&event);
+}
+
+void Geis2GrailBackendBase::SendTouchOwnershipEvent(int touch_id)
+{
+  XEvent event;
+  XGenericEventCookie *xcookie = 0;
+  XITouchOwnershipEvent *ownership_event = 0;
+
+  event.type = GenericEvent;
+  ownership_event = (XITouchOwnershipEvent*)calloc(1, sizeof(XITouchOwnershipEvent));
+  ownership_event->type = GenericEvent;
+  ownership_event->serial = _xevent_serial_number++;
+  ownership_event->display = xmock_display;
+  ownership_event->extension = xmock_xi2_opcode;
+  ownership_event->evtype = XI_TouchOwnership;
+  ownership_event->time = xmock_server_time;
+  ownership_event->deviceid = 0;
+  ownership_event->sourceid = ownership_event->deviceid;
+  ownership_event->touchid = touch_id;
+  ownership_event->root = DefaultRootWindow(xmock_display);
+  ownership_event->event = DefaultRootWindow(xmock_display);
+  ownership_event->child = 0;
+  xcookie = &event.xcookie;
+  xcookie->extension = xmock_xi2_opcode;
+  xcookie->evtype = XI_TouchOwnership;
+  xcookie->data = ownership_event;
+  xmock_add_to_event_queue(&event);
+}
+
+void Geis2GrailBackendBase::AcceptRejectGestureInEvent(
+    GeisEvent event, bool accept)
+{
+  /* We expect only one group with one gesture frame.
+     Multiple groups with several gesture frames is just not supported or used.
+     That is, one GeisEvent is sent for each possible gesture instead of a single
+     GeisEvent with all possible gestures (arranged in groups). */
+
+  GeisAttr attr = geis_event_attr_by_name(event, GEIS_EVENT_ATTRIBUTE_GROUPSET);
+  GeisGroupSet group_set =
+    static_cast<GeisGroupSet>(geis_attr_value_to_pointer(attr));
+
+  ASSERT_EQ(1, geis_groupset_group_count(group_set));
+
+  GeisGroup group = geis_groupset_group(group_set, 0);
+
+  ASSERT_EQ(1, geis_group_frame_count(group));
+
+  GeisFrame frame = geis_group_frame(group, 0);
+
+  GeisStatus status;
+  if (accept)
+  {
+    status = geis_gesture_accept(_geis, group, geis_frame_id(frame));
+  }
+  else
+  {
+    status = geis_gesture_reject(_geis, group, geis_frame_id(frame));
+  }
+  ASSERT_EQ(GEIS_STATUS_SUCCESS, status);
+}
+
+void Geis2GrailBackendBase::AcceptGestureInEvent(GeisEvent event)
+{
+  AcceptRejectGestureInEvent(event, true);
+}
+
+void Geis2GrailBackendBase::RejectGestureInEvent(GeisEvent event)
+{
+  AcceptRejectGestureInEvent(event, false);
+}
+
+
 int main(int argc, char **argv)
 {
     testing::InitGoogleTest(&argc, argv);

=== modified file 'testsuite/geis2/gtest_grail_backend.h'
--- testsuite/geis2/gtest_grail_backend.h	2012-05-18 22:26:00 +0000
+++ testsuite/geis2/gtest_grail_backend.h	2012-06-20 20:08:20 +0000
@@ -28,12 +28,13 @@
 class Geis2GrailBackendBase : public ::testing::Test
 {
  protected:
-  Geis2GrailBackendBase() : _geis(nullptr) {}
+  Geis2GrailBackendBase() : _geis(nullptr),
+                            _xevent_serial_number(0) {}
   virtual ~Geis2GrailBackendBase() {}
 
   /* Dispatch and process Geis events in a loop until there are no more
      events. */
-  void Run(Geis geis);
+  void Run();
 
 
   /* Creates (and activates) a GeisSubscription with the given name
@@ -50,9 +51,21 @@
   virtual void OnEventGestureUpdate(GeisEvent event) {}
   virtual void OnEventGestureEnd(GeisEvent event) {}
 
+  void SendTouchEvent(int event_type, int touch_id, float x, float y);
+  void SendTouchOwnershipEvent(int touch_id);
+
+  void AcceptGestureInEvent(GeisEvent event);
+  void RejectGestureInEvent(GeisEvent event);
+
+  Geis _geis;
  private:
-  Geis _geis;
+
+  /* holds the serial number to be used on the next synthetic XEvent */
+  int _xevent_serial_number;
+
   bool DispatchAndProcessEvents();
+
+  void AcceptRejectGestureInEvent(GeisEvent event, bool accept);
 };
 
 #endif // GTEST_GRAIL_BACKEND_H