Creating second XMLHttpRequest via Components.Constructor throws NS_ERROR_FAILURE

RESOLVED FIXED in Firefox 14

Status

()

Core
XPConnect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Wladimir Palant, Assigned: peterv)

Tracking

(Blocks: 1 bug, {addon-compat, regression})

unspecified
mozilla15
addon-compat, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox14+ verified)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Created attachment 611424 [details]
Restartless extension for testing

A change in the recent nightlies broke subscription downloads in Adblock Plus. The reduced testcase looks like this:

> var XMLHttpRequest = Components.Constructor("@mozilla.org/xmlextras/xmlhttprequest;1", "nsIXMLHttpRequest");
> new XMLHttpRequest();
> new XMLHttpRequest();

The attempt to create a second XMLHttpRequest with the same constructor throws NS_ERROR_FAILURE. This only happens in the context of a JavaScript code module, running the same code in a sandbox or in the window context works fine. Also, it only seems to affect XMLHttpRequest - I tried to create an nsIFile instance and that worked. And using createInstance() directly works as well, the issue only affects Components.Constructor.

A restartless extension to test this is attached. The important code is in Module.js, it will run every time this extension starts up. Normally it should print "XMLHttpRequest objects created successfully" to Error Console, instead it prints "Error creating XMLHttpRequest objects" (an exception occurred in the code).

An Adblock Plus user says that the first hourly build to experience this issue is 1333220903, this means that the regression range is http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=83dce280d871&tochange=bbe5086163c9. I suspect bug 740069 to be the cause.
(Reporter)

Updated

5 years ago
Blocks: 467520
(Assignee)

Comment 1

5 years ago
Created attachment 611438 [details] [diff] [review]
v1
Assignee: nobody → peterv
Status: NEW → ASSIGNED

Comment 2

5 years ago
This doesn't quite seem right...  In particular, wouldn't JS_HasProperty test true for a Window which has a <div id="XMLHttpRequest"> in it?

We really want a JS_HasOwnProperty here.  Maybe.  At least in the window context that should work better.  Modulo whatever happens when you reenter resolve hooks, of course.
(Assignee)

Comment 3

5 years ago
Yup, it should be HasOwnProperty.
(Assignee)

Comment 4

5 years ago
Created attachment 612696 [details] [diff] [review]
v2

For some reason I can't make this fail in an xpcshell test :-/.
Attachment #611438 - Attachment is obsolete: true
Attachment #612696 - Flags: review?(bzbarsky)
(Assignee)

Comment 5

5 years ago
Comment on attachment 612696 [details] [diff] [review]
v2

Actually, I forgot to test the id case.
Attachment #612696 - Flags: review?(bzbarsky)

Comment 6

5 years ago
And my question about resolve hook reentry stands, btw....
(Reporter)

Updated

5 years ago
Duplicate of this bug: 747682

Comment 8

5 years ago
This needs to happen for fx14, since it breaks Adblock Plus if nothing else.
tracking-firefox14: --- → ?
(Reporter)

Comment 9

5 years ago
I stopped using Components.Constructor in Adblock Plus, new version should be released soon. Still, this is quite a bit of platform breakage - XMLHttpRequest should be the most common use of Components.Constructor.
(Assignee)

Comment 10

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #6)
> And my question about resolve hook reentry stands, btw....

The JS engine already protects resolve hooks, see AutoResolving.

Comment 11

5 years ago
Yes, the question was what that protection will end up doing in this case.
(Assignee)

Comment 12

5 years ago
Ah, I think it makes the JS_AlreadyHasOwnProperty not find the property.
Definitively an addon-compat worthy issue. CC'ing Jorge.
Keywords: addon-compat

Updated

5 years ago
tracking-firefox14: ? → +
(Assignee)

Comment 14

5 years ago
Created attachment 620297 [details] [diff] [review]
v2

This doesn't change anything in the order of name resolution on windows from before the landing of the new DOM bindings.

I think I've answered the questions about this patch, let me know if I didn't.
Attachment #612696 - Attachment is obsolete: true
Attachment #620297 - Flags: review?(bzbarsky)

Comment 15

5 years ago
Comment on attachment 620297 [details] [diff] [review]
v2

r=me
Attachment #620297 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/02b78dbc753f
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/02b78dbc753f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 18

5 years ago
Comment on attachment 620297 [details] [diff] [review]
v2

[Approval Request Comment]
Regression caused by (bug #): bug 740069
User impact if declined: Broken addons (Addblock Plus was fixed, but this might affect others)
Testing completed (on m-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): low risk
String changes made by this patch: None
Attachment #620297 - Flags: approval-mozilla-aurora?
(In reply to Peter Van der Beken [:peterv] from comment #18)
> User impact if declined: Broken addons (Addblock Plus was fixed, but this
> might affect others)

It in fact does affect other add-ons, at least 3 of mine, having 2M+ ADU. addon-mxr shows a bunch of other users. Not sure how much are indeed affected, as this requires calling the constructor at least twice.
Comment on attachment 620297 [details] [diff] [review]
v2

[Triage Comment]
New regression in FF14 that has the potential to affect a large population. Approving for Aurora 14.
Attachment #620297 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 21

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/62bc457629b6
status-firefox14: --- → fixed
Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20100101 Firefox/14.0

Verified in F14 beta 8, on Ubuntu 12.04, Windows 7 and Mac OS 10.6 using attachment test case.

Timestamp: 06/22/2012 02:57:09 PM
Error: XMLHttpRequest objects created successfully
Source File: jar:file:///home/virgildicu/.mozilla/firefox/d4sz6vma.qdd/extensions/testtest@adblockplus.org.xpi!/Module.js
Line: 15
status-firefox14: fixed → verified
You need to log in before you can comment on or make changes to this bug.