Bug 1105518: Remove JSPROP_REDEFINE_NONCONFIGURABLE from jsapi. r=jorendorff
authorAndré Bargull <andre.bargull@gmail.com>
Sat, 16 Jun 2018 11:59:04 -0700
changeset 422799 1d498636e0d5b5090691cb05dd7c0af09b4c2949
parent 422798 1950fa5311693d8658f9a79f64ce3ff611c5d0d1
child 422800 6350b1a6097e821b836c7a83924259f3b64d0b70
push id34148
push userccoroiu@mozilla.com
push dateSun, 17 Jun 2018 09:46:48 +0000
treeherdermozilla-central@c40cc0a89bc7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1105518
milestone62.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 1105518: Remove JSPROP_REDEFINE_NONCONFIGURABLE from jsapi. r=jorendorff
js/src/jsapi.h
js/src/vm/NativeObject.cpp
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -491,23 +491,16 @@ static const uint8_t JSPROP_INTERNAL_USE
 
 /* native that can be called as a ctor */
 static const unsigned JSFUN_CONSTRUCTOR =     0x400;
 
 /* | of all the JSFUN_* flags */
 static const unsigned JSFUN_FLAGS_MASK =      0x400;
 
 /*
- * If set, will allow redefining a non-configurable property, but only on a
- * non-DOM global.  This is a temporary hack that will need to go away in bug
- * 1105518.
- */
-static const unsigned JSPROP_REDEFINE_NONCONFIGURABLE = 0x1000;
-
-/*
  * Resolve hooks and enumerate hooks must pass this flag when calling
  * JS_Define* APIs to reify lazily-defined properties.
  *
  * JSPROP_RESOLVING is used only with property-defining APIs. It tells the
  * engine to skip the resolve hook when performing the lookup at the beginning
  * of property definition. This keeps the resolve hook from accidentally
  * triggering itself: unchecked recursion.
  *
@@ -2107,17 +2100,16 @@ class WrappedPtrOperations<JS::PropertyD
     void assertValid() const {
 #ifdef DEBUG
         MOZ_ASSERT((attributes() & ~(JSPROP_ENUMERATE | JSPROP_IGNORE_ENUMERATE |
                                      JSPROP_PERMANENT | JSPROP_IGNORE_PERMANENT |
                                      JSPROP_READONLY | JSPROP_IGNORE_READONLY |
                                      JSPROP_IGNORE_VALUE |
                                      JSPROP_GETTER |
                                      JSPROP_SETTER |
-                                     JSPROP_REDEFINE_NONCONFIGURABLE |
                                      JSPROP_RESOLVING |
                                      SHADOWABLE)) == 0);
         MOZ_ASSERT(!hasAll(JSPROP_IGNORE_ENUMERATE | JSPROP_ENUMERATE));
         MOZ_ASSERT(!hasAll(JSPROP_IGNORE_PERMANENT | JSPROP_PERMANENT));
         if (isAccessorDescriptor()) {
             MOZ_ASSERT(!has(JSPROP_READONLY));
             MOZ_ASSERT(!has(JSPROP_IGNORE_READONLY));
             MOZ_ASSERT(!has(JSPROP_IGNORE_VALUE));
@@ -2129,29 +2121,27 @@ class WrappedPtrOperations<JS::PropertyD
             MOZ_ASSERT(!hasAll(JSPROP_IGNORE_READONLY | JSPROP_READONLY));
             MOZ_ASSERT_IF(has(JSPROP_IGNORE_VALUE), value().isUndefined());
         }
 
         MOZ_ASSERT_IF(has(JSPROP_RESOLVING), !has(JSPROP_IGNORE_ENUMERATE));
         MOZ_ASSERT_IF(has(JSPROP_RESOLVING), !has(JSPROP_IGNORE_PERMANENT));
         MOZ_ASSERT_IF(has(JSPROP_RESOLVING), !has(JSPROP_IGNORE_READONLY));
         MOZ_ASSERT_IF(has(JSPROP_RESOLVING), !has(JSPROP_IGNORE_VALUE));
-        MOZ_ASSERT_IF(has(JSPROP_RESOLVING), !has(JSPROP_REDEFINE_NONCONFIGURABLE));
 #endif
     }
 
     void assertComplete() const {
 #ifdef DEBUG
         assertValid();
         MOZ_ASSERT((attributes() & ~(JSPROP_ENUMERATE |
                                      JSPROP_PERMANENT |
                                      JSPROP_READONLY |
                                      JSPROP_GETTER |
                                      JSPROP_SETTER |
-                                     JSPROP_REDEFINE_NONCONFIGURABLE |
                                      JSPROP_RESOLVING |
                                      SHADOWABLE)) == 0);
         MOZ_ASSERT_IF(isAccessorDescriptor(), has(JSPROP_GETTER) && has(JSPROP_SETTER));
 #endif
     }
 
     void assertCompleteIfFound() const {
 #ifdef DEBUG
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -1702,28 +1702,18 @@ js::NativeDefineProperty(JSContext* cx, 
         // type for this property that doesn't match the value in the slot.
         // Update the type here, even though this DefineProperty call is
         // otherwise a no-op. (See bug 1125624 comment 13.)
         if (!prop.isDenseOrTypedArrayElement() && desc.hasValue())
             UpdateShapeTypeAndValue(cx, obj, prop.shape(), id, desc.value());
         return result.succeed();
     }
 
-    // Non-standard hack: Allow redefining non-configurable properties if
-    // JSPROP_REDEFINE_NONCONFIGURABLE is set _and_ the object is a non-DOM
-    // global. The idea is that a DOM object can never have such a thing on
-    // its proto chain directly on the web, so we should be OK optimizing
-    // access to accessors found on such an object. Bug 1105518 contemplates
-    // removing this hack.
-    bool skipRedefineChecks = (desc.attributes() & JSPROP_REDEFINE_NONCONFIGURABLE) &&
-                              obj->is<GlobalObject>() &&
-                              !obj->getClass()->isDOMClass();
-
     // Step 4.
-    if (!IsConfigurable(shapeAttrs) && !skipRedefineChecks) {
+    if (!IsConfigurable(shapeAttrs)) {
         if (desc.hasConfigurable() && desc.configurable())
             return result.fail(JSMSG_CANT_REDEFINE_PROP);
         if (desc.hasEnumerable() && desc.enumerable() != IsEnumerable(shapeAttrs))
             return result.fail(JSMSG_CANT_REDEFINE_PROP);
     }
 
     // Fill in desc.[[Configurable]] and desc.[[Enumerable]] if missing.
     if (!desc.hasConfigurable())
@@ -1748,79 +1738,77 @@ js::NativeDefineProperty(JSContext* cx, 
             desc.setValue(currentValue);
             desc.setWritable(IsWritable(shapeAttrs));
         } else {
             desc.setGetterObject(prop.shape()->getterObject());
             desc.setSetterObject(prop.shape()->setterObject());
         }
     } else if (desc.isDataDescriptor() != IsDataDescriptor(shapeAttrs)) {
         // Step 6.
-        if (!IsConfigurable(shapeAttrs) && !skipRedefineChecks)
+        if (!IsConfigurable(shapeAttrs))
             return result.fail(JSMSG_CANT_REDEFINE_PROP);
 
         // Fill in desc fields with default values (steps 6.b.i and 6.c.i).
         CompletePropertyDescriptor(&desc);
     } else if (desc.isDataDescriptor()) {
         // Step 7.
         bool frozen = !IsConfigurable(shapeAttrs) && !IsWritable(shapeAttrs);
 
         // Step 7.a.i.1.
-        if (frozen && desc.hasWritable() && desc.writable() && !skipRedefineChecks)
+        if (frozen && desc.hasWritable() && desc.writable())
             return result.fail(JSMSG_CANT_REDEFINE_PROP);
 
         if (frozen || !desc.hasValue()) {
             RootedValue currentValue(cx);
             if (!GetExistingPropertyValue(cx, obj, id, prop, &currentValue))
                 return false;
 
             if (!desc.hasValue()) {
                 // Fill in desc.[[Value]].
                 desc.setValue(currentValue);
             } else {
                 // Step 7.a.i.2.
                 bool same;
                 MOZ_ASSERT(!cx->helperThread());
                 if (!SameValue(cx, desc.value(), currentValue, &same))
                     return false;
-                if (!same && !skipRedefineChecks)
+                if (!same)
                     return result.fail(JSMSG_CANT_REDEFINE_PROP);
             }
         }
 
         // Step 7.a.i.3.
-        if (frozen && !skipRedefineChecks)
+        if (frozen)
             return result.succeed();
 
         // Fill in desc.[[Writable]].
         if (!desc.hasWritable())
             desc.setWritable(IsWritable(shapeAttrs));
     } else {
         // Step 8.
         MOZ_ASSERT(prop.shape()->isAccessorDescriptor());
         MOZ_ASSERT(desc.isAccessorDescriptor());
 
         // The spec says to use SameValue, but since the values in
         // question are objects, we can just compare pointers.
         if (desc.hasSetterObject()) {
             // Step 8.a.i.
             if (!IsConfigurable(shapeAttrs) &&
-                desc.setterObject() != prop.shape()->setterObject() &&
-                !skipRedefineChecks)
+                desc.setterObject() != prop.shape()->setterObject())
             {
                 return result.fail(JSMSG_CANT_REDEFINE_PROP);
             }
         } else {
             // Fill in desc.[[Set]] from shape.
             desc.setSetterObject(prop.shape()->setterObject());
         }
         if (desc.hasGetterObject()) {
             // Step 8.a.ii.
             if (!IsConfigurable(shapeAttrs) &&
-                desc.getterObject() != prop.shape()->getterObject() &&
-                !skipRedefineChecks)
+                desc.getterObject() != prop.shape()->getterObject())
             {
                 return result.fail(JSMSG_CANT_REDEFINE_PROP);
             }
         } else {
             // Fill in desc.[[Get]] from shape.
             desc.setGetterObject(prop.shape()->getterObject());
         }