ES5: Implement Object.seal, Object.isSealed

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: jimb, Assigned: jimb)

Tracking

({dev-doc-complete})

Trunk
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: [fixed-in-tracemonkey])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

8 years ago
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
Assignee: general → jwalden+bmo
Blocks: 445494
blocking2.0: --- → ?

Updated

7 years ago
blocking2.0: ? → beta1+
Duplicate of this bug: 563205

Updated

7 years ago
blocking2.0: beta1+ → beta2+
Keywords: dev-doc-needed

Updated

7 years ago
blocking2.0: beta2+ → betaN+
This bug depends on Brendan's scope-removal work in bug 558451.
Depends on: 558451

Updated

7 years ago
blocking2.0: betaN+ → beta5+
See bug 492849 comment 9.

/be

Updated

7 years ago
blocking2.0: beta5+ → beta6+
Created attachment 470866 [details] [diff] [review]
Patch

This goes atop bug 492849 (extensibility bug), implements both seal and freeze primitives.  Same time/reviewer caveats apply as with that bug/patch.
Attachment #470866 - Flags: review?(jorendorff)
Created attachment 473779 [details] [diff] [review]
Rebased
Attachment #470866 - Attachment is obsolete: true
Attachment #473779 - Flags: review?(brendan)
Attachment #470866 - Flags: review?(jorendorff)
Comment on attachment 473779 [details] [diff] [review]
Rebased

># HG changeset patch
># User Jeff Walden <jwalden@mit.edu>
># 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
Attachment #473779 - Flags: review?(brendan) → review+

Updated

7 years ago
Assignee: jwalden+bmo → jim
(Assignee)

Comment 7

7 years ago
(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.
(Assignee)

Comment 8

7 years ago
http://hg.mozilla.org/tracemonkey/rev/441f83a81fb8
Status: NEW → ASSIGNED
Whiteboard: [fixed-in-tracemonkey]

Comment 9

7 years ago
http://hg.mozilla.org/mozilla-central/rev/441f83a81fb8
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Depends on: 599464
Depends on: 600132
Depends on: 600135
https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Object/seal
https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Object/isSealed
Keywords: dev-doc-needed → dev-doc-complete

Updated

7 years ago
Depends on: 613452
Depends on: 624421
Depends on: 624426
You need to log in before you can comment on or make changes to this bug.