Merge lp:~rodrigo-moya/libubuntuone/avoid-webkit-races into lp:libubuntuone

[Rodrigo Moya]

Don't load pages on the navigation_requested callback, do it in an idle callback to avoid WebKit races

Use WebKitNetworkRequest object to redirect navigation

[Tim Cole (tcole)]

Looks sensible.

review: Approve

[lp:~rodrigo-moya/libubuntuone/avoid-webkit-races]

83. By Rodrigo Moya on 2010-04-08

Do the redirection in an idle callback

84. By Rodrigo Moya on 2010-04-08

Move library overriding to the if...else...block

[Elliot Murphy (statik)]

This fixes my crashing bug.

review: Approve

=== modified file 'libubuntuone/u1-music-store.c'
--- libubuntuone/u1-music-store.c	2010-04-08 15:05:16 +0000
+++ libubuntuone/u1-music-store.c	2010-04-08 18:18:23 +0000
@@ -161,9 +161,8 @@
 	object_class->finalize = u1_music_store_finalize;
 }
 
-
-static void
-load_internal_html_page (WebKitWebView *web_view, const gchar *file_name, const gchar *reload_url)
+static gchar *
+get_internal_html_page_url (WebKitWebView *web_view, const gchar *file_name, const gchar *reload_url)
 {
 	gchar *calculated_file_path, *calculated_file_url, *calculated_file_url_noreload;
 
@@ -177,9 +176,20 @@
 			reload_url);
 		g_free (calculated_file_url_noreload);
 	}
+
+	g_free (calculated_file_path);
+
+	return calculated_file_url;
+}
+
+static void
+load_internal_html_page (WebKitWebView *web_view, const gchar *file_name, const gchar *reload_url)
+{
+	gchar *calculated_file_url;
+
+	calculated_file_url = get_internal_html_page_url (web_view, file_name, reload_url);
 	webkit_web_view_open (web_view, calculated_file_url);
 	g_free (calculated_file_url);
-	g_free (calculated_file_path);
 }
 
 static void
@@ -381,18 +391,54 @@
 	g_free (nrrd);
 }
 
-static void
-fire_authentication (U1MusicStore *music_store, const gchar *uri)
+typedef struct {
+	WebKitWebView *web_view;
+	char *uri;
+} DelayedLoadData;
+
+static gboolean
+html_delayed_load_cb (gpointer user_data)
+{
+	DelayedLoadData *delayed_data = (DelayedLoadData *) user_data;
+
+	webkit_web_view_open (delayed_data->web_view, delayed_data->uri);
+
+	g_object_unref (G_OBJECT (delayed_data->web_view));
+	g_free (delayed_data->uri);
+	g_free (delayed_data);
+
+	return FALSE;
+}
+
+static void
+load_delayed_page (WebKitWebView *web_view, const char *uri, WebKitWebPolicyDecision *policy_decision)
+{
+	DelayedLoadData *delayed_data;
+
+	delayed_data = g_new0 (DelayedLoadData, 1);
+	delayed_data->web_view = g_object_ref (web_view);
+	delayed_data->uri = g_strdup (uri);
+
+	g_idle_add ((GSourceFunc) html_delayed_load_cb, delayed_data);
+
+	webkit_web_policy_decision_ignore (policy_decision);
+}
+
+static void
+fire_authentication (U1MusicStore *music_store, WebKitNetworkRequest *request, WebKitWebPolicyDecision *policy_decision)
 {
 	NotRegisteredReplacementData *nrrd;
+	gchar *internal_uri;
 
 	g_debug ("firing authentication");
 
-	load_internal_html_page (WEBKIT_WEB_VIEW (music_store->priv->web_viewer), U1_CONNECTING_PAGE, NULL);
+	internal_uri = get_internal_html_page_url (WEBKIT_WEB_VIEW (music_store->priv->web_viewer), U1_CONNECTING_PAGE, NULL);
+	load_delayed_page (WEBKIT_WEB_VIEW (music_store->priv->web_viewer), internal_uri, policy_decision);
+	g_free (internal_uri);
 
 	nrrd = g_new0 (NotRegisteredReplacementData, 1);
 	nrrd->music_store = music_store;
-	nrrd->not_registered_url = g_strdup (uri);
+	nrrd->not_registered_url = g_strdup (webkit_network_request_get_uri (request));
 
 	authentication_set_callbacks (G_CALLBACK (authorization_denied_cb),
 				      G_CALLBACK (authorization_denied_cb),
@@ -412,7 +458,7 @@
 {
 	U1MusicStore *music_store = U1_MUSIC_STORE (user_data);
 	SoupURI *parsed_uri;
-	gboolean return_value = FALSE;
+	gchar *internal_uri;
 
 	g_debug ("navigation requested to %s", webkit_network_request_get_uri (request));
 
@@ -433,19 +479,28 @@
 		return FALSE;
 
 	if (g_str_has_prefix (webkit_network_request_get_uri (request), U1_NOT_REGISTERED_URL)) {
-		fire_authentication (music_store, webkit_network_request_get_uri (request));
-
-		return_value = TRUE;
+		fire_authentication (music_store, request, policy_decision);
 	} else if (g_str_has_prefix (parsed_uri->path, "/auth/login") &&
 		   g_strrstr (parsed_uri->query, "next=") != NULL) {
-		fire_authentication (music_store, webkit_network_request_get_uri (request));
-
-		return_value = TRUE;
+		fire_authentication (music_store, request, policy_decision);
 	} else if (g_str_has_prefix (parsed_uri->path, U1_TOKEN_INVALID_URL)) {
 		authentication_clear_tokens ();
-		fire_authentication (music_store, webkit_network_request_get_uri (request));
-
-		return_value = TRUE;
+		fire_authentication (music_store, request, policy_decision);
+	} else if (g_strrstr (parsed_uri->path, "library.aspx") != NULL ||
+		 g_str_has_prefix (parsed_uri->path, "/music/store/library")) {
+		//g_str_has_prefix (webkit_network_request_get_uri (request), "https://one.ubuntu.com/music/store/library")) {
+		const gchar *library_override;
+
+		/* If the URL being loaded is the 7d library page, and we have a library
+		   override set, load ours instead. */
+		library_override = g_getenv ("U1MUSICLIBRARYURL");
+		if (library_override != NULL &&
+		    g_strcmp0 (library_override, webkit_network_request_get_uri (request)) != 0) {
+			g_debug ("overriding the library page to ours");
+			load_delayed_page (web_view, library_override, policy_decision);
+
+			return TRUE;
+		}
 	} else if (!g_strcmp0 ((const gchar *) parsed_uri->host, "www.7digital.com")) {
 		/* host is 7digital.com. It must be in our store. */
 		if (!g_str_has_prefix (parsed_uri->path, "/stores/")) {
@@ -456,10 +511,14 @@
 			to_and_from_error_uris = g_strdup_printf ("%s:::%s", 
 								  webkit_web_view_get_uri (web_view),
 								  webkit_network_request_get_uri (request));
-			load_internal_html_page (web_view, U1_OUTSIDE_DOMAIN_ERROR_PAGE, to_and_from_error_uris);
+
+			internal_uri = get_internal_html_page_url (web_view, U1_OUTSIDE_DOMAIN_ERROR_PAGE, to_and_from_error_uris);
+			load_delayed_page (web_view, internal_uri, policy_decision);
+
+			g_free (internal_uri);
 			g_free (to_and_from_error_uris);
 
-			return_value = TRUE;
+			return TRUE;
 		}
 	} else if (!g_strcmp0 ((const gchar *) parsed_uri->host, "one.ubuntu.com")) {
 		/* host is one.ubuntu.com. It must not be the login page */
@@ -475,14 +534,18 @@
 								  real_url);
 			g_debug("Being led through Ubuntu One login inside Rhythmbox, so throwing an error");
 			if (real_url != NULL) {
-				load_internal_html_page (web_view, U1_TRYING_SSO_ERROR_PAGE, to_and_from_error_uris);
+				internal_uri = get_internal_html_page_url (web_view, U1_TRYING_SSO_ERROR_PAGE, to_and_from_error_uris);
 				g_free (real_url);
 			} else {
-				load_internal_html_page (web_view, U1_TRYING_SSO_ERROR_PAGE, NULL);
+				internal_uri = get_internal_html_page_url (web_view, U1_TRYING_SSO_ERROR_PAGE, NULL);
 			}
+
+		        load_delayed_page (web_view, internal_uri, policy_decision);
+
+			g_free (internal_uri);
 			g_free (to_and_from_error_uris);
 
-			return_value = TRUE;
+			return TRUE;
 		}
 	} else if (!g_strcmp0 ((const gchar *) parsed_uri->host, "login.ubuntu.com")) {
 		gchar *real_url;
@@ -496,42 +559,24 @@
 							  real_url);
 		g_debug("Being led through Ubuntu One login inside Rhythmbox, so throwing an error");
 		if (real_url != NULL) {
-			load_internal_html_page (web_view, U1_TRYING_SSO_ERROR_PAGE, to_and_from_error_uris);
+			internal_uri = get_internal_html_page_url (web_view, U1_TRYING_SSO_ERROR_PAGE, to_and_from_error_uris);
 			g_free (real_url);
 		} else {
-			load_internal_html_page (web_view, U1_TRYING_SSO_ERROR_PAGE, NULL);
+			internal_uri = get_internal_html_page_url (web_view, U1_TRYING_SSO_ERROR_PAGE, NULL);
 		}
+
+		load_delayed_page (web_view, internal_uri, policy_decision);
+
+		g_free (internal_uri);
 		g_free (to_and_from_error_uris);
 
-		return_value = TRUE;
-	} else if (g_strrstr (webkit_network_request_get_uri (request), "library.aspx") != NULL ||
-		 g_str_has_prefix (parsed_uri->path, "/music/store/library")) {
-		//g_str_has_prefix (webkit_network_request_get_uri (request), "https://one.ubuntu.com/music/store/library")) {
-		const gchar *library_override;
-
-		/* If the URL being loaded is the 7d library page, and we have a library
-		   override set, load ours instead. */
-		library_override = g_getenv ("U1MUSICLIBRARYURL");
-		if (library_override != NULL &&
-		    g_strcmp0 (library_override, webkit_network_request_get_uri (request)) != 0) {
-			g_debug ("overriding the library page to ours");
-			webkit_web_view_open (web_view, library_override);
-		}
-
-		return_value = TRUE;
-	}
-
-	if (return_value) {
-		webkit_web_policy_decision_ignore (policy_decision);
-	} else {
-		/* Inform webkit to continue with the loading */
-		webkit_web_policy_decision_use (policy_decision);
+		return TRUE;
 	}
 
 	/* Free memory */
 	soup_uri_free (parsed_uri);
 
-	return return_value;
+	return FALSE;
 }
 
 static void