Same story as everywhere else - we need a ClassSpec in order to support them over Xrays.

This is required in order to avoid exposing resolve hook effects when Object.freeze() is invoked on the global. The freeze() call first enumerates the object, after which point any lazy properties need to be resolve so that we can safely mark the object as non-extensible.

The former is strictly more information, which matters in the case of Error objects where multiple JSProtoKeys share a js::Class.

Comment on attachment 8449889 [details] [diff] [review] Part 1 - Give BackstagePass an Enumerate hook to match its NewResolve hook. v1 Review of attachment 8449889 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/RuntimeService.cpp @@ +1082,5 @@ > } > } > > + // Invoking this function with JSID_VOID means "always resolve". > + bool shouldResolve = aId != JSID_VOID; JSID_IS_VOID(aId) ::: dom/workers/Workers.h @@ +171,5 @@ > > // All of these are implemented in RuntimeService.cpp > + > +// Resolves all of the worker classes onto |aObjp| if one of them matches |aId| > +// or if |aId| is JSID_VOID. A little fugly, but okay.

Comment on attachment 8449890 [details] [diff] [review] Part 2 - Tag JSStdName entries by JSProtoKey rather than a js::Class pointer. v1 Review of attachment 8449890 [details] [diff] [review]: ----------------------------------------------------------------- I want to get rid of this error class sharing at some point, but this is fine in any event.

Comment on attachment 8449892 [details] [diff] [review] Part 4 - Introduce and use ParentKeyForStandardClass. v1 Review of attachment 8449892 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsfriendapi.h @@ +633,5 @@ > return key != keyFromClass; > } > > +// Returns the key for the class inherited by a given standard class (that > +// is to say, the prototype of this standard classes's prototype). class's

Comment on attachment 8449893 [details] [diff] [review] Part 5 - Tweak GenericCreateConstructor to make it usable with Error. v1 Review of attachment 8449893 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/GlobalObject.h @@ +795,5 @@ > GenericCreateConstructor(JSContext *cx, JSProtoKey key) > { > + // Note - We duplicate the trick from ClassName() so that we don't need to > + // include jsatominlines.h here. > + JSAtom *atom = (&cx->names().Null)[key]; PropertyName* please.

Comment on attachment 8449894 [details] [diff] [review] Part 6 - Make Error use a ClassSpec. v1 Review of attachment 8449894 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsexn.cpp @@ +57,5 @@ > +static const JSFunctionSpec exception_methods[] = { > +#if JS_HAS_TOSOURCE > + JS_FN(js_toSource_str, exn_toSource, 0,0), > +#endif > + JS_FN(js_toString_str, exn_toString, 0,0), Don't bother trying to column-align these, they get wrong easily. Let's take the advantage to discard the alignment fetish. @@ +546,2 @@ > { > + JSExnType type = ExnTypeFromProtoKey(key); I'd rather you deferred this til just before it's needed, as it was before, just vice versa. @@ +548,2 @@ > RootedObject errorProto(cx); > + errorProto = GenericCreatePrototype<&ErrorObject::class_>(cx, key); This probably all fits into a single init-at-construction line. @@ +571,5 @@ > +/* static */ JSObject * > +ErrorObject::createConstructor(JSContext *cx, JSProtoKey key) > +{ > + RootedObject ctor(cx); > + ctor = GenericCreateConstructor<Error, 1, JSFunction::ExtendedFinalizeKind>(cx, key); Again might fit on one line.

Thanks for the quick reviews Waldo! Final all-platform try push: https://tbpl.mozilla.org/?tree=Try&rev=5cc932b1916e

Followup for build bustage on MSVC and GCC due to the lack of support for default template parameters on functions: https://tbpl.mozilla.org/?tree=Try&rev=6594e7988d24

Comment on attachment 8449890 [details] [diff] [review] Part 2 - Tag JSStdName entries by JSProtoKey rather than a js::Class pointer. v1 Review of attachment 8449890 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsapi.cpp @@ +1301,5 @@ > > // If this class is anonymous, then it doesn't exist as a global > // property, so we won't resolve anything. > + JSProtoKey key = stdnm ? stdnm->key : JSProto_Null; > + if (key != JSProto_Null && !(ProtoKeyToClass(key)->flags & JSCLASS_IS_ANONYMOUS)) { if (stdnm && stdnm->key != JSProto_Null && !(stdnm->clasp->flags & JSCLASS_IS_ANONYMOUS))?

(In reply to :Ms2ger from comment #16) > if (stdnm && stdnm->key != JSProto_Null && !(stdnm->clasp->flags & > JSCLASS_IS_ANONYMOUS))? I removed the clasp member of stdnm entries in this patch.

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/4ec31a870cb0 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/650197ade3b3 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/3ff6b5e30818 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/30c45b56a2ef remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/a7b254ac4fc4 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/7750f3e24883

https://hg.mozilla.org/mozilla-central/rev/4ec31a870cb0 https://hg.mozilla.org/mozilla-central/rev/650197ade3b3 https://hg.mozilla.org/mozilla-central/rev/3ff6b5e30818 https://hg.mozilla.org/mozilla-central/rev/30c45b56a2ef https://hg.mozilla.org/mozilla-central/rev/a7b254ac4fc4 https://hg.mozilla.org/mozilla-central/rev/7750f3e24883

Warnings introduced by part2: In file included from /home/ben/code/moz/builds/wd64/js/src/Unified_cpp_js_src7.cpp:41:0: /home/ben/code/moz/inbound/js/src/jsapi.cpp:1198:18: warning: ‘DummyClass’ defined but not used [-Wunused-variable] static js::Class DummyClass; ^ /home/ben/code/moz/inbound/js/src/jsapi.cpp:1199:18: warning: ‘SentinelClass’ defined but not used [-Wunused-variable] static js::Class SentinelClass; ^ Easy to fix, and it compiles fine.

Comment on attachment 8451500 [details] [diff] [review] Remove unused DummyClass and SentinelClass I had the same idea: https://hg.mozilla.org/integration/mozilla-inbound/rev/2ec6c5b1d1d2