GNOME.org
 

[gcr] gcr: Pass properties changed back in prompter dbus method responses

commit 0bf6e6e710b1246072d0c4cbc05bca9a5dabafd7
Author: Stef Walter <stefw collabora co uk>
Date:   Mon Oct 31 09:59:45 2011 +0100

    gcr: Pass properties changed back in prompter dbus method responses
    
     * This is to prevent race conditions with the PropertiesChanged
       signal.

 gcr/gcr-system-prompt.c          |  111 +++++++++++++++++----------------
 gcr/gcr-system-prompter.c        |  128 ++++++++++++++------------------------
 gcr/org.gnome.keyring.Prompt.xml |   15 +++--
 3 files changed, 114 insertions(+), 140 deletions(-)
---
diff --git a/gcr/gcr-system-prompt.c b/gcr/gcr-system-prompt.c
index 16e1733..5caac14 100644
--- a/gcr/gcr-system-prompt.c
+++ b/gcr/gcr-system-prompt.c
@@ -73,7 +73,6 @@ struct _GcrSystemPromptPrivate {
 	GHashTable *properties_to_write;
 	GHashTable *property_cache;
 	GDBusProxy *prompt_proxy;
-	gulong prompt_properties_sig;
 	gchar *prompt_path;
 	gboolean exchanged;
 	gboolean begun_prompting;
@@ -222,10 +221,8 @@ gcr_system_prompt_dispose (GObject *obj)
 	g_hash_table_remove_all (self->pv->property_cache);
 
 	if (self->pv->prompt_proxy) {
-		g_signal_handler_disconnect (self->pv->prompt_proxy, self->pv->prompt_properties_sig);
 		g_object_unref (self->pv->prompt_proxy);
 		self->pv->prompt_proxy = NULL;
-		self->pv->prompt_properties_sig = 0;
 	}
 
 	g_clear_object (&self->pv->connection);
@@ -350,38 +347,14 @@ lookup_property_in_caches (GcrSystemPrompt *self,
 	if (variant == NULL && self->pv->prompt_proxy) {
 		variant = g_dbus_proxy_get_cached_property (self->pv->prompt_proxy, property_name);
 		if (variant != NULL)
-			g_hash_table_insert (self->pv->property_cache, (gpointer)property_name, variant);
+			g_hash_table_insert (self->pv->property_cache,
+			                     (gpointer)g_intern_string (property_name),
+			                     variant);
 	}
 
 	return variant;
 }
 
-static void
-on_prompt_properties_changed (GDBusProxy *proxy,
-                              GVariant   *changed_properties,
-                              GStrv       invalidated_properties,
-                              gpointer    user_data)
-{
-	GcrSystemPrompt *self = GCR_SYSTEM_PROMPT (user_data);
-	GVariantIter iter;
-	GVariant *value;
-	gchar *key;
-	guint i;
-
-	g_variant_iter_init (&iter, changed_properties);
-	while (g_variant_iter_next (&iter, "{sv}", &key, &value)) {
-		if (!g_hash_table_lookup (self->pv->properties_to_write, key))
-			g_hash_table_remove (self->pv->property_cache, key);
-		g_free (key);
-		g_variant_unref (value);
-	}
-
-	for (i = 0; invalidated_properties != NULL && invalidated_properties[i] != NULL; i++) {
-		if (!g_hash_table_lookup (self->pv->properties_to_write, invalidated_properties[i]))
-			g_hash_table_remove (self->pv->property_cache, invalidated_properties[i]);
-	}
-}
-
 static const gchar *
 prompt_get_string_property (GcrSystemPrompt *self,
                             const gchar *property_name)
@@ -622,8 +595,6 @@ gcr_system_prompt_real_init (GInitable *initable,
 			return FALSE;
 
 		g_dbus_proxy_set_default_timeout (self->pv->prompt_proxy, G_MAXINT);
-		self->pv->prompt_properties_sig = g_signal_connect (self->pv->prompt_proxy, "g-properties-changed",
-		                                                    G_CALLBACK (on_prompt_properties_changed), self);
 	}
 
 	return TRUE;
@@ -717,9 +688,6 @@ on_prompt_proxy_new (GObject *source,
 
 	if (error == NULL) {
 		g_return_if_fail (self->pv->prompt_proxy != NULL);
-		self->pv->prompt_properties_sig = g_signal_connect (self->pv->prompt_proxy, "g-properties-changed",
-		                                                   G_CALLBACK (on_prompt_properties_changed), self);
-
 		perform_init_async (self, res);
 
 	} else {
@@ -826,26 +794,44 @@ parameter_properties (GcrSystemPrompt *self)
 	GVariantBuilder builder;
 	const gchar *property_name;
 	GVariant *variant;
-	gchar *name;
 
-	g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
+	g_variant_builder_init (&builder, G_VARIANT_TYPE ("a(ssv)"));
 
 	g_hash_table_iter_init (&iter, self->pv->properties_to_write);
 	while (g_hash_table_iter_next (&iter, (gpointer *)&property_name, NULL)) {
 		variant = g_hash_table_lookup (self->pv->property_cache, property_name);
-		if (variant == NULL) {
+		if (variant == NULL)
 			g_warning ("couldn't find prompt property to write: %s", property_name);
-		} else {
-			name = g_strdup_printf ("%s.%s", GCR_DBUS_PROMPT_INTERFACE, property_name);
-			g_variant_builder_add (&builder, "{sv}", name, variant);
-			g_free (name);
-		}
+		else
+			g_variant_builder_add (&builder, "(ssv)", GCR_DBUS_PROMPT_INTERFACE,
+			                       property_name, variant);
 	}
 
 	g_hash_table_remove_all (self->pv->properties_to_write);
 	return g_variant_builder_end (&builder);
 }
 
+static void
+return_properties (GcrSystemPrompt *self,
+                   GVariant *properties)
+{
+	GVariantIter *iter;
+	GVariant *value;
+	gchar *interface;
+	gchar *property_name;
+	gpointer key;
+
+	g_variant_get (properties, "a(ssv)", &iter);
+	while (g_variant_iter_loop (iter, "(ssv)", &interface, &property_name, &value)) {
+		key = (gpointer)g_intern_string (property_name);
+		if (!g_hash_table_lookup (self->pv->properties_to_write, key)) {
+			g_hash_table_insert (self->pv->property_cache,
+			                     key, g_variant_ref (value));
+		}
+	}
+	g_variant_iter_free (iter);
+}
+
 static GVariant *
 parameters_for_password (GcrSystemPrompt *self)
 {
@@ -863,7 +849,7 @@ parameters_for_password (GcrSystemPrompt *self)
 	self->pv->exchanged = TRUE;
 
 	properties = parameter_properties (self);
-	params = g_variant_new ("(@a{sv}s)", properties, input);
+	params = g_variant_new ("(@a(ssv)s)", properties, input);
 	g_free (input);
 
 	return params;
@@ -875,11 +861,14 @@ return_for_password (GcrSystemPrompt *self,
                      GError **error)
 {
 	GcrSecretExchange *exchange;
+	GVariant *properties;
 	const gchar *ret = NULL;
 	gchar *output;
 
 	exchange = gcr_system_prompt_get_secret_exchange (self);
-	g_variant_get (retval, "(s)", &output);
+	g_variant_get (retval, "(@a(ssv)s)", &properties, &output);
+
+	return_properties (self, properties);
 
 	if (output && output[0]) {
 		if (!gcr_secret_exchange_receive (exchange, output)) {
@@ -890,6 +879,7 @@ return_for_password (GcrSystemPrompt *self,
 		}
 	}
 
+	g_variant_unref (properties);
 	g_free (output);
 
 	return ret;
@@ -1000,7 +990,24 @@ parameters_for_confirm (GcrSystemPrompt *self)
 	GVariant *properties;
 
 	properties = parameter_properties (self);
-	return g_variant_new ("(@a{sv})", properties);
+	return g_variant_new ("(@a(ssv))", properties);
+}
+
+static gboolean
+return_for_confirm (GcrSystemPrompt *self,
+                    GVariant *retval,
+                    GError **error)
+{
+	GVariant *properties;
+	gboolean confirm = FALSE;
+
+	g_variant_get (retval, "(@a(ssv)b)", &properties, &confirm);
+
+	return_properties (self, properties);
+
+	g_variant_unref (properties);
+
+	return confirm;
 }
 
 static void
@@ -1012,12 +1019,13 @@ on_prompt_requested_confirm (GObject *source,
 	GcrSystemPrompt *self = GCR_SYSTEM_PROMPT (g_async_result_get_source_object (user_data));
 	GError *error = NULL;
 	GVariant *retval;
+	gboolean ret;
 
 	retval = g_dbus_proxy_call_finish (self->pv->prompt_proxy, result, &error);
 
 	if (retval != NULL) {
-		g_simple_async_result_set_op_res_gpointer (res, retval,
-		                                           (GDestroyNotify)g_variant_unref);
+		ret = return_for_confirm (self, retval, &error);
+		g_simple_async_result_set_op_res_gboolean (res, ret);
 	}
 
 	if (error != NULL)
@@ -1064,7 +1072,6 @@ gcr_system_prompt_confirm_finish (GcrSystemPrompt *self,
                                   GError **error)
 {
 	GSimpleAsyncResult *res;
-	gboolean ret = FALSE;
 
 	g_return_val_if_fail (GCR_IS_SYSTEM_PROMPT (self), FALSE);
 	g_return_val_if_fail (g_simple_async_result_is_valid (result, G_OBJECT (self),
@@ -1074,9 +1081,7 @@ gcr_system_prompt_confirm_finish (GcrSystemPrompt *self,
 	if (g_simple_async_result_propagate_error (res, error))
 		return FALSE;
 
-	g_variant_get (g_simple_async_result_get_op_res_gpointer (res),
-	               "(b)", &ret);
-	return ret;
+	return g_simple_async_result_get_op_res_gboolean (res);
 }
 
 /**
@@ -1120,7 +1125,7 @@ gcr_system_prompt_confirm (GcrSystemPrompt *self,
 	                                 cancellable, error);
 
 	if (retval != NULL) {
-		g_variant_get (retval, "(b)", &ret);
+		ret = return_for_confirm (self, retval, error);
 		g_variant_unref (retval);
 	}
 
diff --git a/gcr/gcr-system-prompter.c b/gcr/gcr-system-prompter.c
index e95e61d..7a60fc7 100644
--- a/gcr/gcr-system-prompter.c
+++ b/gcr/gcr-system-prompter.c
@@ -93,59 +93,27 @@ struct _GcrSystemPrompterPrivate {
 	/* Properties */
 	GHashTable *properties;
 	GQueue *properties_changed;
-	guint changed_source;
 };
 
 static gint signals[LAST_SIGNAL] = { 0, };
 
 G_DEFINE_TYPE (GcrSystemPrompter, gcr_system_prompter, G_TYPE_OBJECT);
 
-static void
-dispatch_changed_properties (GcrSystemPrompter *self)
+static GVariant *
+build_changed_properties (GcrSystemPrompter *self)
 {
 	const gchar *property_name;
 	GVariantBuilder builder;
-	GVariantBuilder invalidated;
 	GVariant *value;
-	GError *error = NULL;
-
-	if (self->pv->changed_source) {
-		g_source_remove (self->pv->changed_source);
-		self->pv->changed_source = 0;
-	}
 
-	g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
-	g_variant_builder_init (&invalidated, G_VARIANT_TYPE ("as"));
+	g_variant_builder_init (&builder, G_VARIANT_TYPE ("a(ssv)"));
 
 	while ((property_name = g_queue_pop_head (self->pv->properties_changed))) {
 		value = g_hash_table_lookup (self->pv->properties, property_name);
-		g_variant_builder_add (&builder, "{sv}", property_name, value);
-	}
-
-	g_dbus_connection_emit_signal (self->pv->connection,
-	                               self->pv->owner_name,
-	                               self->pv->prompt_path,
-	                               "org.freedesktop.DBus.Properties",
-	                               "PropertiesChanged",
-	                               g_variant_new ("(sa{sv}as)", GCR_DBUS_PROMPT_INTERFACE,
-	                                              &builder, &invalidated),
-	                               &error);
-
-	if (error != NULL) {
-		g_warning ("couldn't emit properties changed signal: %s", egg_error_message (error));
-		g_clear_error (&error);
+		g_variant_builder_add (&builder, "(ssv)", GCR_DBUS_PROMPT_INTERFACE, property_name, value);
 	}
-}
-
-static gboolean
-on_idle_dispatch_changed (gpointer data)
-{
-	GcrSystemPrompter *self = GCR_SYSTEM_PROMPTER (data);
-
-	self->pv->changed_source = 0;
-	dispatch_changed_properties (self);
 
-	return FALSE; /* Don't run again */
+	return g_variant_builder_end (&builder);
 }
 
 static void
@@ -159,9 +127,6 @@ prompt_emit_changed (GcrSystemPrompter *self,
 	g_queue_push_tail (self->pv->properties_changed,
 	                   (gpointer)g_intern_string (dbus_property));
 
-	if (!self->pv->changed_source)
-		self->pv->changed_source = g_idle_add (on_idle_dispatch_changed, self);
-
 	if (g_str_equal ("Title", dbus_property))
 		object_property = "title";
 	else if (g_str_equal ("Message", dbus_property))
@@ -247,25 +212,23 @@ static void
 prompt_update_properties (GcrSystemPrompter *self,
                           GVariant *properties)
 {
-	const gchar *prefix = GCR_DBUS_PROMPT_INTERFACE ".";
 	GObject *obj = G_OBJECT (self);
 	GVariantIter *iter;
 	GVariant *variant;
-	gchar *full_property_name;
-	const gchar *property_name;
+	gchar *property_name;
+	gchar *interface;
 
 	g_object_freeze_notify (obj);
 
-	g_variant_get (properties, "a{sv}", &iter);
-	while (g_variant_iter_loop (iter, "{sv}", &full_property_name, &variant)) {
-		if (g_str_has_prefix (full_property_name, prefix)) {
-			property_name = full_property_name + strlen (prefix);
-			if (g_hash_table_lookup (self->pv->properties, property_name)) {
-				g_hash_table_insert (self->pv->properties,
-				                     (gpointer)g_intern_string (property_name),
-				                     g_variant_ref (variant));
-				prompt_emit_changed (self, property_name);
-			}
+	g_variant_get (properties, "a(ssv)", &iter);
+	while (g_variant_iter_loop (iter, "(ssv)", &interface, &property_name, &variant)) {
+		if (g_strcmp0 (interface, GCR_DBUS_PROMPT_INTERFACE) != 0)
+			continue;
+		if (g_hash_table_lookup (self->pv->properties, property_name)) {
+			g_hash_table_insert (self->pv->properties,
+					     (gpointer)g_intern_string (property_name),
+					     g_variant_ref (variant));
+			prompt_emit_changed (self, property_name);
 		}
 	}
 
@@ -289,11 +252,6 @@ prompt_clear_properties (GcrSystemPrompter *self)
 {
 	GVariant *variant;
 
-	if (self->pv->changed_source) {
-		g_source_remove (self->pv->changed_source);
-		self->pv->changed_source = 0;
-	}
-
 	variant = g_variant_ref_sink (g_variant_new_string (""));
 	prompt_clear_property (self, GCR_DBUS_PROMPT_PROPERTY_TITLE, variant);
 	prompt_clear_property (self, GCR_DBUS_PROMPT_PROPERTY_MESSAGE, variant);
@@ -391,17 +349,17 @@ prompt_method_request_confirm (GcrSystemPrompter *self,
 
 static void
 prompt_method_call (GDBusConnection *connection,
-		    const gchar *sender,
-		    const gchar *object_path,
-		    const gchar *interface_name,
-		    const gchar *method_name,
-		    GVariant *parameters,
-		    GDBusMethodInvocation *invocation,
-		    gpointer user_data)
+                    const gchar *sender,
+                    const gchar *object_path,
+                    const gchar *interface_name,
+                    const gchar *method_name,
+                    GVariant *parameters,
+                    GDBusMethodInvocation *invocation,
+                    gpointer user_data)
 {
 	GcrSystemPrompter *self = GCR_SYSTEM_PROMPTER (user_data);
-	GVariant *dict = NULL;
-	gchar *string = NULL;;
+	GVariant *properties = NULL;
+	gchar *string = NULL;
 
 	g_return_if_fail (method_name != NULL);
 
@@ -411,19 +369,19 @@ prompt_method_call (GDBusConnection *connection,
 		                                               "This prompt is owned by another process.");
 
 	} else if (g_str_equal (method_name, "RequestPassword")) {
-		g_variant_get (parameters, "(@a{sv}s)", &dict, &string);
-		prompt_method_request_password (self, invocation, dict, string);
+		g_variant_get (parameters, "(@a(ssv)s)", &properties, &string);
+		prompt_method_request_password (self, invocation, properties, string);
 
 	} else if (g_str_equal (method_name, "RequestConfirm")) {
-		g_variant_get (parameters, "(@a{sv})", &dict);
-		prompt_method_request_confirm (self, invocation, dict);
+		g_variant_get (parameters, "(@a(ssv))", &properties);
+		prompt_method_request_confirm (self, invocation, properties);
 
 	} else {
 		g_return_if_reached ();
 	}
 
-	if (dict)
-		g_variant_unref (dict);
+	if (properties)
+		g_variant_unref (properties);
 	g_free (string);
 }
 
@@ -1067,6 +1025,7 @@ gcr_system_prompter_respond_cancelled (GcrSystemPrompter *self)
 {
 	GDBusMethodInvocation *invocation;
 	const gchar *method;
+	GVariant *properties;
 
 	g_return_if_fail (GCR_IS_SYSTEM_PROMPTER (self));
 	g_return_if_fail (self->pv->invocation != NULL);
@@ -1074,15 +1033,18 @@ gcr_system_prompter_respond_cancelled (GcrSystemPrompter *self)
 	invocation = self->pv->invocation;
 	self->pv->invocation = NULL;
 
-	/* Send back all the properties before we respond */
-	dispatch_changed_properties (self);
+	/* Don't send back any changed properties on cancel */
+	properties = g_variant_new_array (G_VARIANT_TYPE ("(ssv)"), NULL, 0);
 
 	method = g_dbus_method_invocation_get_method_name (invocation);
 	if (method && g_str_equal (method, GCR_DBUS_PROMPT_METHOD_PASSWORD))
-		g_dbus_method_invocation_return_value (invocation, g_variant_new ("(s)", ""));
+		g_dbus_method_invocation_return_value (invocation,
+		                                       g_variant_new ("(@a(ssv)s)",
+		                                                      properties, ""));
 
 	else if (method && g_str_equal (method, GCR_DBUS_PROMPT_METHOD_CONFIRM))
-		g_dbus_method_invocation_return_value (invocation, g_variant_new ("(b)", FALSE));
+		g_dbus_method_invocation_return_value (invocation, g_variant_new ("(@a(ssv)b)",
+		                                                                  properties, FALSE));
 
 	else
 		g_return_if_reached ();
@@ -1104,6 +1066,7 @@ gcr_system_prompter_respond_with_password (GcrSystemPrompter *self,
 {
 	GDBusMethodInvocation *invocation;
 	const gchar *method;
+	GVariant *properties;
 	gchar *exchange;
 
 	g_return_if_fail (GCR_IS_SYSTEM_PROMPTER (self));
@@ -1114,13 +1077,14 @@ gcr_system_prompter_respond_with_password (GcrSystemPrompter *self,
 	self->pv->invocation = NULL;
 
 	/* Send back all the properties before we respond */
-	dispatch_changed_properties (self);
+	properties = build_changed_properties (self);
 
 	method = g_dbus_method_invocation_get_method_name (invocation);
 	g_return_if_fail (method != NULL && g_str_equal (method, GCR_DBUS_PROMPT_METHOD_PASSWORD));
 
 	exchange = gcr_secret_exchange_send (self->pv->exchange, password, -1);
-	g_dbus_method_invocation_return_value (invocation, g_variant_new ("(s)", exchange));
+	g_dbus_method_invocation_return_value (invocation, g_variant_new ("(@a(ssv)s)",
+	                                                                  properties, exchange));
 	g_free (exchange);
 
 	g_signal_emit (self, signals[RESPONDED], 0);
@@ -1137,6 +1101,7 @@ void
 gcr_system_prompter_respond_confirmed (GcrSystemPrompter *self)
 {
 	GDBusMethodInvocation *invocation;
+	GVariant *properties;
 	const gchar *method;
 
 	g_return_if_fail (GCR_IS_SYSTEM_PROMPTER (self));
@@ -1146,11 +1111,12 @@ gcr_system_prompter_respond_confirmed (GcrSystemPrompter *self)
 	self->pv->invocation = NULL;
 
 	/* Send back all the properties before we respond */
-	dispatch_changed_properties (self);
+	properties = build_changed_properties (self);
 
 	method = g_dbus_method_invocation_get_method_name (invocation);
 	g_return_if_fail (method != NULL && g_str_equal (method, GCR_DBUS_PROMPT_METHOD_CONFIRM));
-	g_dbus_method_invocation_return_value (invocation, g_variant_new ("(b)", TRUE));
+	g_dbus_method_invocation_return_value (invocation, g_variant_new ("(@a(ssv)b)",
+	                                                                  properties, TRUE));
 
 	g_signal_emit (self, signals[RESPONDED], 0);
 }
diff --git a/gcr/org.gnome.keyring.Prompt.xml b/gcr/org.gnome.keyring.Prompt.xml
index 0028575..c29635b 100644
--- a/gcr/org.gnome.keyring.Prompt.xml
+++ b/gcr/org.gnome.keyring.Prompt.xml
@@ -21,18 +21,21 @@
 
 		<property name="CallerWindow" type="s" access="readwrite"/>
 
+		<!--
+		 * In addition to PropertiesChanged, we send back changed
+		 * properties with the method response. To avoid race conditions
+		-->
+
 		<method name="RequestPassword">
-			<arg name="properties" type="a{sv}" direction="in"/>
+			<arg name="properties" type="a(ssv)" direction="in"/>
 			<arg name="input" type="s" direction="in"/>
-			xxx
-			<arg name="properties" type="a{sv}" direction="out"/>
+			<arg name="changed" type="a(ssv)" direction="out"/>
 			<arg name="output" type="s" direction="out"/>
 		</method>
 
 		<method name="RequestConfirm">
-			<arg name="properties" type="a{sv}" direction="in"/>
-			xxx
-			<arg name="properties" type="a{sv}" direction="out"/>
+			<arg name="properties" type="a(ssv)" direction="in"/>
+			<arg name="changed" type="a(ssv)" direction="out"/>
 			<arg name="confirmed" type="b" direction="out"/>
 		</method>
 	</interface>
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]