http://msdn.microsoft.com/en-us/library/bb773190%28VS.85%29.aspx#theme_pack
http://en.wikipedia.org/wiki/Cabinet_%28file_format%29

http://www.cabextract.org.uk/ seems to open the file nicely.

Created attachment 142288 [details] [review]
initial patch

Created attachment 142290 [details] [review]
updated patch

Fix leak.

+	res = g_key_file_load_from_data (kf, converted, strlen (converted), 0, &error);

converted can be NULL here. strlen doesn't like that.

+		g_warning ("File '%s' doesn't have a Slideshowgroup", filename);

Missing " ".

+	display_name = g_key_file_get_string (kf, "Theme", "DisplayName", NULL);
+	slideshow_path = g_key_file_get_string (kf, "Slideshow", "ImagesRootPath", NULL);
+	interval = g_key_file_get_integer (kf, "Slideshow", "Interval", NULL);
+	shuffle = g_key_file_get_boolean (kf, "Slideshow", "Shuffle", NULL);

Looks like all of these should be const.

+	if (!g_file_move (src_file, dest_file, G_FILE_COPY_NONE, NULL, NULL, NULL, &error)) {
+		g_warning ("Error while moving from `%s' to `%s': %s",
+			   slideshow_path, new_path, error->message);
+		g_error_free (error);
+		error = NULL;
+	}
+	g_object_unref (dest_file);
+	g_object_unref (src_file);
+	g_object_unref (base_file);
+	g_free (new_path);
+
+	write_backgrounds_xml (xml_path, interval / 1000.0, shuffle);
+	g_free (xml_path);
+
+	ret = TRUE;

This looks like the write and the TRUE return shouldn't happen if the move failed.

+			convert_windows_theme (parent, p);
+		}
+		g_free (p);
+	}
+
+	g_dir_close (d);
+	return TRUE;

Should *at least* return FALSE if all theme conversions fail.

+	gboolean res;
...
-			if (!g_file_move (src_file, new_file, G_FILE_COPY_NONE,
-					  NULL, NULL, NULL, &error)) {
+			res = g_file_move (src_file,
+					   new_file,
+					   G_FILE_COPY_NONE,
+					   NULL, NULL, NULL,
+					   &error);
+			if (! res) {

These changes look like unnecessary code churn. Please drop.

Created attachment 142348 [details] [review]
updated patch

Thanks for the review.  This should fix for the comments and a few other leaks.  I'm not sure what you meant by "consts" though.

(In reply to comment #6)
>  I'm not sure what you meant by "consts" though.

Since you weren't freeing the strings I assumed g_key_file_get_string returned a const char *. Now that you do, it seems that assumption was inccorrect. ;-)

@@ -364,9 +748,13 @@ gnome_theme_install_real (GtkWindow *parent,
 	theme_source_dir = g_file_new_for_path (tmp_dir);
 	theme_dest_dir = g_file_new_for_path (target_dir);
 
-	if (!g_file_move (theme_source_dir, theme_dest_dir,
-			  G_FILE_COPY_OVERWRITE, NULL, NULL,
-			  NULL, &error)) {
+	if (! g_file_move (theme_source_dir,
+			   theme_dest_dir,
+			   G_FILE_COPY_OVERWRITE,
+			   NULL,
+			   NULL,
+			   NULL,
+			   &error)) {
 		gchar *str;

This hunk still looks unnecessary.

Also, I'm wondering about the blocking bugs you entered here. Why are those blocking this one?

Jon, do we still want to support those for GNOME 3?

I'm fine with trying to load those if installed in a special directory (but probably for GNOME 3.2).

This is a similar problem to the one in bug 595890.

A couple of notes on the code:
- what should be the interaction of clicking on themepack links?
- should we present a dialogue asking the user if they're sure about installing it?
- it should definitely use libarchive to unpack the archive rather than "do it by hand" with "7za" or cabextract.
- the themepack mime-type is upstream

I'm going to assume we don't want this anymore. Feel free to reopen if we do.