Bug 1148750, part 8 - Implement ValidateAndApplyPropertyDescriptor step 6. r=efaust.

This also makes some changes to MutableHandle<PropertyDescriptor>, for convenience:

*   Make desc.setGetterObject() and desc.setSetterObject() quietly change *this from a generic or data descriptor into an accessor descriptor, if need be.

*   Make setWritable() clear the JSPROP_IGNORE_READONLY bit if present. (Breaking the symmetry a bit, it won't change *this from an accessor descriptor to a data descriptor. Instead, it asserts you're not doing that.)

--HG--
extra : rebase_source : 2a7cf3d477553838248bfa8bb183985ee369a22e

js/src/jsapi.h

@@ -2709,6 +2709,11 @@ class MutablePropertyDescriptorOperations : public PropertyDescriptorOperations<
         setAttributes((desc()->attrs & ~(JSPROP_IGNORE_ENUMERATE | JSPROP_ENUMERATE)) |
                       (enumerable ? JSPROP_ENUMERATE : 0));
     }
+    void setWritable(bool writable) {
+        MOZ_ASSERT(!(desc()->attrs & (JSPROP_GETTER | JSPROP_SETTER)));
+        setAttributes((desc()->attrs & ~(JSPROP_IGNORE_READONLY | JSPROP_READONLY)) |
+                      (writable ? 0 : JSPROP_READONLY));
+    }
     void setAttributes(unsigned attrs) { desc()->attrs = attrs; }
 
     void setGetter(JSGetterOp op) {
@@ -2719,8 +2724,16 @@ class MutablePropertyDescriptorOperations : public PropertyDescriptorOperations<
         MOZ_ASSERT(op != JS_StrictPropertyStub);
         desc()->setter = op;
     }
-    void setGetterObject(JSObject* obj) { desc()->getter = reinterpret_cast<JSGetterOp>(obj); }
-    void setSetterObject(JSObject* obj) { desc()->setter = reinterpret_cast<JSSetterOp>(obj); }
+    void setGetterObject(JSObject* obj) {
+        desc()->getter = reinterpret_cast<JSGetterOp>(obj);
+        desc()->attrs &= ~(JSPROP_IGNORE_VALUE | JSPROP_IGNORE_READONLY | JSPROP_READONLY);
+        desc()->attrs |= JSPROP_GETTER | JSPROP_SHARED;
+    }
+    void setSetterObject(JSObject* obj) {
+        desc()->setter = reinterpret_cast<JSSetterOp>(obj);
+        desc()->attrs &= ~(JSPROP_IGNORE_VALUE | JSPROP_IGNORE_READONLY | JSPROP_READONLY);
+        desc()->attrs |= JSPROP_SETTER | JSPROP_SHARED;
+    }
 
     JS::MutableHandleObject getterObject() {
         MOZ_ASSERT(this->hasGetterObject());

js/src/vm/NativeObject.cpp

@@ -1249,6 +1249,37 @@ AddOrChangeProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId id,
 
 static bool IsConfigurable(unsigned attrs) { return (attrs & JSPROP_PERMANENT) == 0; }
 static bool IsEnumerable(unsigned attrs) { return (attrs & JSPROP_ENUMERATE) != 0; }
+static bool IsWritable(unsigned attrs) { return (attrs & JSPROP_READONLY) == 0; }
+
+static bool IsAccessorDescriptor(unsigned attrs) {
+    return (attrs & (JSPROP_GETTER | JSPROP_SETTER)) != 0;
+}
+
+static bool IsDataDescriptor(unsigned attrs) {
+    MOZ_ASSERT((attrs & (JSPROP_IGNORE_VALUE | JSPROP_IGNORE_READONLY)) == 0);
+    return !IsAccessorDescriptor(attrs);
+}
+
+template <AllowGC allowGC>
+static MOZ_ALWAYS_INLINE bool
+GetExistingProperty(JSContext* cx,
+                    typename MaybeRooted<JSObject*, allowGC>::HandleType receiver,
+                    typename MaybeRooted<NativeObject*, allowGC>::HandleType obj,
+                    typename MaybeRooted<Shape*, allowGC>::HandleType shape,
+                    typename MaybeRooted<Value, allowGC>::MutableHandleType vp);
+
+static bool
+GetExistingPropertyValue(ExclusiveContext* cx, HandleNativeObject obj, HandleId id,
+                         HandleShape shape, MutableHandleValue vp)
+{
+    if (IsImplicitDenseOrTypedArrayElement(shape)) {
+        vp.set(obj->getDenseOrTypedArrayElement(JSID_TO_INT(id)));
+        return true;
+    }
+    if (!cx->shouldBeJSContext())
+        return false;
+    return GetExistingProperty<CanGC>(cx->asJSContext(), obj, obj, shape, vp);
+}
 
 bool
 js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId id,
@@ -1376,10 +1407,31 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId
     if (!desc.hasEnumerable())
         desc.setEnumerable(IsEnumerable(shapeAttrs));
 
-    // If defining a getter or setter, we must check for its counterpart and
-    // update the attributes and property ops.  A getter or setter is really
-    // only half of a property.
-    if (desc.isAccessorDescriptor()) {
+    // Steps 6-9.
+    if (desc.isGenericDescriptor()) {
+        // Step 6. No further validation is required.
+
+        // Fill in desc. A generic descriptor has none of these fields, so copy
+        // everything from shape.
+        MOZ_ASSERT(!desc.hasValue());
+        MOZ_ASSERT(!desc.hasWritable());
+        MOZ_ASSERT(!desc.hasGetterObject());
+        MOZ_ASSERT(!desc.hasSetterObject());
+        if (IsDataDescriptor(shapeAttrs)) {
+            RootedValue currentValue(cx);
+            if (!GetExistingPropertyValue(cx, obj, id, shape, &currentValue))
+                return false;
+            desc.setValue(currentValue);
+            desc.setWritable(IsWritable(shapeAttrs));
+        } else {
+            desc.setGetterObject(shape->getterObject());
+            desc.setSetterObject(shape->setterObject());
+        }
+    } else if (desc.isAccessorDescriptor()) {
+        // If defining a getter or setter, we must check for its counterpart
+        // and update the attributes and property ops.  A getter or setter is
+        // really only half of a property.
+
         // If we are defining a getter whose setter was already defined, or
         // vice versa, finish the job via obj->changeProperty.
         if (IsImplicitDenseOrTypedArrayElement(shape)) {
@@ -1484,8 +1536,7 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId
     // clear it.
     desc.setAttributes(ApplyOrDefaultAttributes(desc.attributes()) & ~JSPROP_IGNORE_VALUE);
 
-    // At this point, no mutation has happened yet, but all ES6 error cases
-    // have been dealt with.
+    // Step 10.
     if (!AddOrChangeProperty(cx, obj, id, desc))
         return false;
     return result.succeed();