From 9b75358ca14e1ad8a41c364195df748a1a1bbf5d Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 13 Mar 2013 17:16:48 +0000 Subject: stop listening to a widget's signals when it's removed from the menu tree that we're mirroring. This condition was detected by a flood of g_warnings, so add a regression test to confirm that the warning messages no longer appear. --- libdbusmenu-gtk/parser.c | 80 ++++++++++++++++++++------------ tests/Makefile.am | 21 +++++++++ tests/test-gtk-remove.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+), 29 deletions(-) create mode 100644 tests/test-gtk-remove.c diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index 6244b07..9d1b034 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -273,10 +273,47 @@ dbusmenu_gtk_parse_menu_structure (GtkWidget * widget) DbusmenuMenuitem * dbusmenu_gtk_parse_get_cached_item (GtkWidget * widget) { - if (!GTK_IS_MENU_ITEM(widget)) { - return NULL; - } - return DBUSMENU_MENUITEM(g_object_get_data(G_OBJECT(widget), CACHED_MENUITEM)); + GObject * o = NULL; + DbusmenuMenuitem * ret = NULL; + + if (GTK_IS_MENU_ITEM (widget)) + o = g_object_get_data (G_OBJECT(widget), CACHED_MENUITEM); + + if (o && DBUSMENU_IS_MENUITEM(o)) + ret = DBUSMENU_MENUITEM (o); + + return ret; +} + +/* remove our dbusmenuitem's hooks to a GtkWidget, + such as when either of them are being destroyed */ +static void +disconnect_from_widget (GtkWidget * widget) +{ + ParserData * pdata = parser_data_get_from_widget (widget); + + if (pdata && pdata->widget) + { + GObject * o; + + g_assert (pdata->widget == widget); + + /* stop listening to signals from the widget */ + o = G_OBJECT (pdata->widget); + dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_notify_handler_id); + dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_add_handler_id); + dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_accel_handler_id); + dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_toggle_handler_id); + dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_visible_handler_id); + dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_screen_changed_handler_id); + + /* clear the menuitem's widget pointer */ + g_object_remove_weak_pointer (o, (gpointer*)&pdata->widget); + pdata->widget = NULL; + + /* clear the widget's menuitem pointer */ + g_object_steal_data(o, CACHED_MENUITEM); + } } static void @@ -297,17 +334,7 @@ parser_data_free (ParserData * pdata) } if (pdata->widget != NULL) { - GObject * o = G_OBJECT(pdata->widget); - dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_notify_handler_id); - dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_add_handler_id); - dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_accel_handler_id); - dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_toggle_handler_id); - dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_visible_handler_id); - dbusmenu_gtk_clear_signal_handler (o, &pdata->widget_screen_changed_handler_id); - g_object_remove_weak_pointer(o, (gpointer*)&pdata->widget); - - /* since the DbusmenuMenuitem is being destroyed, uncache it from the GtkWidget */ - g_object_steal_data(o, CACHED_MENUITEM); + disconnect_from_widget (pdata->widget); } if (pdata->settings != NULL) { @@ -1387,24 +1414,19 @@ item_inserted_cb (GtkContainer *menu, /* A child item was removed from a menu we're watching. */ static void -item_removed_cb (GtkContainer *menu, GtkWidget *widget, gpointer data) +item_removed_cb (GtkContainer *parent_w, GtkWidget *child_w, gpointer data) { - gpointer pmi = g_object_get_data(G_OBJECT(widget), CACHED_MENUITEM); - if (pmi == NULL) { - return; - } + DbusmenuMenuitem * child_mi; - DbusmenuMenuitem * child = DBUSMENU_MENUITEM(pmi); - - pmi = g_object_get_data(G_OBJECT(menu), CACHED_MENUITEM); - if (pmi == NULL) { - return; - } + if ((child_mi = dbusmenu_gtk_parse_get_cached_item (child_w))) + { + DbusmenuMenuitem * parent_mi; - DbusmenuMenuitem * parent = DBUSMENU_MENUITEM(pmi); + if ((parent_mi = dbusmenu_gtk_parse_get_cached_item (GTK_WIDGET(parent_w)))) + dbusmenu_menuitem_child_delete (parent_mi, child_mi); - dbusmenu_menuitem_child_delete(parent, child); - return; + disconnect_from_widget (child_w); + } } static gboolean diff --git a/tests/Makefile.am b/tests/Makefile.am index 0cd68d0..356d803 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,6 +29,7 @@ TESTS += \ test-gtk-label \ test-gtk-shortcut \ test-gtk-reorder \ + test-gtk-remove \ test-gtk-parser-test # Not working with GTK3 and a critical grab that is in # the GTK3 code. @@ -71,6 +72,7 @@ endif if WANT_LIBDBUSMENUGTK check_PROGRAMS += \ test-gtk-objects \ + test-gtk-remove \ test-gtk-label-client \ test-gtk-label-server \ test-gtk-shortcut-client \ @@ -620,6 +622,25 @@ test-gtk-shortcut-python: test-gtk-shortcut-server test-gtk-shortcut-client.py M EXTRA_DIST += test-gtk-shortcut-client.py CLEANFILES += test-gtk-shortcut-client.pyc +######################### +# Test GTK Remove +######################### + +test_gtk_remove_SOURCES = \ + test-gtk-remove.c + +test_gtk_remove_CFLAGS = \ + -I $(srcdir)/.. \ + $(DBUSMENUGTK_CFLAGS) \ + $(DBUSMENUTESTS_CFLAGS) \ + $(DBUSMENUGLIB_CFLAGS) -Wall -Werror + +test_gtk_remove_LDADD = \ + ../libdbusmenu-glib/libdbusmenu-glib.la \ + ../libdbusmenu-gtk/libdbusmenu-gtk$(VER).la \ + $(DBUSMENUGTK_LIBS) \ + $(DBUSMENUTESTS_LIBS) + ######################### # Test GTK Reorder ######################### diff --git a/tests/test-gtk-remove.c b/tests/test-gtk-remove.c new file mode 100644 index 0000000..0655fc0 --- /dev/null +++ b/tests/test-gtk-remove.c @@ -0,0 +1,118 @@ +/* + Confirm that no warnings/criticals get generated by including + multiple add/removes in a match w/o waiting on the mainloop to iterate. + + Copyright 2013 Canonical Ltd. + + Original sample code by Drew Bliss + Modified for automatic testing by Charles Kerr + */ + +#include /* exit() */ +#include +#include +#include +#include + +static GMainLoop * loop = NULL; +static GtkWidget * top_gtk = NULL; +static DbusmenuMenuitem * top_dbusmenu = NULL; +static DbusmenuServer * menuservice = NULL; + +static void +rebuild_menu (void) +{ + GList * l; + GList * children; + int i; + const int n = 10; + static int count = 0; + + if (top_gtk == NULL) + { + top_gtk = gtk_menu_new (); + gtk_widget_show (top_gtk); + top_dbusmenu = dbusmenu_gtk_parse_menu_structure (top_gtk); + menuservice = dbusmenu_server_new ("/org/ayatana/NotificationItem/test/Menu"); + dbusmenu_server_set_root (menuservice, top_dbusmenu); + } + + // remove all the previous children + children = gtk_container_get_children (GTK_CONTAINER(top_gtk)); + for (l=children; l!=NULL; l=l->next) + gtk_widget_destroy (GTK_WIDGET (l->data)); + + // add a handful of new children + for (i=0; i 5) + { + g_main_loop_quit (loop); + return G_SOURCE_REMOVE; + } + + if (iteration <= 2) + { + rebuild_menu (); + } + else + { + int i; + + for (i=0; i