Last Comment Bug 741267 - UserScript's XMLHttpRequest is undefined in 20120401 nightly
: UserScript's XMLHttpRequest is undefined in 20120401 nightly
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: DOM (show other bugs)
: 14 Branch
: x86_64 Windows 7
: -- normal with 2 votes (vote)
: mozilla15
Assigned To: Peter Van der Beken [:peterv]
:
Mentors:
: 749124 (view as bug list)
Depends on:
Blocks: 740069 741390
  Show dependency treegraph
 
Reported: 2012-04-01 19:22 PDT by dindog
Modified: 2012-05-04 22:08 PDT (History)
18 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments
v1 (1.03 KB, patch)
2012-04-05 14:31 PDT, Peter Van der Beken [:peterv]
no flags Details | Diff | Splinter Review
v1 (31.69 KB, patch)
2012-04-05 14:32 PDT, Peter Van der Beken [:peterv]
no flags Details | Diff | Splinter Review
v1.1 (31.76 KB, patch)
2012-04-20 09:55 PDT, Peter Van der Beken [:peterv]
bzbarsky: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description dindog 2012-04-01 19:22:34 PDT
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-04-01 19:25:13 PDT
Sounds like XrayWrapper fun.
Comment 2 Peter Van der Beken [:peterv] 2012-04-02 07:48:51 PDT
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] 2012-04-03 05:54:13 PDT
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) 2012-04-05 11:20:21 PDT
This is breaking Jetpack tests, so it's a P1 from our view.
Comment 5 Peter Van der Beken [:peterv] 2012-04-05 14:31:40 PDT
Created attachment 612686 [details] [diff] [review]
v1

Not sure how to make the difference between aObject and aGlobal clearer. aDefineOn for aObject?
Comment 6 Peter Van der Beken [:peterv] 2012-04-05 14:32:25 PDT
Created attachment 612688 [details] [diff] [review]
v1

Sorry, wrong patch.
Comment 7 Boris Zbarsky [:bz] 2012-04-05 19:21:33 PDT
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] 2012-04-19 12:11:16 PDT
(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 [:bz] 2012-04-19 12:16:01 PDT
> 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] 2012-04-20 09:55:14 PDT
Created attachment 617019 [details] [diff] [review]
v1.1
Comment 11 Boris Zbarsky [:bz] 2012-04-20 14:20:03 PDT
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 [:bz] 2012-04-20 14:21:28 PDT
Also, does this cause us to preserve the xray?
Comment 13 Dave Townsend [:mossop] 2012-04-26 11:20:46 PDT
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] 2012-04-27 02:52:20 PDT
(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] 2012-04-27 06:07:11 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ed9cc1c4879
Comment 16 Boris Zbarsky [:bz] 2012-04-27 08:47:47 PDT
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] 2012-04-29 13:56:06 PDT
http://hg.mozilla.org/mozilla-central/rev/9ed9cc1c4879
Comment 18 Alex Keybl [:akeybl] 2012-04-30 15:56:37 PDT
Tracking for FF14. Please nominate for Aurora approval asap - my understanding is that this fixes broken jetpack tests.
Comment 19 Alexandre Poirot [:ochameau] 2012-05-01 10:00:29 PDT
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 [:bz] 2012-05-01 10:11:12 PDT
> Exception: Function.prototype.toString called on incompatible object

Bug 742156 covers this.
Comment 21 Boris Zbarsky [:bz] 2012-05-01 10:13:33 PDT
Though the patch in bug 748983 would also fix this for the case of XHR.
Comment 22 Peter Van der Beken [:peterv] 2012-05-02 02:06:58 PDT
(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] 2012-05-02 02:09:36 PDT
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] 2012-05-02 15:27:32 PDT
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] 2012-05-03 06:07:40 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/8378e850186b
Comment 26 Alex 2012-05-04 22:08:09 PDT
*** Bug 749124 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.