703929 - Loading in the Get Add-ons pane a chrome XUL file fails when the add-on manager is reopened

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Florian Quèze [:florian]]

Attachment: Patch

In Instantbird we use a chrome URL for extensions.webservice.discoverURL as our current add-ons website doesn't support that page yet.

Unfortunately, it turns out that using a chrome URL in this preference only works the first time the add-on manager is opened, as the next times the chrome page is cached and the request finishes with aStatus = NS_ERROR_PARSED_DATA_CACHED instead of a success code.

This took me a long time to figure out as that bug couldn't be reproduced in debug builds where this "parsed data cache" seems to be disabled.

The related Instantbird bug is https://bugzilla.instantbird.org/show_bug.cgi?id=872

Comment 1

[Dave Townsend [:mossop]]

Comment on attachment 575702 [details] [diff] [review]
Patch

Review of attachment 575702 [details] [diff] [review]:
-----------------------------------------------------------------

Can we test for this?

Comment 2

[Florian Quèze [:florian]]

(In reply to Dave Townsend (:Mossop) from comment #1)

> Can we test for this?

Do you mean "can we create some unit test?"?

I think a test would need to:
1. set extensions.webservice.discoverURL to a chrome URL,
2. open the add-on manager,
3. select the Get Add-ons pane (if that's not always the tab that shows up by default),
4. wait for the load of browser to complete,
5. check if the selected panel is the browser.
6. close the add-on manager
7. repeat steps 2 to 6.

So it's possible, but I'm not sure it's worth the effort. If you think we really need a test for this, is there an existing test doing something very similar that I could copy and modify?

If you meant "can we reproduce this in Firefox so that QA can verify the fix?", then yes, set the pref extensions.webservice.discoverURL to a chrome URI (I tested with chrome://browser/content/pageinfo/pageInfo.xul but any loadable chrome URI should work) from about:config, open the add-on manager, see page info loaded in there, close the add-on manager, reopen, see the "What are add-ons? [...] When you're connected to the Internet, this page will[...]" pane instead.

With the patch applied, page info should be loaded the second time too.

Comment 3

[Dave Townsend [:mossop]]

browser_discovery.js does a bunch of loading the discovery view with a test file. That same file should also be available as a chrome url so just sticking a couple of extra bits into that test that set the url to the chrome url and attempts to open the add-on manager twice in a row and verifies that it finishes loading should suffice I think.

Comment 4

[Florian Quèze [:florian]]

Attachment: Patch with test

Comment 5

[Florian Quèze [:florian]]

I noticed while writing the test that the bug can only be reproduced with a XUL file, which makes sense as this error code is returned from nsXULDocument::CachedChromeStreamListener::OnStartRequest.

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 578941 [details] [diff] [review]
Patch with test

Review of attachment 578941 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the extra work to test this.

Comment 7

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb6812611234

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/bb6812611234