ayatana-commits team mailing list archive

[Ted Gould <ted@xxxxxxxx>]

Ted Gould has proposed merging lp:~ted/dbusmenu/reordering-all-over-again into lp:dbusmenu.

    Requested reviews:
    DBus Menu Team (dbusmenu-team)


Adds revision tracking of the layout to help fix some of the reordering bugs.
-- 
https://code.launchpad.net/~ted/dbusmenu/reordering-all-over-again/+merge/12367
Your team ayatana-commits is subscribed to branch lp:dbusmenu.

=== modified file 'libdbusmenu-glib/client.c'
--- libdbusmenu-glib/client.c	2009-09-11 16:56:07 +0000
+++ libdbusmenu-glib/client.c	2009-09-24 18:45:21 +0000
@@ -67,6 +67,9 @@
 	DBusGProxy * propproxy;
 	DBusGProxyCall * layoutcall;
 
+	gint current_revision;
+	gint my_revision;
+
 	DBusGProxy * dbusproxy;
 
 	GHashTable * type_handlers;
@@ -91,13 +94,13 @@
 static void set_property (GObject * obj, guint id, const GValue * value, GParamSpec * pspec);
 static void get_property (GObject * obj, guint id, GValue * value, GParamSpec * pspec);
 /* Private Funcs */
-static void layout_update (DBusGProxy * proxy, DbusmenuClient * client);
+static void layout_update (DBusGProxy * proxy, gint revision, DbusmenuClient * client);
 static void id_prop_update (DBusGProxy * proxy, guint id, gchar * property, gchar * value, DbusmenuClient * client);
 static void id_update (DBusGProxy * proxy, guint id, DbusmenuClient * client);
 static void build_proxies (DbusmenuClient * client);
 static guint parse_node_get_id (xmlNodePtr node);
 static DbusmenuMenuitem * parse_layout_xml(DbusmenuClient * client, xmlNodePtr node, DbusmenuMenuitem * item, DbusmenuMenuitem * parent, DBusGProxy * proxy);
-static void parse_layout (DbusmenuClient * client, const gchar * layout);
+static gint parse_layout (DbusmenuClient * client, const gchar * layout);
 static void update_layout_cb (DBusGProxy * proxy, DBusGProxyCall * call, void * data);
 static void update_layout (DbusmenuClient * client);
 static void menuitem_get_properties_cb (DBusGProxy * proxy, GHashTable * properties, GError * error, gpointer data);
@@ -195,6 +198,9 @@
 	priv->propproxy = NULL;
 	priv->layoutcall = NULL;
 
+	priv->current_revision = 0;
+	priv->my_revision = 0;
+
 	priv->dbusproxy = NULL;
 
 	priv->type_handlers = g_hash_table_new_full(g_str_hash, g_str_equal,
@@ -301,9 +307,13 @@
 
 /* Annoying little wrapper to make the right function update */
 static void
-layout_update (DBusGProxy * proxy, DbusmenuClient * client)
+layout_update (DBusGProxy * proxy, gint revision, DbusmenuClient * client)
 {
-	update_layout(client);
+	DbusmenuClientPrivate * priv = DBUSMENU_CLIENT_GET_PRIVATE(client);
+	priv->current_revision = revision;
+	if (priv->current_revision > priv->my_revision) {
+		update_layout(client);
+	}
 	return;
 }
 
@@ -467,7 +477,7 @@
 		priv->dbusproxy = NULL;
 	}
 
-	dbus_g_proxy_add_signal(priv->menuproxy, "LayoutUpdate", G_TYPE_INVALID);
+	dbus_g_proxy_add_signal(priv->menuproxy, "LayoutUpdate", G_TYPE_INT, G_TYPE_INVALID);
 	dbus_g_proxy_connect_signal(priv->menuproxy, "LayoutUpdate", G_CALLBACK(layout_update), client, NULL);
 
 	dbus_g_object_register_marshaller(_dbusmenu_server_marshal_VOID__UINT_STRING_STRING, G_TYPE_NONE, G_TYPE_UINT, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_INVALID);
@@ -482,6 +492,34 @@
 	return;
 }
 
+/* Get the "revision" attribute of the node, parse it and
+   return it.  Also we're checking to ensure the node
+   is a 'menu' here. */
+static gint
+parse_node_get_revision (xmlNodePtr node)
+{
+	if (g_strcmp0((gchar *)node->name, "menu") != 0) {
+		/* This kills some nodes early */
+		g_warning("XML Node is not 'menu' it is '%s'", node->name);
+		return 0;
+	}
+
+	xmlAttrPtr attrib;
+	for (attrib = node->properties; attrib != NULL; attrib = attrib->next) {
+		if (g_strcmp0((gchar *)attrib->name, "revision") == 0) {
+			if (attrib->children != NULL) {
+				guint revision = (guint)g_ascii_strtoull((gchar *)attrib->children->content, NULL, 10);
+				/* g_debug ("Found ID: %d", id); */
+				return revision;
+			}
+			break;
+		}
+	}
+
+	g_warning("Unable to find a revision on the node");
+	return 0;
+}
+
 /* Get the ID attribute of the node, parse it and
    return it.  Also we're checking to ensure the node
    is a 'menu' here. */
@@ -671,7 +709,7 @@
 
 /* Take the layout passed to us over DBus and turn it into
    a set of beautiful objects */
-static void
+static gint
 parse_layout (DbusmenuClient * client, const gchar * layout)
 {
 	DbusmenuClientPrivate * priv = DBUSMENU_CLIENT_GET_PRIVATE(client);
@@ -681,6 +719,7 @@
 	xmldoc = xmlReadMemory(layout, g_utf8_strlen(layout, 16*1024), "dbusmenu.xml", NULL, 0);
 
 	xmlNodePtr root = xmlDocGetRootElement(xmldoc);
+	gint revision = parse_node_get_revision(root);
 
 	DbusmenuMenuitem * oldroot = priv->root;
 	priv->root = parse_layout_xml(client, root, priv->root, NULL, priv->menuproxy);
@@ -694,7 +733,7 @@
 		g_signal_emit(G_OBJECT(client), signals[ROOT_CHANGED], 0, priv->root, TRUE);
 	}
 
-	return;
+	return revision;
 }
 
 /* When the layout property returns, here's where we take care of that. */
@@ -716,11 +755,21 @@
 
 	const gchar * xml = g_value_get_string(&value);
 	/* g_debug("Got layout string: %s", xml); */
-	parse_layout(client, xml);
-
+	gint rev = parse_layout(client, xml);
+
+	if (rev == 0) {
+		g_warning("Unable to parse layout!");
+		return;
+	}
+
+	priv->my_revision = rev;
 	/* g_debug("Root is now: 0x%X", (unsigned int)priv->root); */
 	g_signal_emit(G_OBJECT(client), signals[LAYOUT_UPDATED], 0, TRUE);
 
+	if (priv->my_revision < priv->current_revision) {
+		update_layout(client);
+	}
+
 	return;
 }
 
@@ -803,17 +852,6 @@
 		return NULL;
 	}
 
-#if 0
-/* Seems to be a bug in dbus-glib that assert here, I think because
-   multiple people try and grab it.  We're going to comment this out
-   for now as everyone should be listening to the root changed signal
-   anyway. */
-	if (priv->layoutcall != NULL) {
-		/* Will end the current call and block on it's completion */
-		update_layout_cb(priv->propproxy, priv->layoutcall, client);
-	}
-#endif
-
 	return priv->root;
 }
 

=== modified file 'libdbusmenu-glib/dbus-menu.xml'
--- libdbusmenu-glib/dbus-menu.xml	2009-05-15 20:03:35 +0000
+++ libdbusmenu-glib/dbus-menu.xml	2009-09-24 18:45:21 +0000
@@ -57,6 +57,7 @@
 			<arg type="u" name="id" direction="out" />
 		</signal>
 		<signal name="LayoutUpdate">
+			<arg type="i" name="revision" direction="out" />
 		</signal>
 
 <!-- End of interesting stuff -->

=== modified file 'libdbusmenu-glib/menuitem.c'
--- libdbusmenu-glib/menuitem.c	2009-08-25 16:16:26 +0000
+++ libdbusmenu-glib/menuitem.c	2009-09-24 18:45:21 +0000
@@ -806,6 +806,7 @@
 	dbusmenu_menuitem_buildxml:
 	@mi: #DbusmenuMenuitem to represent in XML
 	@array: A list of string that will be turned into an XML file
+	@revision: The revision of the layout to embed in the XML
 
 	This function will add strings to the array @array.  It will put
 	at least one entry if this menu item has no children.  If it has
@@ -814,17 +815,18 @@
 	children to place their own tags in the array in between those two.
 */
 void
-dbusmenu_menuitem_buildxml (DbusmenuMenuitem * mi, GPtrArray * array)
+dbusmenu_menuitem_buildxml (DbusmenuMenuitem * mi, GPtrArray * array, gint revision)
 {
 	g_return_if_fail(DBUSMENU_IS_MENUITEM(mi));
 
 	GList * children = dbusmenu_menuitem_get_children(mi);
+	/* TODO: Only put revision info in the root node.  Save some bandwidth. */
 	if (children == NULL) {
-		g_ptr_array_add(array, g_strdup_printf("<menu id=\"%d\" />", dbusmenu_menuitem_get_id(mi)));
+		g_ptr_array_add(array, g_strdup_printf("<menu id=\"%d\" revision=\"%d\" />", dbusmenu_menuitem_get_id(mi), revision));
 	} else {
-		g_ptr_array_add(array, g_strdup_printf("<menu id=\"%d\">", dbusmenu_menuitem_get_id(mi)));
+		g_ptr_array_add(array, g_strdup_printf("<menu id=\"%d\" revision=\"%d\">", dbusmenu_menuitem_get_id(mi), revision));
 		for ( ; children != NULL; children = children->next) {
-			dbusmenu_menuitem_buildxml(DBUSMENU_MENUITEM(children->data), array);
+			dbusmenu_menuitem_buildxml(DBUSMENU_MENUITEM(children->data), array, revision);
 		}
 		g_ptr_array_add(array, g_strdup("</menu>"));
 	}

=== modified file 'libdbusmenu-glib/menuitem.h'
--- libdbusmenu-glib/menuitem.h	2009-09-02 14:48:52 +0000
+++ libdbusmenu-glib/menuitem.h	2009-09-24 18:45:21 +0000
@@ -136,7 +136,7 @@
 void dbusmenu_menuitem_set_root (DbusmenuMenuitem * mi, gboolean root);
 gboolean dbusmenu_menuitem_get_root (DbusmenuMenuitem * mi);
 
-void dbusmenu_menuitem_buildxml (DbusmenuMenuitem * mi, GPtrArray * array);
+void dbusmenu_menuitem_buildxml (DbusmenuMenuitem * mi, GPtrArray * array, gint revision);
 void dbusmenu_menuitem_foreach (DbusmenuMenuitem * mi, void (*func) (DbusmenuMenuitem * mi, gpointer data), gpointer data);
 void dbusmenu_menuitem_activate (DbusmenuMenuitem * mi);
 

=== modified file 'libdbusmenu-glib/server.c'
--- libdbusmenu-glib/server.c	2009-07-01 05:36:24 +0000
+++ libdbusmenu-glib/server.c	2009-09-24 18:45:21 +0000
@@ -47,6 +47,7 @@
 {
 	DbusmenuMenuitem * root;
 	gchar * dbusobject;
+	gint layout_revision;
 };
 
 #define DBUSMENU_SERVER_GET_PRIVATE(o) \
@@ -142,6 +143,8 @@
 	/**
 		DbusmenuServer::layout-update:
 		@arg0: The #DbusmenuServer emitting the signal.
+		@arg1: A revision number representing which revision the update
+		       represents itself as.
 
 		This signal is emitted any time the layout of the
 		menuitems under this server is changed.
@@ -151,8 +154,8 @@
 	                                         G_SIGNAL_RUN_LAST,
 	                                         G_STRUCT_OFFSET(DbusmenuServerClass, layout_update),
 	                                         NULL, NULL,
-	                                         g_cclosure_marshal_VOID__VOID,
-	                                         G_TYPE_NONE, 0);
+	                                         g_cclosure_marshal_VOID__INT,
+	                                         G_TYPE_NONE, 1, G_TYPE_INT);
 
 
 	g_object_class_install_property (object_class, PROP_DBUS_OBJECT,
@@ -183,6 +186,7 @@
 
 	priv->root = NULL;
 	priv->dbusobject = NULL;
+	priv->layout_revision = 1;
 
 	return;
 }
@@ -235,7 +239,8 @@
 		} else {
 			g_debug("Setting root node to NULL");
 		}
-		g_signal_emit(obj, signals[LAYOUT_UPDATE], 0, TRUE);
+		priv->layout_revision++;
+		g_signal_emit(obj, signals[LAYOUT_UPDATE], 0, priv->layout_revision, TRUE);
 		break;
 	case PROP_LAYOUT:
 		/* Can't set this, fall through to error */
@@ -275,9 +280,9 @@
 		GPtrArray * xmlarray = g_ptr_array_new();
 		if (priv->root == NULL) {
 			/* g_debug("Getting layout without root node!"); */
-			g_ptr_array_add(xmlarray, g_strdup("<menu />"));
+			g_ptr_array_add(xmlarray, g_strdup_printf("<menu revision=\"%d\" />", priv->layout_revision));
 		} else {
-			dbusmenu_menuitem_buildxml(priv->root, xmlarray);
+			dbusmenu_menuitem_buildxml(priv->root, xmlarray, priv->layout_revision);
 		}
 		g_ptr_array_add(xmlarray, NULL);
 
@@ -310,7 +315,9 @@
 {
 	menuitem_signals_create(child, server);
 	/* TODO: We probably need to group the layout update signals to make the number more reasonble. */
-	g_signal_emit(G_OBJECT(server), signals[LAYOUT_UPDATE], 0, TRUE);
+	DbusmenuServerPrivate * priv = DBUSMENU_SERVER_GET_PRIVATE(server);
+	priv->layout_revision++;
+	g_signal_emit(G_OBJECT(server), signals[LAYOUT_UPDATE], 0, priv->layout_revision, TRUE);
 	return;
 }
 
@@ -319,7 +326,18 @@
 {
 	menuitem_signals_remove(child, server);
 	/* TODO: We probably need to group the layout update signals to make the number more reasonble. */
-	g_signal_emit(G_OBJECT(server), signals[LAYOUT_UPDATE], 0, TRUE);
+	DbusmenuServerPrivate * priv = DBUSMENU_SERVER_GET_PRIVATE(server);
+	priv->layout_revision++;
+	g_signal_emit(G_OBJECT(server), signals[LAYOUT_UPDATE], 0, priv->layout_revision, TRUE);
+	return;
+}
+
+static void 
+menuitem_child_moved (DbusmenuMenuitem * parent, DbusmenuMenuitem * child, guint newpos, guint oldpos, DbusmenuServer * server)
+{
+	DbusmenuServerPrivate * priv = DBUSMENU_SERVER_GET_PRIVATE(server);
+	priv->layout_revision++;
+	g_signal_emit(G_OBJECT(server), signals[LAYOUT_UPDATE], 0, priv->layout_revision, TRUE);
 	return;
 }
 
@@ -330,6 +348,7 @@
 {
 	g_signal_connect(G_OBJECT(mi), DBUSMENU_MENUITEM_SIGNAL_CHILD_ADDED, G_CALLBACK(menuitem_child_added), data);
 	g_signal_connect(G_OBJECT(mi), DBUSMENU_MENUITEM_SIGNAL_CHILD_REMOVED, G_CALLBACK(menuitem_child_removed), data);
+	g_signal_connect(G_OBJECT(mi), DBUSMENU_MENUITEM_SIGNAL_CHILD_MOVED, G_CALLBACK(menuitem_child_moved), data);
 	g_signal_connect(G_OBJECT(mi), DBUSMENU_MENUITEM_SIGNAL_PROPERTY_CHANGED, G_CALLBACK(menuitem_property_changed), data);
 	return;
 }
@@ -341,6 +360,7 @@
 {
 	g_signal_handlers_disconnect_by_func(G_OBJECT(mi), G_CALLBACK(menuitem_child_added), data);
 	g_signal_handlers_disconnect_by_func(G_OBJECT(mi), G_CALLBACK(menuitem_child_removed), data);
+	g_signal_handlers_disconnect_by_func(G_OBJECT(mi), G_CALLBACK(menuitem_child_moved), data);
 	g_signal_handlers_disconnect_by_func(G_OBJECT(mi), G_CALLBACK(menuitem_property_changed), data);
 	return;
 }

=== modified file 'libdbusmenu-glib/server.h'
--- libdbusmenu-glib/server.h	2009-05-12 15:31:59 +0000
+++ libdbusmenu-glib/server.h	2009-09-24 18:45:21 +0000
@@ -71,7 +71,7 @@
 	/* Signals */
 	void (*id_prop_update)(guint id, gchar * property, gchar * value);
 	void (*id_update)(guint id);
-	void (*layout_update)(void);
+	void (*layout_update)(gint revision);
 
 	/* Reserved */
 	void (*dbusmenu_server_reserved1)(void);