Bug 1142775 - Rename NativeSet -> NativeSetExistingDataProperty and simplify it since it is only called for data properties. Delete Shape::set. Add comments. No change in behavior. r=efaust.
☠☠ backed out by c3638d994edd ☠ ☠
authorJason Orendorff <jorendorff@mozilla.com>
Sat, 28 Feb 2015 14:40:07 -0600
changeset 265294 ce0ee37e3ca9a78113075a3f2cb790ad20078580
parent 265293 1519b8f2bbba393de20366c782f0590ddb290b42
child 265295 034f9c8e79ee7a3d54a265324e980c545195c65d
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1142775
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1142775 - Rename NativeSet -> NativeSetExistingDataProperty and simplify it since it is only called for data properties. Delete Shape::set. Add comments. No change in behavior. r=efaust.
js/src/vm/NativeObject.cpp
js/src/vm/Shape-inl.h
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -1072,18 +1072,18 @@ UpdateShapeTypeAndValue(ExclusiveContext
     if (!shape->hasSlot() || !shape->hasDefaultGetter() || !shape->hasDefaultSetter())
         MarkTypePropertyNonData(cx, obj, id);
     if (!shape->writable())
         MarkTypePropertyNonWritable(cx, obj, id);
     return true;
 }
 
 static bool
-NativeSet(JSContext *cx, HandleNativeObject obj, HandleObject receiver,
-          HandleShape shape, MutableHandleValue vp, ObjectOpResult &result);
+NativeSetExistingDataProperty(JSContext *cx, HandleNativeObject obj, HandleObject receiver,
+                              HandleShape shape, MutableHandleValue vp, ObjectOpResult &result);
 
 static inline bool
 DefinePropertyOrElement(ExclusiveContext *cx, HandleNativeObject obj, HandleId id,
                         GetterOp getter, SetterOp setter, unsigned attrs, HandleValue value,
                         bool callSetterAfterwards, ObjectOpResult &result)
 {
     MOZ_ASSERT(getter != JS_PropertyStub);
     MOZ_ASSERT(setter != JS_StrictPropertyStub);
@@ -1162,20 +1162,22 @@ DefinePropertyOrElement(ExclusiveContext
             return result.succeed();
         }
     }
 
     if (!CallAddPropertyHook(cx, obj, shape, value))
         return false;
 
     if (callSetterAfterwards && setter) {
+        MOZ_ASSERT(!(attrs & JSPROP_GETTER));
+        MOZ_ASSERT(!(attrs & JSPROP_SETTER));
         if (!cx->shouldBeJSContext())
             return false;
         RootedValue nvalue(cx, value);
-        return NativeSet(cx->asJSContext(), obj, obj, shape, &nvalue, result);
+        return NativeSetExistingDataProperty(cx->asJSContext(), obj, obj, shape, &nvalue, result);
     }
 
     return result.succeed();
 }
 
 static unsigned
 ApplyOrDefaultAttributes(unsigned attrs, const Shape *shape = nullptr)
 {
@@ -1597,16 +1599,17 @@ js::NativeHasProperty(JSContext *cx, Han
         // that optimization would be incorrect.
         if (!proto->isNative())
             return HasProperty(cx, proto, id, foundp);
 
         pobj = &proto->as<NativeObject>();
     }
 }
 
+
 /*** [[Get]] *************************************************************************************/
 
 static inline bool
 CallGetter(JSContext* cx, HandleObject receiver, HandleShape shape, MutableHandleValue vp)
 {
     MOZ_ASSERT(!shape->hasDefaultGetter());
 
     if (shape->hasGetterValue()) {
@@ -1922,16 +1925,17 @@ js::NativeGetPropertyNoGC(JSContext *cx,
 bool
 js::GetPropertyForNameLookup(JSContext *cx, HandleObject obj, HandleId id, MutableHandleValue vp)
 {
     if (obj->getOps()->getProperty)
         return GeneralizedGetProperty(cx, obj, id, obj, NameLookup, vp);
     return NativeGetPropertyInline<CanGC>(cx, obj.as<NativeObject>(), obj, id, NameLookup, vp);
 }
 
+
 /*** [[Set]] *************************************************************************************/
 
 static bool
 MaybeReportUndeclaredVarAssignment(JSContext *cx, JSString *propname)
 {
     {
         jsbytecode *pc;
         JSScript *script = cx->currentScript(&pc, JSContext::ALLOW_CROSS_COMPARTMENT);
@@ -2093,66 +2097,61 @@ SetDenseOrTypedArrayElement(JSContext *c
     if (!obj->maybeCopyElementsForWrite(cx))
         return false;
 
     obj->setDenseElementWithType(cx, index, vp);
     return result.succeed();
 }
 
 /*
- * Finish assignment to a shapeful property of a native object obj. This
+ * Finish assignment to a shapeful data property of a native object obj. This
  * conforms to no standard and there is a lot of legacy baggage here.
  */
 static bool
-NativeSet(JSContext *cx, HandleNativeObject obj, HandleObject receiver,
-          HandleShape shape, MutableHandleValue vp, ObjectOpResult &result)
+NativeSetExistingDataProperty(JSContext *cx, HandleNativeObject obj, HandleObject receiver,
+                              HandleShape shape, MutableHandleValue vp, ObjectOpResult &result)
 {
     MOZ_ASSERT(obj->isNative());
+    MOZ_ASSERT(shape->isDataDescriptor());
 
-    if (shape->hasSlot()) {
-        /* If shape has a stub setter, just store vp. */
-        if (shape->hasDefaultSetter()) {
+    if (shape->hasDefaultSetter()) {
+        if (shape->hasSlot()) {
+            // The common path. Standard data property.
+
             // Global properties declared with 'var' will be initially
             // defined with an undefined value, so don't treat the initial
             // assignments to such properties as overwrites.
             bool overwriting = !obj->is<GlobalObject>() || !obj->getSlot(shape->slot()).isUndefined();
             obj->setSlotWithType(cx, shape, vp, overwriting);
             return result.succeed();
         }
+
+        // Bizarre: shared (slotless) property that's writable but has no
+        // JSSetterOp. JS code can't define such a property, but it can be done
+        // through the JSAPI. Treat it as non-writable.
+        return result.fail(JSMSG_GETTER_ONLY);
     }
 
-    if (!shape->hasSlot()) {
-        /*
-         * Allow API consumers to create shared properties with stub setters.
-         * Such properties effectively function as data descriptors which are
-         * not writable, so attempting to set such a property should do nothing
-         * or throw if we're in strict mode.
-         */
-        if (!shape->hasGetterValue() && shape->hasDefaultSetter())
-            return result.fail(JSMSG_GETTER_ONLY);
-    }
-
-    RootedValue ovp(cx, vp);
+    MOZ_ASSERT(!obj->is<DynamicWithObject>());  // See bug 1128681.
 
     uint32_t sample = cx->runtime()->propertyRemovals;
-    if (!shape->set(cx, obj, receiver, vp, result))
+    RootedId id(cx, shape->propid());
+    if (!CallJSSetterOp(cx, shape->setterOp(), obj, id, vp, result))
         return false;
 
-    /*
-     * Update any slot for the shape with the value produced by the setter,
-     * unless the setter deleted the shape.
-     */
+    // Update any slot for the shape with the value produced by the setter,
+    // unless the setter deleted the shape.
     if (shape->hasSlot() &&
         (MOZ_LIKELY(cx->runtime()->propertyRemovals == sample) ||
          obj->contains(cx, shape)))
     {
         obj->setSlot(shape->slot(), vp);
     }
 
-    return true;  // result is populated by shape->set() above.
+    return true;  // result is populated by CallJSSetterOp above.
 }
 
 /*
  * Finish the assignment `receiver[id] = vp` when an existing property (shape)
  * has been found on a native object (pobj). This implements ES6 draft rev 32
  * (2015 Feb 2) 9.1.9 steps 5 and 6.
  *
  * It is necessary to pass both id and shape because shape could be an implicit
@@ -2188,33 +2187,33 @@ SetExistingProperty(JSContext *cx, Handl
             // the lookup in step 5.c again, as our caller just did it for us. The
             // result is |shape|.
 
             // Steps 5.e.i-ii.
             if (pobj->is<ArrayObject>() && id == NameToId(cx->names().length)) {
                 Rooted<ArrayObject*> arr(cx, &pobj->as<ArrayObject>());
                 return ArraySetLength(cx, arr, id, shape->attributes(), vp, result);
             }
-            return NativeSet(cx, obj, receiver, shape, vp, result);
+            return NativeSetExistingDataProperty(cx, obj, receiver, shape, vp, result);
         }
 
         // SpiderMonkey special case: assigning to an inherited slotless
         // property causes the setter to be called, instead of shadowing,
         // unless the existing property is JSPROP_SHADOWABLE (see bug 552432)
         // or it's the array length property.
         if (!shape->hasSlot() &&
             !shape->hasShadowable() &&
             !(pobj->is<ArrayObject>() && id == NameToId(cx->names().length)))
         {
             // Even weirder sub-special-case: inherited slotless data property
             // with default setter. Wut.
             if (shape->hasDefaultSetter())
                 return result.succeed();
 
-            return shape->set(cx, obj, receiver, vp, result);
+            return CallJSSetterOp(cx, shape->setterOp(), obj, id, vp, result);
         }
 
         // Shadow pobj[id] by defining a new data property receiver[id].
         // Delegate everything to SetPropertyByDefining.
         return SetPropertyByDefining(cx, obj, receiver, id, vp, obj == pobj, result);
     }
 
     // Steps 6-11.
--- a/js/src/vm/Shape-inl.h
+++ b/js/src/vm/Shape-inl.h
@@ -34,40 +34,16 @@ StackBaseShape::StackBaseShape(Exclusive
 
 inline Shape *
 Shape::search(ExclusiveContext *cx, jsid id)
 {
     ShapeTable::Entry *_;
     return search(cx, this, id, &_);
 }
 
-inline bool
-Shape::set(JSContext* cx, HandleNativeObject obj, HandleObject receiver, MutableHandleValue vp,
-           ObjectOpResult &result)
-{
-    MOZ_ASSERT_IF(hasDefaultSetter(), hasGetterValue());
-    MOZ_ASSERT(!obj->is<DynamicWithObject>());  // See bug 1128681.
-
-    if (attrs & JSPROP_SETTER) {
-        Value fval = setterValue();
-        if (!InvokeGetterOrSetter(cx, receiver, fval, 1, vp.address(), vp))
-            return false;
-        return result.succeed();
-    }
-
-    if (attrs & JSPROP_GETTER)
-        return result.fail(JSMSG_GETTER_ONLY);
-
-    if (!setterOp())
-        return result.succeed();
-
-    RootedId id(cx, propid());
-    return CallJSSetterOp(cx, setterOp(), obj, id, vp, result);
-}
-
 /* static */ inline Shape *
 Shape::search(ExclusiveContext *cx, Shape *start, jsid id, ShapeTable::Entry **pentry, bool adding)
 {
     if (start->inDictionary()) {
         *pentry = &start->table().search(id, adding);
         return (*pentry)->shape();
     }