Merge lp:~smspillaz/unity/unity.fix_1180970 into lp:unity

[Sam Spilsbury]

Use gtk_icon_info_free on gtk < 3.8

GtkIconInfo is a GObject, but on < 3.8 callers are meant to free it with
gtk_icon_info_unref according to the documentation for it:

GtkIconInfo * gtk_icon_theme_lookup_icon (GtkIconTheme *icon_theme,
                                          const gchar *icon_name,
                                          gint size,
                                          GtkIconLookupFlags flags);
Looks up a named icon and returns a structure containing information such
as the filename of the icon. The icon can then be rendered into a pixbuf
using gtk_icon_info_load_icon(). (gtk_icon_theme_load_icon() combines
these two steps if all you need is the pixbuf.)

...

Returns :
a GtkIconInfo structure containing information about the icon, or NULL if
the icon wasn't found. Free with gtk_icon_info_free()

On > 3.8 the function is deprecated and g_object_unref should be used.

GtkUtilDeathTest::NoDeathOnUnreference verifies that there is no crash-on-unref.

(LP: #1180790)

Use gtk_icon_info_free on gtk < 3.8

GtkIconInfo is a GObject, but on < 3.8 callers are meant to free it with
gtk_icon_info_unref according to the documentation for it:

GtkIconInfo * gtk_icon_theme_lookup_icon (GtkIconTheme *icon_theme,
                                          const gchar *icon_name,
                                          gint size,
                                          GtkIconLookupFlags flags);
Looks up a named icon and returns a structure containing information such
as the filename of the icon. The icon can then be rendered into a pixbuf
using gtk_icon_info_load_icon(). (gtk_icon_theme_load_icon() combines
these two steps if all you need is the pixbuf.)

...

Returns :
a GtkIconInfo structure containing information about the icon, or NULL if
the icon wasn't found. Free with gtk_icon_info_free()

On > 3.8 the function is deprecated and g_object_unref should be used.

GtkUtilDeathTest::NoDeathOnUnreference verifies that there is no crash-on-unref.

(LP: #1180790)

I haven't used glib::Object<> here. As much as I would have liked to - it doesn't support a concept of custom deleters which would be required to support anything like GtkIconInfo. Given that this is the only usecase so far and its actually just for a previous ubuntu version, it probably didn't make sense to add a new constructor there.

[Ted Gould (ted)]

Actually it's deprecated:

https://developer.gnome.org/gtk3/stable/GtkIconTheme.html#gtk-icon-info-free

review: Needs Fixing

[Ted Gould (ted)]

Proposed a different merge to handle the bad object in bug 1180970.

https://code.launchpad.net/~ted/unity/null-icon-info/+merge/164214

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3325
http://jenkins.qa.ubuntu.com/job/unity-ci/58/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-amd64-ci/58
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-armhf-ci/58
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-i386-ci/58

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/58/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

Please move everything to glib::Object<GtkIconInfo>.

[Sam Spilsbury (smspillaz)]

g_object_clear / g_object_unref is broken on < 3.8 and gtk_icon_info_free is deprecated on >= 3.8. I like it when libraries do this. We'll have to ifdef it.

review: Needs Resubmitting

[Sam Spilsbury (smspillaz)]

Ah, frick. Forgot to add the file.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3326
http://jenkins.qa.ubuntu.com/job/unity-ci/66/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-raring-amd64-ci/70/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-raring-armhf-ci/66/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-raring-i386-ci/66/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/66/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3326
http://jenkins.qa.ubuntu.com/job/unity-ci/67/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-raring-amd64-ci/71/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-armhf-ci/67
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-raring-i386-ci/67/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/67/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3327
http://jenkins.qa.ubuntu.com/job/unity-ci/68/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-amd64-ci/72
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-armhf-ci/68
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-i386-ci/68

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/68/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

I'd prefer to have a wrapper for this, I'm submitting a proposal to superseed this.

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-05-16 23:30:28 +0000
+++ CMakeLists.txt	2013-05-17 06:49:34 +0000
@@ -161,6 +161,14 @@
 execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} indicator3-0.4 --variable indicatordir OUTPUT_VARIABLE INDICATORDIR OUTPUT_STRIP_TRAILING_WHITESPACE)
 execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} indicator3-0.4 --variable iconsdir OUTPUT_VARIABLE INDICATORICONDIR OUTPUT_STRIP_TRAILING_WHITESPACE)
 
+pkg_check_modules (GTK_3_8 gtk+-3.0>=3.8)
+
+if (GTK_3_8_FOUND)
+    set (HAVE_GTK_3_8 TRUE)
+else (GTK_3_8_FOUND)
+    set (HAVE_GTK_3_8 FALSE)
+endif (GTK_3_8_FOUND)
+
 configure_file (${CMAKE_SOURCE_DIR}/config.h.cmake ${CMAKE_BINARY_DIR}/config.h)
 
 #

=== modified file 'config.h.cmake'
--- config.h.cmake	2013-02-18 19:38:46 +0000
+++ config.h.cmake	2013-05-17 06:49:34 +0000
@@ -20,5 +20,6 @@
 #ifndef INDICATOR_SERVICE_DIR
 #cmakedefine INDICATOR_SERVICE_DIR "@INDICATOR_SERVICE_DIR@"
 #endif
+#cmakedefine HAVE_GTK_3_8
 
 #endif // CONFIG_H

=== modified file 'dash/ResultRenderer.cpp'
--- dash/ResultRenderer.cpp	2013-05-16 10:58:57 +0000
+++ dash/ResultRenderer.cpp	2013-05-17 06:49:34 +0000
@@ -26,6 +26,8 @@
 #include <unity-protocol.h>
 #include <NuxGraphics/GdkGraphics.h>
 
+#include "unity-shared/GtkUtil.h"
+
 namespace unity
 {
 namespace dash
@@ -109,7 +111,7 @@
     pbuf = NULL;
   }
 
-  g_object_unref(G_OBJECT(info));
+  gtk::UnreferenceIconInfo(info);
   return pbuf;
 }
 

=== modified file 'launcher/LauncherIcon.cpp'
--- launcher/LauncherIcon.cpp	2013-05-16 10:58:57 +0000
+++ launcher/LauncherIcon.cpp	2013-05-17 06:49:34 +0000
@@ -32,6 +32,7 @@
 #include "LauncherIcon.h"
 #include "Launcher.h"
 #include "unity-shared/TimeUtil.h"
+#include "unity-shared/GtkUtil.h"
 
 #include "QuicklistManager.h"
 #include "QuicklistMenuItem.h"
@@ -341,7 +342,7 @@
   if (g_strrstr(gtk_icon_info_get_filename(info), "ubuntu-mono") != NULL)
     _current_theme_is_mono = (int)true;
 
-  g_object_unref(G_OBJECT(info));
+  gtk::UnreferenceIconInfo(info);
   return (bool)_current_theme_is_mono;
 
 }
@@ -396,7 +397,7 @@
                                                             bool update_glow_colors,
                                                             bool is_default_theme)
 {
-  GtkIconInfo* info;
+  GtkIconInfo* info = NULL;
   nux::BaseTexture* result = NULL;
   GIcon* icon;
   GtkIconLookupFlags flags = (GtkIconLookupFlags) 0;
@@ -429,7 +430,7 @@
 
   glib::Error error;
   glib::Object<GdkPixbuf> pbuf(gtk_icon_info_load_icon(info, &error));
-  g_object_unref(G_OBJECT(info));
+  gtk::UnreferenceIconInfo(info);
 
   if (GDK_IS_PIXBUF(pbuf.RawPtr()))
   {

=== modified file 'launcher/MockLauncherIcon.h'
--- launcher/MockLauncherIcon.h	2013-05-14 15:54:28 +0000
+++ launcher/MockLauncherIcon.h	2013-05-17 06:49:34 +0000
@@ -32,6 +32,7 @@
 
 #include <libdbusmenu-glib/menuitem.h>
 #include "unity-shared/ApplicationManager.h"
+#include "unity-shared/GtkUtil.h"
 
 #include "AbstractLauncherIcon.h"
 
@@ -365,6 +366,8 @@
       g_error_free(error);
     }
 
+    gtk::UnreferenceIconInfo(info);
+
     return result;
   }
 

=== modified file 'panel/PanelIndicatorEntryView.cpp'
--- panel/PanelIndicatorEntryView.cpp	2013-05-14 15:54:28 +0000
+++ panel/PanelIndicatorEntryView.cpp	2013-05-17 06:49:34 +0000
@@ -36,7 +36,7 @@
 #include "PanelIndicatorEntryView.h"
 #include "unity-shared/PanelStyle.h"
 #include "unity-shared/WindowManager.h"
-
+#include "unity-shared/GtkUtil.h"
 
 namespace unity
 {
@@ -214,7 +214,7 @@
     if (info)
     {
       pixbuf = gtk_icon_info_load_icon(info, nullptr);
-      g_object_unref(G_OBJECT(info));
+      gtk::UnreferenceIconInfo(info);
     }
   }
 

=== modified file 'tests/test_icon_loader.cpp'
--- tests/test_icon_loader.cpp	2013-04-03 19:42:06 +0000
+++ tests/test_icon_loader.cpp	2013-05-17 06:49:34 +0000
@@ -21,6 +21,7 @@
 #include <sigc++/sigc++.h>
 
 #include "IconLoader.h"
+#include "unity-shared/GtkUtil.h"
 #include "test_utils.h"
 
 using namespace testing;
@@ -210,5 +211,31 @@
   CheckResults(results);
 }
 
+TEST(GtkIconUtilDeathTest, NoDeathOnUnreference)
+{
+  GList *icons = gtk_icon_theme_list_icons(gtk_icon_theme_get_default(), "Emblems");
+
+  // Get the first icon name
+  if (icons)
+  {
+    const char *icon_name = static_cast <const char *>(icons->data);
+    GtkIconInfo *info = gtk_icon_theme_lookup_icon(gtk_icon_theme_get_default(), icon_name, 32, (GtkIconLookupFlags) 0);
+    ASSERT_THAT(info, NotNull());
+    EXPECT_EXIT({
+        unity::gtk::UnreferenceIconInfo(info);
+        fprintf(stderr, "Success\n");
+        exit(0);
+    }, ExitedWithCode(0), "Success");
+
+    unity::gtk::UnreferenceIconInfo(info);
+  }
+
+  // Free all the icons
+  for (GList *it = icons; it != NULL; it = it->next)
+    g_free (it->data);
+
+  g_list_free (icons);
+}
+
 
 }

=== modified file 'tests/test_result_renderer.cpp'
--- tests/test_result_renderer.cpp	2013-05-16 10:58:57 +0000
+++ tests/test_result_renderer.cpp	2013-05-17 06:49:34 +0000
@@ -24,6 +24,7 @@
 
 #include "unity-shared/DashStyle.h"
 #include "unity-shared/UnitySettings.h"
+#include "unity-shared/GtkUtil.h"
 #include "UnityCore/Result.h"
 #include "dash/ResultRendererTile.h"
 
@@ -59,7 +60,7 @@
       g_error_free (error);
       pbuf = NULL;
     }
-    g_object_unref(G_OBJECT(info));
+    gtk::UnreferenceIconInfo(info);
   }
 
   return pbuf;

=== added file 'unity-shared/GtkUtil.h'
--- unity-shared/GtkUtil.h	1970-01-01 00:00:00 +0000
+++ unity-shared/GtkUtil.h	2013-05-17 06:49:34 +0000
@@ -0,0 +1,43 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+/*
+* Copyright (C) 2013 Canonical Ltd
+*
+* This program is free software: you can redistribute it and/or modify
+* it under the terms of the GNU General Public License version 3 as
+* published by the Free Software Foundation.
+*
+* This program is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU General Public License for more details.
+*
+* You should have received a copy of the GNU General Public License
+* along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*
+* Authored by: Sam Spilsbury <[e-mail]>
+*/
+
+#ifndef GTK_UTIL_H
+#define GTK_UTIL_H
+
+#include <glib-2.0/glib-object.h>
+#include <gtk/gtk.h>
+#include <config.h>
+
+namespace unity
+{
+namespace gtk
+{
+inline void UnreferenceIconInfo(GtkIconInfo *info)
+{
+#ifdef HAVE_GTK_3_8
+  g_object_clear(info);
+#else
+  if (info)
+    gtk_icon_info_free(info);
+#endif
+}
+}
+}
+
+#endif

=== modified file 'unity-shared/IconLoader.cpp'
--- unity-shared/IconLoader.cpp	2013-05-14 15:54:28 +0000
+++ unity-shared/IconLoader.cpp	2013-05-17 06:49:34 +0000
@@ -34,6 +34,7 @@
 #include <UnityCore/GLibSignal.h>
 
 #include "unity-shared/Timer.h"
+#include "unity-shared/GtkUtil.h"
 
 namespace unity
 {
@@ -124,8 +125,7 @@
 
     ~IconLoaderTask()
     {
-      if (icon_info)
-        ::g_object_unref(G_OBJECT(icon_info));
+      gtk::UnreferenceIconInfo(icon_info);
       if (helper_handle != 0)
         impl->DisconnectHandle(helper_handle);
     }