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

[Daniel d'Andrada]

Fixes bug 1045785.

Improved handling of gesture configuration options for GeisSubscriptions

- You can now call geis_subscription_set_configuration() independently of whether the subscription is active or not. Before that, setting configuration options on an inactive subscription would fail.

- configuration options remain valid for devices that become available *after* the subscription is configured.

Also a number of memory leaks were plugged along the way.

[Chase Douglas (chasedouglas)]

* It doesn't make much sense to include the current actual outcome in the test description because it will no longer be the actual outcome after the branch is merged :). The test could also fail in other ways in the future.

* Typo in a test comment: "Set a huge threshold (in meters) so that there's no *change* a drag can be"

Everything else looks good to me! This is a great improvement over the current functionality :).

review: Approve

[lp:~dandrader/geis/lp1045785]

289. By Daniel d'Andrada on 2012-09-05

Fix comments in tests

* Remove the "actual outcome" from the test description. It doesn't make sense
to have it there and it's also no longer the actual outcome.

* Correct comment on the drag threshold

=== modified file 'libgeis/backend/dbus/geis_dbus_backend.c'
--- libgeis/backend/dbus/geis_dbus_backend.c	2012-07-24 16:04:12 +0000
+++ libgeis/backend/dbus/geis_dbus_backend.c	2012-09-05 18:11:18 +0000
@@ -156,6 +156,11 @@
   return GEIS_STATUS_UNKNOWN_ERROR;
 }
 
+static void
+_geis_dbus_token_free_subscription_pdata(GeisBackendToken token GEIS_UNUSED,
+                                         GeisSubscription subscription GEIS_UNUSED)
+{
+}
 
 static struct GeisBackendTokenVtable _token_vtbl = {
   _geis_dbus_token_clone,
@@ -163,6 +168,7 @@
   _geis_dbus_token_compose,
   _geis_dbus_token_activate,
   _geis_dbus_token_deactivate,
+  _geis_dbus_token_free_subscription_pdata
 };
 
 
 
=== modified file 'libgeis/backend/grail/geis_grail_backend.c'
--- libgeis/backend/grail/geis_grail_backend.c	2012-08-27 21:58:31 +0000
+++ libgeis/backend/grail/geis_grail_backend.c	2012-09-05 18:11:18 +0000
@@ -106,29 +106,27 @@
   GeisBoolean               send_synchronous_events;
 };
 
-
 /**
- * A table of supported grail cofiguration properties and their GEIS
- * equivalents.
+  Holds backend-specific information regarding a GeisSubscription
  */
-static const struct GeisGrailConfigProperties
+struct GeisGrailSubscriptionData
 {
-  GeisString             geis_config_name;
-  GeisAttrType           geis_value_type;
-  UGSubscriptionProperty grail_property_name;
-} _grail_be_config_properties[] = {
-  { GEIS_CONFIG_DRAG_TIMEOUT,     GEIS_ATTR_TYPE_INTEGER, UGSubscriptionPropertyDragTimeout },
-  { GEIS_CONFIG_DRAG_THRESHOLD,   GEIS_ATTR_TYPE_FLOAT,   UGSubscriptionPropertyDragThreshold },
-  { GEIS_CONFIG_PINCH_TIMEOUT,    GEIS_ATTR_TYPE_INTEGER, UGSubscriptionPropertyPinchTimeout },
-  { GEIS_CONFIG_PINCH_THRESHOLD,  GEIS_ATTR_TYPE_FLOAT,   UGSubscriptionPropertyPinchThreshold },
-  { GEIS_CONFIG_ROTATE_TIMEOUT,   GEIS_ATTR_TYPE_INTEGER, UGSubscriptionPropertyRotateTimeout },
-  { GEIS_CONFIG_ROTATE_THRESHOLD, GEIS_ATTR_TYPE_FLOAT,   UGSubscriptionPropertyRotateThreshold },
-  { GEIS_CONFIG_TAP_TIMEOUT,      GEIS_ATTR_TYPE_INTEGER, UGSubscriptionPropertyTapTimeout },
-  { GEIS_CONFIG_TAP_THRESHOLD,    GEIS_ATTR_TYPE_FLOAT,   UGSubscriptionPropertyTapThreshold },
-  { NULL, 0, 0 }
+  GeisUGSubscriptionStore ugstore;
+
+  /* configuration options
+     NULL if not set by user, in which case grail defaults are used */
+  uint64_t *drag_timeout;
+  float *drag_threshold;
+  uint64_t *pinch_timeout;
+  float *pinch_threshold;
+  uint64_t *rotate_timeout;
+  float *rotate_threshold;
+  uint64_t *tap_timeout;
+  float *tap_threshold;
 };
 
 
+
 static GeisStatus
 _grail_be_activate_for_device(GeisGrailBackend gbe,
                               GeisFilter       filter,
@@ -839,7 +837,8 @@
        it != geis_subscription_bag_end(gbe->subscription_bag);
        it = geis_subscription_bag_iterator_next(gbe->subscription_bag, it))
   {
-   GeisUGSubscriptionStore ugstore = geis_subscription_pdata(*it);
+   struct GeisGrailSubscriptionData *subscription_data = geis_subscription_pdata(*it);
+   GeisUGSubscriptionStore ugstore = subscription_data->ugstore;
    for (GeisFilterIterator fit = geis_subscription_filter_begin(*it);
          fit != geis_subscription_filter_end(*it);
          fit = geis_subscription_filter_next(*it, fit))
@@ -1702,12 +1701,11 @@
  * subscriptions have the same property value setting.
  */
 static GeisStatus
-_grail_be_get_ugsub_property(GeisSubscription       geis_subscription,
+_grail_be_get_ugsub_property(GeisUGSubscriptionStore ugstore,
                              UGSubscriptionProperty grail_property,
                              GeisPointer            grail_value)
 {
   GeisStatus retval = GEIS_STATUS_UNKNOWN_ERROR;
-  GeisUGSubscriptionStore ugstore = geis_subscription_pdata(geis_subscription);
   if (ugstore)
   {
     for (GeisSize i = 0; i < geis_ugsubscription_count(ugstore); ++i)
@@ -1731,29 +1729,65 @@
  * Sets a given grail property to a specified value.
  */
 static GeisStatus
-_grail_be_set_ugsub_property(GeisSubscription       geis_subscription,
+_grail_be_set_ugsub_property(GeisUGSubscriptionStore ugstore,
                              UGSubscriptionProperty grail_property,
                              GeisPointer            grail_value)
 {
-  GeisStatus retval = GEIS_STATUS_UNKNOWN_ERROR;
-  GeisUGSubscriptionStore ugstore = geis_subscription_pdata(geis_subscription);
-  if (ugstore)
+  /* OBS: it's still a success if there's no grail subscription */
+  GeisStatus retval = GEIS_STATUS_SUCCESS;
+  for (GeisSize i = 0; i < geis_ugsubscription_count(ugstore); ++i)
   {
-    for (GeisSize i = 0; i < geis_ugsubscription_count(ugstore); ++i)
-    {
-      UGSubscription ugsub = geis_ugsubscription_get_ugsubscription_at(ugstore,
-                                                                       i);
-      if (UGStatusSuccess == grail_subscription_set_property(ugsub,
-                                                             grail_property,
-                                                             grail_value))
-      {
-        retval = GEIS_STATUS_SUCCESS;
-      }
+    UGSubscription ugsub = geis_ugsubscription_get_ugsubscription_at(ugstore,
+                                                                     i);
+    if (UGStatusSuccess == grail_subscription_set_property(ugsub,
+                                                           grail_property,
+                                                           grail_value))
+    {
+      retval = GEIS_STATUS_SUCCESS;
+    }
+    else
+    {
+      retval = GEIS_STATUS_UNKNOWN_ERROR;
     }
   }
   return retval;
 }
 
+static GeisStatus
+_grail_be_get_integer_configuration(GeisUGSubscriptionStore ugstore,
+                                    uint64_t *prop,
+                                    UGSubscriptionProperty grail_prop,
+                                    GeisPointer geis_value)
+{
+  if (prop)
+  {
+    *((GeisInteger*)geis_value) = *prop;
+    return GEIS_STATUS_SUCCESS;
+  }
+  else
+  {
+    return _grail_be_get_ugsub_property(ugstore, grail_prop, geis_value);
+  }
+
+}
+
+static GeisStatus
+_grail_be_get_float_configuration(GeisUGSubscriptionStore ugstore,
+                                  float *prop,
+                                  UGSubscriptionProperty grail_prop,
+                                  GeisPointer geis_value)
+{
+  if (prop)
+  {
+    *((GeisFloat*)geis_value) = *prop;
+    return GEIS_STATUS_SUCCESS;
+  }
+  else
+  {
+    return _grail_be_get_ugsub_property(ugstore, grail_prop, geis_value);
+  }
+
+}
 
 /*
  * Dispatches the get-configuration call.
@@ -1765,42 +1799,75 @@
                             GeisPointer      item_value)
 {
   GeisStatus retval = GEIS_STATUS_NOT_SUPPORTED;
-  for (int i = 0; _grail_be_config_properties[i].geis_config_name; ++i)
+
+  struct GeisGrailSubscriptionData *subscription_data =
+    geis_subscription_pdata(subscription);
+  if (!subscription_data)
   {
-    if (0 == strcmp(item_name, _grail_be_config_properties[i].geis_config_name))
-    {
-      switch (_grail_be_config_properties[i].geis_value_type)
-      {
-        case GEIS_ATTR_TYPE_INTEGER:
-        {
-          uint64_t value;
-          retval = _grail_be_get_ugsub_property(subscription,
-                            _grail_be_config_properties[i].grail_property_name,
-                            &value);
-          *(GeisInteger*)item_value = value;
-          break;
-        }
-        case GEIS_ATTR_TYPE_FLOAT:
-        {
-          float value;
-          retval = _grail_be_get_ugsub_property(subscription,
-                            _grail_be_config_properties[i].grail_property_name,
-                            &value);
-          *(GeisFloat*)item_value = value;
-          break;
-        }
-        default:
-          geis_error("configuration value of unknown type requested");
-          retval = GEIS_STATUS_UNKNOWN_ERROR;
-          break;
-      }
-      break;
-    }
-  }
+    return retval;
+  }
+
+  #define GEIS_GRAIL_CHECK_GESTURE_CONFIG(gesture, Gesture, GESTURE) \
+  if (strcmp(item_name, GEIS_CONFIG_##GESTURE##_TIMEOUT) == 0) \
+  { \
+    retval = _grail_be_get_integer_configuration( \
+        subscription_data->ugstore, \
+        subscription_data->gesture##_timeout, \
+        UGSubscriptionProperty##Gesture##Timeout, \
+        item_value); \
+  } \
+  else if (strcmp(item_name, GEIS_CONFIG_##GESTURE##_THRESHOLD) == 0) \
+  { \
+    retval = _grail_be_get_float_configuration( \
+        subscription_data->ugstore, \
+        subscription_data->gesture##_threshold, \
+        UGSubscriptionProperty##Gesture##Threshold, \
+        item_value); \
+  }
+
+  GEIS_GRAIL_CHECK_GESTURE_CONFIG(drag, Drag, DRAG)
+  else GEIS_GRAIL_CHECK_GESTURE_CONFIG(pinch, Pinch, PINCH)
+  else GEIS_GRAIL_CHECK_GESTURE_CONFIG(rotate, Rotate, ROTATE)
+  else GEIS_GRAIL_CHECK_GESTURE_CONFIG(tap, Tap, TAP)
+
+  #undef GEIS_GRAIL_CHECK_GESTURE_CONFIG
 
   return retval;
 }
 
+static GeisStatus
+_grail_be_set_integer_configuration(GeisUGSubscriptionStore ugstore,
+                                    uint64_t **prop,
+                                    UGSubscriptionProperty grail_prop,
+                                    GeisPointer geis_value)
+{
+  if (!*prop)
+    *prop = malloc(sizeof(uint64_t));
+
+  **prop = *((GeisInteger*)geis_value);
+
+  if (ugstore)
+    return _grail_be_set_ugsub_property(ugstore, grail_prop, *prop);
+  else
+    return GEIS_STATUS_SUCCESS;
+}
+
+static GeisStatus
+_grail_be_set_float_configuration(GeisUGSubscriptionStore ugstore,
+                                  float **prop,
+                                  UGSubscriptionProperty grail_prop,
+                                  GeisPointer geis_value)
+{
+  if (!*prop)
+    *prop = malloc(sizeof(float));
+
+  **prop = *((GeisFloat*)geis_value);
+
+  if (ugstore)
+    return _grail_be_set_ugsub_property(ugstore, grail_prop, *prop);
+  else
+    return GEIS_STATUS_SUCCESS;
+}
 
 /*
  * Dispatches the set-configuration call.
@@ -1813,41 +1880,42 @@
 {
   GeisStatus retval = GEIS_STATUS_NOT_SUPPORTED;
 
-  for (int i = 0; _grail_be_config_properties[i].geis_config_name; ++i)
+  struct GeisGrailSubscriptionData *subscription_data =
+    geis_subscription_pdata(subscription);
+  if (!subscription_data)
   {
-    if (0 == strcmp(item_name, _grail_be_config_properties[i].geis_config_name))
-    {
-      switch (_grail_be_config_properties[i].geis_value_type)
-      {
-        case GEIS_ATTR_TYPE_INTEGER:
-        {
-          uint64_t value = *(GeisInteger *)item_value;
-          retval = _grail_be_set_ugsub_property(subscription,
-                            _grail_be_config_properties[i].grail_property_name,
-                            &value);
-          break;
-        }
-        case GEIS_ATTR_TYPE_FLOAT:
-        {
-          float value = *(GeisFloat *)item_value;
-          retval = _grail_be_set_ugsub_property(subscription,
-                            _grail_be_config_properties[i].grail_property_name,
-                            &value);
-          break;
-        }
-        default:
-          geis_error("configuration value of unknown type requested");
-          retval = GEIS_STATUS_UNKNOWN_ERROR;
-          break;
-      }
-      break;
-    }
-  }
+    subscription_data = calloc(1, sizeof(struct GeisGrailSubscriptionData));
+    geis_subscription_set_pdata(subscription, subscription_data);
+  }
+
+  #define GEIS_GRAIL_CHECK_GESTURE_CONFIG(gesture, Gesture, GESTURE) \
+  if (strcmp(item_name, GEIS_CONFIG_##GESTURE##_TIMEOUT) == 0) \
+  { \
+    retval = _grail_be_set_integer_configuration( \
+        subscription_data->ugstore, \
+        &(subscription_data->gesture##_timeout), \
+        UGSubscriptionProperty##Gesture##Timeout, \
+        item_value); \
+  } \
+  else if (strcmp(item_name, GEIS_CONFIG_##GESTURE##_THRESHOLD) == 0) \
+  { \
+    retval = _grail_be_set_float_configuration( \
+        subscription_data->ugstore, \
+        &(subscription_data->gesture##_threshold), \
+        UGSubscriptionProperty##Gesture##Threshold, \
+        item_value); \
+  }
+
+  GEIS_GRAIL_CHECK_GESTURE_CONFIG(drag, Drag, DRAG)
+  else GEIS_GRAIL_CHECK_GESTURE_CONFIG(pinch, Pinch, PINCH)
+  else GEIS_GRAIL_CHECK_GESTURE_CONFIG(rotate, Rotate, ROTATE)
+  else GEIS_GRAIL_CHECK_GESTURE_CONFIG(tap, Tap, TAP)
+
+  #undef GEIS_GRAIL_CHECK_GESTURE_CONFIG
 
   return retval;
 }
 
-
 static struct GeisBackendVtable gbe_vtbl = {
   _geis_grail_backend_construct,
   _geis_grail_backend_finalize,
@@ -1855,10 +1923,9 @@
   _grail_be_accept_gesture,
   _grail_be_reject_gesture,
   _grail_be_get_configuration,
-  _grail_be_set_configuration,
+  _grail_be_set_configuration
 };
 
-
 static GeisStatus
 _geis_grail_filter_gestures(GeisGrailBackend gbe,
                             GeisFilter       filter,
@@ -2029,6 +2096,45 @@
 }
 
 
+static void
+_geis_grail_set_ugsubscription_properties(GeisGrailBackend gbe,
+                                          UGSubscription ugsub,
+                                          GeisSubscription subscription)
+{
+  struct GeisGrailSubscriptionData *subscription_data = geis_subscription_pdata(subscription);
+
+  GeisBoolean geis_use_atomic_gestures = GEIS_FALSE;
+  geis_get_configuration(gbe->geis,
+                         GEIS_CONFIG_ATOMIC_GESTURES,
+                         &geis_use_atomic_gestures);
+  int grail_use_atomic_gestures = (geis_use_atomic_gestures == GEIS_TRUE);
+  grail_subscription_set_property(ugsub,
+                                  UGSubscriptionPropertyAtomicGestures,
+                                  &grail_use_atomic_gestures);
+
+
+  #define GEIS_GRAIL_SYNC_GESTURE_PROPERTIES(gesture, Gesture) \
+  if (subscription_data->gesture##_timeout) \
+  { \
+    grail_subscription_set_property(ugsub, \
+                                    UGSubscriptionProperty##Gesture##Timeout, \
+                                    subscription_data->gesture##_timeout); \
+  } \
+  if (subscription_data->gesture##_threshold) \
+  { \
+    grail_subscription_set_property(ugsub, \
+                                    UGSubscriptionProperty##Gesture##Threshold, \
+                                    subscription_data->gesture##_threshold); \
+  }
+
+  GEIS_GRAIL_SYNC_GESTURE_PROPERTIES(drag, Drag);
+  GEIS_GRAIL_SYNC_GESTURE_PROPERTIES(pinch, Pinch);
+  GEIS_GRAIL_SYNC_GESTURE_PROPERTIES(rotate, Rotate);
+  GEIS_GRAIL_SYNC_GESTURE_PROPERTIES(tap, Tap);
+
+  #undef GEIS_GRAIL_SYNC_GESTURE_PROPERTIES
+}
+
 /**
  * Activates a subscription for a (device, region).
  */
@@ -2042,56 +2148,45 @@
   GeisStatus status = GEIS_STATUS_UNKNOWN_ERROR;
   GeisInteger device_id = geis_device_id(device);
   UFDevice ufdevice = _grail_be_ufdevice_from_device_id(gbe, device_id);
-  GeisUGSubscriptionStore ugstore = geis_subscription_pdata(subscription);
+  struct GeisGrailSubscriptionData *subscription_data = geis_subscription_pdata(subscription);
+  GeisUGSubscriptionStore ugstore = subscription_data->ugstore;
   UGSubscription ugsub = geis_ugsubscription_get_ugsubscription(ugstore,
                                                                 filter,
                                                                 ufdevice,
                                                                 window_id);
+
+  if (!ugsub)
+  {
+    ugsub = geis_ugsubscription_create_ugsubscription(ugstore,
+                                                      filter,
+                                                      ufdevice,
+                                                      window_id);
+    _geis_grail_set_ugsubscription_properties(gbe, ugsub, subscription);
+  }
+
   if (!ugsub)
   {
     geis_error("can not retrieve UGSubscription for (device, window)");
     goto final_exit;
   }
 
-  UGStatus ugstatus = UGStatusErrorGeneric;
-  GeisBoolean geis_use_atomic_gestures = GEIS_FALSE;
-  geis_get_configuration(gbe->geis,
-                         GEIS_CONFIG_ATOMIC_GESTURES,
-                         &geis_use_atomic_gestures);
-  int grail_use_atomic_gestures = (geis_use_atomic_gestures == GEIS_TRUE);
-  ugstatus = grail_subscription_set_property(ugsub,
-                                             UGSubscriptionPropertyAtomicGestures,
-                                             &grail_use_atomic_gestures);
-
   status = _geis_grail_filter_gestures(gbe, filter, subscription, ugsub);
 
-  UGGestureTypeMask ugmask;
-  ugstatus = grail_subscription_get_property(ugsub,
-                                             UGSubscriptionPropertyMask,
-                                             &ugmask);
-  if (ugstatus != UGStatusSuccess)
-  {
-    geis_error("failed to get UGSubscription mask");
-    goto final_exit;
-  }
-
   if (filter)
     geis_debug("subscription='%s' filter='%s' device=%d '%s' window=0x%08x "
-               "ugsub=%p atomic=%d",
+               "ugsub=%p",
                geis_subscription_name(subscription),
                geis_filter_name(filter),
                device_id, geis_device_name(device),
                window_id,
-               (void *)ugsub,
-               grail_use_atomic_gestures);
+               (void *)ugsub);
   else
     geis_debug("subscription='%s' no-filter device=%d '%s' window=0x%08x "
-               "ugsub=%p atomic=%d",
+               "ugsub=%p",
                geis_subscription_name(subscription),
                device_id, geis_device_name(device),
                window_id,
-               (void *)ugsub,
-               grail_use_atomic_gestures);
+               (void *)ugsub);
 
   status = geis_grail_window_grab_store_grab(gbe->window_grabs, window_id);
   if (status != GEIS_STATUS_SUCCESS)
@@ -2100,7 +2195,7 @@
     goto final_exit;
   }
 
-  ugstatus = grail_subscription_activate(gbe->grail, ugsub);
+  UGStatus ugstatus = grail_subscription_activate(gbe->grail, ugsub);
   if (ugstatus != UGStatusSuccess)
   {
     status = GEIS_STATUS_UNKNOWN_ERROR;
@@ -2264,16 +2359,22 @@
     goto final_exit;
   }
 
-  GeisUGSubscriptionStore ugstore = geis_subscription_pdata(subscription);
-  if (ugstore == NULL)
-  {
-    ugstore = geis_ugsubscription_store_new();
-    if (!ugstore)
+  struct GeisGrailSubscriptionData *subscription_data =
+    geis_subscription_pdata(subscription);
+  if (!subscription_data)
+  {
+    subscription_data = calloc(1, sizeof(struct GeisGrailSubscriptionData));
+    geis_subscription_set_pdata(subscription, subscription_data);
+  }
+
+  if (subscription_data->ugstore == NULL)
+  {
+    subscription_data->ugstore = geis_ugsubscription_store_new();
+    if (!subscription_data->ugstore)
     {
       geis_error("error creating grail subscription store");
       goto final_exit;
     }
-    geis_subscription_set_pdata(subscription, ugstore);
   }
 
   if (geis_subscription_filter_count(subscription) > 0)
@@ -2313,7 +2414,9 @@
     goto final_exit;
   }
 
-  GeisUGSubscriptionStore ugstore = geis_subscription_pdata(subscription);
+  struct GeisGrailSubscriptionData *subscription_data =
+    geis_subscription_pdata(subscription);
+  GeisUGSubscriptionStore ugstore = subscription_data->ugstore;
   if (ugstore)
   {
     for (GeisSize i = 0; i < geis_ugsubscription_count(ugstore); ++i)
@@ -2347,6 +2450,34 @@
   return status;
 }
 
+/*
+  Frees the memory allocated for the GEIS subscription private data
+ */
+void
+geis_grail_backend_free_subscription_pdata(GeisGrailBackend gbe GEIS_UNUSED,
+                                           GeisSubscription subscription)
+{
+  struct GeisGrailSubscriptionData *subscription_data =
+    geis_subscription_pdata(subscription);
+  if (!subscription_data)
+    return;
+
+  if (subscription_data->ugstore)
+    geis_ugsubscription_delete(subscription_data->ugstore);
+
+  free(subscription_data->drag_timeout);
+  free(subscription_data->drag_threshold);
+  free(subscription_data->pinch_timeout);
+  free(subscription_data->pinch_threshold);
+  free(subscription_data->rotate_timeout);
+  free(subscription_data->rotate_threshold);
+  free(subscription_data->tap_timeout);
+  free(subscription_data->tap_threshold);
+
+  free(subscription_data);
+
+  geis_subscription_set_pdata(subscription, NULL);
+}
 
 /**
  * Registers the back end with the GEIS back end registry.
 
=== modified file 'libgeis/backend/grail/geis_grail_backend.h'
--- libgeis/backend/grail/geis_grail_backend.h	2012-03-31 16:15:35 +0000
+++ libgeis/backend/grail/geis_grail_backend.h	2012-09-05 18:11:18 +0000
@@ -40,4 +40,11 @@
 geis_grail_backend_deactivate_subscription(GeisGrailBackend gbe,
                                            GeisSubscription subscription);
 
+/**
+ * Frees the memory allocated for the GEIS subscription private data
+ */
+void
+geis_grail_backend_free_subscription_pdata(GeisGrailBackend gbe,
+                                           GeisSubscription subscription);
+
 #endif /* GEIS_BACKEND_GRAIL_BACKEND_H_ */
 
=== modified file 'libgeis/backend/grail/geis_grail_token.c'
--- libgeis/backend/grail/geis_grail_token.c	2012-03-31 16:15:35 +0000
+++ libgeis/backend/grail/geis_grail_token.c	2012-09-05 18:11:18 +0000
@@ -126,6 +126,15 @@
   return status;
 }
 
+/**
+ * Frees the memory allocated for the GEIS subscription private data
+ */
+static void
+_geis_grail_token_free_subscription_pdata(GeisBackendToken token, GeisSubscription sub)
+{
+  GeisGrailToken gdt = _geis_grail_token_from_geis_token(token);
+  geis_grail_backend_free_subscription_pdata(gdt->be, sub);
+}
 
 /**
  * Creates Grail-back-end-specific back end token.
@@ -140,6 +149,7 @@
     _geis_grail_token_compose,
     _geis_grail_token_activate,
     _geis_grail_token_deactivate,
+    _geis_grail_token_free_subscription_pdata
   };
 
   GeisGrailToken token = _geis_grail_token_allocate();
 
=== modified file 'libgeis/backend/grail/geis_ugsubscription_store.c'
--- libgeis/backend/grail/geis_ugsubscription_store.c	2012-06-21 21:17:25 +0000
+++ libgeis/backend/grail/geis_ugsubscription_store.c	2012-09-05 18:11:18 +0000
@@ -59,7 +59,8 @@
 {
   for (GeisSize i = 0; i < geis_bag_count(store); ++i)
   {
-    grail_subscription_delete(*(UGSubscription*)geis_bag_at(store, i));
+    struct GeisUGSubscription * s = (struct GeisUGSubscription *)geis_bag_at(store, i);
+    grail_subscription_delete(s->ugsub);
   }
   geis_bag_delete(store);
 }
@@ -86,6 +87,53 @@
   return ((GeisUGSubscription)geis_bag_at(store, index))->ugsub;
 }
 
+/*
+ * Creates a new UGSubscription and adds it to the store.
+ */
+UGSubscription
+geis_ugsubscription_create_ugsubscription(GeisUGSubscriptionStore store,
+                                          GeisFilter              filter,
+                                          UFDevice                device,
+                                          GeisInteger             region_id)
+{
+  UGSubscription ugsub = NULL;
+
+  UGStatus ugstatus = grail_subscription_new(&ugsub);
+  if (ugstatus != UGStatusSuccess)
+  {
+    geis_error("failed to create grail subscription");
+    goto final_exit;
+  }
+
+  ugstatus = grail_subscription_set_property(ugsub,
+      UGSubscriptionPropertyDevice,
+      &device);
+  if (ugstatus != UGStatusSuccess)
+  {
+    geis_error("failed to set UGSubscription device property");
+  }
+
+  UFWindowId window_id = frame_x11_create_window_id(region_id);
+  ugstatus = grail_subscription_set_property(ugsub,
+      UGSubscriptionPropertyWindow,
+      &window_id);
+  if (ugstatus != UGStatusSuccess)
+  {
+    geis_error("failed to set UGSubscription window property");
+  }
+
+  struct GeisUGSubscription s = {
+    .filter    = filter,
+    .device    = device,
+    .region_id = region_id,
+    .ugsub     = ugsub
+  };
+  geis_bag_append(store, &s);
+
+ final_exit:
+  return ugsub;
+}
+
 
 /*
  * Gets a UGSubscription from the store.
@@ -97,6 +145,7 @@
                                        GeisInteger             region_id)
 {
   UGSubscription ugsub = NULL;
+
   for (GeisSize i = 0; i < geis_bag_count(store); ++i)
   {
     GeisUGSubscription s = (GeisUGSubscription)geis_bag_at(store, i);
@@ -107,42 +156,6 @@
     }
   }
 
-  if (!ugsub)
-  {
-    UGStatus ugstatus = grail_subscription_new(&ugsub);
-    if (ugstatus != UGStatusSuccess)
-    {
-      geis_error("failed to create grail subscription");
-      goto final_exit;
-    }
-
-    ugstatus = grail_subscription_set_property(ugsub,
-                                               UGSubscriptionPropertyDevice,
-                                               &device);
-    if (ugstatus != UGStatusSuccess)
-    {
-      geis_error("failed to set UGSubscription device property");
-    }
-
-    UFWindowId window_id = frame_x11_create_window_id(region_id);
-    ugstatus = grail_subscription_set_property(ugsub,
-                                               UGSubscriptionPropertyWindow,
-                                               &window_id);
-    if (ugstatus != UGStatusSuccess)
-    {
-      geis_error("failed to set UGSubscription window property");
-    }
-
-    struct GeisUGSubscription s = {
-      .filter    = filter,
-      .device    = device,
-      .region_id = region_id,
-      .ugsub     = ugsub
-    };
-    geis_bag_append(store, &s);
-  }
-
-final_exit:
   return ugsub;
 }
 
 
=== modified file 'libgeis/backend/grail/geis_ugsubscription_store.h'
--- libgeis/backend/grail/geis_ugsubscription_store.h	2012-06-21 21:17:25 +0000
+++ libgeis/backend/grail/geis_ugsubscription_store.h	2012-09-05 18:11:18 +0000
@@ -64,8 +64,7 @@
  * @param[in] store    The grail subscription store.
  * @param[in] index    Indicates a ugsub.
  *
- * Gets a UGSubscription from the store for the identified device and region.
- * If no such UGSubscription is available, it creates one first.
+ * Gets a UGSubscription from the store at the given index.
  *
  * @returns a valid UGSubscription or NULL to indicate failure.
  */
@@ -73,6 +72,27 @@
 geis_ugsubscription_get_ugsubscription_at(GeisUGSubscriptionStore store,
                                           GeisSize                index);
 
+/*
+ * Creates a new UGSubscription and adds it to the store.
+ * @param[in] store     The grail subscription store.
+ * @param[in] filter    Identifies a filter.
+ * @param[in] device    Identifies a device.
+ * @param[in] region_id Identifies a region.
+ *
+ * Creates a new UGSubscription for the identified filter, device,
+ * and region. It's added to the store. A separate grail subscription
+ * is required for each (filter, device, window) because filters on a
+ * geis subscription are ORed, meaning each filter may have a different
+ * minimum touch requirement.
+ *
+ * @returns a valid UGSubscription or NULL to indicate failure.
+ */
+UGSubscription
+geis_ugsubscription_create_ugsubscription(GeisUGSubscriptionStore store,
+                                          GeisFilter              filter,
+                                          UFDevice                device,
+                                          GeisInteger             region_id);
+
 /**
  * Gets a UGSubscription from the store.
  * @param[in] store     The grail subscription store.
@@ -85,8 +105,6 @@
  * device, window) because filters on a geis subscription are ORed, meaning each
  * filter may have a different minimum touch requirement.
  *
- * If no such UGSubscription is available, it creates one first.
- *
  * @returns a valid UGSubscription or NULL to indicate failure.
  */
 UGSubscription
 
=== modified file 'libgeis/backend/test_fixture/geis_backend_test_fixture.c'
--- libgeis/backend/test_fixture/geis_backend_test_fixture.c	2012-07-24 16:04:12 +0000
+++ libgeis/backend/test_fixture/geis_backend_test_fixture.c	2012-09-05 18:11:18 +0000
@@ -82,6 +82,7 @@
 static void             _token_compose(GeisBackendToken, GeisBackendToken);
 static GeisStatus       _token_activate(GeisBackendToken, GeisSubscription);
 static GeisStatus       _token_deactivate(GeisBackendToken, GeisSubscription);
+static void             _token_free_subscription_pdata(GeisBackendToken, GeisSubscription);
 
 static struct GeisBackendTokenVtable _token_vtbl = {
   _token_clone,
@@ -89,6 +90,7 @@
   _token_compose,
   _token_activate,
   _token_deactivate,
+  _token_free_subscription_pdata
 };
 
 static GeisGestureClass g_poke_class = NULL;
@@ -424,6 +426,11 @@
   return status;
 }
 
+void
+_token_free_subscription_pdata(GeisBackendToken gbtoken GEIS_UNUSED,
+                               GeisSubscription subscription GEIS_UNUSED)
+{
+}
 
 __attribute__((constructor))
 static void _register_test_fixture()
 
=== modified file 'libgeis/geis_backend_protected.h'
--- libgeis/geis_backend_protected.h	2012-04-17 23:43:04 +0000
+++ libgeis/geis_backend_protected.h	2012-09-05 18:11:18 +0000
@@ -58,6 +58,7 @@
   void             (* compose)(GeisBackendToken, GeisBackendToken);
   GeisStatus       (* activate)(GeisBackendToken, GeisSubscription);
   GeisStatus       (* deactivate)(GeisBackendToken, GeisSubscription);
+  void             (* free_subscription_pdata)(GeisBackendToken, GeisSubscription);
 } *GeisBackendTokenVtable;
 
 
 
=== modified file 'libgeis/geis_backend_token.c'
--- libgeis/geis_backend_token.c	2011-10-17 16:52:22 +0000
+++ libgeis/geis_backend_token.c	2012-09-05 18:11:18 +0000
@@ -77,4 +77,12 @@
   return token->vtbl->deactivate(token, subscription);
 }
 
-
+/**
+ * Frees the private data that the backend allocated for a given subscription
+ */
+void
+geis_backend_token_free_subscription_pdata(GeisBackendToken token,
+                                           GeisSubscription subscription)
+{
+  token->vtbl->free_subscription_pdata(token, subscription);
+}
 
=== modified file 'libgeis/geis_backend_token.h'
--- libgeis/geis_backend_token.h	2011-10-17 16:52:22 +0000
+++ libgeis/geis_backend_token.h	2012-09-05 18:11:18 +0000
@@ -101,4 +101,15 @@
 geis_backend_token_deactivate(GeisBackendToken token,
                               GeisSubscription subscription);
 
+/**
+ * Frees the private data that the backend allocated for a given subscription
+ *
+ * @param[in] token          The backend token.
+ * @param[in] subscription  The subscription that will have its backend private data
+ *                          freed
+ */
+void
+geis_backend_token_free_subscription_pdata(GeisBackendToken token,
+                                           GeisSubscription subscription);
+
 #endif /* GEIS_BACKEND_TOKEN_H_ */
 
=== modified file 'libgeis/geis_subscription.c'
--- libgeis/geis_subscription.c	2012-06-21 21:17:25 +0000
+++ libgeis/geis_subscription.c	2012-09-05 18:11:18 +0000
@@ -337,8 +337,6 @@
   if (sub->sub_backend_token)
   {
     status = geis_backend_token_deactivate(sub->sub_backend_token, sub);
-    geis_backend_token_delete(sub->sub_backend_token);
-    sub->sub_backend_token = NULL;
   }
   return status;
 }
@@ -426,6 +424,14 @@
 {
   geis_debug("destroying subscription \"%s\" id %d", sub->sub_name, sub->sub_id);
   geis_subscription_deactivate(sub);
+
+  if (sub->sub_backend_token)
+  {
+    geis_backend_token_free_subscription_pdata(sub->sub_backend_token, sub);
+    geis_backend_token_delete(sub->sub_backend_token);
+    sub->sub_backend_token = NULL;
+  }
+
   if (sub->sub_geis != NULL)
   {
     geis_unref(sub->sub_geis);
@@ -507,6 +513,13 @@
     geis_error("error adding filter to subscription");
     goto error_exit;
   }
+  else
+  {
+    /* since we're taking ownership of the filter, we're removing the initial
+       reference that belonged to the API user, created in geis_filter_new().
+       Now the only remaining reference is the one held by our geis_filter_bag */
+    geis_filter_unref(filter);
+  }
 
 error_exit:
   return status;
 
=== modified file 'testsuite/geis2/Makefile.am'
--- testsuite/geis2/Makefile.am	2012-07-24 22:18:36 +0000
+++ testsuite/geis2/Makefile.am	2012-09-05 18:11:18 +0000
@@ -48,6 +48,7 @@
 gtest_geis2_grail_backend_SOURCES = \
 	gtest_grail_backend.h gtest_grail_backend.cpp \
 	gtest_gbe_accept_ended_grail_gesture.cpp \
+	gtest_gbe_configure_new_devices.cpp \
 	gtest_gbe_deactivate_sub.cpp \
 	gtest_gbe_direct_touch_coords.cpp \
 	gtest_gbe_construction_finished.cpp \
 
=== added file 'testsuite/geis2/gtest_gbe_configure_new_devices.cpp'
--- testsuite/geis2/gtest_gbe_configure_new_devices.cpp	1970-01-01 00:00:00 +0000
+++ testsuite/geis2/gtest_gbe_configure_new_devices.cpp	2012-09-05 18:11:18 +0000
@@ -0,0 +1,201 @@
+#include "gtest_grail_backend.h"
+#include "x11_mocks.h"
+
+/*
+  Regression test for bug LP#1045785
+  https://bugs.launchpad.net/geis/+bug/1045785
+
+  Steps to reproduce the issue:
+    1 - create a subscription, set some configuration options
+        (geis_subscription_set_configuration) and activate it.
+    2 - connect a new multitouch device to your computer (e.g. pair an Apple
+        Magic Trackpad)
+
+  Expected outcome:
+    Gestures from that new multitouch device follow the configurations set by
+    the subscription.
+ */
+
+class ConfigureNewDevice : public Geis2GrailBackendBase
+{
+ protected:
+  ConfigureNewDevice();
+
+  virtual void OnEventInitComplete(GeisEvent event);
+  virtual void OnEventClassAvailable(GeisEvent event);
+  virtual void OnEventGestureBegin(GeisEvent event);
+
+  void CreateSubscription();
+  void SendXInput2Events();
+
+  /* signals that a new device has been added */
+  void SendXIDeviceAddedEvent();
+
+  GeisSubscription _subscription;
+  GeisGestureClass _drag_class;
+};
+
+ConfigureNewDevice::ConfigureNewDevice()
+    : _subscription(nullptr),
+      _drag_class(nullptr)
+{
+}
+
+void ConfigureNewDevice::OnEventInitComplete(GeisEvent event)
+{
+  CreateSubscription();
+
+  /* The new device comes only after the subscription has been created and
+     configured */
+  CreateXMockTouchScreenDevice();
+  SendXIDeviceAddedEvent();
+
+  SendXInput2Events();
+}
+
+void ConfigureNewDevice::OnEventClassAvailable(GeisEvent event)
+{
+  GeisAttr attr = geis_event_attr_by_name(event, GEIS_EVENT_ATTRIBUTE_CLASS);
+  GeisGestureClass gesture_class =
+    reinterpret_cast<GeisGestureClass>(geis_attr_value_to_pointer(attr));
+
+  if (strcmp(geis_gesture_class_name(gesture_class), GEIS_GESTURE_DRAG) == 0)
+  {
+    _drag_class = gesture_class;
+    geis_gesture_class_ref(gesture_class);
+  }
+}
+
+void ConfigureNewDevice::OnEventGestureBegin(GeisEvent event)
+{
+  /* There should be no drag gesture since we set a huge drag threshold. */
+
+  GeisAttr attr = geis_event_attr_by_name(event, GEIS_EVENT_ATTRIBUTE_GROUPSET);
+  GeisGroupSet group_set =
+    reinterpret_cast<GeisGroupSet>(geis_attr_value_to_pointer(attr));
+  GeisGroup group = geis_groupset_group(group_set, 0);
+  GeisFrame frame = geis_group_frame(group, 0);
+
+  ASSERT_NE(nullptr, _drag_class);
+  GeisBoolean is_drag = geis_frame_is_class(frame, _drag_class);
+  ASSERT_FALSE(is_drag);
+}
+
+void ConfigureNewDevice::CreateSubscription()
+{
+  GeisStatus status = GEIS_STATUS_UNKNOWN_ERROR;
+  GeisFilter filter = nullptr;
+
+  _subscription = geis_subscription_new(_geis, "2-fingers drag",
+      GEIS_SUBSCRIPTION_NONE);
+
+  filter = geis_filter_new(_geis, "filter");
+  ASSERT_NE(nullptr, filter);
+
+  status = geis_filter_add_term(filter,
+      GEIS_FILTER_CLASS,
+      GEIS_CLASS_ATTRIBUTE_NAME, GEIS_FILTER_OP_EQ, GEIS_GESTURE_DRAG,
+      GEIS_GESTURE_ATTRIBUTE_TOUCHES, GEIS_FILTER_OP_EQ, 2,
+      nullptr);
+  ASSERT_EQ(GEIS_STATUS_SUCCESS, status);
+
+  status = geis_filter_add_term(filter,
+      GEIS_FILTER_REGION,
+      GEIS_REGION_ATTRIBUTE_WINDOWID, GEIS_FILTER_OP_EQ,
+      DefaultRootWindow(xmock_display),
+      nullptr);
+  ASSERT_EQ(GEIS_STATUS_SUCCESS, status);
+
+  status = geis_subscription_add_filter(_subscription, filter);
+  ASSERT_EQ(GEIS_STATUS_SUCCESS, status);
+
+  /* Set a huge threshold (in meters) so that no drag can be recognized */
+  GeisFloat drag_threshold = 1000.0f;
+  status = geis_subscription_set_configuration(_subscription,
+      GEIS_CONFIG_DRAG_THRESHOLD, &drag_threshold);
+  ASSERT_EQ(GEIS_STATUS_SUCCESS, status);
+
+  status = geis_subscription_activate(_subscription);
+  ASSERT_EQ(GEIS_STATUS_SUCCESS, status);
+}
+
+void ConfigureNewDevice::SendXInput2Events()
+{
+  /* Emulate a simple 2 fingers drag */
+
+  /* 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 += 2;
+
+  /* touch id  */
+  SendTouchOwnershipEvent(1);
+  SendTouchOwnershipEvent(2);
+
+  xmock_server_time += 20;
+
+  SendTouchEvent(XI_TouchUpdate, 1, 10.0f, 25.0f);
+  SendTouchEvent(XI_TouchUpdate, 2, 20.0f, 25.0f);
+
+  xmock_server_time += 20;
+
+  SendTouchEvent(XI_TouchUpdate, 1, 10.0f, 35.0f);
+  SendTouchEvent(XI_TouchUpdate, 2, 20.0f, 35.0f);
+
+  xmock_server_time += 20;
+
+  SendTouchEvent(XI_TouchEnd, 1, 10.0f, 50.0f);
+  SendTouchEvent(XI_TouchEnd, 2, 20.0f, 50.0f);
+}
+
+void ConfigureNewDevice::SendXIDeviceAddedEvent()
+{
+  XEvent event;
+  XGenericEventCookie *xcookie = 0;
+  XIHierarchyEvent *hierarchy_event;
+  XIHierarchyInfo *info;
+
+  info = (XIHierarchyInfo*)calloc(1, sizeof(XIHierarchyInfo));
+  info->deviceid = xmock_devices[0].deviceid;
+  info->enabled = True;
+  info->flags = XISlaveAdded;
+
+  hierarchy_event = (XIHierarchyEvent*)calloc(1, sizeof(XIHierarchyEvent));
+  hierarchy_event->type = GenericEvent;
+  hierarchy_event->serial = _xevent_serial_number++;
+  hierarchy_event->display = xmock_display;
+  hierarchy_event->extension = xmock_xi2_opcode;
+  hierarchy_event->evtype = XI_HierarchyChanged;
+  hierarchy_event->time = xmock_server_time;
+  hierarchy_event->flags = XISlaveAdded;
+  hierarchy_event->num_info = 1;
+  hierarchy_event->info = info;
+
+  event.type = GenericEvent;
+  xcookie = &event.xcookie;
+  xcookie->extension = xmock_xi2_opcode;
+  xcookie->evtype = XI_HierarchyChanged;
+  xcookie->data = hierarchy_event;
+  xmock_add_to_event_queue(&event);
+}
+
+TEST_F(ConfigureNewDevice, Test)
+{
+  _geis = geis_new(GEIS_INIT_GRAIL_BACKEND,
+                   GEIS_INIT_NO_ATOMIC_GESTURES,
+                   nullptr);
+  ASSERT_NE(nullptr, _geis);
+
+  Run();
+
+  if (_subscription)
+    geis_subscription_delete(_subscription);
+
+  if (_drag_class)
+    geis_gesture_class_unref(_drag_class);
+
+  geis_delete(_geis);
+
+  DestroyXMockDevices();
+}
 
=== modified file 'testsuite/geis2/gtest_grail_backend.cpp'
--- testsuite/geis2/gtest_grail_backend.cpp	2012-07-05 17:13:06 +0000
+++ testsuite/geis2/gtest_grail_backend.cpp	2012-09-05 18:11:18 +0000
@@ -168,6 +168,9 @@
       case GEIS_EVENT_INIT_COMPLETE:
         OnEventInitComplete(event);
         break;
+      case GEIS_EVENT_CLASS_AVAILABLE:
+        OnEventClassAvailable(event);
+        break;
       case GEIS_EVENT_GESTURE_BEGIN:
         OnEventGestureBegin(event);
         break;
 
=== modified file 'testsuite/geis2/gtest_grail_backend.h'
--- testsuite/geis2/gtest_grail_backend.h	2012-07-05 17:13:06 +0000
+++ testsuite/geis2/gtest_grail_backend.h	2012-09-05 18:11:18 +0000
@@ -47,6 +47,7 @@
   void DestroyXMockDevices();
 
   virtual void OnEventInitComplete(GeisEvent event) {}
+  virtual void OnEventClassAvailable(GeisEvent event) {}
   virtual void OnEventGestureBegin(GeisEvent event) {}
   virtual void OnEventGestureUpdate(GeisEvent event) {}
   virtual void OnEventGestureEnd(GeisEvent event) {}
@@ -59,11 +60,12 @@
   void GetGestureTimestampInEvent(GeisInteger *timestamp, GeisEvent event);
 
   Geis _geis;
- private:
 
   /* holds the serial number to be used on the next synthetic XEvent */
   int _xevent_serial_number;
 
+ private:
+
   bool DispatchAndProcessEvents();
 
   void AcceptRejectGestureInEvent(GeisEvent event, bool accept);
 
=== modified file 'testsuite/x11_mocks/x11_mocks.c'
--- testsuite/x11_mocks/x11_mocks.c	2012-04-25 17:23:11 +0000
+++ testsuite/x11_mocks/x11_mocks.c	2012-09-05 18:11:18 +0000
@@ -194,6 +194,14 @@
       free(device_event->valuators.mask);
       free(device_event->valuators.values);
     }
+    else if (cookie->evtype == XI_HierarchyChanged)
+    {
+      XIHierarchyEvent *hierarchy_event = (XIHierarchyEvent*) cookie->data;
+      for (int i = 0; i < hierarchy_event->num_info; ++i)
+      {
+        free(&(hierarchy_event->info[i])); 
+      }
+    }
     free(cookie->data);
   }
 }