Bug 1133081, part 5 - Remove non-asserting PropertyDescriptor accessors in favor of the new PropDesc-inspired asserting accessors. r=efaust.
authorJason Orendorff <jorendorff@mozilla.com>
Sun, 15 Feb 2015 06:18:30 -0600
changeset 233538 c78a9d1273c5d4ddf7a804eab41a92ff90ce0ec5
parent 233537 63dbcc4fd0f02324f31e55c69409d61bc10116c0
child 233539 35ad2e5b036b9d141a0bbf71571b7b4936e23b7e
push id14482
push userryanvm@gmail.com
push dateFri, 13 Mar 2015 20:14:13 +0000
treeherderb2g-inbound@d14bd865880c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1133081
milestone39.0a1
Bug 1133081, part 5 - Remove non-asserting PropertyDescriptor accessors in favor of the new PropDesc-inspired asserting accessors. r=efaust. value() can't assert hasValue() because too many places have plausible reasons for calling it on a PropertyDescriptor they basically know nothing about. One such place is CompartmentChecker::check(Handle<JSPropertyDescriptor>). Another is DefinePropertyByDescriptor. Maybe this will change with time. In some cases we do things like `desc.hasWritable() && desc.writable() != existing_desc.writable()`. It is OK to write it this way, even though we have not checked existing_desc.hasWritable(), because in these cases we already know existingDesc is a complete property descriptor.
dom/base/nsDocument.cpp
dom/base/nsGlobalWindow.cpp
js/src/asmjs/AsmJSLink.cpp
js/src/builtin/Object.cpp
js/src/jsapi-tests/testDefineGetterSetterNonEnumerable.cpp
js/src/jsapi-tests/testDefinePropertyIgnoredAttributes.cpp
js/src/jsapi.h
js/src/jsiter.cpp
js/src/jsobj.cpp
js/src/proxy/BaseProxyHandler.cpp
js/src/proxy/ScriptedDirectProxyHandler.cpp
js/src/proxy/ScriptedIndirectProxyHandler.cpp
js/src/vm/Interpreter-inl.h
js/xpconnect/src/Sandbox.cpp
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/AddonWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -6241,18 +6241,17 @@ nsDocument::RegisterElement(JSContext* a
       // This check will go through a wrapper, but as we checked above
       // it should be transparent or an xray. This should be fine for now,
       // until the spec is sorted out.
       if (!JS_GetPropertyDescriptor(aCx, protoObject, "constructor", desc)) {
         rv.Throw(NS_ERROR_UNEXPECTED);
         return;
       }
 
-      // Check if non-configurable
-      if (desc.isPermanent()) {
+      if (!desc.configurable()) {
         rv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
         return;
       }
 
       JS::Handle<JSObject*> svgProto(
         SVGElementBinding::GetProtoObjectHandle(aCx, global));
       if (!svgProto) {
         rv.Throw(NS_ERROR_OUT_OF_MEMORY);
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -792,17 +792,17 @@ nsOuterWindowProxy::defineProperty(JSCon
     // to the caller to decide whether to throw a TypeError.
     return result.failCantDefineWindowElement();
   }
 
   // For now, allow chrome code to define non-configurable properties
   // on windows, until we sort out what exactly the addon SDK is
   // doing.  In the meantime, this still allows us to test web compat
   // behavior.
-  if (false && desc.isPermanent() && !nsContentUtils::IsCallerChrome()) {
+  if (false && !desc.configurable() && !nsContentUtils::IsCallerChrome()) {
     return ThrowErrorMessage(cx, MSG_DEFINE_NON_CONFIGURABLE_PROP_ON_WINDOW);
   }
 
   return js::Wrapper::defineProperty(cx, proxy, id, desc, result);
 }
 
 bool
 nsOuterWindowProxy::ownPropertyKeys(JSContext *cx,
--- a/js/src/asmjs/AsmJSLink.cpp
+++ b/js/src/asmjs/AsmJSLink.cpp
@@ -91,17 +91,17 @@ GetDataProperty(JSContext *cx, HandleVal
     Rooted<PropertyDescriptor> desc(cx);
     RootedId id(cx, NameToId(field));
     if (!GetPropertyDescriptor(cx, obj, id, &desc))
         return false;
 
     if (!desc.object())
         return LinkFail(cx, "property not present on object");
 
-    if (desc.hasGetterOrSetterObject())
+    if (!desc.isDataDescriptor())
         return LinkFail(cx, "property is not a data property");
 
     v.set(desc.value());
     return true;
 }
 
 static bool
 HasPureCoercion(JSContext *cx, HandleValue v)
--- a/js/src/builtin/Object.cpp
+++ b/js/src/builtin/Object.cpp
@@ -91,17 +91,17 @@ obj_propertyIsEnumerable(JSContext *cx, 
         return false;
 
     /* Step 3. */
     Rooted<PropertyDescriptor> desc(cx);
     if (!GetOwnPropertyDescriptor(cx, obj, idRoot, &desc))
         return false;
 
     /* Steps 4-5. */
-    args.rval().setBoolean(desc.object() && desc.isEnumerable());
+    args.rval().setBoolean(desc.object() && desc.enumerable());
     return true;
 }
 
 #if JS_HAS_TOSOURCE
 static bool
 obj_toSource(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
@@ -191,17 +191,17 @@ js::ObjectToSource(JSContext *cx, Handle
     for (size_t i = 0; i < idv.length(); ++i) {
         RootedId id(cx, idv[i]);
         Rooted<PropertyDescriptor> desc(cx);
         if (!GetOwnPropertyDescriptor(cx, obj, id, &desc))
             return nullptr;
 
         int valcnt = 0;
         if (desc.object()) {
-            if (desc.hasGetterOrSetterObject()) {
+            if (desc.isAccessorDescriptor()) {
                 if (desc.hasGetterObject() && desc.getterObject()) {
                     val[valcnt].setObject(*desc.getterObject());
                     gsop[valcnt].set(cx->names().get);
                     valcnt++;
                 }
                 if (desc.hasSetterObject() && desc.setterObject()) {
                     val[valcnt].setObject(*desc.setterObject());
                     gsop[valcnt].set(cx->names().set);
--- a/js/src/jsapi-tests/testDefineGetterSetterNonEnumerable.cpp
+++ b/js/src/jsapi-tests/testDefineGetterSetterNonEnumerable.cpp
@@ -45,14 +45,14 @@ BEGIN_TEST(testDefineGetterSetterNonEnum
                             JS_DATA_TO_FUNC_PTR(JSNative, (JSObject*) funGetObj),
                             JS_DATA_TO_FUNC_PTR(JSNative, (JSObject*) funSetObj)));
 
     JS::Rooted<JSPropertyDescriptor> desc(cx);
     CHECK(JS_GetOwnPropertyDescriptor(cx, vObject, PROPERTY_NAME, &desc));
     CHECK(desc.object());
     CHECK(desc.hasGetterObject());
     CHECK(desc.hasSetterObject());
-    CHECK(desc.isPermanent());
-    CHECK(!desc.isEnumerable());
+    CHECK(!desc.configurable());
+    CHECK(!desc.enumerable());
 
     return true;
 }
 END_TEST(testDefineGetterSetterNonEnumerable)
--- a/js/src/jsapi-tests/testDefinePropertyIgnoredAttributes.cpp
+++ b/js/src/jsapi-tests/testDefinePropertyIgnoredAttributes.cpp
@@ -19,27 +19,33 @@ static const unsigned ValueWithConfigura
 static bool
 Getter(JSContext *cx, unsigned argc, JS::Value *vp)
 {
     JS::CallArgs args = CallArgsFromVp(argc, vp);
     args.rval().setBoolean(true);
     return true;
 }
 
+enum PropertyDescriptorKind {
+    DataDescriptor, AccessorDescriptor
+};
+
 static bool
-CheckDescriptor(JS::Handle<JSPropertyDescriptor> desc, bool enumerable,
-                bool writable, bool configurable)
+CheckDescriptor(JS::Handle<JSPropertyDescriptor> desc, PropertyDescriptorKind kind,
+                bool enumerable, bool writable, bool configurable)
 {
     if (!desc.object())
         return false;
-    if (desc.isEnumerable() != enumerable)
+    if (!(kind == DataDescriptor ? desc.isDataDescriptor() : desc.isAccessorDescriptor()))
+        return false;
+    if (desc.enumerable() != enumerable)
         return false;
-    if (desc.isReadonly() == writable)
+    if (kind == DataDescriptor && desc.writable() != writable)
         return false;
-    if (desc.isPermanent() == configurable)
+    if (desc.configurable() != configurable)
         return false;
     return true;
 }
 
 BEGIN_TEST(testDefinePropertyIgnoredAttributes)
 {
     JS::RootedObject obj(cx, JS_NewPlainObject(cx));
     JS::Rooted<JSPropertyDescriptor> desc(cx);
@@ -47,50 +53,49 @@ BEGIN_TEST(testDefinePropertyIgnoredAttr
 
     // Try a getter. Allow it to fill in the defaults.
     CHECK(JS_DefineProperty(cx, obj, "foo", defineValue,
                             IgnoreAll | JSPROP_SHARED,
                             Getter));
 
     CHECK(JS_GetPropertyDescriptor(cx, obj, "foo", &desc));
 
-    // Note that since JSPROP_READONLY means nothing for accessor properties, we will actually
-    // claim to be writable, since the flag is not included in the mask.
-    CHECK(CheckDescriptor(desc, false, true, false));
+    // Note that JSPROP_READONLY is meaningless for accessor properties.
+    CHECK(CheckDescriptor(desc, AccessorDescriptor, false, true, false));
 
     // Install another configurable property, so we can futz with it.
     CHECK(JS_DefineProperty(cx, obj, "bar", defineValue,
                             AllowConfigure | JSPROP_SHARED,
                             Getter));
     CHECK(JS_GetPropertyDescriptor(cx, obj, "bar", &desc));
-    CHECK(CheckDescriptor(desc, false, true, true));
+    CHECK(CheckDescriptor(desc, AccessorDescriptor, false, true, true));
 
     // Rewrite the descriptor to now be enumerable, ensuring that the lack of
     // configurablity stayed.
     CHECK(JS_DefineProperty(cx, obj, "bar", defineValue,
                             AllowEnumerate |
                             JSPROP_ENUMERATE |
                             JSPROP_SHARED,
                             Getter));
     CHECK(JS_GetPropertyDescriptor(cx, obj, "bar", &desc));
-    CHECK(CheckDescriptor(desc, true, true, true));
+    CHECK(CheckDescriptor(desc, AccessorDescriptor, true, true, true));
 
     // Now try the same game with a value property
     defineValue.setObject(*obj);
     CHECK(JS_DefineProperty(cx, obj, "baz", defineValue, IgnoreWithValue));
     CHECK(JS_GetPropertyDescriptor(cx, obj, "baz", &desc));
-    CHECK(CheckDescriptor(desc, false, false, false));
+    CHECK(CheckDescriptor(desc, DataDescriptor, false, false, false));
 
     // Now again with a configurable property
     CHECK(JS_DefineProperty(cx, obj, "quox", defineValue, ValueWithConfigurable));
     CHECK(JS_GetPropertyDescriptor(cx, obj, "quox", &desc));
-    CHECK(CheckDescriptor(desc, false, false, true));
+    CHECK(CheckDescriptor(desc, DataDescriptor, false, false, true));
 
     // Just make it writable. Leave the old value and everythign else alone.
     defineValue.setUndefined();
     CHECK(JS_DefineProperty(cx, obj, "quox", defineValue, AllowWritable));
     CHECK(JS_GetPropertyDescriptor(cx, obj, "quox", &desc));
-    CHECK(CheckDescriptor(desc, false, true, true));
+    CHECK(CheckDescriptor(desc, DataDescriptor, false, true, true));
     CHECK_SAME(JS::ObjectValue(*obj), desc.value());
 
     return true;
 }
 END_TEST(testDefinePropertyIgnoredAttributes)
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2529,67 +2529,73 @@ struct JSPropertyDescriptor {
 
 namespace JS {
 
 template <typename Outer>
 class PropertyDescriptorOperations
 {
     const JSPropertyDescriptor * desc() const { return static_cast<const Outer*>(this)->extract(); }
 
+    bool has(unsigned bit) const {
+        MOZ_ASSERT(bit != 0);
+        MOZ_ASSERT((bit & (bit - 1)) == 0);  // only a single bit
+        return (desc()->attrs & bit) != 0;
+    }
+
+    bool hasAny(unsigned bits) const {
+        return (desc()->attrs & bits) != 0;
+    }
+
   public:
     // Descriptors with JSGetterOp/JSSetterOp are considered data
     // descriptors. It's complicated.
-    bool isAccessorDescriptor() const { return hasGetterOrSetterObject(); }
+    bool isAccessorDescriptor() const { return hasAny(JSPROP_GETTER | JSPROP_SETTER); }
     bool isGenericDescriptor() const {
         return (desc()->attrs &
                 (JSPROP_GETTER | JSPROP_SETTER | JSPROP_IGNORE_READONLY | JSPROP_IGNORE_VALUE)) ==
                (JSPROP_IGNORE_READONLY | JSPROP_IGNORE_VALUE);
     }
     bool isDataDescriptor() const { return !isAccessorDescriptor() && !isGenericDescriptor(); }
 
-    bool hasConfigurable() const { return !hasAttributes(JSPROP_IGNORE_PERMANENT); }
-    bool configurable() const { MOZ_ASSERT(hasConfigurable()); return !isPermanent(); }
-    bool hasEnumerable() const { return !hasAttributes(JSPROP_IGNORE_ENUMERATE); }
-    bool enumerable() const { MOZ_ASSERT(hasEnumerable()); return isEnumerable(); }
-    bool hasValue() const { return !isAccessorDescriptor() && !hasAttributes(JSPROP_IGNORE_VALUE); }
-    bool hasWritable() const { return !isAccessorDescriptor() && !hasAttributes(JSPROP_IGNORE_READONLY); }
-    bool writable() const { MOZ_ASSERT(hasWritable()); return !isReadonly(); }
-
-    bool isEnumerable() const { return desc()->attrs & JSPROP_ENUMERATE; }
-    bool isReadonly() const { return desc()->attrs & JSPROP_READONLY; }
-    bool isPermanent() const { return desc()->attrs & JSPROP_PERMANENT; }
-    bool hasGetterObject() const { return desc()->attrs & JSPROP_GETTER; }
-    bool hasSetterObject() const { return desc()->attrs & JSPROP_SETTER; }
-    bool hasGetterOrSetterObject() const { return desc()->attrs & (JSPROP_GETTER | JSPROP_SETTER); }
+    bool hasConfigurable() const { return !has(JSPROP_IGNORE_PERMANENT); }
+    bool configurable() const { MOZ_ASSERT(hasConfigurable()); return !has(JSPROP_PERMANENT); }
+
+    bool hasEnumerable() const { return !has(JSPROP_IGNORE_ENUMERATE); }
+    bool enumerable() const { MOZ_ASSERT(hasEnumerable()); return has(JSPROP_ENUMERATE); }
+
+    bool hasValue() const { return !isAccessorDescriptor() && !has(JSPROP_IGNORE_VALUE); }
+    JS::HandleValue value() const {
+        return JS::HandleValue::fromMarkedLocation(&desc()->value);
+    }
+
+    bool hasWritable() const { return !isAccessorDescriptor() && !has(JSPROP_IGNORE_READONLY); }
+    bool writable() const { MOZ_ASSERT(hasWritable()); return !has(JSPROP_READONLY); }
+
+    bool hasGetterObject() const { return has(JSPROP_GETTER); }
+    JS::HandleObject getterObject() const {
+        MOZ_ASSERT(hasGetterObject());
+        return JS::HandleObject::fromMarkedLocation(
+                reinterpret_cast<JSObject *const *>(&desc()->getter));
+    }
+    bool hasSetterObject() const { return has(JSPROP_SETTER); }
+    JS::HandleObject setterObject() const {
+        MOZ_ASSERT(hasSetterObject());
+        return JS::HandleObject::fromMarkedLocation(
+                reinterpret_cast<JSObject *const *>(&desc()->setter));
+    }
+
     bool hasGetterOrSetter() const { return desc()->getter || desc()->setter; }
-    bool isShared() const { return desc()->attrs & JSPROP_SHARED; }
-    bool isIndex() const { return desc()->attrs & JSPROP_INDEX; }
-    bool hasAttributes(unsigned attrs) const { return desc()->attrs & attrs; }
-
-    bool isWritable() const { MOZ_ASSERT(isDataDescriptor()); return !isReadonly(); }
+    bool isShared() const { return has(JSPROP_SHARED); }
 
     JS::HandleObject object() const {
         return JS::HandleObject::fromMarkedLocation(&desc()->obj);
     }
     unsigned attributes() const { return desc()->attrs; }
     JSGetterOp getter() const { return desc()->getter; }
     JSSetterOp setter() const { return desc()->setter; }
-    JS::HandleObject getterObject() const {
-        MOZ_ASSERT(hasGetterObject());
-        return JS::HandleObject::fromMarkedLocation(
-                reinterpret_cast<JSObject *const *>(&desc()->getter));
-    }
-    JS::HandleObject setterObject() const {
-        MOZ_ASSERT(hasSetterObject());
-        return JS::HandleObject::fromMarkedLocation(
-                reinterpret_cast<JSObject *const *>(&desc()->setter));
-    }
-    JS::HandleValue value() const {
-        return JS::HandleValue::fromMarkedLocation(&desc()->value);
-    }
 };
 
 template <typename Outer>
 class MutablePropertyDescriptorOperations : public PropertyDescriptorOperations<Outer>
 {
     JSPropertyDescriptor * desc() { return static_cast<Outer*>(this)->extractMutable(); }
 
   public:
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -322,17 +322,17 @@ Snapshot(JSContext *cx, HandleObject pob
                 for (size_t n = 0, len = proxyProps.length(); n < len; n++) {
                     bool enumerable = false;
 
                     // We need to filter, if the caller just wants enumerable
                     // symbols.
                     if (!(flags & JSITER_HIDDEN)) {
                         if (!Proxy::getOwnPropertyDescriptor(cx, pobj, proxyProps[n], &desc))
                             return false;
-                        enumerable = desc.isEnumerable();
+                        enumerable = desc.enumerable();
                     }
 
                     if (!Enumerate(cx, pobj, proxyProps[n], enumerable, flags, ht, props))
                         return false;
                 }
             } else {
                 // Returns enumerable property names (no symbols).
                 if (!Proxy::getOwnEnumerablePropertyKeys(cx, pobj, proxyProps))
@@ -1170,17 +1170,17 @@ SuppressDeletedPropertyHelper(JSContext 
                         if (!ValueToId<CanGC>(cx, idv, &id))
                             return false;
 
                         Rooted<PropertyDescriptor> desc(cx);
                         if (!GetPropertyDescriptor(cx, proto, id, &desc))
                             return false;
 
                         if (desc.object()) {
-                            if (desc.isEnumerable())
+                            if (desc.enumerable())
                                 continue;
                         }
                     }
 
                     /*
                      * If GetPropertyDescriptorById above removed a property from
                      * ni, start over.
                      */
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1065,18 +1065,18 @@ js::TestIntegrityLevel(JSContext *cx, Ha
         if (!GetOwnPropertyDescriptor(cx, obj, id, &desc))
             return false;
 
         // Step 9.c.
         if (!desc.object())
             continue;
 
         // Steps 9.c.i-ii.
-        if (!desc.isPermanent() ||
-            (level == IntegrityLevel::Frozen && desc.isDataDescriptor() && desc.isWritable()))
+        if (desc.configurable() ||
+            (level == IntegrityLevel::Frozen && desc.isDataDescriptor() && desc.writable()))
         {
             *result = false;
             return true;
         }
     }
 
     // Step 10.
     *result = true;
@@ -3134,17 +3134,17 @@ js::GetOwnPropertyDescriptor(JSContext *
         return false;
     if (!shape) {
         desc.object().set(nullptr);
         return true;
     }
 
     bool doGet = true;
     desc.setAttributes(GetShapeAttributes(obj, shape));
-    if (desc.hasGetterOrSetterObject()) {
+    if (desc.isAccessorDescriptor()) {
         MOZ_ASSERT(desc.isShared());
         doGet = false;
 
         // The result of GetOwnPropertyDescriptor() must be either undefined or
         // a complete property descriptor (per ES6 draft rev 32 (2015 Feb 2)
         // 6.1.7.3, Invariants of the Essential Internal Methods).
         //
         // It is an unfortunate fact that in SM, properties can exist that have
--- a/js/src/proxy/BaseProxyHandler.cpp
+++ b/js/src/proxy/BaseProxyHandler.cpp
@@ -112,17 +112,17 @@ js::SetPropertyIgnoringNamedGetter(JSCon
         ownDesc.clear();
         ownDesc.setAttributes(JSPROP_ENUMERATE);
     }
 
     // Step 5.
     if (ownDesc.isDataDescriptor()) {
         // Steps 5.a-b, adapted to our nonstandard implementation of ES6
         // [[Set]] return values.
-        if (!ownDesc.isWritable())
+        if (!ownDesc.writable())
             return result.fail(JSMSG_READ_ONLY);
 
         // Nonstandard SpiderMonkey special case: setter ops.
         SetterOp setter = ownDesc.setter();
         MOZ_ASSERT(setter != JS_StrictPropertyStub);
         if (setter && setter != JS_StrictPropertyStub)
             return CallSetter(cx, receiver, id, setter, ownDesc.attributes(), vp, result);
 
@@ -178,17 +178,17 @@ BaseProxyHandler::getOwnEnumerableProper
         id = props[j];
         if (JSID_IS_SYMBOL(id))
             continue;
 
         AutoWaivePolicy policy(cx, proxy, id, BaseProxyHandler::GET);
         Rooted<PropertyDescriptor> desc(cx);
         if (!getOwnPropertyDescriptor(cx, proxy, id, &desc))
             return false;
-        if (desc.object() && desc.isEnumerable())
+        if (desc.object() && desc.enumerable())
             props[i++].set(id);
     }
 
     MOZ_ASSERT(i <= props.length());
     props.resize(i);
 
     return true;
 }
--- a/js/src/proxy/ScriptedDirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedDirectProxyHandler.cpp
@@ -44,17 +44,18 @@ ValidatePropertyDescriptor(JSContext *cx
         !desc.hasGetterObject() && !desc.hasSetterObject() &&
         !desc.hasEnumerable() && !desc.hasConfigurable())
     {
         *bp = true;
         return true;
     }
 
     // step 4
-    if ((!desc.hasWritable() || desc.writable() == !current.isReadonly()) &&
+    if ((!desc.hasWritable() ||
+         (current.hasWritable() && desc.writable() == current.writable())) &&
         (!desc.hasGetterObject() || desc.getter() == current.getter()) &&
         (!desc.hasSetterObject() || desc.setter() == current.setter()) &&
         (!desc.hasEnumerable() || desc.enumerable() == current.enumerable()) &&
         (!desc.hasConfigurable() || desc.configurable() == current.configurable()))
     {
         if (!desc.hasValue()) {
             *bp = true;
             return true;
@@ -131,17 +132,17 @@ static bool
 IsSealed(JSContext* cx, HandleObject obj, HandleId id, bool *bp)
 {
     // step 1
     Rooted<PropertyDescriptor> desc(cx);
     if (!GetOwnPropertyDescriptor(cx, obj, id, &desc))
         return false;
 
     // steps 2-3
-    *bp = desc.object() && desc.isPermanent();
+    *bp = desc.object() && !desc.configurable();
     return true;
 }
 
 // Get the [[ProxyHandler]] of a scripted direct proxy.
 static JSObject *
 GetDirectProxyHandlerObject(JSObject *proxy)
 {
     MOZ_ASSERT(proxy->as<ProxyObject>().handler() == &ScriptedDirectProxyHandler::singleton);
@@ -479,17 +480,17 @@ ScriptedDirectProxyHandler::getOwnProper
     if (trapResult.isUndefined()) {
         // substep a
         if (!targetDesc.object()) {
             desc.object().set(nullptr);
             return true;
         }
 
         // substep b
-        if (targetDesc.isPermanent()) {
+        if (!targetDesc.configurable()) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NC_AS_NE);
             return false;
         }
 
         // substep c-e
         bool extensibleTarget;
         if (!IsExtensible(cx, target, &extensibleTarget))
             return false;
@@ -529,17 +530,17 @@ ScriptedDirectProxyHandler::getOwnProper
 
     // step 21
     if (!resultDesc.configurable()) {
         if (!targetDesc.object()) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NE_AS_NC);
             return false;
         }
 
-        if (!targetDesc.isPermanent()) {
+        if (targetDesc.configurable()) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_REPORT_C_AS_NC);
             return false;
         }
     }
 
     // step 22
     desc.set(resultDesc);
     desc.object().set(proxy);
@@ -600,36 +601,34 @@ ScriptedDirectProxyHandler::defineProper
         return false;
 
     // step 15-16
     bool extensibleTarget;
     if (!IsExtensible(cx, target, &extensibleTarget))
         return false;
 
     // step 17-18
-    // FIXME bug 1133081: settingConfigFalse should be false if we have
-    // JSPROP_IGNORE_PERMANENT.
-    bool settingConfigFalse = desc.isPermanent();
+    bool settingConfigFalse = desc.hasConfigurable() && !desc.configurable();
     if (!targetDesc.object()) {
         // step 19.a
         if (!extensibleTarget) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_NEW);
             return false;
         }
         // step 19.b
         if (settingConfigFalse) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_NE_AS_NC);
             return false;
         }
     } else {
         // step 20
         bool valid;
         if (!ValidatePropertyDescriptor(cx, extensibleTarget, desc, targetDesc, &valid))
             return false;
-        if (!valid || (settingConfigFalse && !targetDesc.isPermanent())) {
+        if (!valid || (settingConfigFalse && targetDesc.configurable())) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_INVALID);
             return false;
         }
     }
 
     // step 21
     return result.succeed();
 }
@@ -724,17 +723,17 @@ ScriptedDirectProxyHandler::delete_(JSCo
         return result.fail(JSMSG_PROXY_DELETE_RETURNED_FALSE);
 
     // steps 12-13
     Rooted<PropertyDescriptor> desc(cx);
     if (!GetOwnPropertyDescriptor(cx, target, id, &desc))
         return false;
 
     // step 14-15
-    if (desc.object() && desc.isPermanent()) {
+    if (desc.object() && !desc.configurable()) {
         RootedValue v(cx, IdToValue(id));
         ReportValueError(cx, JSMSG_CANT_DELETE, JSDVG_IGNORE_STACK, v, js::NullPtr());
         return false;
     }
 
     // step 16
     return result.succeed();
 }
@@ -827,17 +826,17 @@ ScriptedDirectProxyHandler::has(JSContex
 
     // step 11
     if (!success) {
         Rooted<PropertyDescriptor> desc(cx);
         if (!GetOwnPropertyDescriptor(cx, target, id, &desc))
             return false;
 
         if (desc.object()) {
-            if (desc.isPermanent()) {
+            if (!desc.configurable()) {
                 JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NC_AS_NE);
                 return false;
             }
 
             bool extensible;
             if (!IsExtensible(cx, target, &extensible))
                 return false;
             if (!extensible) {
@@ -893,27 +892,27 @@ ScriptedDirectProxyHandler::get(JSContex
 
     // step 10-11
     Rooted<PropertyDescriptor> desc(cx);
     if (!GetOwnPropertyDescriptor(cx, target, id, &desc))
         return false;
 
     // step 12
     if (desc.object()) {
-        if (IsDataDescriptor(desc) && desc.isPermanent() && desc.isReadonly()) {
+        if (desc.isDataDescriptor() && !desc.configurable() && !desc.writable()) {
             bool same;
             if (!SameValue(cx, trapResult, desc.value(), &same))
                 return false;
             if (!same) {
                 JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_MUST_REPORT_SAME_VALUE);
                 return false;
             }
         }
 
-        if (IsAccessorDescriptor(desc) && desc.isPermanent() && desc.getterObject() == nullptr) {
+        if (desc.isAccessorDescriptor() && !desc.configurable() && desc.getterObject() == nullptr) {
             if (!trapResult.isUndefined()) {
                 JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_MUST_REPORT_UNDEFINED);
                 return false;
             }
         }
     }
 
     // step 13
@@ -963,27 +962,27 @@ ScriptedDirectProxyHandler::set(JSContex
 
     // step 12-13
     Rooted<PropertyDescriptor> desc(cx);
     if (!GetOwnPropertyDescriptor(cx, target, id, &desc))
         return false;
 
     // step 14
     if (desc.object()) {
-        if (IsDataDescriptor(desc) && desc.isPermanent() && desc.isReadonly()) {
+        if (desc.isDataDescriptor() && !desc.configurable() && !desc.writable()) {
             bool same;
             if (!SameValue(cx, vp, desc.value(), &same))
                 return false;
             if (!same) {
                 JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_SET_NW_NC);
                 return false;
             }
         }
 
-        if (IsAccessorDescriptor(desc) && desc.isPermanent() && desc.setterObject() == nullptr) {
+        if (desc.isAccessorDescriptor() && !desc.configurable() && desc.setterObject() == nullptr) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_SET_WO_SETTER);
             return false;
         }
     }
 
     // step 15
     return result.succeed();
 }
--- a/js/src/proxy/ScriptedIndirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedIndirectProxyHandler.cpp
@@ -348,17 +348,17 @@ ScriptedIndirectProxyHandler::derivedSet
     }
 
     MOZ_ASSERT_IF(descIsOwn, desc.object());
     if (desc.object()) {
         MOZ_ASSERT(desc.getter() != JS_PropertyStub);
         MOZ_ASSERT(desc.setter() != JS_StrictPropertyStub);
 
         // Check for read-only properties.
-        if (desc.isReadonly())
+        if (desc.isDataDescriptor() && !desc.writable())
             return result.fail(descIsOwn ? JSMSG_READ_ONLY : JSMSG_CANT_REDEFINE_PROP);
 
         if (desc.hasSetterObject() || desc.setter()) {
             if (!CallSetter(cx, receiver, id, desc.setter(), desc.attributes(), vp, result))
                 return false;
             if (!result)
                 return true;
             if (!proxy->is<ProxyObject>() ||
--- a/js/src/vm/Interpreter-inl.h
+++ b/js/src/vm/Interpreter-inl.h
@@ -361,20 +361,21 @@ DefVarOrConstOperation(JSContext *cx, Ha
          */
         RootedId id(cx, NameToId(dn));
         Rooted<PropertyDescriptor> desc(cx);
         if (!GetOwnPropertyDescriptor(cx, obj2, id, &desc))
             return false;
 
         JSAutoByteString bytes;
         if (AtomToPrintableString(cx, dn, &bytes)) {
+            bool isConst = desc.hasWritable() && !desc.writable();
             JS_ALWAYS_FALSE(JS_ReportErrorFlagsAndNumber(cx, JSREPORT_ERROR,
                                                          GetErrorMessage,
                                                          nullptr, JSMSG_REDECLARED_VAR,
-                                                         desc.isReadonly() ? "const" : "var",
+                                                         isConst ? "const" : "var",
                                                          bytes.ptr()));
         }
         return false;
     }
 
     return true;
 }
 
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -435,17 +435,17 @@ sandbox_addProperty(JSContext *cx, Handl
     // However, in the case of |const x = 3|, we get called once for
     // JSOP_DEFCONST and once for JSOP_SETCONST. The first one creates the
     // property as readonly and configurable. The second one changes the
     // attributes to readonly and not configurable. If we use JS_SetPropertyById
     // for the second call, it will throw an exception because the property is
     // readonly. We have to use JS_CopyPropertyFrom since it ignores the
     // readonly attribute (as it calls JSObject::defineProperty). See bug
     // 1019181.
-    if (pd.object() && pd.isPermanent()) {
+    if (pd.object() && !pd.configurable()) {
         if (!JS_SetPropertyById(cx, proto, id, vp))
             return false;
     } else {
         if (!JS_CopyPropertyFrom(cx, id, unwrappedProto, obj,
                                  MakeNonConfigurableIntoConfigurable))
             return false;
     }
 
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -356,17 +356,17 @@ ExposedPropertiesOnly::check(JSContext *
         return false;
     }
 
     Access access = NO_ACCESS;
 
     if (!JS_GetPropertyDescriptorById(cx, hallpass, id, &desc)) {
         return false; // Error
     }
-    if (!desc.object() || !desc.isEnumerable())
+    if (!desc.object() || !desc.enumerable())
         return false;
 
     if (!desc.value().isString()) {
         EnterAndThrow(cx, wrapper, "property must be a string");
         return false;
     }
 
     JSFlatString *flat = JS_FlattenString(cx, desc.value().toString());
--- a/js/xpconnect/wrappers/AddonWrapper.cpp
+++ b/js/xpconnect/wrappers/AddonWrapper.cpp
@@ -127,19 +127,19 @@ AddonWrapper<Base>::set(JSContext *cx, J
     if (!Interpose(cx, wrapper, nullptr, id, &desc))
         return false;
 
     if (!desc.object())
         return Base::set(cx, wrapper, receiver, id, vp, result);
 
     if (desc.setter()) {
         MOZ_ASSERT(desc.hasSetterObject());
-        MOZ_ASSERT(!desc.isReadonly());
         JS::AutoValueVector args(cx);
-        args.append(vp);
+        if (!args.append(vp))
+            return false;
         RootedValue fval(cx, ObjectValue(*desc.setterObject()));
         if (!JS_CallFunctionValue(cx, receiver, fval, args, vp))
             return false;
         return result.succeed();
     }
 
     return result.failCantSetInterposed();
 }
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1974,28 +1974,28 @@ 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()) {
+    if (existing_desc.object() == wrapper && !existing_desc.configurable()) {
         // 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()))
+        if (existing_desc.isAccessorDescriptor() || desc.isAccessorDescriptor() ||
+            (desc.hasEnumerable() && existing_desc.enumerable() != desc.enumerable()) ||
+            (desc.hasWritable() && !existing_desc.writable() && desc.writable()))
         {
             // We should technically report non-configurability in strict mode, but
             // doing that via JSAPI used to be a lot of trouble. See bug 1135997.
             return result.succeed();
         }
-        if (existing_desc.isReadonly()) {
+        if (!existing_desc.writable()) {
             // Same as the above for non-writability.
             return result.succeed();
         }
     }
 
     bool defined = false;
     if (!Traits::singleton.defineProperty(cx, wrapper, id, desc, existing_desc, result, &defined))
         return false;