Closed Bug 1164188 Opened 6 years ago Closed 3 years ago

Expose the XMLHttpRequest (XHR) constructor in system scopes by default


(Core :: DOM: Core & HTML, defect, P5)




Tracking Status
firefox41 --- affected


(Reporter: bzbarsky, Assigned: bzbarsky)




(3 files, 2 obsolete files)

It can already be imported, so there's no reason not to do it.
Hrm.  So maybe we can't do this because of addons.  :(  There's "const XMLHttpRequest" all over addon code.  :(

Mostly from "resources/api-utils/lib/self!.js" and "boostrap.js", which I think is addon SDK stuff, so _maybe_ wouldn't be an issue?  But there are some other uses too, looks like.  This is why we can't have nice things.

Once bug 589199 is fixed we can do this.
Depends on: 589199
Attached patch Patch for posterity (obsolete) — Splinter Review
Assignee: nobody → bzbarsky
Bug 589199 seems to have landed
Yeah, need to test that the way that was actually fixed for addons will still work with the "const XMLHttpRequest" stuff...  I'm not quite sure how to go about doing that, and this is pretty low-priority for me.  If someone wants to do the testing and get the patch in, please feel free to steal.
All add-ons using "const XMLHttpRequest" should have been rewritten due to global lexical scope for let/const. If not, it is already broken.
Duplicate of this bug: 439286
Priority: -- → P5
Is this still something we want to do, bz? (I'm not sure if it squares with our efforts to minimize memory usage).
Flags: needinfo?(bzbarsky)
So first off [Exposed=System] adds a bit of data to the table used by ResolveSystemBinding.  It doesn't really cause memory usage other than that if the global in question doesn't use the relevant binding.

Past that, [Exposed=System] exposes the API in three places:

* ContentFrameMessageManager globals
* ProcessGlobal globals
* BackstagePass globals

The first two are going to go away in bug 1480244.  For BackstagePass, it looks like we create globals using it in:

1) ipc::XPCShellEnvironment::Init
2) mozJSComponentLoader::CreateLoaderGlobal
3) XRE_XPCShellMain

We don't really care about #1 and #3.  CreateLoaderGlobal can be called from:

a) mozJSComponentLoader::GetSharedGlobal
b) mozJSComponentLoader::PrepareObjectForLocation

There's only one shared global.  PrepareObjectForLocation uses the shared global unless ReuseGlobal(aURI) returns false.  Since we set pref("jsloader.shareGlobal", true), we use the shared global for everything except "resource://gre/modules/commonjs/toolkit/loader.js", resource://gre/modules/jsdebugger.jsm, and resource://specialpowers/*.

In a release build I expect we don't have specialpowers bits.  The "resource://gre/modules/commonjs/toolkit/loader.js" thing no longer exists.  So apart from the shared global we'd only need to worry about exposing in jsdebugger.jsm...

I suspect we should in fact just expose this to System.  Kris, any objections?
Flags: needinfo?(bzbarsky) → needinfo?(kmaglione+bmo)
Agreed, I can't think of any reason not to do this. If anything, it will probably save memory by preventing people from eagerly importing it before it's needed.
Flags: needinfo?(kmaglione+bmo)
That all sounds good to me. Is there a reason to not try landing the patch, then?
Try run of patch plus some cleanup is at

Once I make sure it's green I will post things for review.
OK, so the try run is very orange, because if I expose XHR to System, then the Window-only-exposed .responseXML getter is _not_ exposed to System and getting .responseXML from XHRs fails in system scopes.

And exposing that getter to System requires that its return type (Document) be exposed to System.

And that requires, by the same argument, that the return types of all properties of Document, and all consequential interfaces of all those types, and all the return types of those, etc, be exposed to System.

So we end up needing to expose a lot of things to System.  I'll post a patch that does that, in case we want to.
Another option would be to just expose everything that's exposed in Window in system scopes....
Attachment #8604914 - Attachment is obsolete: true
The JSM global has this by default now.
The JSM global has this by default now.
Attachment #9001481 - Attachment is obsolete: true
This got fixed in bug 1489301.
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1489301
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.