Bug 1090537, part 5 - Remove legacy special case in baseops::SetPropertyHelper that cloned the getter and setter of JSPROP_SHADOWABLE properties when shadowed on another native object. r=Waldo.
authorJason Orendorff <jorendorff@mozilla.com>
Wed, 15 Oct 2014 14:05:29 -0500
changeset 226123 9317cf955dc77e6dbd914b0afd47303087d31dc5
parent 226122 be9e874aea12cbb0d4b3cc67e7870530d245439b
child 226124 1791020afc6f362725b8008869c85cb9a97ce01a
push id36
push userdburns@mozilla.com
push dateMon, 10 Nov 2014 15:14:02 +0000
reviewersWaldo
bugs1090537
milestone36.0a1
Bug 1090537, part 5 - Remove legacy special case in baseops::SetPropertyHelper that cloned the getter and setter of JSPROP_SHADOWABLE properties when shadowed on another native object. r=Waldo.
js/src/vm/NativeObject.cpp
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -2008,20 +2008,16 @@ baseops::SetPropertyHelper(typename Exec
         }
     }
 
     /*
      * Now either shape is null, meaning id was not found in obj or one of its
      * prototypes; or shape is non-null, meaning id was found directly in pobj.
      */
     unsigned attrs = JSPROP_ENUMERATE;
-    const Class *clasp = obj->getClass();
-    PropertyOp getter = clasp->getProperty;
-    StrictPropertyOp setter = clasp->setProperty;
-
     if (IsImplicitDenseOrTypedArrayElement(shape)) {
         /* ES5 8.12.4 [[Put]] step 2, for a dense data property on pobj. */
         if (pobj != obj)
             shape = nullptr;
     } else if (shape) {
         /* ES5 8.12.4 [[Put]] step 2. */
         if (shape->isAccessorDescriptor()) {
             if (shape->hasDefaultSetter()) {
@@ -2048,53 +2044,32 @@ baseops::SetPropertyHelper(typename Exec
                 if (strict)
                     return JSObject::reportReadOnly(cx, id, JSREPORT_ERROR);
                 if (cx->compartment()->options().extraWarnings(cx))
                     return JSObject::reportReadOnly(cx, id, JSREPORT_STRICT | JSREPORT_WARNING);
                 return true;
             }
         }
 
-        attrs = shape->attributes();
-        if (pobj != obj) {
-            /*
-             * We found id in a prototype object: prepare to share or shadow.
-             */
+        if (pobj == obj) {
+            attrs = shape->attributes();
+        } else {
+            // We found id in a prototype object: prepare to share or shadow.
+
             if (!shape->shadowable()) {
                 if (shape->hasDefaultSetter() && !shape->hasGetterValue())
                     return true;
 
                 if (mode == ParallelExecution)
                     return false;
 
                 return shape->set(cxArg->asJSContext(), obj, receiver, strict, vp);
             }
 
-            /*
-             * Preserve attrs except JSPROP_SHARED, getter, and setter when
-             * shadowing any property that has no slot (is shared). We must
-             * clear the shared attribute for the shadowing shape so that the
-             * property in obj that it defines has a slot to retain the value
-             * being set, in case the setter simply cannot operate on instances
-             * of obj's class by storing the value in some class-specific
-             * location.
-             */
-            if (!shape->hasSlot()) {
-                attrs &= ~JSPROP_SHARED;
-                getter = shape->getter();
-                setter = shape->setter();
-            } else {
-                /* Restore attrs to the ECMA default for new properties. */
-                attrs = JSPROP_ENUMERATE;
-            }
-
-            /*
-             * Forget we found the proto-property now that we've copied any
-             * needed member values.
-             */
+            // Forget we found the proto-property since we're shadowing it.
             shape = nullptr;
         }
     }
 
     if (IsImplicitDenseOrTypedArrayElement(shape)) {
         uint32_t index = JSID_TO_INT(id);
 
         if (IsAnyTypedArray(obj)) {
@@ -2167,31 +2142,33 @@ baseops::SetPropertyHelper(typename Exec
             if (mode == SequentialExecution &&
                 cxArg->asJSContext()->compartment()->options().extraWarnings(cxArg->asJSContext()))
             {
                 return obj->reportNotExtensible(cxArg, JSREPORT_STRICT | JSREPORT_WARNING);
             }
             return true;
         }
 
+        const Class *clasp = obj->getClass();
         if (mode == ParallelExecution) {
             if (obj->isDelegate())
                 return false;
 
-            if (getter != JS_PropertyStub || !types::HasTypePropertyId(obj, id, vp))
+            if (clasp->getProperty != JS_PropertyStub || !types::HasTypePropertyId(obj, id, vp))
                 return false;
         } else {
             JSContext *cx = cxArg->asJSContext();
 
             /* Purge the property cache of now-shadowed id in obj's scope chain. */
             if (!PurgeScopeChain(cx, obj, id))
                 return false;
         }
 
-        return DefinePropertyOrElement<mode>(cxArg, obj, id, getter, setter,
+        return DefinePropertyOrElement<mode>(cxArg, obj, id,
+                                             clasp->getProperty, clasp->setProperty,
                                              attrs, vp, true, strict);
     }
 
     return NativeSet<mode>(cxArg, obj, receiver, shape, strict, vp);
 }
 
 template bool
 baseops::SetPropertyHelper<SequentialExecution>(JSContext *cx, HandleNativeObject obj,