Unity

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

  1. unity.fix_1180970
  2. Merge into trunk
Proposed by Sam Spilsbury
Status: Superseded
Proposed branch: lp:~smspillaz/unity/unity.fix_1180970
Merge into: lp:unity
Diff against target: 220 lines (+52/-9)
9 files modified
CMakeLists.txt (+8/-0)
config.h.cmake (+1/-0)
dash/ResultRenderer.cpp (+3/-1)
launcher/LauncherIcon.cpp (+4/-3)
launcher/MockLauncherIcon.h (+3/-0)
panel/PanelIndicatorEntryView.cpp (+2/-2)
tests/test_icon_loader.cpp (+27/-0)
tests/test_result_renderer.cpp (+2/-1)
unity-shared/IconLoader.cpp (+2/-2)
To merge this branch: bzr merge lp:~smspillaz/unity/unity.fix_1180970
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Sam Spilsbury (community) Needs Resubmitting
Ted Gould (community) Needs Fixing
Review via email: mp+164189@code.launchpad.net

This proposal has been superseded by a proposal from 2013-05-17.

Commit message

Use gtk_icon_info_free instead of g_object_unref

GtkIconInfo is a GObject, but 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()

(LP: #1180790)

Description of the change

Use gtk_icon_info_free instead of g_object_unref

GtkIconInfo is a GObject, but 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()

(LP: #1180790)

To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote :

Actually it's deprecated:

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

review: Needs Fixing
Revision history for this message
Ted Gould (ted) wrote :

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

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

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

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

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'CMakeLists.txt'
--- CMakeLists.txt 2013-05-16 23:30:28 +0000
+++ CMakeLists.txt 2013-05-17 06:24:29 +0000
@@ -161,6 +161,14 @@
161execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} indicator3-0.4 --variable indicatordir OUTPUT_VARIABLE INDICATORDIR OUTPUT_STRIP_TRAILING_WHITESPACE)161execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} indicator3-0.4 --variable indicatordir OUTPUT_VARIABLE INDICATORDIR OUTPUT_STRIP_TRAILING_WHITESPACE)
162execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} indicator3-0.4 --variable iconsdir OUTPUT_VARIABLE INDICATORICONDIR OUTPUT_STRIP_TRAILING_WHITESPACE)162execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} indicator3-0.4 --variable iconsdir OUTPUT_VARIABLE INDICATORICONDIR OUTPUT_STRIP_TRAILING_WHITESPACE)
163163
164pkg_check_modules (GTK_3_8 gtk+-3.0>=3.8)
165
166if (GTK_3_8_FOUND)
167 set (HAVE_GTK_3_8 TRUE)
168else (GTK_3_8_FOUND)
169 set (HAVE_GTK_3_8 FALSE)
170endif (GTK_3_8_FOUND)
171
164configure_file (${CMAKE_SOURCE_DIR}/config.h.cmake ${CMAKE_BINARY_DIR}/config.h)172configure_file (${CMAKE_SOURCE_DIR}/config.h.cmake ${CMAKE_BINARY_DIR}/config.h)
165173
166#174#
167175
=== modified file 'config.h.cmake'
--- config.h.cmake 2013-02-18 19:38:46 +0000
+++ config.h.cmake 2013-05-17 06:24:29 +0000
@@ -20,5 +20,6 @@
20#ifndef INDICATOR_SERVICE_DIR20#ifndef INDICATOR_SERVICE_DIR
21#cmakedefine INDICATOR_SERVICE_DIR "@INDICATOR_SERVICE_DIR@"21#cmakedefine INDICATOR_SERVICE_DIR "@INDICATOR_SERVICE_DIR@"
22#endif22#endif
23#cmakedefine HAVE_GTK_3_8
2324
24#endif // CONFIG_H25#endif // CONFIG_H
2526
=== modified file 'dash/ResultRenderer.cpp'
--- dash/ResultRenderer.cpp 2013-05-16 10:58:57 +0000
+++ dash/ResultRenderer.cpp 2013-05-17 06:24:29 +0000
@@ -26,6 +26,8 @@
26#include <unity-protocol.h>26#include <unity-protocol.h>
27#include <NuxGraphics/GdkGraphics.h>27#include <NuxGraphics/GdkGraphics.h>
2828
29#include "unity-shared/GtkUtil.h"
30
29namespace unity31namespace unity
30{32{
31namespace dash33namespace dash
@@ -109,7 +111,7 @@
109 pbuf = NULL;111 pbuf = NULL;
110 }112 }
111113
112 g_object_unref(G_OBJECT(info));114 gtk::UnreferenceIconInfo(info);
113 return pbuf;115 return pbuf;
114}116}
115117
116118
=== modified file 'launcher/LauncherIcon.cpp'
--- launcher/LauncherIcon.cpp 2013-05-16 10:58:57 +0000
+++ launcher/LauncherIcon.cpp 2013-05-17 06:24:29 +0000
@@ -32,6 +32,7 @@
32#include "LauncherIcon.h"32#include "LauncherIcon.h"
33#include "Launcher.h"33#include "Launcher.h"
34#include "unity-shared/TimeUtil.h"34#include "unity-shared/TimeUtil.h"
35#include "unity-shared/GtkUtil.h"
3536
36#include "QuicklistManager.h"37#include "QuicklistManager.h"
37#include "QuicklistMenuItem.h"38#include "QuicklistMenuItem.h"
@@ -341,7 +342,7 @@
341 if (g_strrstr(gtk_icon_info_get_filename(info), "ubuntu-mono") != NULL)342 if (g_strrstr(gtk_icon_info_get_filename(info), "ubuntu-mono") != NULL)
342 _current_theme_is_mono = (int)true;343 _current_theme_is_mono = (int)true;
343344
344 g_object_unref(G_OBJECT(info));345 gtk::UnreferenceIconInfo(info);
345 return (bool)_current_theme_is_mono;346 return (bool)_current_theme_is_mono;
346347
347}348}
@@ -396,7 +397,7 @@
396 bool update_glow_colors,397 bool update_glow_colors,
397 bool is_default_theme)398 bool is_default_theme)
398{399{
399 GtkIconInfo* info;400 GtkIconInfo* info = NULL;
400 nux::BaseTexture* result = NULL;401 nux::BaseTexture* result = NULL;
401 GIcon* icon;402 GIcon* icon;
402 GtkIconLookupFlags flags = (GtkIconLookupFlags) 0;403 GtkIconLookupFlags flags = (GtkIconLookupFlags) 0;
@@ -429,7 +430,7 @@
429430
430 glib::Error error;431 glib::Error error;
431 glib::Object<GdkPixbuf> pbuf(gtk_icon_info_load_icon(info, &error));432 glib::Object<GdkPixbuf> pbuf(gtk_icon_info_load_icon(info, &error));
432 g_object_unref(G_OBJECT(info));433 gtk::UnreferenceIconInfo(info);
433434
434 if (GDK_IS_PIXBUF(pbuf.RawPtr()))435 if (GDK_IS_PIXBUF(pbuf.RawPtr()))
435 {436 {
436437
=== modified file 'launcher/MockLauncherIcon.h'
--- launcher/MockLauncherIcon.h 2013-05-14 15:54:28 +0000
+++ launcher/MockLauncherIcon.h 2013-05-17 06:24:29 +0000
@@ -32,6 +32,7 @@
3232
33#include <libdbusmenu-glib/menuitem.h>33#include <libdbusmenu-glib/menuitem.h>
34#include "unity-shared/ApplicationManager.h"34#include "unity-shared/ApplicationManager.h"
35#include "unity-shared/GtkUtil.h"
3536
36#include "AbstractLauncherIcon.h"37#include "AbstractLauncherIcon.h"
3738
@@ -365,6 +366,8 @@
365 g_error_free(error);366 g_error_free(error);
366 }367 }
367368
369 gtk::UnreferenceIconInfo(info);
370
368 return result;371 return result;
369 }372 }
370373
371374
=== modified file 'panel/PanelIndicatorEntryView.cpp'
--- panel/PanelIndicatorEntryView.cpp 2013-05-14 15:54:28 +0000
+++ panel/PanelIndicatorEntryView.cpp 2013-05-17 06:24:29 +0000
@@ -36,7 +36,7 @@
36#include "PanelIndicatorEntryView.h"36#include "PanelIndicatorEntryView.h"
37#include "unity-shared/PanelStyle.h"37#include "unity-shared/PanelStyle.h"
38#include "unity-shared/WindowManager.h"38#include "unity-shared/WindowManager.h"
3939#include "unity-shared/GtkUtil.h"
4040
41namespace unity41namespace unity
42{42{
@@ -214,7 +214,7 @@
214 if (info)214 if (info)
215 {215 {
216 pixbuf = gtk_icon_info_load_icon(info, nullptr);216 pixbuf = gtk_icon_info_load_icon(info, nullptr);
217 g_object_unref(G_OBJECT(info));217 gtk::UnreferenceIconInfo(info);
218 }218 }
219 }219 }
220220
221221
=== 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:24:29 +0000
@@ -21,6 +21,7 @@
21#include <sigc++/sigc++.h>21#include <sigc++/sigc++.h>
2222
23#include "IconLoader.h"23#include "IconLoader.h"
24#include "unity-shared/GtkUtil.h"
24#include "test_utils.h"25#include "test_utils.h"
2526
26using namespace testing;27using namespace testing;
@@ -210,5 +211,31 @@
210 CheckResults(results);211 CheckResults(results);
211}212}
212213
214TEST(GtkIconUtilDeathTest, NoDeathOnUnreference)
215{
216 GList *icons = gtk_icon_theme_list_icons(gtk_icon_theme_get_default(), "Emblems");
217
218 // Get the first icon name
219 if (icons)
220 {
221 const char *icon_name = static_cast <const char *>(icons->data);
222 GtkIconInfo *info = gtk_icon_theme_lookup_icon(gtk_icon_theme_get_default(), icon_name, 32, (GtkIconLookupFlags) 0);
223 ASSERT_THAT(info, NotNull());
224 EXPECT_EXIT({
225 unity::gtk::UnreferenceIconInfo(info);
226 fprintf(stderr, "Success\n");
227 exit(0);
228 }, ExitedWithCode(0), "Success");
229
230 unity::gtk::UnreferenceIconInfo(info);
231 }
232
233 // Free all the icons
234 for (GList *it = icons; it != NULL; it = it->next)
235 g_free (it->data);
236
237 g_list_free (icons);
238}
239
213240
214}241}
215242
=== 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:24:29 +0000
@@ -24,6 +24,7 @@
2424
25#include "unity-shared/DashStyle.h"25#include "unity-shared/DashStyle.h"
26#include "unity-shared/UnitySettings.h"26#include "unity-shared/UnitySettings.h"
27#include "unity-shared/GtkUtil.h"
27#include "UnityCore/Result.h"28#include "UnityCore/Result.h"
28#include "dash/ResultRendererTile.h"29#include "dash/ResultRendererTile.h"
2930
@@ -59,7 +60,7 @@
59 g_error_free (error);60 g_error_free (error);
60 pbuf = NULL;61 pbuf = NULL;
61 }62 }
62 g_object_unref(G_OBJECT(info));63 gtk::UnreferenceIconInfo(info);
63 }64 }
6465
65 return pbuf;66 return pbuf;
6667
=== 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:24:29 +0000
@@ -34,6 +34,7 @@
34#include <UnityCore/GLibSignal.h>34#include <UnityCore/GLibSignal.h>
3535
36#include "unity-shared/Timer.h"36#include "unity-shared/Timer.h"
37#include "unity-shared/GtkUtil.h"
3738
38namespace unity39namespace unity
39{40{
@@ -124,8 +125,7 @@
124125
125 ~IconLoaderTask()126 ~IconLoaderTask()
126 {127 {
127 if (icon_info)128 gtk::UnreferenceIconInfo(icon_info);
128 ::g_object_unref(G_OBJECT(icon_info));
129 if (helper_handle != 0)129 if (helper_handle != 0)
130 impl->DisconnectHandle(helper_handle);130 impl->DisconnectHandle(helper_handle);
131 }131 }