814505 - Firefox hangs and unresponsive script dialog appears when installing large unpacked addon

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

Attachment: screenshot showing unresponsive script dialog

When Firefox installs a large addon that needs to be unpacked, like Firefox OS Simulator <https://ftp.mozilla.org/pub/mozilla.org/labs/r2d2b2g/>, Firefox will hang for multiple seconds, and an unresponsive script dialog will frequently appear.

Such addons are presumably few and far between.  Nevertheless, they do exist, and they're supported by the browser; so Firefox shouldn't hang, and that dialog shouldn't appear, when installing them.

I see this on Windows, Mac, and Linux.  The dialog doesn't always appear, presumably because of differences in system speed and load; but it shows up regularly.  It's most reproducible with the Linux package of the Simulator, which is larger than the other packages (and which I install on slower VMs).

Comment 1

[Kris Maglione [:kmag]]

It should be possible to move the IO to a background thread, but the add-on installation API is designed to be synchronous. It seems like it takes about the same amount of time for Firefox to extract the XPI as it does for BSD tar, so that's probably the only option.

Comment 2

[Dave Townsend [:mossop]]

Actually the API should support asynchronous extraction but the ZIP API makes doing it complicated. I think there are also a few cases (during startup) where we did need to do it synchronously so after we implemented packed XPI installation I mostly gave up thinking about it much.

Comment 3

[Philipp Kewisch [:Fallen][📱📆]]

Note that if the script is stopped, it sometimes leaves the addon in limbo and recovering means deleting a few files manually. Reinstalling the addon via UI doesn't work then.

Comment 4

[Kris Maglione [:kmag]]

Right, at startup there would obviously be no point. I don't think it would be especially complicated to do asynchronous extraction from the zip API, though. Its input streams should work with an async stream copier, it's just not perhaps as nice as the extract() method.

Comment 5

[Dave Townsend [:mossop]]

(In reply to Kris Maglione [:kmag] from comment #4)
> Right, at startup there would obviously be no point. I don't think it would
> be especially complicated to do asynchronous extraction from the zip API,
> though. Its input streams should work with an async stream copier, it's just
> not perhaps as nice as the extract() method.

That would work yes, but extract() also applies the correct file permissions to the created file and right now there is no method on nsIZipReader to get those.

Comment 6

[Kris Maglione [:kmag]]

Hrm. In my experience, if XPIs have permissions on them, they're usually wrong and tend to just do evil things like preventing the owner from reading or deleting them on Unix-ish filesystems. Anyway, shouldn't be too hard to add the necessary fields to nsIZipEntry.

Comment 7

[Dave Townsend [:mossop]]

(In reply to Kris Maglione [:kmag] from comment #6)
> Hrm. In my experience, if XPIs have permissions on them, they're usually
> wrong and tend to just do evil things like preventing the owner from reading
> or deleting them on Unix-ish filesystems. Anyway, shouldn't be too hard to
> add the necessary fields to nsIZipEntry.

Mostly true, in fact after extracting the file we force the permissions to make the files readable and writeable. The only exception is the executable permission which is important to set write for a bunch of extensions.

Comment 8

[Dave Townsend [:mossop]]

Now that bug 562674 is fixed I think this is largely fixed as well. Re-open if you can still reproduce though.

Comment 9

[Mihai Morar, (:MihaiMorar)]

Not able to reproduce on Latest Nightly 28 using Windows 7 x64 and Mac OS 10.8.4:
BuildID: 20131105030206