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
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2013-05-16 23:30:28 +0000
3+++ CMakeLists.txt 2013-05-17 06:24:29 +0000
4@@ -161,6 +161,14 @@
5 execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} indicator3-0.4 --variable indicatordir OUTPUT_VARIABLE INDICATORDIR OUTPUT_STRIP_TRAILING_WHITESPACE)
6 execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} indicator3-0.4 --variable iconsdir OUTPUT_VARIABLE INDICATORICONDIR OUTPUT_STRIP_TRAILING_WHITESPACE)
7
8+pkg_check_modules (GTK_3_8 gtk+-3.0>=3.8)
9+
10+if (GTK_3_8_FOUND)
11+ set (HAVE_GTK_3_8 TRUE)
12+else (GTK_3_8_FOUND)
13+ set (HAVE_GTK_3_8 FALSE)
14+endif (GTK_3_8_FOUND)
15+
16 configure_file (${CMAKE_SOURCE_DIR}/config.h.cmake ${CMAKE_BINARY_DIR}/config.h)
17
18 #
19
20=== modified file 'config.h.cmake'
21--- config.h.cmake 2013-02-18 19:38:46 +0000
22+++ config.h.cmake 2013-05-17 06:24:29 +0000
23@@ -20,5 +20,6 @@
24 #ifndef INDICATOR_SERVICE_DIR
25 #cmakedefine INDICATOR_SERVICE_DIR "@INDICATOR_SERVICE_DIR@"
26 #endif
27+#cmakedefine HAVE_GTK_3_8
28
29 #endif // CONFIG_H
30
31=== modified file 'dash/ResultRenderer.cpp'
32--- dash/ResultRenderer.cpp 2013-05-16 10:58:57 +0000
33+++ dash/ResultRenderer.cpp 2013-05-17 06:24:29 +0000
34@@ -26,6 +26,8 @@
35 #include <unity-protocol.h>
36 #include <NuxGraphics/GdkGraphics.h>
37
38+#include "unity-shared/GtkUtil.h"
39+
40 namespace unity
41 {
42 namespace dash
43@@ -109,7 +111,7 @@
44 pbuf = NULL;
45 }
46
47- g_object_unref(G_OBJECT(info));
48+ gtk::UnreferenceIconInfo(info);
49 return pbuf;
50 }
51
52
53=== modified file 'launcher/LauncherIcon.cpp'
54--- launcher/LauncherIcon.cpp 2013-05-16 10:58:57 +0000
55+++ launcher/LauncherIcon.cpp 2013-05-17 06:24:29 +0000
56@@ -32,6 +32,7 @@
57 #include "LauncherIcon.h"
58 #include "Launcher.h"
59 #include "unity-shared/TimeUtil.h"
60+#include "unity-shared/GtkUtil.h"
61
62 #include "QuicklistManager.h"
63 #include "QuicklistMenuItem.h"
64@@ -341,7 +342,7 @@
65 if (g_strrstr(gtk_icon_info_get_filename(info), "ubuntu-mono") != NULL)
66 _current_theme_is_mono = (int)true;
67
68- g_object_unref(G_OBJECT(info));
69+ gtk::UnreferenceIconInfo(info);
70 return (bool)_current_theme_is_mono;
71
72 }
73@@ -396,7 +397,7 @@
74 bool update_glow_colors,
75 bool is_default_theme)
76 {
77- GtkIconInfo* info;
78+ GtkIconInfo* info = NULL;
79 nux::BaseTexture* result = NULL;
80 GIcon* icon;
81 GtkIconLookupFlags flags = (GtkIconLookupFlags) 0;
82@@ -429,7 +430,7 @@
83
84 glib::Error error;
85 glib::Object<GdkPixbuf> pbuf(gtk_icon_info_load_icon(info, &error));
86- g_object_unref(G_OBJECT(info));
87+ gtk::UnreferenceIconInfo(info);
88
89 if (GDK_IS_PIXBUF(pbuf.RawPtr()))
90 {
91
92=== modified file 'launcher/MockLauncherIcon.h'
93--- launcher/MockLauncherIcon.h 2013-05-14 15:54:28 +0000
94+++ launcher/MockLauncherIcon.h 2013-05-17 06:24:29 +0000
95@@ -32,6 +32,7 @@
96
97 #include <libdbusmenu-glib/menuitem.h>
98 #include "unity-shared/ApplicationManager.h"
99+#include "unity-shared/GtkUtil.h"
100
101 #include "AbstractLauncherIcon.h"
102
103@@ -365,6 +366,8 @@
104 g_error_free(error);
105 }
106
107+ gtk::UnreferenceIconInfo(info);
108+
109 return result;
110 }
111
112
113=== modified file 'panel/PanelIndicatorEntryView.cpp'
114--- panel/PanelIndicatorEntryView.cpp 2013-05-14 15:54:28 +0000
115+++ panel/PanelIndicatorEntryView.cpp 2013-05-17 06:24:29 +0000
116@@ -36,7 +36,7 @@
117 #include "PanelIndicatorEntryView.h"
118 #include "unity-shared/PanelStyle.h"
119 #include "unity-shared/WindowManager.h"
120-
121+#include "unity-shared/GtkUtil.h"
122
123 namespace unity
124 {
125@@ -214,7 +214,7 @@
126 if (info)
127 {
128 pixbuf = gtk_icon_info_load_icon(info, nullptr);
129- g_object_unref(G_OBJECT(info));
130+ gtk::UnreferenceIconInfo(info);
131 }
132 }
133
134
135=== modified file 'tests/test_icon_loader.cpp'
136--- tests/test_icon_loader.cpp 2013-04-03 19:42:06 +0000
137+++ tests/test_icon_loader.cpp 2013-05-17 06:24:29 +0000
138@@ -21,6 +21,7 @@
139 #include <sigc++/sigc++.h>
140
141 #include "IconLoader.h"
142+#include "unity-shared/GtkUtil.h"
143 #include "test_utils.h"
144
145 using namespace testing;
146@@ -210,5 +211,31 @@
147 CheckResults(results);
148 }
149
150+TEST(GtkIconUtilDeathTest, NoDeathOnUnreference)
151+{
152+ GList *icons = gtk_icon_theme_list_icons(gtk_icon_theme_get_default(), "Emblems");
153+
154+ // Get the first icon name
155+ if (icons)
156+ {
157+ const char *icon_name = static_cast <const char *>(icons->data);
158+ GtkIconInfo *info = gtk_icon_theme_lookup_icon(gtk_icon_theme_get_default(), icon_name, 32, (GtkIconLookupFlags) 0);
159+ ASSERT_THAT(info, NotNull());
160+ EXPECT_EXIT({
161+ unity::gtk::UnreferenceIconInfo(info);
162+ fprintf(stderr, "Success\n");
163+ exit(0);
164+ }, ExitedWithCode(0), "Success");
165+
166+ unity::gtk::UnreferenceIconInfo(info);
167+ }
168+
169+ // Free all the icons
170+ for (GList *it = icons; it != NULL; it = it->next)
171+ g_free (it->data);
172+
173+ g_list_free (icons);
174+}
175+
176
177 }
178
179=== modified file 'tests/test_result_renderer.cpp'
180--- tests/test_result_renderer.cpp 2013-05-16 10:58:57 +0000
181+++ tests/test_result_renderer.cpp 2013-05-17 06:24:29 +0000
182@@ -24,6 +24,7 @@
183
184 #include "unity-shared/DashStyle.h"
185 #include "unity-shared/UnitySettings.h"
186+#include "unity-shared/GtkUtil.h"
187 #include "UnityCore/Result.h"
188 #include "dash/ResultRendererTile.h"
189
190@@ -59,7 +60,7 @@
191 g_error_free (error);
192 pbuf = NULL;
193 }
194- g_object_unref(G_OBJECT(info));
195+ gtk::UnreferenceIconInfo(info);
196 }
197
198 return pbuf;
199
200=== modified file 'unity-shared/IconLoader.cpp'
201--- unity-shared/IconLoader.cpp 2013-05-14 15:54:28 +0000
202+++ unity-shared/IconLoader.cpp 2013-05-17 06:24:29 +0000
203@@ -34,6 +34,7 @@
204 #include <UnityCore/GLibSignal.h>
205
206 #include "unity-shared/Timer.h"
207+#include "unity-shared/GtkUtil.h"
208
209 namespace unity
210 {
211@@ -124,8 +125,7 @@
212
213 ~IconLoaderTask()
214 {
215- if (icon_info)
216- ::g_object_unref(G_OBJECT(icon_info));
217+ gtk::UnreferenceIconInfo(icon_info);
218 if (helper_handle != 0)
219 impl->DisconnectHandle(helper_handle);
220 }