942211 - Remove usages of NewObjectWithClassProto

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Till Schneidereit [:till]]

Removes those uses of NewObjectWithClassProto that I could tell were safe to remove. I'm sure most or all the others can be removed with some more work (or even just analysis), too, but this seems like a decent first step.

Also, it fixes the Object.getPrototypeOf(Math) issue from bug 919095.

Comment 1

[Till Schneidereit [:till]]

Attachment: Remove usages of NewObjectWithClassProto

Comment 2

[Till Schneidereit [:till]]

Try run with some preparatory patches here: https://tbpl.mozilla.org/?tree=Try&rev=529794f7ff7f

Comment 3

[Jeff Walden [:Waldo]]

Comment on attachment 8336868 [details] [diff] [review]
Remove usages of NewObjectWithClassProto

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

::: js/src/builtin/TypedObject.cpp
@@ +1253,5 @@
>                         HandleObject module,
>                         typename T::TypeRepr::Type type,
>                         HandlePropertyName className)
>  {
>      RootedObject funcProto(cx, JS_GetFunctionPrototype(cx, global));

global->getOrCreateFunctionPrototype(cx), or however it's spelled

::: js/src/jsmath.cpp
@@ +1474,5 @@
>  
>  JSObject *
>  js_InitMathClass(JSContext *cx, HandleObject obj)
>  {
> +    RootedObject proto(cx, obj->as<GlobalObject>().getOrCreateObjectPrototype(cx));

This needs a null-check -- you're not guaranteed that this will have been initialized already.

::: js/src/jsreflect.cpp
@@ +3312,5 @@
>          JS_FS_END
>      };
>  
>      RootedObject obj(cx, objArg);
> +    RootedObject proto(cx, obj->as<GlobalObject>().getOrCreateObjectPrototype(cx));

Needs a null-check.

::: js/src/vm/GlobalObject.cpp
@@ +388,5 @@
>          intrinsicsHolder = self;
>      } else {
> +        RootedObject proto(cx, self->getOrCreateObjectPrototype(cx));
> +        intrinsicsHolder = NewObjectWithClassProto(cx, &JSObject::class_, proto, self,
> +                                                   TenuredObject);

Is it necessary for the intrinsics holder to have a non-null prototype?  I can't think of a reason for this.  Maybe in a followup patch change this to do that?

Comment 4

[Jeff Walden [:Waldo]]

Oh, you should add a jsapi-test for the Math.[[Prototype]] issue.

Comment 5

[Till Schneidereit [:till]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e4c2a0b98917

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e4c2a0b98917