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

[Daniel d'Andrada]

grail_backend: Properly activate a subscription that has no filters.

Fixes bug https://launchpad.net/bugs/939679

[Stephen M. Webb (bregma)]

In the non-filter debug statement (on or around line 432), could you please make the fact that there is no filter on the subscription explicit? This is important debug information and just leaving it out makes it harder to spot during bug analysis without reading the code.

Other than that, this is good.

review: Approve

=== modified file 'libutouch-geis/backend/grail/geis_grail_backend.c'
--- libutouch-geis/backend/grail/geis_grail_backend.c	2012-02-17 23:51:04 +0000
+++ libutouch-geis/backend/grail/geis_grail_backend.c	2012-02-24 11:49:19 +0000
@@ -1232,11 +1232,11 @@
                                    | UGGestureTypeTap
                                    | UGGestureTypeTouch;
   UGGestureTypeMask ugmask = 0;
-  unsigned int start_touches = 0;
+  unsigned int start_touches = 1;
   unsigned int min_touches = 1;
   unsigned int max_touches = 5;
 
-  for (GeisSize i = 0; i < geis_filter_term_count(filter); ++i)
+  for (GeisSize i = 0; filter && i < geis_filter_term_count(filter); ++i)
   {
     GeisFilterTerm term = geis_filter_term(filter, i);
     if (GEIS_FILTER_CLASS == geis_filter_term_facility(term))
@@ -1419,14 +1419,23 @@
     goto final_exit;
   }
 
-  geis_debug("subscription='%s' filter='%s' device=%d '%s' window=0x%08x "
-             "ugsub=%p atomic=%d",
-             geis_subscription_name(subscription),
-             geis_filter_name(filter),
-             device_id, geis_device_name(device),
-             window_id,
-             (void *)ugsub,
-             grail_use_atomic_gestures);
+  if (filter)
+    geis_debug("subscription='%s' filter='%s' device=%d '%s' window=0x%08x "
+               "ugsub=%p atomic=%d",
+               geis_subscription_name(subscription),
+               geis_filter_name(filter),
+               device_id, geis_device_name(device),
+               window_id,
+               (void *)ugsub,
+               grail_use_atomic_gestures);
+  else
+    geis_debug("subscription='%s' device=%d '%s' window=0x%08x "
+               "ugsub=%p atomic=%d",
+               geis_subscription_name(subscription),
+               device_id, geis_device_name(device),
+               window_id,
+               (void *)ugsub,
+               grail_use_atomic_gestures);
 
   geis_grail_window_grab_store_grab(gbe->window_grabs, window_id);
   ugstatus = grail_subscription_activate(gbe->grail, ugsub);
@@ -1455,35 +1464,39 @@
   {
     GeisDevice device = geis_device_bag_device(device_bag, d);
     GeisBoolean has_region_term = GEIS_FALSE;
-    for (GeisSize i = 0; i < geis_filter_term_count(filter); ++i)
+    if (filter)
     {
-      GeisFilterTerm term = geis_filter_term(filter, i);
-      if (GEIS_FILTER_REGION == geis_filter_term_facility(term))
+      for (GeisSize i = 0; i < geis_filter_term_count(filter); ++i)
       {
-        GeisInteger         window_id = 0;
-        GeisAttr            attr      = geis_filter_term_attr(term);
-        GeisString          name      = geis_attr_name(attr);
-        GeisFilterOperation operation = geis_filter_term_operation(term);
+        GeisFilterTerm term = geis_filter_term(filter, i);
+        if (GEIS_FILTER_REGION == geis_filter_term_facility(term))
+        {
+          GeisInteger         window_id = 0;
+          GeisAttr            attr      = geis_filter_term_attr(term);
+          GeisString          name      = geis_attr_name(attr);
+          GeisFilterOperation operation = geis_filter_term_operation(term);
 
-        if (0 == strcmp(name, GEIS_REGION_ATTRIBUTE_WINDOWID)
-          && operation == GEIS_FILTER_OP_EQ)
-        {
-          window_id = geis_attr_value_to_integer(attr);
-          has_region_term = GEIS_TRUE;
-          status = _geis_grail_activate_for_device_region(gbe,
-                                                          filter,
-                                                          device,
-                                                          window_id,
-                                                          subscription);
-        }
-        else
-        {
-          geis_warning("unhandled region filter term");
+          if (0 == strcmp(name, GEIS_REGION_ATTRIBUTE_WINDOWID)
+              && operation == GEIS_FILTER_OP_EQ)
+          {
+            window_id = geis_attr_value_to_integer(attr);
+            has_region_term = GEIS_TRUE;
+            status = _geis_grail_activate_for_device_region(gbe,
+                filter,
+                device,
+                window_id,
+                subscription);
+          }
+          else
+          {
+            geis_warning("unhandled region filter term");
+          }
         }
       }
     }
 
-    /* If no region terms were found, use the root window. */
+    /* If no region terms were found, or there's no filter, use the root
+       window. */
     if (!has_region_term)
     {
       status = _geis_grail_activate_for_device_region(gbe,
@@ -1496,28 +1509,12 @@
   return status;
 }
 
-
-/*
- * Activates a GEIS subscription on the back end.
- */
-GeisStatus
-geis_grail_backend_activate_subscription(GeisGrailBackend gbe,
-                                         GeisSubscription subscription)
+static GeisStatus
+_geis_grail_activate_with_filters(GeisGrailBackend gbe,
+                                  GeisSubscription subscription)
 {
   GeisStatus status = GEIS_STATUS_UNKNOWN_ERROR;
 
-  GeisUGSubscriptionStore ugstore = geis_subscription_pdata(subscription);
-  if (ugstore == NULL)
-  {
-    ugstore = geis_ugsubscription_store_new();
-    if (!ugstore)
-    {
-      geis_error("error creating grail subscription store");
-      goto final_exit;
-    }
-    geis_subscription_set_pdata(subscription, ugstore);
-  }
-
   for (GeisFilterIterator it = geis_subscription_filter_begin(subscription);
        it != geis_subscription_filter_end(subscription);
        it = geis_subscription_filter_next(subscription, it))
@@ -1563,6 +1560,39 @@
     geis_device_bag_delete(selected_devices);
   }
 
+  return status;
+}
+
+/*
+ * Activates a GEIS subscription on the back end.
+ */
+GeisStatus
+geis_grail_backend_activate_subscription(GeisGrailBackend gbe,
+                                         GeisSubscription subscription)
+{
+  GeisStatus status = GEIS_STATUS_UNKNOWN_ERROR;
+
+  GeisUGSubscriptionStore ugstore = geis_subscription_pdata(subscription);
+  if (ugstore == NULL)
+  {
+    ugstore = geis_ugsubscription_store_new();
+    if (!ugstore)
+    {
+      geis_error("error creating grail subscription store");
+      goto final_exit;
+    }
+    geis_subscription_set_pdata(subscription, ugstore);
+  }
+
+  if (geis_subscription_filter_count(subscription) > 0) {
+    status = _geis_grail_activate_with_filters(gbe, subscription);
+  } else {
+    status = _geis_grail_activate_for_devices(gbe,
+        NULL, /* no filter */
+        geis_devices(gbe->geis),
+        subscription);
+  }
+
 final_exit:
   return status;
 }