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.

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.

Yup, it should be HasOwnProperty.

For some reason I can't make this fail in an xpcshell test :-/.

Comment on attachment 612696 [details] [diff] [review] v2 Actually, I forgot to test the id case.

And my question about resolve hook reentry stands, btw....

This needs to happen for fx14, since it breaks Adblock Plus if nothing else.

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.

(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.

Yes, the question was what that protection will end up doing in this case.

Ah, I think it makes the JS_AlreadyHasOwnProperty not find the property.

Definitively an addon-compat worthy issue. CC'ing Jorge.

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.

Comment on attachment 620297 [details] [diff] [review] v2 r=me

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

(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.

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