[network-manager-applet] editor: serialize PolicyKit secrets requests (rh #462944) (lp:462944)

[Dan Williams <dcbw src gnome org>]

commit 325ec4ffd1ed19506c9e9896436f3ee7c1b82cde
Author: Dan Williams <dcbw redhat com>
Date:   Sat Dec 5 10:43:34 2009 -0800

    editor: serialize PolicyKit secrets requests (rh #462944) (lp:462944)
    
    PolicyKit doesn't queue up authorization requests internally.  Instead,
    if there's a pending authorization request, subsequent requests for that
    same authorization will return NotAuthorized+Challenge.  That's pretty
    inconvenient and it would be a lot nicer if PK just queued up subsequent
    authorization requests and executed them when the first one was finished.
    But since it doesn't do that, we have to serialize the authorization
    requests ourselves to get the right authorization result.
    
    This can be reverted whenever (if) PK starts serializing the requests
    internally.

 src/connection-editor/nm-connection-editor.c |  151 ++++++++++++++++++--------
 src/connection-editor/nm-connection-editor.h |    8 +-
 2 files changed, 110 insertions(+), 49 deletions(-)
---
diff --git a/src/connection-editor/nm-connection-editor.c b/src/connection-editor/nm-connection-editor.c
index 5fc761d..b59e476 100644
--- a/src/connection-editor/nm-connection-editor.c
+++ b/src/connection-editor/nm-connection-editor.c
@@ -84,13 +84,12 @@ static gboolean nm_connection_editor_set_connection (NMConnectionEditor *editor,
                                                      NMConnection *connection,
                                                      GError **error);
 
-typedef struct {
+struct GetSecretsInfo {
 	NMConnectionEditor *self;
 	CEPage *page;
 	char *setting_name;
 	gboolean canceled;
-} GetSecretsInfo;
-
+};
 
 static void
 nm_connection_editor_update_title (NMConnectionEditor *editor)
@@ -144,6 +143,12 @@ ui_to_setting (NMConnectionEditor *editor)
 	return TRUE;
 }
 
+static gboolean
+editor_is_initialized (NMConnectionEditor *editor)
+{
+	return (g_slist_length (editor->initializing_pages) == 0);
+}
+
 static void
 update_sensitivity (NMConnectionEditor *editor)
 {
@@ -157,7 +162,8 @@ update_sensitivity (NMConnectionEditor *editor)
 	/* Can't modify read-only connections; can't modify anything before the
 	 * editor is initialized either.
 	 */
-	if (!nm_setting_connection_get_read_only (s_con) && editor->initialized) {
+	if (   !nm_setting_connection_get_read_only (s_con)
+	    && editor_is_initialized (editor)) {
 		if (editor->system_settings_can_modify) {
 			actionable = ce_polkit_button_get_actionable (CE_POLKIT_BUTTON (editor->ok_button));
 			authorized = ce_polkit_button_get_authorized (CE_POLKIT_BUTTON (editor->ok_button));
@@ -208,7 +214,7 @@ connection_editor_validate (NMConnectionEditor *editor)
 	gboolean valid = FALSE;
 	GSList *iter;
 
-	if (!editor->initialized)
+	if (!editor_is_initialized (editor))
 		goto done;
 
 	s_con = NM_SETTING_CONNECTION (nm_connection_get_setting (editor->connection, NM_TYPE_SETTING_CONNECTION));
@@ -311,23 +317,38 @@ nm_connection_editor_init (NMConnectionEditor *editor)
 }
 
 static void
+get_secrets_info_free (GetSecretsInfo *info)
+{
+	g_free (info->setting_name);
+	g_free (info);
+}
+
+static void
 dispose (GObject *object)
 {
 	NMConnectionEditor *editor = NM_CONNECTION_EDITOR (object);
 	GSList *iter;
 
+	editor->disposed = TRUE;
+
+	g_slist_foreach (editor->initializing_pages, (GFunc) g_object_unref, NULL);
+	g_slist_free (editor->initializing_pages);
+	editor->initializing_pages = NULL;
+
 	g_slist_foreach (editor->pages, (GFunc) g_object_unref, NULL);
 	g_slist_free (editor->pages);
 	editor->pages = NULL;
 
-	/* Mark any in-progress secrets calls as canceled; they will clean up
-	 * after themselves.
-	 */
-	for (iter = editor->secrets_calls; iter; iter = g_slist_next (iter)) {
-		GetSecretsInfo *info = iter->data;
-		info->canceled = TRUE;
+	/* Mark any in-progress secrets call as canceled; it will clean up after itself. */
+	if (editor->secrets_call)
+		editor->secrets_call->canceled = TRUE;
+
+	/* Kill any pending secrets calls */
+	for (iter = editor->pending_secrets_calls; iter; iter = g_slist_next (iter)) {
+		get_secrets_info_free ((GetSecretsInfo *) iter->data);
 	}
-	g_slist_free (editor->secrets_calls);
+	g_slist_free (editor->pending_secrets_calls);
+	editor->pending_secrets_calls = NULL;
 
 	if (editor->connection) {
 		g_object_unref (editor->connection);
@@ -505,20 +526,14 @@ idle_validate (gpointer user_data)
 static void
 recheck_initialization (NMConnectionEditor *editor)
 {
-	GSList *iter;
-
-	/* Check if all pages are initialized; if not, desensitize the editor */
-	for (iter = editor->pages; iter; iter = g_slist_next (iter)) {
-		if (!ce_page_get_initialized (CE_PAGE (iter->data))) {
-			update_sensitivity (editor);
-			return;
-		}
-	}
-
-	editor->initialized = TRUE;
+	if (!editor_is_initialized (editor))
+		return;
 
 	populate_connection_ui (editor);
 
+	/* When everything is initialized, re-present the window to ensure it's on top */
+	nm_connection_editor_present (editor);
+
 	/* Validate the connection from an idle handler to ensure that stuff like
 	 * GtkFileChoosers have had a chance to asynchronously find their files.
 	 */
@@ -534,7 +549,6 @@ page_initialized (CEPage *page, gpointer unused, GError *error, gpointer user_da
 	GtkWidget *label;
 
 	if (error) {
-		g_object_unref (page);
 		gtk_widget_hide (editor->window);
 		g_signal_emit (editor, editor_signals[EDITOR_DONE], 0, GTK_RESPONSE_NONE, error);
 		return;
@@ -545,11 +559,16 @@ page_initialized (CEPage *page, gpointer unused, GError *error, gpointer user_da
 	label = gtk_label_new (ce_page_get_title (page));
 	widget = ce_page_get_page (page);
 	gtk_notebook_append_page (GTK_NOTEBOOK (notebook), widget, label);
+
+	/* Move the page from the initializing list to the main page list */
+	editor->initializing_pages = g_slist_remove (editor->initializing_pages, page);
 	editor->pages = g_slist_append (editor->pages, page);
 
 	recheck_initialization (editor);
 }
 
+static void request_secrets (GetSecretsInfo *info);
+
 static void
 get_secrets_cb (NMSettingsConnectionInterface *connection,
                 GHashTable *secrets,
@@ -557,14 +576,54 @@ get_secrets_cb (NMSettingsConnectionInterface *connection,
                 gpointer user_data)
 {
 	GetSecretsInfo *info = user_data;
+	NMConnectionEditor *self;
 
-	if (!info->canceled) {
-		info->self->secrets_calls = g_slist_remove (info->self->secrets_calls, info);
-		ce_page_complete_init (info->page, info->setting_name, secrets, error);
+	if (info->canceled) {
+		get_secrets_info_free (info);
+		return;
 	}
 
-	g_free (info->setting_name);
-	g_free (info);
+	self = info->self;
+
+	/* Complete this secrets request; completion can actually dispose of the
+	 * dialog if there was an error.
+	 */
+	self->secrets_call = NULL;
+	ce_page_complete_init (info->page, info->setting_name, secrets, error);
+	get_secrets_info_free (info);
+
+	/* Kick off the next secrets request if there is one queued; if the dialog
+	 * was disposed of by the completion above we don't need to do anything.
+	 */
+	if (!self->disposed && self->pending_secrets_calls) {
+		self->secrets_call = g_slist_nth_data (self->pending_secrets_calls, 0);
+		self->pending_secrets_calls = g_slist_remove (self->pending_secrets_calls, self->secrets_call);
+
+		request_secrets (self->secrets_call);
+	}
+}
+
+static void
+request_secrets (GetSecretsInfo *info)
+{
+	NMSettingsConnectionInterface *connection;
+	gboolean success;
+	GError *error;
+
+	g_return_if_fail (info != NULL);
+
+	connection = NM_SETTINGS_CONNECTION_INTERFACE (info->self->orig_connection);
+	success = nm_settings_connection_interface_get_secrets (connection,
+	                                                        info->setting_name,
+	                                                        NULL,
+	                                                        FALSE,
+	                                                        get_secrets_cb,
+	                                                        info);
+	if (!success) {
+		error = g_error_new_literal (0, 0, _("Failed to update connection secrets due to an unknown error."));
+		get_secrets_cb (connection, NULL, error, info);
+		g_error_free (error);
+	}
 }
 
 static void
@@ -573,7 +632,6 @@ get_secrets_for_page (NMConnectionEditor *self,
                       const char *setting_name)
 {
 	GetSecretsInfo *info;
-	gboolean success = FALSE;
 
 	if (!setting_name) {
 		/* page doesn't need any secrets */
@@ -599,24 +657,22 @@ get_secrets_for_page (NMConnectionEditor *self,
 	info->page = page;
 	info->setting_name = g_strdup (setting_name);
 
-	success = nm_settings_connection_interface_get_secrets (NM_SETTINGS_CONNECTION_INTERFACE (self->orig_connection),
-	                                                        setting_name,
-	                                                        NULL,
-	                                                        FALSE,
-	                                                        get_secrets_cb,
-	                                                        info);
-	if (success) {
-		self->secrets_calls = g_slist_prepend (self->secrets_calls, info);
-	} else {
-		GError *error;
-
-		error = g_error_new_literal (0, 0, _("Failed to update connection secrets due to an unknown error."));
-		get_secrets_cb (NM_SETTINGS_CONNECTION_INTERFACE (self->orig_connection), NULL, error, info);
-		g_error_free (error);
+	/* PolicyKit doesn't queue up authorization requests internally.  Instead,
+	 * if there's a pending authorization request, subsequent requests for that
+	 * same authorization will return NotAuthorized+Challenge.  That's pretty
+	 * inconvenient and it would be a lot nicer if PK just queued up subsequent
+	 * authorization requests and executed them when the first one was finished.
+	 * But it since it doesn't do that, we have to serialize the authorization
+	 * requests ourselves to get the right authorization result.
+	 */
 
-		/* Free the secrets info */
-		g_free (info->setting_name);
-		g_free (info);
+	/* If there's already an in-progress call, queue up the new one */
+	if (self->secrets_call)
+		self->pending_secrets_calls = g_slist_append (self->pending_secrets_calls, info);
+	else {
+		/* Request secrets for this page */
+		self->secrets_call = info;
+		request_secrets (info);
 	}
 }
 
@@ -637,6 +693,7 @@ add_page (NMConnectionEditor *editor,
 	if (!page)
 		return FALSE;
 
+	editor->initializing_pages = g_slist_prepend (editor->initializing_pages, page);
 	g_signal_connect (page, "changed", G_CALLBACK (page_changed), editor);
 	g_signal_connect (page, "initialized", G_CALLBACK (page_initialized), editor);
 
diff --git a/src/connection-editor/nm-connection-editor.h b/src/connection-editor/nm-connection-editor.h
index 492cddb..63e1c63 100644
--- a/src/connection-editor/nm-connection-editor.h
+++ b/src/connection-editor/nm-connection-editor.h
@@ -34,21 +34,25 @@
 #define NM_IS_CONNECTION_EDITOR(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_CONNECTION_EDITOR))
 #define NM_CONNECTION_EDITOR(obj)    (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_CONNECTION_EDITOR, NMConnectionEditor))
 
+typedef struct GetSecretsInfo GetSecretsInfo;
+
 typedef struct {
 	GObject parent;
+	gboolean disposed;
 
 	/* private data */
 	NMConnection *connection;
 	NMConnection *orig_connection;
-	gboolean initialized;
 
 	NMConnectionScope orig_scope;
 
-	GSList *secrets_calls;
+	GetSecretsInfo *secrets_call;
+	GSList *pending_secrets_calls;
 
 	GtkWidget *system_checkbutton;
 	gboolean system_settings_can_modify;
 
+	GSList *initializing_pages;
 	GSList *pages;
 	GladeXML *xml;
 	GtkWidget *window;