Seal/freeze objects without converting to dictionary mode, bug 708641. r=luke
authorBrian Hackett <bhackett1024@gmail.com>
Thu, 05 Jan 2012 19:51:08 -0800
changeset 86359 8dc46cdc401b476c0904498fb29363d98f91f194
parent 86358 bb37803ae6c89e8d826ff709d71665627af5204b
child 86360 4779d8df054b671e7e44deaed81a83ae9bb6b0e0
push idunknown
push userunknown
push dateunknown
reviewersluke
bugs708641
milestone12.0a1
Seal/freeze objects without converting to dictionary mode, bug 708641. r=luke
js/src/jit-test/tests/basic/write-frozen-property.js
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/write-frozen-property.js
@@ -0,0 +1,8 @@
+
+function foo(x) {
+  x.a = 10;
+  assertEq(x.a, 0);
+}
+x = {a:0,b:1};
+Object.freeze(x);
+foo(x);
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -2693,16 +2693,25 @@ obj_preventExtensions(JSContext *cx, uin
     vp->setObject(*obj);
     if (!obj->isExtensible())
         return true;
 
     AutoIdVector props(cx);
     return obj->preventExtensions(cx, &props);
 }
 
+/* static */ inline uintN
+JSObject::getSealedOrFrozenAttributes(uintN attrs, ImmutabilityType it)
+{
+    /* Make all attributes permanent; if freezing, make data attributes read-only. */
+    if (it == FREEZE && !(attrs & (JSPROP_GETTER | JSPROP_SETTER)))
+        return JSPROP_PERMANENT | JSPROP_READONLY;
+    return JSPROP_PERMANENT;
+}
+
 bool
 JSObject::sealOrFreeze(JSContext *cx, ImmutabilityType it)
 {
     assertSameCompartment(cx, this);
     JS_ASSERT(it == SEAL || it == FREEZE);
 
     RootedVarObject self(cx, this);
 
@@ -2713,37 +2722,72 @@ JSObject::sealOrFreeze(JSContext *cx, Im
     } else {
         if (!GetPropertyNames(cx, this, JSITER_HIDDEN | JSITER_OWNONLY, &props))
             return false;
     }
 
     /* preventExtensions must slowify dense arrays, so we can assign to holes without checks. */
     JS_ASSERT(!self->isDenseArray());
 
-    for (size_t i = 0, len = props.length(); i < len; i++) {
-        jsid id = props[i];
-
-        uintN attrs;
-        if (!self->getGenericAttributes(cx, id, &attrs))
+    if (isNative() && !inDictionaryMode()) {
+        /*
+         * Seal/freeze non-dictionary objects by constructing a new shape
+         * hierarchy mirroring the original one, which can be shared if many
+         * objects with the same structure are sealed/frozen. If we use the
+         * generic path below then any non-empty object will be converted to
+         * dictionary mode.
+         */
+        Shape *last = EmptyShape::getInitialShape(cx, self->getClass(),
+                                                  self->getProto(),
+                                                  self->getParent(),
+                                                  self->getAllocKind(),
+                                                  self->lastProperty()->getObjectFlags());
+        if (!last)
             return false;
 
-        /* Make all attributes permanent; if freezing, make data attributes read-only. */
-        uintN new_attrs;
-        if (it == FREEZE && !(attrs & (JSPROP_GETTER | JSPROP_SETTER)))
-            new_attrs = JSPROP_PERMANENT | JSPROP_READONLY;
-        else
-            new_attrs = JSPROP_PERMANENT;
-
-        /* If we already have the attributes we need, skip the setAttributes call. */
-        if ((attrs | new_attrs) == attrs)
-            continue;
-
-        attrs |= new_attrs;
-        if (!self->setGenericAttributes(cx, id, &attrs))
-            return false;
+        /* Get an in order list of the shapes in this object. */
+        AutoShapeVector shapes(cx);
+        for (Shape::Range r = self->lastProperty()->all(); !r.empty(); r.popFront()) {
+            if (!shapes.append(&r.front()))
+                return false;
+        }
+        Reverse(shapes.begin(), shapes.end());
+
+        for (size_t i = 0; i < shapes.length(); i++) {
+            StackShape child(shapes[i]);
+            child.attrs |= getSealedOrFrozenAttributes(child.attrs, it);
+
+            if (!JSID_IS_EMPTY(child.propid))
+                MarkTypePropertyConfigured(cx, self, child.propid);
+
+            last = JS_PROPERTY_TREE(cx).getChild(cx, last, self->numFixedSlots(), child);
+            if (!last)
+                return NULL;
+        }
+
+        JS_ASSERT(self->lastProperty()->slotSpan() == last->slotSpan());
+        JS_ALWAYS_TRUE(setLastProperty(cx, last));
+    } else {
+        for (size_t i = 0; i < props.length(); i++) {
+            jsid id = props[i];
+
+            uintN attrs;
+            if (!self->getGenericAttributes(cx, id, &attrs))
+                return false;
+
+            uintN new_attrs = getSealedOrFrozenAttributes(attrs, it);
+
+            /* If we already have the attributes we need, skip the setAttributes call. */
+            if ((attrs | new_attrs) == attrs)
+                continue;
+
+            attrs |= new_attrs;
+            if (!self->setGenericAttributes(cx, id, &attrs))
+                return false;
+        }
     }
 
     return true;
 }
 
 bool
 JSObject::isSealedOrFrozen(JSContext *cx, ImmutabilityType it, bool *resultp)
 {
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -963,16 +963,18 @@ struct JSObject : js::gc::Cell
      * object as non-extensible, and adjust each property's attributes appropriately: each
      * property becomes non-configurable, and if |freeze|, data properties become
      * read-only as well.
      */
     bool sealOrFreeze(JSContext *cx, ImmutabilityType it);
 
     bool isSealedOrFrozen(JSContext *cx, ImmutabilityType it, bool *resultp);
 
+    static inline uintN getSealedOrFrozenAttributes(uintN attrs, ImmutabilityType it);
+
     inline void *&privateRef(uint32_t nfixed) const;
 
   public:
     inline bool isExtensible() const;
     bool preventExtensions(JSContext *cx, js::AutoIdVector *props);
 
     /* ES5 15.2.3.8: non-extensible, all props non-configurable */
     inline bool seal(JSContext *cx) { return sealOrFreeze(cx, SEAL); }
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -295,17 +295,16 @@ JSObject::enclosingScope()
  * Property read barrier for deferred cloning of compiler-created function
  * objects optimized as typically non-escaping, ad-hoc methods in obj.
  */
 inline js::Shape *
 JSObject::methodReadBarrier(JSContext *cx, const js::Shape &shape, js::Value *vp)
 {
     JS_ASSERT(nativeContains(cx, shape));
     JS_ASSERT(shape.isMethod());
-    JS_ASSERT(shape.writable());
     JS_ASSERT(shape.hasSlot());
     JS_ASSERT(shape.hasDefaultSetter());
     JS_ASSERT(!isGlobal());  /* i.e. we are not changing the global shape */
 
     JSFunction *fun = vp->toObject().toFunction();
     JS_ASSERT(!fun->isClonedMethod());
     JS_ASSERT(fun->isNullClosure());