492845 - ES5: Implement Object.seal, Object.isSealed

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jim Blandy :jimb]

Object.seal and Object.isSealed are specified by 15.2.3.8 and 15.2.3.11 in the ES5 draft:
http://wiki.ecmascript.org/doku.php?id=es3.1:es3.1_proposal_working_draft

Comment 2

[Jeff Walden [:Waldo]]

This bug depends on Brendan's scope-removal work in bug 558451.

Comment 3

[Brendan Eich [:brendan]]

See bug 492849 comment 9.

/be

Comment 4

[Jeff Walden [:Waldo]]

Attachment: Patch

This goes atop bug 492849 (extensibility bug), implements both seal and freeze primitives.  Same time/reviewer caveats apply as with that bug/patch.

Comment 5

[Jeff Walden [:Waldo]]

Attachment: Rebased

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 473779 [details] [diff] [review]
Rebased

># HG changeset patch
># User Jeff Walden <[e-mail]>
># Date 1277925101 25200
># Node ID 48df17e99d8632f4e6a3fd58a32e9ec78875bf4e
># Parent  284cb62bbb72c525e7638c86fbfc64e904410883
>Implement Object.isSealed, Object.isFrozen, Object.seal, Object.freeze.  NOT REVIEWED YET
>
>diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
>--- a/js/src/jsapi.cpp
>+++ b/js/src/jsapi.cpp
>@@ -2928,6 +2928,12 @@ JS_NewObjectForConstructor(JSContext *cx
> }
> 
> JS_PUBLIC_API(JSBool)
>+JS_IsExtensible(JSObject *obj)
>+{
>+    return obj->isExtensible();
>+}
>+
>+JS_PUBLIC_API(JSBool)
> JS_FreezeObject(JSContext *cx, JSObject *obj)
> {
>     CHECK_REQUEST(cx);
>diff --git a/js/src/jsapi.h b/js/src/jsapi.h
>--- a/js/src/jsapi.h
>+++ b/js/src/jsapi.h
>@@ -1887,6 +1887,10 @@ extern JS_PUBLIC_API(JSObject *)
> JS_NewObjectWithGivenProto(JSContext *cx, JSClass *clasp, JSObject *proto,
>                            JSObject *parent);
> 
>+/* Queries the [[Extensible]] property of the object. */
>+extern JS_PUBLIC_API(JSBool)
>+JS_IsExtensible(JSObject *obj);
>+
> /*
>  * Equivalent to Object.freeze (called recursively, if needed).  Deprecated
>  * as this previously-existing name is a clear false cognate of ES5's
>diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp
>--- a/js/src/jsobj.cpp
>+++ b/js/src/jsobj.cpp
>@@ -2560,7 +2560,7 @@ JSObject::modifyAllPropertyAttributes(JS
>                 return false;
> 
>             if ((attrs & JSPROP_PERMANENT) &&
>-                (!(attrs & (JSPROP_GETTER | JSPROP_SETTER)) || (attrs & JSPROP_READONLY))) {
>+                ((attrs & (JSPROP_GETTER | JSPROP_SETTER)) || (attrs & JSPROP_READONLY))) {
>                 continue;
>             }
> 

Oops, missed this -- it looks like a bug in the patch for bug 492849, so should go in a new patch for that bug, not this one.

>+static JSBool
>+obj_seal(JSContext *cx, uintN argc, Value *vp)
>+{
>+    JSObject *obj;
>+    if (!GetFirstArgumentAsObject(cx, argc, vp, "Object.seal", &obj))
>+        return false;
>+    vp->setObject(*obj);
>+
>+    return obj->seal(cx);

Nit: simplest style for control flow like this would put the blank line after the return false; line, not after the vp->setObject(*obj).

>+obj_freeze(JSContext *cx, uintN argc, Value *vp)
>+{
>+    JSObject *obj;
>+    if (!GetFirstArgumentAsObject(cx, argc, vp, "Object.freeze", &obj))
>+        return false;
>+    vp->setObject(*obj);
>+
>+    return obj->freeze(cx);

Ditto.

Looks great otherwise -- r=me with fixes.

/be

Comment 7

[Jim Blandy :jimb]

(In reply to comment #6)
> >             if ((attrs & JSPROP_PERMANENT) &&
> >-                (!(attrs & (JSPROP_GETTER | JSPROP_SETTER)) || (attrs & JSPROP_READONLY))) {
> >+                ((attrs & (JSPROP_GETTER | JSPROP_SETTER)) || (attrs & JSPROP_READONLY))) {
> >                 continue;
> >             }
> > 
> 
> Oops, missed this -- it looks like a bug in the patch for bug 492849, so should
> go in a new patch for that bug, not this one.

Already noticed and fixed in the patch for that bug.

> >+static JSBool
> >+obj_seal(JSContext *cx, uintN argc, Value *vp)
> >+{
> >+    JSObject *obj;
> >+    if (!GetFirstArgumentAsObject(cx, argc, vp, "Object.seal", &obj))
> >+        return false;
> >+    vp->setObject(*obj);
> >+
> >+    return obj->seal(cx);
> 
> Nit: simplest style for control flow like this would put the blank line after
> the return false; line, not after the vp->setObject(*obj).

Fixed.

Comment 8

[Jim Blandy :jimb]

http://hg.mozilla.org/tracemonkey/rev/441f83a81fb8

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/441f83a81fb8

Comment 10

[Fabian (Crash) Grutschus]

https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Object/seal
https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Object/isSealed