Part of Xray-to-JS. This may get pretty involved.

Attaching the patches that need to be reviewed by luke. The rest are coming in a couple of minutes.

This is the last part of our Q2 goal. \o/ https://tbpl.mozilla.org/?tree=Try&rev=673316e5f6ae

Comment on attachment 8441577 [details] [diff] [review] Part 1 - Add a mechanism to identify a standard constructor. v1 Review of attachment 8441577 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsobj.cpp @@ +3451,5 @@ > + > + GlobalObject &global = obj->global(); > + for (size_t k = 0; k < JSProto_LIMIT; ++k) { > + JSProtoKey key = static_cast<JSProtoKey>(k); > + if (global.getConstructor(key) == ObjectValue(*obj)) Could you use JSCLASS_CACHED_PROTO_KEY? You'd still need to test the identity of the constructor function against global.getConstructor(key), but at least that way you know the key and don't need an O(n) search. I'm not sure if every standard constructor's class JSCLASS_HAS_CACHED_PROTO, but it seems like you could assert this in initBuiltinConstructor() and fix the stragglers.

Comment on attachment 8441578 [details] [diff] [review] Part 2 - Add jsfriendapi accessors for function name and length. v1 Review of attachment 8441578 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsfriendapi.cpp @@ +553,5 @@ > > +JS_FRIEND_API(size_t) > +js::FunctionLength(JSObject *fun) > +{ > + return fun->as<JSFunction>().nargs(); I think you can use: JS_GetFunctionArity(JS_GetObjectFunction(fun)) instead of adding this.

Comment on attachment 8441577 [details] [diff] [review] Part 1 - Add a mechanism to identify a standard constructor. v1 Ah, I forgot that these Class here is just the Function Class, not the Class of the constructed objects, so there is no CACHED_PROTO.

Note - part 2 is now obsolete thanks to JS_GetFunctionArity and JS_GetFunctionId.

Final all-platform try push: https://tbpl.mozilla.org/?tree=Try&rev=728f17338811

(In reply to Bobby Holley (:bholley) from comment #14) > Final all-platform try push: > https://tbpl.mozilla.org/?tree=Try&rev=728f17338811 Something odd happened with Windows Opt xpcshell, but thankfully it went away with a fresh push: https://tbpl.mozilla.org/?tree=Try&rev=d00179b9b23c This is now green. Once it's reviewed, we can land it, and mark that Q2 goal as done.

Comment on attachment 8441603 [details] [diff] [review] Part 3 - Implement Xrays to Function objects. v1 Review of attachment 8441603 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/wrappers/XrayWrapper.cpp @@ +347,5 @@ > > static bool call(JSContext *cx, HandleObject wrapper, > const JS::CallArgs &args, js::Wrapper& baseInstance) > { > + auto self = JSXrayTraits::singleton; https://i.imgflip.com/9not0.jpg Seriously though for this single ensureHolder call I don't think you should use anything fancy. Do we have an actual policy for the usage of all the new C++ features? If I missed out something on this topic feel free to ignore my opinion here.

Comment on attachment 8441665 [details] [diff] [review] Part 5 - Support the .name and .length properties of Function instances. v2 Review of attachment 8441665 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/wrappers/XrayWrapper.cpp @@ +846,5 @@ > props.infallibleAppend(INT_TO_JSID(i)); > } else if (key == JSProto_Function) { > + if (!props.append(GetRTIdByIndex(cx, XPCJSRuntime::IDX_LENGTH))) > + return false; > + if (!props.append(GetRTIdByIndex(cx, XPCJSRuntime::IDX_NAME))) NIT: you could merge these two ifs with ||

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #17) > > } else if (key == JSProto_Function) { > > + if (!props.append(GetRTIdByIndex(cx, XPCJSRuntime::IDX_LENGTH))) > > + return false; > > + if (!props.append(GetRTIdByIndex(cx, XPCJSRuntime::IDX_NAME))) > > NIT: you could merge these two ifs with || Yeah, but with the required bracing style for multi-line conditionals it ends up longer that way. remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/51ab4d67ca8a remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fa086a70cfbf remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/a471415834ae remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/206d7f502e14 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/871d570bc7c9

(In reply to Bobby Holley (:bholley) from comment #18) > Yeah, but with the required bracing style for multi-line conditionals it > ends up longer that way. Ah, OK then.

https://hg.mozilla.org/mozilla-central/rev/51ab4d67ca8a https://hg.mozilla.org/mozilla-central/rev/fa086a70cfbf https://hg.mozilla.org/mozilla-central/rev/a471415834ae https://hg.mozilla.org/mozilla-central/rev/206d7f502e14 https://hg.mozilla.org/mozilla-central/rev/871d570bc7c9