Merge lp:~larsu/indicator-appmenu/lp1075263 into lp:indicator-appmenu/13.04

[Lars Karlitski]

Remove window-destroy timeout

The windows were hashed by xid, and the entries in the hash table weren't
removed until five seconds after the window was destroyed. Thus, restarting an
application within these five seconds and being lucky enough to get the same
xid again would lead to the menu of the old instance being shown. Activating
menu items doesn't work in that case, because the action group is on the dbus
connection of the new instance.

This was initially introduced to work around a bug in BAMF (lp:718926), which
has since been fixed.

[Charles Kerr (charlesk)]

Nice detective work. :)

All this looks right to me, but for completeness' sake I should point out that though this patch seems to mostly be a reversion of https://code.launchpad.net/~ted/indicator-appmenu/destruction-will-happen-in-time/+merge/56641 which was added for lp:718926, this patch and the pre-718926 code have some minor differences (such as in new_window()) and some major ones (such as in unregister_window()).

review: Approve

[Ted Gould (ted)]

review: Approve

=== modified file 'src/indicator-appmenu.c'
--- src/indicator-appmenu.c	2012-05-08 00:26:54 +0000
+++ src/indicator-appmenu.c	2012-11-07 15:02:19 +0000
@@ -106,8 +106,6 @@
 	GDBusConnection * bus;
 	guint owner_id;
 	guint dbus_registration;
-
-	GHashTable * destruction_timers;
 };
 
 
@@ -203,7 +201,6 @@
                                                                       gpointer user_data);
 static void menus_destroyed                                          (GObject * menus,
                                                                       gpointer user_data);
-static void source_unregister                                        (gpointer user_data);
 static GVariant * unregister_window                                  (IndicatorAppmenu * iapp,
                                                                       guint windowid);
 
@@ -287,9 +284,6 @@
 	self->desktop_windows = g_hash_table_new(g_direct_hash, g_direct_equal);
 	self->desktop_menu = NULL; /* Starts NULL until found */
 
-	/* Set up the hashtable of destruction timers */
-	self->destruction_timers = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, source_unregister);
-
 	build_window_menus(self);
 
 	/* Get the default BAMF matcher */
@@ -408,14 +402,6 @@
 		iapp->dbus_registration = 0;
 	}
 
-	if (iapp->destruction_timers != NULL) {
-		/* These are in dispose and not finalize becuase the dereference
-		   function removes timers that could need the object to be in
-		   a valid state, so it's better to have them in dispose */
-		g_hash_table_destroy(iapp->destruction_timers);
-		iapp->destruction_timers = NULL;
-	}
-
 	if (iapp->bus != NULL) {
 		g_object_unref(iapp->bus);
 		iapp->bus = NULL;
@@ -627,9 +613,6 @@
 	IndicatorAppmenu * iapp = INDICATOR_APPMENU(user_data);
 	guint32 xid = bamf_window_get_xid(window);
 
-	/* Make sure we don't destroy it later */
-	g_hash_table_remove(iapp->destruction_timers, GUINT_TO_POINTER(xid));
-
 	if (bamf_window_get_window_type(window) != BAMF_WINDOW_DESKTOP) {
 		return;
 	}
@@ -651,32 +634,6 @@
 	return;
 }
 
-typedef struct _destroy_data_t destroy_data_t;
-struct _destroy_data_t {
-	IndicatorAppmenu * iapp;
-	guint32 xid;
-};
-
-/* Timeout to finally cleanup the window.  Causes is to ignore glitches that
-   come from BAMF/WNCK. */
-static gboolean
-destroy_window_timeout (gpointer user_data)
-{
-	destroy_data_t * destroy_data = (destroy_data_t *)user_data;
-	g_hash_table_steal(destroy_data->iapp->destruction_timers, GUINT_TO_POINTER(destroy_data->xid));
-	unregister_window(destroy_data->iapp, destroy_data->xid);
-	return FALSE; /* free's data through source deregistration */
-}
-
-/* Unregisters the source in the hash table when it gets removed.  This ensure
-   we don't leave any timeouts around */
-static void
-source_unregister (gpointer user_data)
-{
-	g_source_remove(GPOINTER_TO_UINT(user_data));
-	return;
-}
-
 /* When windows leave us, this function gets called */
 static void
 old_window (BamfMatcher * matcher, BamfView * view, gpointer user_data)
@@ -689,12 +646,7 @@
 	BamfWindow * window = BAMF_WINDOW(view);
 	guint32 xid = bamf_window_get_xid(window);
 
-	destroy_data_t * destroy_data = g_new0(destroy_data_t, 1);
-	destroy_data->iapp = iapp;
-	destroy_data->xid = xid;
-
-	guint source_id = g_timeout_add_seconds_full(G_PRIORITY_LOW, 5, destroy_window_timeout, destroy_data, g_free);
-	g_hash_table_replace(iapp->destruction_timers, GUINT_TO_POINTER(xid), GUINT_TO_POINTER(source_id));
+	unregister_window(iapp, xid);
 
 	return;
 }
@@ -1178,9 +1130,6 @@
 {
 	g_debug("Registering window ID %d with path %s from %s", windowid, objectpath, sender);
 
-	/* Shouldn't do anything, but let's be sure */
-	g_hash_table_remove(iapp->destruction_timers, GUINT_TO_POINTER(windowid));
-
 	if (g_hash_table_lookup(iapp->apps, GUINT_TO_POINTER(windowid)) == NULL && windowid != 0) {
 		WindowMenu * wm = WINDOW_MENU(window_menu_dbusmenu_new(windowid, sender, objectpath));
 		g_return_val_if_fail(wm != NULL, FALSE);
@@ -1230,9 +1179,6 @@
 	g_return_val_if_fail(IS_INDICATOR_APPMENU(iapp), NULL);
 	g_return_val_if_fail(iapp->matcher != NULL, NULL);
 
-	/* Make sure we don't destroy it later */
-	g_hash_table_remove(iapp->destruction_timers, GUINT_TO_POINTER(windowid));
-
 	/* If it's a desktop window remove it from that table as well */
 	g_hash_table_remove(iapp->desktop_windows, GUINT_TO_POINTER(windowid));