Merge lp:~bregma/libgrip/lp-827958 into lp:libgrip

[Stephen M. Webb]

Registers and removes only those widgets whose toplevel was just mapped in the window-mapped callback.

Previously, an attempt to register ALL pending widgets regardless of toplevel was causing an occasional segfault.

[Stephen M. Webb (bregma)]

A quick re-review of this fix reveals it introduces another bug.

review: Needs Resubmitting

[lp:~bregma/libgrip/lp-827958]

65. By Stephen M. Webb on 2011-10-06

Renamed window_mapped_cb to toplevel_mapped_cb to reduce confusion.

[Stephen M. Webb (bregma)]

NOTE: made a final commit to change the name of the callback function to reduce future confusion.

[Chase Douglas (chasedouglas)]

The code looks reasonable to me, but I really don't know gtk well enough to approve it myself, especially given how close we are to release. I am have subscribed Michael Terry and Sebastien Bacher to review since they would know more and have commented in the bug report.

As soon as someone with more gtk knowledge approves, I want to get this path added to libgrip and uploaded. I am off until Tuesday, so please work with Seb, Didier, or someone else as appropriate to get it into Oneiric.

Great work figuring this bug out!

[Michael Terry (mterry)]

So the code changes themselves looks fine. I haven't tested that it fixes the segfault.

review: Approve

=== modified file 'src/gripgesturemanager.c'
--- src/gripgesturemanager.c	2011-09-22 19:55:37 +0000
+++ src/gripgesturemanager.c	2011-10-06 19:41:26 +0000
@@ -163,9 +163,9 @@
                             GeisSize           attr_count,
                             GeisGestureAttr   *attrs);
 
-static void window_mapped_cb (GtkWidget       *widget,
-                              GdkEvent        *event,
-                              gpointer         user_data);
+static void toplevel_mapped_cb (GtkWidget       *toplevel,
+                                GdkEvent        *event,
+                                gpointer         user_data);
 
 static GeisGestureFuncs gesture_funcs = {
   gesture_added,
@@ -1253,7 +1253,7 @@
       GtkWidget *toplevel = gtk_widget_get_toplevel (widget);
       GripRegistrationRequest *req = (GripRegistrationRequest *)user_data;
 
-      if (GTK_IS_WINDOW (toplevel))
+      if (gtk_widget_is_toplevel (toplevel))
         {
           g_signal_handlers_disconnect_by_func (widget,
                                                 G_CALLBACK (toplevel_notify_cb),
@@ -1280,7 +1280,7 @@
 
               g_signal_connect (toplevel,
                                 "map-event",
-                                G_CALLBACK (window_mapped_cb),
+                                G_CALLBACK (toplevel_mapped_cb),
                                 req->manager);
             }
         }
@@ -1316,7 +1316,7 @@
 {
   GtkWidget *toplevel = gtk_widget_get_toplevel (widget);
 
-  if (GTK_IS_WINDOW (toplevel))
+  if (gtk_widget_is_toplevel (toplevel))
     {
       register_internal (manager,
                          widget,
@@ -1348,35 +1348,39 @@
 }
 
 static void
-window_mapped_cb (GtkWidget   *widget,
-                  GdkEvent    *event G_GNUC_UNUSED,
-                  gpointer     user_data)
+toplevel_mapped_cb (GtkWidget   *toplevel,
+                    GdkEvent    *event G_GNUC_UNUSED,
+                    gpointer     user_data)
 {
   GripGestureManager *manager = (GripGestureManager *)user_data;
   GripGestureManagerPrivate *priv = manager->priv;
-  GList *tmp;
+  GList *request = priv->requests;
 
-  for (tmp = priv->requests; tmp != NULL; tmp = tmp->next)
+  while (request)
     {
-      GripRegistrationRequest *req = tmp->data;
-
-      register_widget (req->manager,
-                       req->widget,
-                       req->gesture_type,
-                       req->device_type,
-                       req->touch_points,
-                       req->callback,
-                       req->user_data,
-                       req->destroy);
-
-      g_free (req);
+      GList *next = request->next;
+      GripRegistrationRequest *req = request->data;
+
+      if (gtk_widget_get_toplevel (req->widget) == toplevel)
+        {
+	  register_widget (req->manager,
+                           req->widget,
+                           req->gesture_type,
+                           req->device_type,
+                           req->touch_points,
+                           req->callback,
+                           req->user_data,
+                           req->destroy);
+
+          priv->requests = g_list_remove_link (priv->requests, request);
+          g_free (req);
+          g_list_free_1 (request);
+        }
+      request = next;
     }
 
-  g_list_free (priv->requests);
-  priv->requests = NULL;
-
-  g_signal_handlers_disconnect_by_func (widget,
-                                        window_mapped_cb,
+  g_signal_handlers_disconnect_by_func (toplevel,
+                                        toplevel_mapped_cb,
                                         user_data);
 }
 
@@ -1445,7 +1449,7 @@
 
           g_signal_connect (toplevel,
                             "map-event",
-                            G_CALLBACK (window_mapped_cb),
+                            G_CALLBACK (toplevel_mapped_cb),
                             manager);
         }
     }