Bug 1020609 - Make configurability check in Xray defineProperty match the spec. r=bz, a=lmandel
authorBobby Holley <bobbyholley@gmail.com>
Wed, 11 Jun 2014 15:16:06 -0700
changeset 208214 43fe37d13da6f7e5c1cbff924c3126990d055f42
parent 208213 f916b5650b573e8ef8cd2106a2495cba01fef13a
child 208215 e1f7922776650a02579e800e93a7ee3b13b82f58
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, lmandel
bugs1020609
milestone32.0a2
Bug 1020609 - Make configurability check in Xray defineProperty match the spec. r=bz, a=lmandel This code is basically emulating the ES semantics with respect to non-configurable properties. Non-configurable value properties can still be writable, in which case their value and writability may be updated.
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -2309,18 +2309,32 @@ XrayWrapper<Base, Traits>::definePropert
         return false;
 
     // Note that the check here is intended to differentiate between own and
     // non-own properties, since the above lookup is not limited to own
     // properties. At present, this may not always do the right thing because
     // we often lie (sloppily) about where we found properties and set
     // desc.object() to |wrapper|. Once we fully fix our Xray prototype semantics,
     // this should work as intended.
-    if (existing_desc.object() == wrapper && existing_desc.isPermanent())
-        return true; // silently ignore attempt to overwrite native property
+    if (existing_desc.object() == wrapper && existing_desc.isPermanent()) {
+        // We have a non-configurable property. See if the caller is trying to
+        // re-configure it in any way other than making it non-writable.
+        if (existing_desc.hasGetterOrSetterObject() || desc.hasGetterOrSetterObject() ||
+            existing_desc.isEnumerable() != desc.isEnumerable() ||
+            (existing_desc.isReadonly() && !desc.isReadonly()))
+        {
+            // We should technically report non-configurability in strict mode, but
+            // doing that via JSAPI is a lot of trouble.
+            return true;
+        }
+        if (existing_desc.isReadonly()) {
+            // Same as the above for non-writability.
+            return true;
+        }
+    }
 
     bool defined = false;
     if (!Traits::singleton.defineProperty(cx, wrapper, id, desc, existing_desc, &defined))
         return false;
     if (defined)
         return true;
 
     // We're placing an expando. The expando objects live in the target