Merge lp:~dshea/timezonemap/location-cycle into lp:timezonemap

[David Shea]

An anaconda user noticed (https://bugzilla.redhat.com/show_bug.cgi?id=1190265) that if you repeatedly click the same spot on the map, it will choose different locations for each click (which is intentional), to the point where clicks will start jumping wildly across the map as the search area expands outwards (which is probably less intentional). While looking into this, I found that the handling of the repeated clicks resulted in a memory leak in priv->distances, and if you managed to get to the end of the list it would crash. The first patch addresses the memory problems, and the second limits the location search to a 50px radius.

[Adam Williamson (awilliamson)]

Aw, no is_canada() ? :P

[Iain Lane (laney)]

Cool, thanks

review: Approve

=== modified file 'src/cc-timezone-map.c'
--- src/cc-timezone-map.c	2013-12-16 17:27:20 +0000
+++ src/cc-timezone-map.c	2015-03-31 20:04:30 +0000
@@ -68,6 +68,8 @@
   CcTimezoneLocation *location;
   GHashTable *alias_db;
   GList *distances;
+  /* Store the head of the list separately so it can be freed later */
+  GList *distances_head;
 
   gint previous_x;
   gint previous_y;
@@ -594,9 +596,10 @@
       g_hash_table_destroy (priv->alias_db);
       priv->alias_db = NULL;
     }
-  if (priv->distances)
+  if (priv->distances_head)
     {
-      g_list_free (priv->distances);
+      g_list_free (priv->distances_head);
+      priv->distances_head = NULL;
       priv->distances = NULL;
     }
 
@@ -991,10 +994,15 @@
   if (x == priv->previous_x && y == priv->previous_y) 
     {
       priv->distances = g_list_next (priv->distances);
+      if (!priv->distances)
+          priv->distances = priv->distances_head;
+
       location = (CcTimezoneLocation*) priv->distances->data;
     } else {
-      g_list_free (priv->distances);
-      priv->distances = NULL;
+      GList *node;
+
+      g_list_free (priv->distances_head);
+      priv->distances_head = NULL;
       for (i = 0; i < array->len; i++)
         {
           gdouble pointx, pointy, dx, dy;
@@ -1007,9 +1015,29 @@
           dy = pointy - y;
 
           cc_timezone_location_set_dist(loc, (gdouble) dx * dx + dy * dy);
-          priv->distances = g_list_prepend (priv->distances, loc);
-        }
-      priv->distances = g_list_sort (priv->distances, (GCompareFunc) sort_locations);
+          priv->distances_head = g_list_prepend (priv->distances_head, loc);
+        }
+      priv->distances_head = g_list_sort (priv->distances_head, (GCompareFunc) sort_locations);
+
+      /* Remove locations from the list with a distance of greater than 50px,
+       * so that repeated clicks cycle through a smaller area instead of
+       * jumping all over the map. Start with the second element to ensure
+       * that at least one element stays in the list.
+       */
+      node = priv->distances_head->next;
+      while (node != NULL)
+        {
+          if (cc_timezone_location_get_dist(node->data) > (50 * 50))
+            {
+              /* Cut the list off here */
+              node->prev->next = NULL;
+              g_list_free(node);
+            }
+
+          node = g_list_next(node);
+        }
+
+      priv->distances = priv->distances_head;
       location = (CcTimezoneLocation*) priv->distances->data;
       priv->previous_x = x;
       priv->previous_y = y;