741267 - UserScript's XMLHttpRequest is undefined in 20120401 nightly

Status: RESOLVED FIXED

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

Description

[dindog]

Both Greasemonkey and Scriptish run userscript with XMLHttpRequest will dump "ReferenceError: XMLHttpRequest is not defined".

I'm pretty sure those script work fine in 20120330 nightly build.

Probably regression from Bug 741163.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Sounds like XrayWrapper fun.

Comment 2

[Peter Van der Beken [:peterv]]

We're resolving on the global of the XPCWrappedNativeScope we get from the sandbox global's proto, but we should resolve on the proto itself.

Comment 3

[Alexandre Poirot [:ochameau]]

Looks like we are hitting the same bug in Jetpack, because of this:

  var Cu = Components.utils;
  var s = Cu.Sandbox(content);
  s.win = content;
  Cu.evalInSandbox("win.XMLHttpRequest()", s);
  // Exception on nightly: win.XMLHttpRequest is not a function
  // While it runs without exception in FF11

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This is breaking Jetpack tests, so it's a P1 from our view.

Comment 5

[Peter Van der Beken [:peterv]]

Attachment: v1

Not sure how to make the difference between aObject and aGlobal clearer. aDefineOn for aObject?

Comment 6

[Peter Van der Beken [:peterv]]

Attachment: v1

Sorry, wrong patch.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 612688 [details] [diff] [review]
v1

So is the |obj| passed to CreateInterfaceObjects sometimes not a global now?  Does using it as the parent for the protos and interface objects still make sense?  Or should we use the global there?  Will |obj| in practice end up being a security wrapper here, or the window itself?

As far as naming goes, perhaps aReceiver for aObject?

Comment 8

[Peter Van der Beken [:peterv]]

(In reply to Boris Zbarsky (:bz) from comment #7)
> So is the |obj| passed to CreateInterfaceObjects sometimes not a global now?

Yes. It can be a wrapper for a global, set as the proto of the sandbox global.

> Does using it as the parent for the protos and interface objects still make
> sense?  Or should we use the global there?

I think it's fine, but are you worried about something in particular.

> Will |obj| in practice end up
> being a security wrapper here, or the window itself?

Security wrapper.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> I think it's fine, but are you worried about something in particular.

Are we putting these things that are parented to "obj" in the list attached to global?

Comment 10

[Peter Van der Beken [:peterv]]

Attachment: v1.1

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 617019 [details] [diff] [review]
v1.1

Please write a more useful checkin comment, one that describes the receiver/global split, and r=me

One question, though: With this setup we stash the interface/proto object pointers in the sandbox global's proto array, right?  But we resolve them on the receiver (the xray for the window), and stick the actual JSObject refs into the sandbox's holder?  And we parent the objects to the sandbox global?  Is that basically the setup?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Also, does this cause us to preserve the xray?

Comment 13

[Dave Townsend [:mossop]]

This breaks XHR from jetpack right now so we'd really like to get this landed a.s.a.p. and look into putting it on aurora too.

Comment 14

[Peter Van der Beken [:peterv]]

(In reply to Boris Zbarsky (:bz) from comment #11)
> One question, though: With this setup we stash the interface/proto object
> pointers in the sandbox global's proto array, right?  But we resolve them on
> the receiver (the xray for the window), and stick the actual JSObject refs
> into the sandbox's holder?  And we parent the objects to the sandbox global?

Yes, assuming that by "into the sandbox's holder" you mean "on the receiver's holder".

(In reply to Boris Zbarsky (:bz) from comment #12)
> Also, does this cause us to preserve the xray?

Not preserve as in PreserveWrapper. Of course the prototypes hold their parent (the sandbox global) and the global holds its prototype.

Comment 15

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ed9cc1c4879

Comment 16

[Boris Zbarsky [:bzbarsky]]

I guess what I was really trying to ask is this.  If another Xray for the window is created, will it have those objects already in its holder?  Or do we not coalesce Xrays?

Comment 17

[Ryan VanderMeulen [:RyanVM]]

http://hg.mozilla.org/mozilla-central/rev/9ed9cc1c4879

Comment 18

[Alex Keybl [:akeybl]]

Tracking for FF14. Please nominate for Aurora approval asap - my understanding is that this fixes broken jetpack tests.

Comment 19

[Alexandre Poirot [:ochameau]]

It doesn't only fix jetpack tests but fix XMLHttpRequest usages jetpack addons too!
So it is really important to get this included Aurora.

Otherwise, XMLHttpRequest() now works, but some other stuff are still broken, like:
  var Cu = Components.utils;
  var s = Cu.Sandbox(content);
  s.win = content;
  Cu.evalInSandbox("win.XMLHttpRequest.toString()", s);
/*
Exception: Function.prototype.toString called on incompatible object
@Scratchpad:4
*/

Should I open another bug? Or reopen this one?

Comment 20

[Boris Zbarsky [:bzbarsky]]

> Exception: Function.prototype.toString called on incompatible object

Bug 742156 covers this.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Though the patch in bug 748983 would also fix this for the case of XHR.

Comment 22

[Peter Van der Beken [:peterv]]

(In reply to Boris Zbarsky (:bz) from comment #16)
> I guess what I was really trying to ask is this.  If another Xray for the
> window is created, will it have those objects already in its holder?

Every Xray has its own holder.

Comment 23

[Peter Van der Beken [:peterv]]

Comment on attachment 617019 [details] [diff] [review]
v1.1

[Approval Request Comment]
Regression caused by (bug #): bug 740069
User impact if declined: Jetpack-based addons using XMLHttpRequest will be broken
Testing completed (on m-c, etc.): landed on m-c on 2012-04-29, has an automated test (and is also tested by Jetpack tests)
Risk to taking this patch (and alternatives if risky): low-risk
String changes made by this patch: None

Comment 24

[Alex Keybl [:akeybl]]

Comment on attachment 617019 [details] [diff] [review]
v1.1

[Triage Comment]
Given where we are in the dev cycle, and our concern with the impact of this bug, approving for Aurora 14.

Comment 25

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8378e850186b