Merge lp:~charlesk/overlay-scrollbar/lp-1058205 into lp:overlay-scrollbar

[Charles Kerr]

This is based off Thomas' fix-mem-leak-bug-1058205 branch.

While I was testing Thomas' branch, I noticed his test case reported the overlay scrollbar created these Priv structs while the scrollbars were being disposed. This has two problems -- first is the unnecessary work of creating and populating a Priv struct only to dispose it; but more importantly, we're allocating memory and setting it in an object's qdata after the object's been disposed.

So, this branch contains Thomas' slice_free() fix and also avoids creating Privs in our hijacked_scrollbar_dispose() callback.

[Andrea Cimitan (cimi)]

Fine for me, there's an extra white line before the new static function :)

review: Approve

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:358
http://jenkins.qa.ubuntu.com/job/ayatana-scrollbar-ci/5/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ayatana-scrollbar-ci/./build=pbuilder,distribution=quantal,flavor=amd64/5/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ayatana-scrollbar-ci/./build=pbuilder,distribution=quantal,flavor=i386/5/console

review: Needs Fixing (continuous-integration)

[Thomas Bechtold (toabctl)]

Looks good to me. The Jenkins links are all 404 :(

review: Approve

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:358
http://jenkins.qa.ubuntu.com/job/ayatana-scrollbar-ci/6/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ayatana-scrollbar-ci/./build=pbuilder,distribution=quantal,flavor=amd64/6/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ayatana-scrollbar-ci/./build=pbuilder,distribution=quantal,flavor=i386/6/console

review: Approve (continuous-integration)

=== modified file 'os/os-scrollbar.c'
--- os/os-scrollbar.c	2012-08-29 16:09:23 +0000
+++ os/os-scrollbar.c	2012-10-04 18:48:20 +0000
@@ -607,14 +607,29 @@
 }
 #endif
 
-/* Get the private struct. */
+
+/* destroy the private struct */
+static void
+destroy_private (gpointer priv)
+{
+  g_slice_free (OsScrollbarPrivate, priv);
+}
+
+/* Get the private struct. If there isn't one, return NULL */
+static OsScrollbarPrivate*
+lookup_private (GtkWidget *widget)
+{
+  return g_object_get_qdata (G_OBJECT (widget), os_quark_qdata);
+}
+
+/* Get the private struct. If there isn't one, create it */
 static OsScrollbarPrivate*
 get_private (GtkWidget *widget)
 {
   OsScrollbarPrivate *priv;
 
   /* Fetch the private qdata struct. */
-  priv = g_object_get_qdata (G_OBJECT (widget), os_quark_qdata);
+  priv = lookup_private (widget);
 
   if (!priv)
     {
@@ -665,7 +680,7 @@
                                            scrolling_cb, scrolling_end_cb, widget);
 
       /* Store qdata. */
-      g_object_set_qdata (G_OBJECT (widget), os_quark_qdata, qdata);
+      g_object_set_qdata_full (G_OBJECT (widget), os_quark_qdata, qdata, destroy_private);
       priv = qdata;
 
       /* Create adjustment and thumb. */
@@ -3173,31 +3188,6 @@
       OsScrollbarPrivate *priv;
 
       scrollbar = GTK_SCROLLBAR (object);
-      priv = get_private (GTK_WIDGET (scrollbar));
-
-      if (priv->source_deactivate_bar_id != 0)
-        {
-          g_source_remove (priv->source_deactivate_bar_id);
-          priv->source_deactivate_bar_id = 0;
-        }
-
-      if (priv->source_hide_thumb_id != 0)
-        {
-          g_source_remove (priv->source_hide_thumb_id);
-          priv->source_hide_thumb_id = 0;
-        }
-
-      if (priv->source_show_thumb_id != 0)
-        {
-          g_source_remove (priv->source_show_thumb_id);
-          priv->source_show_thumb_id = 0;
-        }
-
-      if (priv->source_unlock_thumb_id != 0)
-        {
-          g_source_remove (priv->source_unlock_thumb_id);
-          priv->source_unlock_thumb_id = 0;
-        }
 
       os_root_list = g_slist_remove (os_root_list, scrollbar);
 
@@ -3213,26 +3203,54 @@
                                     root_filter_func, NULL);
         }
 
-      if (priv->animation != NULL)
-        {
-          g_object_unref (priv->animation);
-          priv->animation = NULL;
-        }
-
-      if (priv->bar != NULL)
-        {
-          g_object_unref (priv->bar);
-          priv->bar = NULL;
-        }
-
-      if (priv->window_group != NULL)
-        {
-          g_object_unref (priv->window_group);
-          priv->window_group = NULL;
-        }
-
-      swap_adjustment (scrollbar, NULL);
-      swap_thumb (scrollbar, NULL);
+      priv = lookup_private (GTK_WIDGET(scrollbar));
+      if (priv != NULL)
+        {
+          if (priv->source_deactivate_bar_id != 0)
+            {
+              g_source_remove (priv->source_deactivate_bar_id);
+              priv->source_deactivate_bar_id = 0;
+            }
+
+          if (priv->source_hide_thumb_id != 0)
+            {
+              g_source_remove (priv->source_hide_thumb_id);
+              priv->source_hide_thumb_id = 0;
+            }
+
+          if (priv->source_show_thumb_id != 0)
+            {
+              g_source_remove (priv->source_show_thumb_id);
+              priv->source_show_thumb_id = 0;
+            }
+
+          if (priv->source_unlock_thumb_id != 0)
+            {
+              g_source_remove (priv->source_unlock_thumb_id);
+              priv->source_unlock_thumb_id = 0;
+            }
+
+          if (priv->animation != NULL)
+            {
+              g_object_unref (priv->animation);
+              priv->animation = NULL;
+            }
+
+          if (priv->bar != NULL)
+            {
+              g_object_unref (priv->bar);
+              priv->bar = NULL;
+            }
+
+          if (priv->window_group != NULL)
+            {
+              g_object_unref (priv->window_group);
+              priv->window_group = NULL;
+            }
+
+          swap_adjustment (scrollbar, NULL);
+          swap_thumb (scrollbar, NULL);
+        }
     }
 
   (* pre_hijacked_scrollbar_dispose) (object);