Bug 1469217 part 6 - Remove JSPROP_SHADOWABLE, address review comments. r=anba
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 21 Jun 2018 11:05:42 +0200
changeset 477488 422829b56d7fe2bcd0e1d0e2bcc74c119c6a0f6c
parent 477487 4db8094fadc1b5577ab3668e1f49875af9f758f6
child 477489 98710e23e09bb5445c7668fc69d9d6e1620b2a5b
push id9385
push userdluca@mozilla.com
push dateFri, 22 Jun 2018 15:47:18 +0000
treeherdermozilla-beta@82a9a1027e2b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1469217
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 1469217 part 6 - Remove JSPROP_SHADOWABLE, address review comments. r=anba
js/src/builtin/Array.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/vm/ArgumentsObject.cpp
js/src/vm/Debugger.cpp
js/src/vm/JSObject.cpp
js/src/vm/JSObject.h
js/src/vm/NativeObject.cpp
js/src/vm/NativeObject.h
js/src/vm/Shape.h
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/js/src/builtin/Array.cpp
+++ b/js/src/builtin/Array.cpp
@@ -1022,17 +1022,17 @@ AddLengthProperty(JSContext* cx, HandleA
      * as accesses to 'length' will use the elements header.
      */
 
     RootedId lengthId(cx, NameToId(cx->names().length));
     MOZ_ASSERT(!obj->lookup(cx, lengthId));
 
     return NativeObject::addAccessorProperty(cx, obj, lengthId,
                                              array_length_getter, array_length_setter,
-                                             JSPROP_PERMANENT | JSPROP_SHADOWABLE);
+                                             JSPROP_PERMANENT);
 }
 
 static bool
 IsArrayConstructor(const JSObject* obj)
 {
     // This must only return true if v is *the* Array constructor for the
     // current compartment; we rely on the fact that any other Array
     // constructor would be represented as a wrapper.
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -2246,32 +2246,16 @@ JS_PUBLIC_API(bool)
 JS_DefinePropertyById(JSContext* cx, HandleObject obj, HandleId id, double valueArg,
                       unsigned attrs)
 {
     Value value = NumberValue(valueArg);
     return DefineDataPropertyById(cx, obj, id, HandleValue::fromMarkedLocation(&value), attrs);
 }
 
 static bool
-DefineAccessorProperty(JSContext* cx, HandleObject obj, const char* name,
-                       const JSNativeWrapper& getter, const JSNativeWrapper& setter,
-                       unsigned attrs)
-{
-    AutoRooterGetterSetter gsRoot(cx, attrs, const_cast<JSNative*>(&getter.op),
-                                  const_cast<JSNative*>(&setter.op));
-
-    JSAtom* atom = Atomize(cx, name, strlen(name));
-    if (!atom)
-        return false;
-    RootedId id(cx, AtomToId(atom));
-
-    return DefineAccessorPropertyById(cx, obj, id, getter, setter, attrs);
-}
-
-static bool
 DefineDataProperty(JSContext* cx, HandleObject obj, const char* name, HandleValue value,
                    unsigned attrs)
 {
     JSAtom* atom = Atomize(cx, name, strlen(name));
     if (!atom)
         return false;
     RootedId id(cx, AtomToId(atom));
 
@@ -2284,18 +2268,22 @@ JS_DefineProperty(JSContext* cx, HandleO
 {
     return DefineDataProperty(cx, obj, name, value, attrs);
 }
 
 JS_PUBLIC_API(bool)
 JS_DefineProperty(JSContext* cx, HandleObject obj, const char* name, Native getter, Native setter,
                   unsigned attrs)
 {
-    return DefineAccessorProperty(cx, obj, name, NativeOpWrapper(getter), NativeOpWrapper(setter),
-                                  attrs);
+    JSAtom* atom = Atomize(cx, name, strlen(name));
+    if (!atom)
+        return false;
+    RootedId id(cx, AtomToId(atom));
+    return DefineAccessorPropertyById(cx, obj, id, NativeOpWrapper(getter),
+                                      NativeOpWrapper(setter), attrs);
 }
 
 JS_PUBLIC_API(bool)
 JS_DefineProperty(JSContext* cx, HandleObject obj, const char* name, HandleObject getter,
                   HandleObject setter, unsigned attrs)
 {
     JSAtom* atom = Atomize(cx, name, strlen(name));
     if (!atom)
@@ -2463,19 +2451,16 @@ JS_DefineElement(JSContext* cx, HandleOb
 {
     return ::DefineDataElement(cx, obj, index, value, attrs);
 }
 
 JS_PUBLIC_API(bool)
 JS_DefineElement(JSContext* cx, HandleObject obj, uint32_t index, HandleObject getter,
                  HandleObject setter, unsigned attrs)
 {
-    assertSameCompartment(cx, obj, getter, setter);
-    AssertHeapIsIdle();
-    CHECK_REQUEST(cx);
     RootedId id(cx);
     if (!IndexToId(cx, index, &id))
         return false;
     return DefineAccessorPropertyById(cx, obj, id, getter, setter, attrs);
 }
 
 JS_PUBLIC_API(bool)
 JS_DefineElement(JSContext* cx, HandleObject obj, uint32_t index, HandleObject valueArg,
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2011,19 +2011,16 @@ class WrappedPtrOperations<JS::PropertyD
     bool hasAny(unsigned bits) const {
         return (desc().attrs & bits) != 0;
     }
 
     bool hasAll(unsigned bits) const {
         return (desc().attrs & bits) == bits;
     }
 
-    // Non-API attributes bit used internally for arguments objects.
-    enum { SHADOWABLE = JSPROP_INTERNAL_USE_BIT };
-
   public:
     // Descriptors with JSGetterOp/JSSetterOp are considered data
     // descriptors. It's complicated.
     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);
@@ -2070,24 +2067,24 @@ class WrappedPtrOperations<JS::PropertyD
 #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_RESOLVING |
-                                     SHADOWABLE)) == 0);
+                                     JSPROP_INTERNAL_USE_BIT)) == 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));
-            MOZ_ASSERT(!has(SHADOWABLE));
+            MOZ_ASSERT(!has(JSPROP_INTERNAL_USE_BIT));
             MOZ_ASSERT(value().isUndefined());
             MOZ_ASSERT_IF(!has(JSPROP_GETTER), !getter());
             MOZ_ASSERT_IF(!has(JSPROP_SETTER), !setter());
         } else {
             MOZ_ASSERT(!hasAll(JSPROP_IGNORE_READONLY | JSPROP_READONLY));
             MOZ_ASSERT_IF(has(JSPROP_IGNORE_VALUE), value().isUndefined());
         }
 
@@ -2102,17 +2099,17 @@ class WrappedPtrOperations<JS::PropertyD
 #ifdef DEBUG
         assertValid();
         MOZ_ASSERT((attributes() & ~(JSPROP_ENUMERATE |
                                      JSPROP_PERMANENT |
                                      JSPROP_READONLY |
                                      JSPROP_GETTER |
                                      JSPROP_SETTER |
                                      JSPROP_RESOLVING |
-                                     SHADOWABLE)) == 0);
+                                     JSPROP_INTERNAL_USE_BIT)) == 0);
         MOZ_ASSERT_IF(isAccessorDescriptor(), has(JSPROP_GETTER) && has(JSPROP_SETTER));
 #endif
     }
 
     void assertCompleteIfFound() const {
 #ifdef DEBUG
         if (object())
             assertComplete();
--- a/js/src/vm/ArgumentsObject.cpp
+++ b/js/src/vm/ArgumentsObject.cpp
@@ -571,17 +571,17 @@ MappedArgumentsObject::obj_resolve(JSCon
             return true;
 
         if (!DefineArgumentsIterator(cx, argsobj))
             return false;
         *resolvedp = true;
         return true;
     }
 
-    unsigned attrs = JSPROP_SHADOWABLE | JSPROP_RESOLVING;
+    unsigned attrs = JSPROP_RESOLVING;
     if (JSID_IS_INT(id)) {
         uint32_t arg = uint32_t(JSID_TO_INT(id));
         if (arg >= argsobj->initialLength() || argsobj->isElementDeleted(arg))
             return true;
 
         attrs |= JSPROP_ENUMERATE;
     } else if (JSID_IS_ATOM(id, cx->names().length)) {
         if (argsobj->hasOverriddenLength())
@@ -771,45 +771,48 @@ UnmappedArgumentsObject::obj_resolve(JSC
             return true;
 
         if (!DefineArgumentsIterator(cx, argsobj))
             return false;
         *resolvedp = true;
         return true;
     }
 
-    unsigned attrs = JSPROP_SHADOWABLE;
-    GetterOp getter = UnmappedArgGetter;
-    SetterOp setter = UnmappedArgSetter;
+    if (JSID_IS_ATOM(id, cx->names().callee)) {
+        RootedObject throwTypeError(cx, GlobalObject::getOrCreateThrowTypeError(cx, cx->global()));
+        if (!throwTypeError)
+            return false;
 
+        unsigned attrs = JSPROP_RESOLVING | JSPROP_PERMANENT | JSPROP_GETTER | JSPROP_SETTER;
+        if (!NativeDefineAccessorProperty(cx, argsobj, id, throwTypeError, throwTypeError, attrs))
+            return false;
+
+        *resolvedp = true;
+        return true;
+    }
+
+    unsigned attrs = JSPROP_RESOLVING;
     if (JSID_IS_INT(id)) {
         uint32_t arg = uint32_t(JSID_TO_INT(id));
         if (arg >= argsobj->initialLength() || argsobj->isElementDeleted(arg))
             return true;
 
         attrs |= JSPROP_ENUMERATE;
     } else if (JSID_IS_ATOM(id, cx->names().length)) {
         if (argsobj->hasOverriddenLength())
             return true;
     } else {
-        if (!JSID_IS_ATOM(id, cx->names().callee))
-            return true;
-
-        JSObject* throwTypeError = GlobalObject::getOrCreateThrowTypeError(cx, cx->global());
-        if (!throwTypeError)
-            return false;
-
-        attrs = JSPROP_PERMANENT | JSPROP_GETTER | JSPROP_SETTER;
-        getter = CastAsGetterOp(throwTypeError);
-        setter = CastAsSetterOp(throwTypeError);
+        return true;
     }
 
-    attrs |= JSPROP_RESOLVING;
-    if (!NativeDefineAccessorProperty(cx, argsobj, id, getter, setter, attrs))
+    if (!NativeDefineAccessorProperty(cx, argsobj, id, UnmappedArgGetter, UnmappedArgSetter,
+                                      attrs))
+    {
         return false;
+    }
 
     *resolvedp = true;
     return true;
 }
 
 /* static */ bool
 UnmappedArgumentsObject::obj_enumerate(JSContext* cx, HandleObject obj)
 {
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -8426,18 +8426,17 @@ DebuggerArguments::create(JSContext* cx,
     for (unsigned i = 0; i < fargc; i++) {
         RootedFunction getobj(cx);
         getobj = NewNativeFunction(cx, DebuggerArguments_getArg, 0, nullptr,
                                    gc::AllocKind::FUNCTION_EXTENDED);
         if (!getobj)
             return nullptr;
         id = INT_TO_JSID(i);
         if (!getobj ||
-            !NativeDefineAccessorProperty(cx, obj, id,
-                                          JS_DATA_TO_FUNC_PTR(GetterOp, getobj.get()), nullptr,
+            !NativeDefineAccessorProperty(cx, obj, id, getobj, nullptr,
                                           JSPROP_ENUMERATE | JSPROP_GETTER))
         {
             return nullptr;
         }
         getobj->setExtendedSlot(0, Int32Value(i));
     }
 
     return obj;
--- a/js/src/vm/JSObject.cpp
+++ b/js/src/vm/JSObject.cpp
@@ -2804,18 +2804,18 @@ js::DefineProperty(JSContext* cx, Handle
 bool
 js::DefineAccessorProperty(JSContext* cx, HandleObject obj, HandleId id,
                            HandleObject getter, HandleObject setter, unsigned attrs,
                            ObjectOpResult& result)
 {
     Rooted<PropertyDescriptor> desc(cx);
 
     {
-        JSGetterOp getterOp = JS_DATA_TO_FUNC_PTR(GetterOp, getter.get());
-        JSSetterOp setterOp = JS_DATA_TO_FUNC_PTR(SetterOp, setter.get());
+        GetterOp getterOp = JS_DATA_TO_FUNC_PTR(GetterOp, getter.get());
+        SetterOp setterOp = JS_DATA_TO_FUNC_PTR(SetterOp, setter.get());
         desc.initFields(nullptr, UndefinedHandleValue, attrs, getterOp, setterOp);
     }
 
     if (DefinePropertyOp op = obj->getOpsDefineProperty()) {
         MOZ_ASSERT(!cx->helperThread());
         return op(cx, obj, id, desc, result);
     }
     return NativeDefineProperty(cx, obj.as<NativeObject>(), id, desc, result);
--- a/js/src/vm/JSObject.h
+++ b/js/src/vm/JSObject.h
@@ -1072,28 +1072,16 @@ extern JSObject*
 CreateThis(JSContext* cx, const js::Class* clasp, js::HandleObject callee);
 
 extern JSObject*
 CloneObject(JSContext* cx, HandleObject obj, Handle<js::TaggedProto> proto);
 
 extern JSObject*
 DeepCloneObjectLiteral(JSContext* cx, HandleObject obj, NewObjectKind newKind = GenericObject);
 
-inline JSGetterOp
-CastAsGetterOp(JSObject* object)
-{
-    return JS_DATA_TO_FUNC_PTR(JSGetterOp, object);
-}
-
-inline JSSetterOp
-CastAsSetterOp(JSObject* object)
-{
-    return JS_DATA_TO_FUNC_PTR(JSSetterOp, object);
-}
-
 /* ES6 draft rev 32 (2015 Feb 2) 6.2.4.5 ToPropertyDescriptor(Obj) */
 bool
 ToPropertyDescriptor(JSContext* cx, HandleValue descval, bool checkAccessors,
                      MutableHandle<JS::PropertyDescriptor> desc);
 
 /*
  * Throw a TypeError if desc.getterObject() or setterObject() is not
  * callable. This performs exactly the checks omitted by ToPropertyDescriptor
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -1819,49 +1819,71 @@ js::NativeDefineProperty(JSContext* cx, 
     if (!AddOrChangeProperty<IsAddOrChange::AddOrChange>(cx, obj, id, desc))
         return false;
 
     // Step 10.
     return result.succeed();
 }
 
 bool
-js::NativeDefineAccessorProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
-                                 GetterOp getter, SetterOp setter, unsigned attrs,
-                                 ObjectOpResult& result)
-{
-    Rooted<PropertyDescriptor> desc(cx);
-    desc.initFields(nullptr, UndefinedHandleValue, attrs, getter, setter);
-    return NativeDefineProperty(cx, obj, id, desc, result);
-}
-
-bool
 js::NativeDefineDataProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
                              HandleValue value, unsigned attrs, ObjectOpResult& result)
 {
     Rooted<PropertyDescriptor> desc(cx);
     desc.initFields(nullptr, value, attrs, nullptr, nullptr);
     return NativeDefineProperty(cx, obj, id, desc, result);
 }
 
 bool
 js::NativeDefineAccessorProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
-                                 JSGetterOp getter, JSSetterOp setter, unsigned attrs)
+                                 GetterOp getter, SetterOp setter, unsigned attrs)
 {
+    Rooted<PropertyDescriptor> desc(cx);
+    desc.initFields(nullptr, UndefinedHandleValue, attrs, getter, setter);
+
     ObjectOpResult result;
-    if (!NativeDefineAccessorProperty(cx, obj, id, getter, setter, attrs, result))
+    if (!NativeDefineProperty(cx, obj, id, desc, result))
         return false;
+
     if (!result) {
         // Off-thread callers should not get here: they must call this
         // function only with known-valid arguments. Populating a new
         // PlainObject with configurable properties is fine.
         MOZ_ASSERT(!cx->helperThread());
         result.reportError(cx, obj, id);
         return false;
     }
+
+    return true;
+}
+
+bool
+js::NativeDefineAccessorProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
+                                 HandleObject getter, HandleObject setter, unsigned attrs)
+{
+    Rooted<PropertyDescriptor> desc(cx);
+    {
+        GetterOp getterOp = JS_DATA_TO_FUNC_PTR(GetterOp, getter.get());
+        SetterOp setterOp = JS_DATA_TO_FUNC_PTR(SetterOp, setter.get());
+        desc.initFields(nullptr, UndefinedHandleValue, attrs, getterOp, setterOp);
+    }
+
+    ObjectOpResult result;
+    if (!NativeDefineProperty(cx, obj, id, desc, result))
+        return false;
+
+    if (!result) {
+        // Off-thread callers should not get here: they must call this
+        // function only with known-valid arguments. Populating a new
+        // PlainObject with configurable properties is fine.
+        MOZ_ASSERT(!cx->helperThread());
+        result.reportError(cx, obj, id);
+        return false;
+    }
+
     return true;
 }
 
 bool
 js::NativeDefineDataProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
                              HandleValue value, unsigned attrs)
 {
     ObjectOpResult result;
@@ -1874,24 +1896,16 @@ js::NativeDefineDataProperty(JSContext* 
         MOZ_ASSERT(!cx->helperThread());
         result.reportError(cx, obj, id);
         return false;
     }
     return true;
 }
 
 bool
-js::NativeDefineAccessorProperty(JSContext* cx, HandleNativeObject obj, PropertyName* name,
-                                 JSGetterOp getter, JSSetterOp setter, unsigned attrs)
-{
-    RootedId id(cx, NameToId(name));
-    return NativeDefineAccessorProperty(cx, obj, id, getter, setter, attrs);
-}
-
-bool
 js::NativeDefineDataProperty(JSContext* cx, HandleNativeObject obj, PropertyName* name,
                              HandleValue value, unsigned attrs)
 {
     RootedId id(cx, NameToId(name));
     return NativeDefineDataProperty(cx, obj, id, value, attrs);
 }
 
 static bool
@@ -2689,28 +2703,16 @@ SetExistingProperty(JSContext* cx, Handl
             // Steps 5.e.i-ii.
             if (pobj->is<ArrayObject>() && id == NameToId(cx->names().length)) {
                 Rooted<ArrayObject*> arr(cx, &pobj->as<ArrayObject>());
                 return ArraySetLength(cx, arr, id, shape->attributes(), v, result);
             }
             return NativeSetExistingDataProperty(cx, pobj, shape, v, result);
         }
 
-        // SpiderMonkey special case: assigning to an inherited slotless
-        // property causes the setter to be called, instead of shadowing,
-        // unless the existing property is JSPROP_SHADOWABLE (see bug 552432).
-        if (!shape->isDataProperty() && !shape->hasShadowable()) {
-            // Even weirder sub-special-case: inherited slotless data property
-            // with default setter. Wut.
-            if (shape->hasDefaultSetter())
-                return result.succeed();
-
-            return CallJSSetterOp(cx, shape->setterOp(), obj, id, v, result);
-        }
-
         // Shadow pobj[id] by defining a new data property receiver[id].
         // Delegate everything to SetPropertyByDefining.
         return SetPropertyByDefining(cx, id, v, receiver, result);
     }
 
     // Steps 6-11.
     MOZ_ASSERT(shape->isAccessorDescriptor());
     MOZ_ASSERT_IF(!shape->hasSetterObject(), shape->hasDefaultSetter());
--- a/js/src/vm/NativeObject.h
+++ b/js/src/vm/NativeObject.h
@@ -1524,38 +1524,33 @@ NativeObject::privateWriteBarrierPre(voi
  */
 
 extern bool
 NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
                      Handle<JS::PropertyDescriptor> desc,
                      ObjectOpResult& result);
 
 extern bool
-NativeDefineAccessorProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
-                             JSGetterOp getter, JSSetterOp setter, unsigned attrs,
-                             ObjectOpResult& result);
-
-extern bool
 NativeDefineDataProperty(JSContext* cx, HandleNativeObject obj, HandleId id, HandleValue value,
                          unsigned attrs, ObjectOpResult& result);
 
 /* If the result out-param is omitted, throw on failure. */
 extern bool
 NativeDefineAccessorProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
-                             JSGetterOp getter, JSSetterOp setter, unsigned attrs);
+                             GetterOp getter, SetterOp setter, unsigned attrs);
+
+extern bool
+NativeDefineAccessorProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
+                             HandleObject getter, HandleObject setter, unsigned attrs);
 
 extern bool
 NativeDefineDataProperty(JSContext* cx, HandleNativeObject obj, HandleId id, HandleValue value,
                          unsigned attrs);
 
 extern bool
-NativeDefineAccessorProperty(JSContext* cx, HandleNativeObject obj, PropertyName* name,
-                             JSGetterOp getter, JSSetterOp setter, unsigned attrs);
-
-extern bool
 NativeDefineDataProperty(JSContext* cx, HandleNativeObject obj, PropertyName* name,
                          HandleValue value, unsigned attrs);
 
 extern bool
 NativeHasProperty(JSContext* cx, HandleNativeObject obj, HandleId id, bool* foundp);
 
 extern bool
 NativeGetOwnPropertyDescriptor(JSContext* cx, HandleNativeObject obj, HandleId id,
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -383,21 +383,16 @@ class MOZ_RAII AutoKeepShapeTables
     void operator=(const AutoKeepShapeTables&) = delete;
 
   public:
     explicit inline AutoKeepShapeTables(JSContext* cx);
     inline ~AutoKeepShapeTables();
 };
 
 /*
- * Use the reserved attribute bit to mean shadowability.
- */
-#define JSPROP_SHADOWABLE       JSPROP_INTERNAL_USE_BIT
-
-/*
  * Shapes encode information about both a property lineage *and* a particular
  * property. This information is split across the Shape and the BaseShape
  * at shape->base(). Both Shape and BaseShape can be either owned or unowned
  * by, respectively, the Object or Shape referring to them.
  *
  * Owned Shapes are used in dictionary objects, and form a doubly linked list
  * whose entries are all owned by that dictionary. Unowned Shapes are all in
  * the property tree.
@@ -1073,18 +1068,16 @@ class Shape : public gc::TenuredCell
 
     bool isDataDescriptor() const {
         return (attrs & (JSPROP_SETTER | JSPROP_GETTER)) == 0;
     }
     bool isAccessorDescriptor() const {
         return (attrs & (JSPROP_SETTER | JSPROP_GETTER)) != 0;
     }
 
-    bool hasShadowable() const { return attrs & JSPROP_SHADOWABLE; }
-
     uint32_t entryCount() {
         JS::AutoCheckCannotGC nogc;
         if (ShapeTable* table = maybeTable(nogc))
             return table->entryCount();
         uint32_t count = 0;
         for (Shape::Range<NoGC> r(this); !r.empty(); r.popFront())
             ++count;
         return count;
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -416,23 +416,22 @@ TryResolvePropertyFromSpecs(JSContext* c
         // JS_GetPropertyById at the top of JSXrayTraits::resolveOwnProperty.
         //
         // Note also that the public-facing API here doesn't give us a way to
         // pass along JITInfo. It's probably ok though, since Xrays are already
         // pretty slow.
         desc.value().setUndefined();
         unsigned flags = psMatch->flags;
         if (psMatch->isAccessor()) {
-            RootedObject getterObj(cx);
-            RootedObject setterObj(cx);
             if (psMatch->isSelfHosted()) {
                 JSFunction* getterFun = JS::GetSelfHostedFunction(cx, psMatch->accessors.getter.selfHosted.funname, id, 0);
                 if (!getterFun)
                     return false;
-                getterObj = JS_GetFunctionObject(getterFun);
+                RootedObject getterObj(cx, JS_GetFunctionObject(getterFun));
+                RootedObject setterObj(cx);
                 if (psMatch->accessors.setter.selfHosted.funname) {
                     MOZ_ASSERT(flags & JSPROP_SETTER);
                     JSFunction* setterFun = JS::GetSelfHostedFunction(cx, psMatch->accessors.setter.selfHosted.funname, id, 0);
                     if (!setterFun)
                         return false;
                     setterObj = JS_GetFunctionObject(setterFun);
                 }
                 if (!JS_DefinePropertyById(cx, holder, id, getterObj, setterObj, flags))