Closed Bug 665214 Opened 9 years ago Closed 4 years ago

Provide a js::GetOwnPropertyDescriptor function that populates a PropertyDescriptor

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jorendorff, Assigned: jorendorff)

Details

Attachments

(2 files)

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.
Attached patch v1Splinter Review
Assignee: general → jorendorff
Attachment #540168 - Flags: review?(jwalden+bmo)
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 */
Attachment #540168 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/af9974df716c (This took too long to find)
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.