DBus Menu

Merge lp:~charlesk/libdbusmenu/lp-977803 into lp:libdbusmenu/0.6

  1. lp-977803
  2. Merge into trunk.0.6
Proposed by Charles Kerr
Status: Merged
Approved by: Ted Gould
Approved revision: 409
Merged at revision: 408
Proposed branch: lp:~charlesk/libdbusmenu/lp-977803
Merge into: lp:libdbusmenu/0.6
Diff against target: 356 lines (+114/-93)
1 file modified
libdbusmenu-gtk/parser.c (+114/-93)
To merge this branch: bzr merge lp:~charlesk/libdbusmenu/lp-977803
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Review via email: mp+101401@code.launchpad.net

Description of the change

I'm still suspicious about parser.c's signal handling, but this morning's bug #977803 shows that my disconnect-smoke-tests from yesterday have too many noisy false positives.

This commit remembers the handler_id of each signal that we connect to so that we can give more useful warnings at disconnect time.

To post a comment you must log in.
lp:~charlesk/libdbusmenu/lp-977803 updated
409. By Charles Kerr

switch the smoke test to a g_debug statement

Revision history for this message
Ted Gould (ted) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libdbusmenu-gtk/parser.c'
2--- libdbusmenu-gtk/parser.c 2012-04-09 16:16:07 +0000
3+++ libdbusmenu-gtk/parser.c 2012-04-10 16:09:19 +0000
4@@ -39,11 +39,27 @@
5 typedef struct _ParserData
6 {
7 GtkWidget *label;
8+ gulong label_notify_handler_id;
9+
10 GtkAction *action;
11- GtkWidget *widget;
12+ gulong action_notify_handler_id;
13+
14 GtkWidget *shell;
15+ gulong item_inserted_handler_id;
16+ gulong item_removed_handler_id;
17+
18 GtkWidget *image;
19+ gulong image_notify_handler_id;
20+
21 AtkObject *accessible;
22+ gulong a11y_handler_id;
23+
24+ GtkWidget *widget;
25+ gulong widget_notify_handler_id;
26+ gulong widget_add_handler_id;
27+ gulong widget_accel_handler_id;
28+ gulong widget_toggle_handler_id;
29+ gulong widget_visible_handler_id;
30
31 } ParserData;
32
33@@ -160,6 +176,41 @@
34 ****
35 ***/
36
37+static void
38+dbusmenu_gtk_clear_signal_handler (gpointer instance, gulong *handler_id)
39+{
40+ if (handler_id && *handler_id) {
41+ /* complain if we thought we were connected but aren't */
42+ if (!g_signal_handler_is_connected (instance, *handler_id)) {
43+ g_debug ("%s tried to disconnect signal handler %lu from disconnected %p", G_STRLOC, *handler_id, instance);
44+ } else {
45+ g_signal_handler_disconnect (instance, *handler_id);
46+ *handler_id = 0;
47+ }
48+ }
49+}
50+
51+/* get the ParserData associated with the specified DbusmenuMenuitem */
52+static ParserData*
53+parser_data_get_from_menuitem (DbusmenuMenuitem * item)
54+{
55+ return (ParserData *) g_object_get_data(G_OBJECT(item), PARSER_DATA);
56+}
57+
58+/* get the ParserData associated with the specified widget */
59+static ParserData*
60+parser_data_get_from_widget (GtkWidget * widget)
61+{
62+ DbusmenuMenuitem * item = dbusmenu_gtk_parse_get_cached_item (widget);
63+ if (item != NULL)
64+ return parser_data_get_from_menuitem (item);
65+ return NULL;
66+}
67+
68+/***
69+****
70+***/
71+
72 /**
73 * dbusmenu_gtk_parse_menu_structure:
74 * @widget: A #GtkMenuItem or #GtkMenuShell to turn into a #DbusmenuMenuitem
75@@ -222,64 +273,47 @@
76 g_return_if_fail (pdata != NULL);
77
78 if (pdata->label != NULL) {
79- gint i = 0;
80- i += g_signal_handlers_disconnect_matched(pdata->label, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
81- 0, 0, NULL, G_CALLBACK(label_notify_cb), NULL);
82- g_warn_if_fail (i != 1);
83- g_object_remove_weak_pointer(G_OBJECT(pdata->label), (gpointer*)&pdata->label);
84+ GObject * o = G_OBJECT(pdata->label);
85+ dbusmenu_gtk_clear_signal_handler (o, &pdata->label_notify_handler_id);
86+ g_object_remove_weak_pointer(o, (gpointer*)&pdata->label);
87 }
88
89 if (pdata->action != NULL) {
90- gint i = 0;
91- i += g_signal_handlers_disconnect_matched(pdata->action, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
92- 0, 0, NULL, G_CALLBACK(action_notify_cb), NULL);
93- g_warn_if_fail (i != 1);
94- g_object_remove_weak_pointer(G_OBJECT(pdata->action), (gpointer*)&pdata->action);
95+ GObject * o = G_OBJECT(pdata->action);
96+ dbusmenu_gtk_clear_signal_handler (o, &pdata->action_notify_handler_id);
97+ g_object_remove_weak_pointer(o, (gpointer*)&pdata->action);
98 }
99
100 if (pdata->widget != NULL) {
101 GObject * o = G_OBJECT(pdata->widget);
102- gint i = 0;
103- i += g_signal_handlers_disconnect_matched(o, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
104- 0, 0, NULL, G_CALLBACK(widget_notify_cb), NULL);
105- i += g_signal_handlers_disconnect_matched(o, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
106- 0, 0, NULL, G_CALLBACK(widget_add_cb), NULL);
107- i += g_signal_handlers_disconnect_matched(o, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
108- 0, 0, NULL, G_CALLBACK(accel_changed), NULL);
109- i += g_signal_handlers_disconnect_matched(o, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
110- 0, 0, NULL, G_CALLBACK(checkbox_toggled), NULL);
111- i += g_signal_handlers_disconnect_matched(o, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
112- 0, 0, NULL, G_CALLBACK(menuitem_notify_cb), NULL);
113- g_warn_if_fail (i != 5);
114+ dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_notify_handler_id);
115+ dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_add_handler_id);
116+ dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_accel_handler_id);
117+ dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_toggle_handler_id);
118+ dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_visible_handler_id);
119 g_object_remove_weak_pointer(o, (gpointer*)&pdata->widget);
120+
121 /* since the DbusmenuMenuitem is being destroyed, uncache it from the GtkWidget */
122 g_object_steal_data(o, CACHED_MENUITEM);
123 }
124
125 if (pdata->shell != NULL) {
126- gint i = 0;
127- i += g_signal_handlers_disconnect_matched(pdata->shell, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
128- 0, 0, NULL, G_CALLBACK(item_inserted_cb), NULL);
129- i += g_signal_handlers_disconnect_matched(pdata->shell, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
130- 0, 0, NULL, G_CALLBACK(item_removed_cb), NULL);
131- g_warn_if_fail (i != 2);
132- g_object_remove_weak_pointer(G_OBJECT(pdata->shell), (gpointer*)&pdata->shell);
133+ GObject * o = G_OBJECT(pdata->shell);
134+ dbusmenu_gtk_clear_signal_handler (o, &pdata->item_inserted_handler_id);
135+ dbusmenu_gtk_clear_signal_handler (o, &pdata->item_removed_handler_id);
136+ g_object_remove_weak_pointer(o, (gpointer*)&pdata->shell);
137 }
138
139 if (pdata->image != NULL) {
140- gint i = 0;
141- i += g_signal_handlers_disconnect_matched(pdata->image, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
142- 0, 0, NULL, G_CALLBACK(image_notify_cb), NULL);
143- g_warn_if_fail (i != 1);
144- g_object_remove_weak_pointer(G_OBJECT(pdata->image), (gpointer*)&pdata->image);
145+ GObject * o = G_OBJECT(pdata->image);
146+ dbusmenu_gtk_clear_signal_handler (o, &pdata->image_notify_handler_id);
147+ g_object_remove_weak_pointer(o, (gpointer*)&pdata->image);
148 }
149
150 if (pdata->accessible != NULL) {
151- gint i = 0;
152- i += g_signal_handlers_disconnect_matched(pdata->accessible, (GSignalMatchType)G_SIGNAL_MATCH_FUNC,
153- 0, 0, NULL, G_CALLBACK(a11y_name_notify_cb), NULL);
154- g_warn_if_fail (i != 1);
155- g_object_remove_weak_pointer(G_OBJECT(pdata->accessible), (gpointer*)&pdata->accessible);
156+ GObject * o = G_OBJECT(pdata->accessible);
157+ dbusmenu_gtk_clear_signal_handler (o, &pdata->a11y_handler_id);
158+ g_object_remove_weak_pointer(o, (gpointer*)&pdata->accessible);
159 }
160
161 g_free(pdata);
162@@ -347,10 +381,10 @@
163 g_return_if_fail(DBUSMENU_IS_MENUITEM(mi));
164 g_return_if_fail(GTK_IS_MENU_SHELL(menu));
165
166- ParserData *pdata = (ParserData *)g_object_get_data(G_OBJECT(mi), PARSER_DATA);
167+ ParserData *pdata = parser_data_get_from_menuitem (mi);
168
169 pdata->shell = menu;
170- g_signal_connect (G_OBJECT (menu),
171+ pdata->item_inserted_handler_id = g_signal_connect (G_OBJECT (menu),
172 #ifdef HAVE_GTK3
173 "insert",
174 #else
175@@ -358,10 +392,10 @@
176 #endif
177 G_CALLBACK (item_inserted_cb),
178 mi);
179- g_signal_connect (G_OBJECT (menu),
180- "remove",
181- G_CALLBACK (item_removed_cb),
182- mi);
183+ pdata->item_removed_handler_id = g_signal_connect (G_OBJECT (menu),
184+ "remove",
185+ G_CALLBACK (item_removed_cb),
186+ mi);
187 g_object_add_weak_pointer(G_OBJECT (menu), (gpointer*)&pdata->shell);
188
189 /* Some apps (notably Eclipse RCP apps) don't fill contents of submenus
190@@ -447,17 +481,18 @@
191 thisitem = construct_dbusmenu_for_widget (widget);
192
193 if (!gtk_widget_get_visible (widget)) {
194- g_signal_connect (G_OBJECT (widget),
195- "notify::visible",
196- G_CALLBACK (menuitem_notify_cb),
197- recurse->toplevel);
198- }
199+ ParserData *pdata = parser_data_get_from_menuitem (thisitem);
200+ pdata->widget_visible_handler_id = g_signal_connect (G_OBJECT (widget),
201+ "notify::visible",
202+ G_CALLBACK (menuitem_notify_cb),
203+ recurse->toplevel);
204+ }
205
206 if (GTK_IS_TEAROFF_MENU_ITEM (widget)) {
207 dbusmenu_menuitem_property_set_bool (thisitem,
208 DBUSMENU_MENUITEM_PROP_VISIBLE,
209 FALSE);
210- }
211+ }
212 }
213
214 /* Check to see if we're in our parents list of children, if we have
215@@ -584,10 +619,8 @@
216 }
217 else
218 {
219- g_signal_connect (widget,
220- "accel-closures-changed",
221- G_CALLBACK (accel_changed),
222- mi);
223+ pdata->widget_accel_handler_id = g_signal_connect (widget, "accel-closures-changed",
224+ G_CALLBACK (accel_changed), mi);
225
226 if (GTK_IS_CHECK_MENU_ITEM (widget))
227 {
228@@ -599,10 +632,7 @@
229 DBUSMENU_MENUITEM_PROP_TOGGLE_STATE,
230 gtk_check_menu_item_get_active (GTK_CHECK_MENU_ITEM (widget)) ? DBUSMENU_MENUITEM_TOGGLE_STATE_CHECKED : DBUSMENU_MENUITEM_TOGGLE_STATE_UNCHECKED);
231
232- g_signal_connect (widget,
233- "activate",
234- G_CALLBACK (checkbox_toggled),
235- mi);
236+ pdata->widget_toggle_handler_id = g_signal_connect (widget, "activate", G_CALLBACK (checkbox_toggled), mi);
237 }
238
239 if (GTK_IS_IMAGE_MENU_ITEM (widget))
240@@ -626,10 +656,7 @@
241 g_free (text);
242
243 pdata->label = label;
244- g_signal_connect (G_OBJECT (label),
245- "notify",
246- G_CALLBACK (label_notify_cb),
247- mi);
248+ pdata->label_notify_handler_id = g_signal_connect (G_OBJECT (label), "notify", G_CALLBACK(label_notify_cb), mi);
249 g_object_add_weak_pointer(G_OBJECT (label), (gpointer*)&pdata->label);
250
251 AtkObject *accessible = gtk_widget_get_accessible (widget);
252@@ -646,10 +673,10 @@
253 // An application may set an alternate accessible name in the future,
254 // so we had better watch out for it.
255 pdata->accessible = accessible;
256- g_signal_connect (G_OBJECT (accessible),
257- "notify::accessible-name",
258- G_CALLBACK (a11y_name_notify_cb),
259- mi);
260+ pdata->a11y_handler_id = g_signal_connect (G_OBJECT (accessible),
261+ "notify::accessible-name",
262+ G_CALLBACK (a11y_name_notify_cb),
263+ mi);
264 g_object_add_weak_pointer(G_OBJECT (accessible), (gpointer*)&pdata->accessible);
265 }
266
267@@ -667,10 +694,9 @@
268 sensitive = gtk_action_is_sensitive (action);
269
270 pdata->action = action;
271- g_signal_connect_object (action, "notify",
272- G_CALLBACK (action_notify_cb),
273- mi,
274- G_CONNECT_AFTER);
275+ pdata->action_notify_handler_id = g_signal_connect_object (action, "notify",
276+ G_CALLBACK (action_notify_cb), mi,
277+ G_CONNECT_AFTER);
278 g_object_add_weak_pointer(G_OBJECT (action), (gpointer*)&pdata->action);
279 }
280 }
281@@ -714,15 +740,11 @@
282 DBUSMENU_MENUITEM_PROP_ENABLED,
283 sensitive);
284
285- g_signal_connect (widget,
286- "notify",
287- G_CALLBACK (widget_notify_cb),
288- mi);
289+ pdata->widget_notify_handler_id = g_signal_connect (widget, "notify",
290+ G_CALLBACK (widget_notify_cb), mi);
291
292- g_signal_connect (widget,
293- "add",
294- G_CALLBACK (widget_add_cb),
295- mi);
296+ pdata->widget_add_handler_id = g_signal_connect (widget, "add",
297+ G_CALLBACK (widget_add_cb), mi);
298
299 return mi;
300 }
301@@ -742,16 +764,15 @@
302 if (pspec->name == interned_str_visible)
303 {
304 GtkWidget * new_toplevel = gtk_widget_get_toplevel (widget);
305- GtkWidget * old_toplevel = GTK_WIDGET(data);
306+ GtkWidget * old_toplevel = GTK_WIDGET(data);
307
308 if (new_toplevel == old_toplevel) {
309 /* TODO: Figure this out -> rebuild (context->bridge, window); */
310 }
311
312 /* We only care about this once, so let's disconnect now. */
313- g_signal_handlers_disconnect_by_func (widget,
314- G_CALLBACK (menuitem_notify_cb),
315- data);
316+ ParserData * pdata = parser_data_get_from_widget (widget);
317+ dbusmenu_gtk_clear_signal_handler (widget, &pdata->widget_visible_handler_id);
318 }
319 }
320
321@@ -785,17 +806,18 @@
322 if (image != GTK_IMAGE(pdata->image)) {
323
324 if (pdata->image != NULL) {
325- g_signal_handlers_disconnect_by_func(pdata->image, G_CALLBACK(image_notify_cb), menuitem);
326- g_object_remove_weak_pointer(G_OBJECT(pdata->image), (gpointer*)&pdata->image);
327+ GObject * o = G_OBJECT(pdata->image);
328+ dbusmenu_gtk_clear_signal_handler (o, &pdata->image_notify_handler_id);
329+ g_object_remove_weak_pointer(o, (gpointer*)&pdata->image);
330 }
331
332 pdata->image = GTK_WIDGET(image);
333
334 if (pdata->image != NULL) {
335- g_signal_connect (G_OBJECT (pdata->image),
336- "notify",
337- G_CALLBACK (image_notify_cb),
338- menuitem);
339+ pdata->image_notify_handler_id = g_signal_connect (G_OBJECT (pdata->image),
340+ "notify",
341+ G_CALLBACK (image_notify_cb),
342+ menuitem);
343 g_object_add_weak_pointer(G_OBJECT (pdata->image), (gpointer*)&pdata->image);
344 }
345 }
346@@ -1205,9 +1227,8 @@
347 */
348 if (GTK_WIDGET (g_value_get_object (&prop_value)) == NULL)
349 {
350- g_signal_handlers_disconnect_by_func (widget,
351- G_CALLBACK (widget_notify_cb),
352- child);
353+ ParserData *pdata = parser_data_get_from_menuitem (child);
354+ dbusmenu_gtk_clear_signal_handler (widget, &pdata->widget_notify_handler_id);
355
356 DbusmenuMenuitem *parent = dbusmenu_menuitem_get_parent (child);
357

Subscribers

People subscribed via source and target branches

to all changes: