Bug 1058869 - Don't forget about Arrays for attribute-only Object.defineProperty calls. (r=jorendorff)
authorEric Faust <efaustbmo@gmail.com>
Fri, 29 Aug 2014 14:59:51 -0700
changeset 224163 721ba95253972546a15af237593d8efdf0881569
parent 224162 2a11f29863a1a59ad2b13d4d72f6e9c5577f2c76
child 224164 480cdabe848fd5f99187b031e2b301918bb89bea
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1058869
milestone34.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 1058869 - Don't forget about Arrays for attribute-only Object.defineProperty calls. (r=jorendorff)
js/src/jsobj.cpp
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -4274,30 +4274,48 @@ js::DefineNativeProperty(ExclusiveContex
         }
     } else if (!(attrs & JSPROP_IGNORE_VALUE)) {
         /*
          * We might still want to ignore redefining some of our attributes, if the
          * request came through a proxy after Object.defineProperty(), but only if we're redefining
          * a data property.
          * FIXME: All this logic should be removed when Proxies use PropDesc, but we need to
          *        remove JSPropertyOp getters and setters first.
+         * FIXME: This is still wrong for various array types, and will set the wrong attributes
+         *        by accident, but we can't use NativeLookupOwnProperty in this case, because of resolve
+         *        loops.
          */
         shape = obj->nativeLookup(cx, id);
         if (shape && shape->isDataDescriptor())
             attrs = ApplyOrDefaultAttributes(attrs, shape);
     } else {
         /*
          * We have been asked merely to update some attributes by a caller of
          * Object.defineProperty, laundered through the proxy system, and returning here. We can
          * get away with just using JSObject::changeProperty here.
          */
         if (!NativeLookupOwnProperty(cx, obj, id, &shape))
             return false;
 
         if (shape) {
+            // Don't forget about arrays.
+            if (IsImplicitDenseOrTypedArrayElement(shape)) {
+                if (obj->is<TypedArrayObject>()) {
+                    /*
+                     * Silently ignore attempts to change individial index attributes.
+                     * FIXME: Uses the same broken behavior as for accessors. This should
+                     *        probably throw.
+                     */
+                    return true;
+                }
+                if (!JSObject::sparsifyDenseElement(cx, obj, JSID_TO_INT(id)))
+                    return false;
+                shape = obj->nativeLookup(cx, id);
+            }
+
             attrs = ApplyOrDefaultAttributes(attrs, shape);
 
             /* Keep everything from the shape that isn't the things we're changing */
             unsigned attrMask = ~(JSPROP_ENUMERATE | JSPROP_READONLY | JSPROP_PERMANENT);
             shape = JSObject::changeProperty<SequentialExecution>(cx, obj, shape, attrs, attrMask,
                                                                   shape->getter(), shape->setter());
             if (!shape)
                 return false;