Closed Bug 942211 Opened 7 years ago Closed 7 years ago

Remove usages of NewObjectWithClassProto

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: till, Assigned: till)

Details

(Whiteboard: [qa-])

Attachments

(1 file)

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.
Attachment #8336868 - Flags: review?(jwalden+bmo)
Try run with some preparatory patches here: https://tbpl.mozilla.org/?tree=Try&rev=529794f7ff7f
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?
Attachment #8336868 - Flags: review?(jwalden+bmo) → review+
Oh, you should add a jsapi-test for the Math.[[Prototype]] issue.
https://hg.mozilla.org/mozilla-central/rev/e4c2a0b98917
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.