665214 - Provide a js::GetOwnPropertyDescriptor function that populates a PropertyDescriptor

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

We already have a js_GetOwnPropertyDescriptor, but it goes all the way and creates the property descriptor Object. I want to split that into two parts, one that does the inspection (producing a js::PropertyDescriptor) and one that creates the Object.

I need this for jsdbg2; I'll attach the patch that needs it in case a reviewer is interested.

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: v1

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: the patch that needs this, fyi

Comment 3

[Jeff Walden [:Waldo]]

Comment on attachment 540168 [details] [diff] [review]
v1

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

::: js/src/jsobj.cpp
@@ +1779,5 @@
> +        !obj->defineProperty(cx, ATOM_TO_JSID(atomState.configurableAtom),
> +                             BooleanValue((attrs & JSPROP_PERMANENT) == 0),
> +                             PropertyStub, StrictPropertyStub, JSPROP_ENUMERATE)) {
> +        return false;
> +    }

It occurs to me that we could define enumerable/configurable before get/set or value/writable, to share more property tree.  If you want to move it, feel free.  It probably doesn't matter that much in the grand scheme of things, but it wouldn't hurt.

@@ +1826,5 @@
> +
> +bool
> +GetOwnPropertyDescriptor(JSContext *cx, JSObject *obj, jsid id, Value *vp)
> +{
> +    AutoPropertyDescriptorRooter desc(cx);

I don't believe the rooter is needed here any more, with conservative stack scanning, but it doesn't hurt to have it, right?

::: js/src/jsobj.h
@@ +1719,5 @@
> +
> +bool
> +NewPropertyDescriptorObject(JSContext *cx, const PropertyDescriptor *desc, Value *vp);
> +
> +}

/* namespace js */

Comment 4

[Tom Schuster [:evilpie]]

https://hg.mozilla.org/mozilla-central/rev/af9974df716c (This took too long to find)