AppMenu GTK+

Merge lp:~mterry/appmenu-gtk/cancellable-register-window into lp:appmenu-gtk/0.4

  1. cancellable-register-window
  2. Merge into trunk
Proposed by Michael Terry
Status: Merged
Approved by: Ted Gould
Approved revision: 122
Merge reported by: Sebastien Bacher
Merged at revision: not available
Proposed branch: lp:~mterry/appmenu-gtk/cancellable-register-window
Merge into: lp:appmenu-gtk/0.4
Diff against target: 180 lines (+66/-41)
1 file modified
src/bridge.c (+66/-41)
To merge this branch: bzr merge lp:~mterry/appmenu-gtk/cancellable-register-window
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Review via email: mp+52248@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Michael Terry (mterry) wrote :

A quick comment about this branch: it fixed the bug for the reporter, though that user then experienced a different crash that he has not been able to reproduce. So this should get a thorough lookover.

Logic seems sound to me though. Basically, the callback for RegisterWindow needs to be cancelled on dispose.

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

Looks good to me.

  review approve
  merge approve

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/bridge.c'
--- src/bridge.c 2011-03-03 17:19:12 +0000
+++ src/bridge.c 2011-03-04 20:07:05 +0000
@@ -73,6 +73,7 @@
73 gboolean registered;73 gboolean registered;
74 AppMenuBridge *bridge;74 AppMenuBridge *bridge;
75 GHashTable *lookup;75 GHashTable *lookup;
76 GCancellable *cancel;
76};77};
7778
78struct _AppMenuBridgePrivate79struct _AppMenuBridgePrivate
@@ -154,24 +155,13 @@
154}155}
155156
156static void157static void
157app_menu_bridge_dispose (GObject *object)158context_dispose (AppWindowContext *context)
158{159{
159 AppMenuBridge *bridge = APP_MENU_BRIDGE (object);160 if (context->cancel != NULL)
160161 {
161 if (bridge->priv->appmenuproxy)162 g_cancellable_cancel (context->cancel);
162 {163 g_object_unref (context->cancel);
163 g_object_unref (bridge->priv->appmenuproxy);164 context->cancel = NULL;
164 bridge->priv->appmenuproxy = NULL;
165 }
166}
167
168static void
169free_context_contents (AppWindowContext *context)
170{
171 if (context->path != NULL)
172 {
173 g_free (context->path);
174 context->path = NULL;
175 }165 }
176166
177 if (context->server != NULL)167 if (context->server != NULL)
@@ -184,11 +174,41 @@
184 {174 {
185 g_signal_handlers_disconnect_by_func(context->window, G_CALLBACK(mnemonic_shown_cb), context);175 g_signal_handlers_disconnect_by_func(context->window, G_CALLBACK(mnemonic_shown_cb), context);
186 g_object_remove_weak_pointer(G_OBJECT(context->window), (gpointer *)&(context->window));176 g_object_remove_weak_pointer(G_OBJECT(context->window), (gpointer *)&(context->window));
177 context->window = NULL;
187 }178 }
188179
189 if (context->lookup)180 if (context->lookup != NULL)
190 {181 {
191 g_hash_table_unref (context->lookup);182 g_hash_table_unref (context->lookup);
183 context->lookup = NULL;
184 }
185}
186
187static void
188context_free (AppWindowContext *context)
189{
190 context_dispose (context);
191
192 if (context->path != NULL)
193 {
194 g_free (context->path);
195 context->path = NULL;
196 }
197
198 g_free (context);
199}
200
201static void
202app_menu_bridge_dispose (GObject *object)
203{
204 AppMenuBridge *bridge = APP_MENU_BRIDGE (object);
205
206 g_list_foreach (bridge->priv->windows, (GFunc)context_dispose, NULL);
207
208 if (bridge->priv->appmenuproxy)
209 {
210 g_object_unref (bridge->priv->appmenuproxy);
211 bridge->priv->appmenuproxy = NULL;
192 }212 }
193}213}
194214
@@ -196,17 +216,8 @@
196app_menu_bridge_finalize (GObject *object)216app_menu_bridge_finalize (GObject *object)
197{217{
198 AppMenuBridge *bridge = APP_MENU_BRIDGE (object);218 AppMenuBridge *bridge = APP_MENU_BRIDGE (object);
199 GList *tmp = NULL;219
200220 g_list_foreach (bridge->priv->windows, (GFunc)context_free, NULL);
201 for (tmp = bridge->priv->windows; tmp != NULL; tmp = tmp->next)
202 {
203 AppWindowContext *context = tmp->data;
204
205 free_context_contents (context);
206
207 g_free (context);
208 }
209
210 g_list_free (bridge->priv->windows);221 g_list_free (bridge->priv->windows);
211 bridge->priv->windows = NULL;222 bridge->priv->windows = NULL;
212223
@@ -225,11 +236,8 @@
225236
226 if (context)237 if (context)
227 {238 {
228 free_context_contents (context);
229
230 context->bridge->priv->windows = g_list_remove (context->bridge->priv->windows, context);239 context->bridge->priv->windows = g_list_remove (context->bridge->priv->windows, context);
231240 context_free (context);
232 g_free (context);
233 }241 }
234}242}
235243
@@ -308,19 +316,33 @@
308 g_variant_unref(variants);316 g_variant_unref(variants);
309 }317 }
310318
319 if (error != NULL &&
320 error->domain == G_IO_ERROR && error->code == G_IO_ERROR_CANCELLED)
321 {
322 /* If we were cancelled, we've been disposed (and possibly finalized) and
323 shouldn't trust anything in context. This is because cancel callbacks
324 are done in the idle loop for GIO functions. */
325 g_error_free(error);
326 return;
327 }
328
329 if (context->cancel != NULL)
330 {
331 g_object_unref (context->cancel);
332 context->cancel = NULL;
333 }
334
311 if (error != NULL)335 if (error != NULL)
312 {336 {
313 g_warning("Unable to register window with path '%s': %s", context->path, error->message);337 g_warning("Unable to register window with path '%s': %s", context->path, error->message);
314 g_error_free(error);338 g_error_free(error);
315339
340 context->registered = FALSE;
341
316 if (context->bridge != NULL)342 if (context->bridge != NULL)
317 {
318 context->registered = FALSE;
319
320 app_menu_bridge_set_show_local (context->bridge, TRUE);343 app_menu_bridge_set_show_local (context->bridge, TRUE);
321344
322 return;345 return;
323 }
324 }346 }
325347
326 context->registered = TRUE;348 context->registered = TRUE;
@@ -347,8 +369,11 @@
347 }369 }
348 }370 }
349371
350 if (!context->registered && context->server != NULL && GTK_IS_WINDOW (widget) && bridge->priv->appmenuproxy != NULL)372 if (!context->registered && context->server != NULL &&
373 context->cancel == NULL && GTK_IS_WINDOW (widget) &&
374 bridge->priv->appmenuproxy != NULL)
351 {375 {
376 context->cancel = g_cancellable_new ();
352 g_dbus_proxy_call(bridge->priv->appmenuproxy,377 g_dbus_proxy_call(bridge->priv->appmenuproxy,
353 "RegisterWindow",378 "RegisterWindow",
354 g_variant_new("(uo)",379 g_variant_new("(uo)",
@@ -356,7 +381,7 @@
356 context->path),381 context->path),
357 G_DBUS_CALL_FLAGS_NONE,382 G_DBUS_CALL_FLAGS_NONE,
358 -1, /* timeout */383 -1, /* timeout */
359 NULL, /* cancelable */384 context->cancel,
360 register_application_window_cb,385 register_application_window_cb,
361 context);386 context);
362 }387 }

Subscribers

People subscribed via source and target branches