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 233524 c78a9d1273c5d4ddf7a804eab41a92ff90ce0ec5
parent 233523 63dbcc4fd0f02324f31e55c69409d61bc10116c0
child 233525 35ad2e5b036b9d141a0bbf71571b7b4936e23b7e
push id28417
push userryanvm@gmail.com
push dateFri, 13 Mar 2015 19:52:44 +0000
treeherdermozilla-central@977add19414a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1133081
milestone39.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 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;