1135428 - OdinMonkey: remove compileAndGo restriction

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

IIUC, JS::CloneFunctionObject is only used for XBL, not any content-visible scripts; is that right bz (due to all the with-wrapped scope chain work)?

Currently JS::CloneFunctionObject on asm.js throws a dynamic error, so we disabled Odin on !compileAndGo (a precondition for calling CloneFunctionObject) back when content was cloneable, but now that it's not, I think we can simply remove this restriction.

Comment 1

[Boris Zbarsky [:bzbarsky]]

So the situation in Gecko right now is that CloneFunctionObject is only used for XBL methods/getters/setter/eventhandlers.

These are not "content" in the sense of being available on the web, but they can appear in extensions obviously.

That said, the JSFunctions involved are just created via JS::CompileFunction, so would they ever be asm.js functions anyway?

Comment 2

[Luke Wagner [:luke]]

(In reply to Boris Zbarsky [:bz] from comment #1)
> These are not "content" in the sense of being available on the web, but they
> can appear in extensions obviously.

Right, we wouldn't crash, just throw a safe exception.  If anyone really needed this feature, we could actually add cloning support w/o too much work since we already have the raw functionality to clone an AsmJSModule.

> That said, the JSFunctions involved are just created via
> JS::CompileFunction, so would they ever be asm.js functions anyway?

Yep, asm.js can appear at top-level in a CompileFunction call or nested anywhere inside.

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK.

I think it's not a problem if things that try to use asm.js in xbl get exceptions, especially if we have AMO check for that.  I can't think of any cases when it would be reasonable to use asm in XBL stuff.

Comment 4

[David Bruant]

Related to bug 679939 ?

Comment 5

[Luke Wagner [:luke]]

Attachment: rm-compile-and-go-limit

Looks like we already have tests for the attempted-cross-compartment-clone case and the same-compartment-clone case.

Comment 6

[Luke Wagner [:luke]]

(In reply to David Bruant from comment #4)
> Related to bug 679939 ?

Yeah, this removes a remaining use.

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8578263 [details] [diff] [review]
rm-compile-and-go-limit

Review of attachment 8578263 [details] [diff] [review]:
-----------------------------------------------------------------

Cool.

Comment 8

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e246f0d09dbe

Comment 9

[Luke Wagner [:luke]]

Oops, return 'nullptr' not 'false':
https://hg.mozilla.org/integration/mozilla-inbound/rev/d74279a9b861

Comment 10

[Benjamin Bouvier [:bbouvier] (inactive)]

Fix the bustage on --baseline-eager --no-fpu, on x86:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4113f4e7e3e3

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/e246f0d09dbe
https://hg.mozilla.org/mozilla-central/rev/d74279a9b861
https://hg.mozilla.org/mozilla-central/rev/4113f4e7e3e3