Merge lp:~ted/libindicate/show-hide-signals into lp:libindicate/0.6

[Ted Gould]

Change from using cached properties to requesting it. This matches what clients expect and gets better values anyway.

[lp:~ted/libindicate/show-hide-signals]

412. By Ted Gould on 2011-02-02

This is a dead code path, might get used later, but people can find it in Bazaar

413. By Ted Gould on 2011-02-03

Attaching bug

[Mikkel Kamstrup Erlandsen (kamstrup)]

The code looks good, but I still don't entirely get why the properties from the GDBusProxy aren't good enough..?

review: Approve

[Ted Gould (ted)]

On Thu, 2011-02-03 at 11:38 +0000, Mikkel Kamstrup Erlandsen wrote:
> The code looks good, but I still don't entirely get why the properties
> from the GDBusProxy aren't good enough..?

I'm not entirely sure either. I need to investigate that more, but this
does put us in a more stable situation. I think they're not getting
updated properly.

=== modified file 'libindicate/listener.c'
--- libindicate/listener.c	2011-02-01 20:04:05 +0000
+++ libindicate/listener.c	2011-02-03 01:52:09 +0000
@@ -809,7 +809,9 @@
 		proxy_indicator_added(id, proxyt);
 	}
 
-	g_variant_unref(retval);
+	if (retval != NULL) {
+		g_variant_unref(retval);
+	}
 
 	return;
 }
@@ -997,7 +999,9 @@
 	}
 	}
 
-	g_variant_unref(retvalue);
+	if (retvalue != NULL) {
+		g_variant_unref(retvalue);
+	}
 	g_free(get_property_data->property);
 	g_free(get_property_data);
 
@@ -1217,6 +1221,79 @@
 	return;
 }
 
+/* Callback data structure */
+typedef struct _get_server_prop_data_t get_server_prop_data_t;
+struct _get_server_prop_data_t {
+	IndicateListener * listener;
+	IndicateListenerServer * server;
+	indicate_listener_get_server_property_cb callback;
+	indicate_listener_get_server_uint_property_cb callback_uint;
+	gpointer data;
+};
+
+/* This does the actual work of calling the callbacks.  It's split out
+   so that we can use it in the DBus function callback but also in the
+   case where we already have the cached value. */
+static void
+get_server_property_work (get_server_prop_data_t * prop_t, GVariant * prop)
+{
+	if (prop == NULL) {
+		if (prop_t->callback == NULL) {
+			prop_t->callback_uint(prop_t->listener, prop_t->server, 0, prop_t->data);
+		} else {
+			prop_t->callback(prop_t->listener, prop_t->server, NULL, prop_t->data);
+		}
+		return;
+	}
+
+	if (g_variant_is_of_type(prop, G_VARIANT_TYPE_STRING) && prop_t->callback != NULL) {
+		prop_t->callback(prop_t->listener, prop_t->server, g_variant_get_string(prop, NULL), prop_t->data);
+	} else if (g_variant_is_of_type(prop, G_VARIANT_TYPE_OBJECT_PATH) && prop_t->callback != NULL) {
+		prop_t->callback(prop_t->listener, prop_t->server, g_variant_get_string(prop, NULL), prop_t->data);
+	} else if (g_variant_is_of_type(prop, G_VARIANT_TYPE_UINT32) && prop_t->callback_uint != NULL) {
+		prop_t->callback_uint(prop_t->listener, prop_t->server, g_variant_get_uint32(prop), prop_t->data);
+	} else {
+		g_warning("Really?  This can't happen.  WTF!  %s", g_variant_get_type_string(prop));
+	}
+
+	return;
+}
+
+/* Callback from getting the property off of DBus.  Try to complete
+   the call and then pass it up to the worker, clearing out the
+   memory allocations caused here. */
+static void
+get_server_property_cb (GObject * obj, GAsyncResult * res, gpointer user_data)
+{
+	GError * error = NULL;
+	get_server_prop_data_t * prop_t = (get_server_prop_data_t *)user_data;
+
+	GVariant * prop = g_dbus_connection_call_finish(G_DBUS_CONNECTION(obj), res, &error);
+	if (error != NULL) {
+		g_warning("Error getting property! %s", error->message);
+		g_error_free(error);
+	}
+
+	GVariant * unwrap = NULL;
+	if (prop != NULL) {
+		unwrap = g_variant_get_child_value(prop, 0);
+	}
+
+	if (unwrap != NULL && g_variant_is_of_type(unwrap, G_VARIANT_TYPE_VARIANT)) {
+		get_server_property_work(prop_t, g_variant_get_variant(unwrap));
+	} else {
+		get_server_property_work(prop_t, NULL);
+	}
+
+	if (prop != NULL) {
+		g_variant_unref(prop);
+	}
+	
+	g_object_unref(G_OBJECT(prop_t->listener));
+	g_free(prop_t);
+	return;
+}
+
 /* This is a helper function for all the functions that
    get properties from the server.  They all need to have
    a callback setup with an intermediary data structure
@@ -1225,26 +1302,31 @@
 static void
 get_server_property (IndicateListener * listener, IndicateListenerServer * server, indicate_listener_get_server_property_cb callback, indicate_listener_get_server_uint_property_cb callback_uint, const gchar * property_name, gpointer data)
 {
-	GVariant * prop = g_dbus_proxy_get_cached_property(server->proxy, property_name);
-
-	if (prop == NULL) {
-		if (callback == NULL) {
-			callback_uint(listener, server, 0, data);
-		} else {
-			callback(listener, server, NULL, data);
-		}
-		return;
-	}
-
-	if (g_variant_is_of_type(prop, G_VARIANT_TYPE_STRING) && callback != NULL) {
-		callback(listener, server, g_variant_get_string(prop, NULL), data);
-	} else if (g_variant_is_of_type(prop, G_VARIANT_TYPE_OBJECT_PATH) && callback != NULL) {
-		callback(listener, server, g_variant_get_string(prop, NULL), data);
-	} else if (g_variant_is_of_type(prop, G_VARIANT_TYPE_UINT32) && callback_uint != NULL) {
-		callback_uint(listener, server, g_variant_get_uint32(prop), data);
-	} else {
-		g_warning("Really?  This can't happen.  WTF!  %s", g_variant_get_type_string(prop));
-	}
+	IndicateListenerPrivate * priv = INDICATE_LISTENER_GET_PRIVATE(listener);
+	get_server_prop_data_t * prop_t = g_new0(get_server_prop_data_t, 1);
+
+	prop_t->listener = listener;
+	prop_t->server = server;
+	prop_t->callback = callback;
+	prop_t->callback_uint = callback_uint;
+	prop_t->data = data;
+
+	g_object_ref(G_OBJECT(listener));
+
+	/* If we don't have the property cached, let's go and find
+	   it on the bus. */
+	g_dbus_connection_call(priv->session_bus,
+	                       g_dbus_proxy_get_name(server->proxy),
+	                       g_dbus_proxy_get_object_path(server->proxy),
+	                       "org.freedesktop.DBus.Properties",
+	                       "Get",
+	                       g_variant_new("(ss)", INDICATE_DBUS_IFACE, property_name),
+	                       G_VARIANT_TYPE("(v)"),
+	                       G_DBUS_CALL_FLAGS_NONE,
+	                       -1,
+	                       NULL,
+	                       get_server_property_cb,
+	                       prop_t);
 
 	return;
 }