Bug 1082672, part 4 - Change XrayWrapper code to be able to resolve symbol-keyed methods. r=bz, r=bholley.
authorJason Orendorff <jorendorff@mozilla.com>
Thu, 18 Sep 2014 12:30:38 -0500
changeset 210828 9a7fd8fd00b3245271a3b09d0399176efd5e770e
parent 210827 f2214b9e3333a6ff9f588a2bdade024c6e87d074
child 210829 dc2d310c063c22c3f24a389d17f4af4b48e30b40
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbz, bholley
bugs1082672
milestone36.0a1
Bug 1082672, part 4 - Change XrayWrapper code to be able to resolve symbol-keyed methods. r=bz, r=bholley.
dom/bindings/BindingUtils.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/vm/Runtime.h
js/xpconnect/tests/chrome/test_xrayToJS.xul
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -1401,18 +1401,21 @@ XrayAttributeOrMethodKeys(JSContext* cx,
                           unsigned flags, JS::AutoIdVector& props)
 {
   for (; list->specs; ++list) {
     if (list->isEnabled(cx, obj)) {
       // Set i to be the index into our full list of ids/specs that we're
       // looking at now.
       size_t i = list->specs - specList;
       for ( ; ids[i] != JSID_VOID; ++i) {
+        // Skip non-enumerable properties and symbol-keyed properties unless
+        // they are specially requested via flags.
         if (((flags & JSITER_HIDDEN) ||
              (specList[i].flags & JSPROP_ENUMERATE)) &&
+            ((flags & JSITER_SYMBOLS) || !JSID_IS_SYMBOL(ids[i])) &&
             !props.append(ids[i])) {
           return false;
         }
       }
     }
   }
   return true;
 }
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3239,23 +3239,31 @@ JS_DefineConstDoubles(JSContext *cx, Han
     return DefineConstScalar(cx, obj, cds);
 }
 JS_PUBLIC_API(bool)
 JS_DefineConstIntegers(JSContext *cx, HandleObject obj, const JSConstIntegerSpec *cis)
 {
     return DefineConstScalar(cx, obj, cis);
 }
 
+static JS::SymbolCode
+PropertySpecNameToSymbolCode(const char *name)
+{
+    MOZ_ASSERT(JS::PropertySpecNameIsSymbol(name));
+    uintptr_t u = reinterpret_cast<uintptr_t>(name);
+    return JS::SymbolCode(u - 1);
+}
+
 static bool
 PropertySpecNameToId(JSContext *cx, const char *name, MutableHandleId id,
                      js::InternBehavior ib = js::DoNotInternAtom)
 {
     if (JS::PropertySpecNameIsSymbol(name)) {
-        uintptr_t u = reinterpret_cast<uintptr_t>(name);
-        id.set(SYMBOL_TO_JSID(cx->wellKnownSymbols().get(u - 1)));
+        JS::SymbolCode which = PropertySpecNameToSymbolCode(name);
+        id.set(SYMBOL_TO_JSID(cx->wellKnownSymbols().get(which)));
     } else {
         JSAtom *atom = Atomize(cx, name, strlen(name), ib);
         if (!atom)
             return false;
         id.set(AtomToId(atom));
     }
     return true;
 }
@@ -5511,16 +5519,43 @@ JS::GetSymbolCode(Handle<Symbol*> symbol
 }
 
 JS_PUBLIC_API(JS::Symbol *)
 JS::GetWellKnownSymbol(JSContext *cx, JS::SymbolCode which)
 {
     return cx->runtime()->wellKnownSymbols->get(uint32_t(which));
 }
 
+static bool
+PropertySpecNameIsDigits(const char *s) {
+    if (JS::PropertySpecNameIsSymbol(s))
+        return false;
+    if (!*s)
+        return false;
+    for (; *s; s++) {
+        if (*s < '0' || *s > '9')
+            return false;
+    }
+    return true;
+}
+
+JS_PUBLIC_API(bool)
+JS::PropertySpecNameEqualsId(const char *name, HandleId id)
+{
+    if (JS::PropertySpecNameIsSymbol(name)) {
+        if (!JSID_IS_SYMBOL(id))
+            return false;
+        Symbol *sym = JSID_TO_SYMBOL(id);
+        return sym->isWellKnownSymbol() && sym->code() == PropertySpecNameToSymbolCode(name);
+    }
+
+    MOZ_ASSERT(!PropertySpecNameIsDigits(name));
+    return JSID_IS_ATOM(id) && JS_FlatStringEqualsAscii(JSID_TO_ATOM(id), name);
+}
+
 JS_PUBLIC_API(bool)
 JS_Stringify(JSContext *cx, MutableHandleValue vp, HandleObject replacer,
              HandleValue space, JSONWriteCallback callback, void *data)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, replacer, space);
     StringBuffer sb(cx);
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -4512,16 +4512,19 @@ GetWellKnownSymbol(JSContext *cx, Symbol
  */
 inline bool
 PropertySpecNameIsSymbol(const char *name)
 {
     uintptr_t u = reinterpret_cast<uintptr_t>(name);
     return u != 0 && u - 1 < WellKnownSymbolLimit;
 }
 
+JS_PUBLIC_API(bool)
+PropertySpecNameEqualsId(const char *name, HandleId id);
+
 /*
  * Create a jsid that does not need to be marked for GC.
  *
  * 'name' is a JSPropertySpec::name or JSFunctionSpec::name value. The
  * resulting jsid, on success, is either an interned string or a well-known
  * symbol; either way it is immune to GC so there is no need to visit *idp
  * during GC marking.
  */
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -418,20 +418,24 @@ namespace js {
  *
  * Well-known symbols are never GC'd. The description() of each well-known
  * symbol is a permanent atom.
  */
 struct WellKnownSymbols
 {
     js::ImmutableSymbolPtr iterator;
 
-    ImmutableSymbolPtr &get(size_t i) {
-        MOZ_ASSERT(i < JS::WellKnownSymbolLimit);
+    ImmutableSymbolPtr &get(size_t u) {
+        MOZ_ASSERT(u < JS::WellKnownSymbolLimit);
         ImmutableSymbolPtr *symbols = reinterpret_cast<ImmutableSymbolPtr *>(this);
-        return symbols[i];
+        return symbols[u];
+    }
+
+    ImmutableSymbolPtr &get(JS::SymbolCode code) {
+        return get(size_t(code));
     }
 };
 
 #define NAME_OFFSET(name)       offsetof(JSAtomState, name)
 
 inline HandlePropertyName
 AtomStateOffsetToName(const JSAtomState &atomState, size_t offset)
 {
--- a/js/xpconnect/tests/chrome/test_xrayToJS.xul
+++ b/js/xpconnect/tests/chrome/test_xrayToJS.xul
@@ -140,16 +140,17 @@ https://bugzilla.mozilla.org/show_bug.cg
   // Maintain a static list of the properties that are available on each standard
   // prototype, so that we make sure to audit any new ones to make sure they're
   // Xray-safe.
   //
   // DO NOT CHANGE WTIHOUT REVIEW FROM AN XPCONNECT PEER.
   var version = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULAppInfo).version;
   var isNightlyBuild = version.endsWith("a1");
   var isReleaseBuild = !version.contains("a");
+  const jsHasSymbols = typeof Symbol === "function";
   var gPrototypeProperties = {};
   gPrototypeProperties['Date'] =
     ["getTime", "getTimezoneOffset", "getYear", "getFullYear", "getUTCFullYear",
     "getMonth", "getUTCMonth", "getDate", "getUTCDate", "getDay", "getUTCDay",
     "getHours", "getUTCHours", "getMinutes", "getUTCMinutes", "getSeconds",
     "getUTCSeconds", "getMilliseconds", "getUTCMilliseconds", "setTime",
     "setYear", "setFullYear", "setUTCFullYear", "setMonth", "setUTCMonth",
     "setDate", "setUTCDate", "setHours", "setUTCHours", "setMinutes",
@@ -192,29 +193,33 @@ https://bugzilla.mozilla.org/show_bug.cg
 
   gPrototypeProperties['Function'] =
     ["constructor", "toSource", "toString", "apply", "call", "bind",
      "isGenerator", "length", "name", "arguments", "caller"];
   gPrototypeProperties['Function'] =
     ["constructor", "toSource", "toString", "apply", "call", "bind",
      "isGenerator", "length", "name", "arguments", "caller"];
 
+  // Sort an array that may contain symbols as well as strings.
+  function sortProperties(arr) {
+    function sortKey(prop) {
+      return typeof prop + ":" + prop.toString();
+    }
+    arr.sort((a, b) => sortKey(a) < sortKey(b) ? -1 : +1);
+  }
+
   // Sort all the lists so we don't need to mutate them later (or copy them
   // again to sort them).
   for (var c of Object.keys(gPrototypeProperties))
-    gPrototypeProperties[c].sort();
+    sortProperties(gPrototypeProperties[c]);
 
   function filterOut(array, props) {
     return array.filter(p => props.indexOf(p) == -1);
   }
 
-  function appendUnique(array, vals) {
-    filterOut(vals, array).forEach(v => array.push(v));
-  }
-
   function isTypedArrayClass(classname) {
     return typedArrayClasses.indexOf(classname) >= 0;
   }
 
   function propertyIsGetter(obj, name, classname) {
     return !!Object.getOwnPropertyDescriptor(obj, name).get;
   }
 
@@ -262,29 +267,40 @@ https://bugzilla.mozilla.org/show_bug.cg
     let xrayProto = Object.getPrototypeOf(xray);
     let localProto = window[classname].prototype;
     let desiredProtoProps = Object.getOwnPropertyNames(localProto).sort();
 
     is(desiredProtoProps.toSource(),
        gPrototypeProperties[classname].filter(id => typeof id === "string").toSource(),
        "A property on the " + classname +
        " prototype has changed! You need a security audit from an XPConnect peer");
+    if (jsHasSymbols) {
+      is(Object.getOwnPropertySymbols(localProto).map(uneval).sort().toSource(),
+         gPrototypeProperties[classname].filter(id => typeof id !== "string").map(uneval).sort().toSource(),
+         "A symbol-keyed property on the " + classname +
+         " prototype has been changed! You need a security audit from an XPConnect peer");
+    }
 
     let protoProps = filterOut(desiredProtoProps, propsToSkip);
     let protoCallables = protoProps.filter(name => propertyIsGetter(localProto, name, classname) ||
                                                    typeof localProto[name] == 'function' &&
                                                    name != 'constructor');
     ok(protoCallables.length > 0, "Need something to test");
     is(xrayProto, iwin[classname].prototype, "Xray proto is correct");
     is(xrayProto, xray.__proto__, "Proto accessors agree");
     var protoProto = classname == "Object" ? null : iwin.Object.prototype;
     is(Object.getPrototypeOf(xrayProto), protoProto, "proto proto is correct");
     testProtoCallables(protoCallables, xray, xrayProto, localProto);
     is(Object.getOwnPropertyNames(xrayProto).sort().toSource(),
        protoProps.toSource(), "getOwnPropertyNames works");
+    if (jsHasSymbols) {
+      is(Object.getOwnPropertySymbols(xrayProto).map(uneval).sort().toSource(),
+         gPrototypeProperties[classname].filter(id => typeof id !== "string").map(uneval).sort().toSource(),
+         protoProps.toSource(), "getOwnPropertySymbols works");
+    }
 
     is(xrayProto.constructor, iwin[classname], "constructor property works");
 
     xrayProto.expando = 42;
     is(xray.expando, 42, "Xrayed instances see proto expandos");
     is(xray2.expando, 42, "Xrayed instances see proto expandos");
   }
 
@@ -298,57 +314,73 @@ https://bugzilla.mozilla.org/show_bug.cg
     // Test the self-hosted toLocaleString.
     var d = new iwin.Date();
     isnot(d.toLocaleString, Cu.unwaiveXrays(d.wrappedJSObject.toLocaleString), "Different function identities");
     is(Cu.getGlobalForObject(d.toLocaleString), window, "Xray global is correct");
     is(Cu.getGlobalForObject(d.wrappedJSObject.toLocaleString), iwin, "Underlying global is correct");
     is(d.toLocaleString('de-DE'), d.wrappedJSObject.toLocaleString('de-DE'), "Results match");
   }
 
+  var uniqueSymbol;
+
   function testObject() {
     testXray('Object', Cu.unwaiveXrays(Cu.waiveXrays(iwin).Object.create(new iwin.Object())),
              new iwin.Object(), []);
 
     // Construct an object full of tricky things.
+    let symbolProps = '';
+    if (jsHasSymbols) {
+      uniqueSymbol = iwin.eval('var uniqueSymbol = Symbol("uniqueSymbol"); uniqueSymbol');
+      symbolProps = `, [uniqueSymbol]: 43,
+                     [Symbol.for("registrySymbolProp")]: 44`;
+    }
     var trickyObject =
-      iwin.eval('new Object({ \
-                   primitiveProp: 42, objectProp: { foo: 2 }, \
-                   xoProp: top.location, hasOwnProperty: 10, \
-                   get getterProp() { return 2; }, \
-                   set setterProp(x) { }, \
-                   get getterSetterProp() { return 3; }, \
-                   set getterSetterProp(x) { }, \
-                   callableProp: function() { }, \
-                   nonXrayableProp: new WeakMap() })');
+      iwin.eval(`new Object({
+                   primitiveProp: 42, objectProp: { foo: 2 },
+                   xoProp: top.location, hasOwnProperty: 10,
+                   get getterProp() { return 2; },
+                   set setterProp(x) { },
+                   get getterSetterProp() { return 3; },
+                   set getterSetterProp(x) { },
+                   callableProp: function() { },
+                   nonXrayableProp: new WeakMap()
+                   ${symbolProps}
+                 })`);
     testTrickyObject(trickyObject);
-
   }
 
-function testArray() {
+  function testArray() {
     // The |length| property is generally very weird, especially with respect
     // to its behavior on the prototype. Array.prototype is actually an Array
     // instance, and therefore has a vestigial .length. But we don't want to
     // show that over Xrays, and generally want .length to just appear as an
     // |own| data property. So we add it to the ignore list here, and check it
     // separately.
     let propsToSkip = ['length'];
     testXray('Array', new iwin.Array(20), new iwin.Array(), propsToSkip);
 
+    let symbolProps = '';
+    if (jsHasSymbols) {
+      uniqueSymbol = iwin.eval('var uniqueSymbol = Symbol("uniqueSymbol"); uniqueSymbol');
+      symbolProps = `trickyArray[uniqueSymbol] = 43;
+                     trickyArray[Symbol.for("registrySymbolProp")] = 44;`;
+    }
     var trickyArray =
-      iwin.eval("var trickyArray = []; \
-                 trickyArray.primitiveProp = 42; \
-                 trickyArray.objectProp = { foo: 2 }; \
-                 trickyArray.xoProp = top.location; \
-                 trickyArray.hasOwnProperty = 10; \
-                 Object.defineProperty(trickyArray, 'getterProp', { get: function() { return 2; }}); \
-                 Object.defineProperty(trickyArray, 'setterProp', { set: function(x) {}}); \
-                 Object.defineProperty(trickyArray, 'getterSetterProp', { get: function() { return 3; }, set: function(x) {}}); \
-                 trickyArray.callableProp = function() {}; \
-                 trickyArray.nonXrayableProp = new WeakMap(); \
-                 trickyArray;");
+      iwin.eval(`var trickyArray = [];
+                 trickyArray.primitiveProp = 42;
+                 trickyArray.objectProp = { foo: 2 };
+                 trickyArray.xoProp = top.location;
+                 trickyArray.hasOwnProperty = 10;
+                 Object.defineProperty(trickyArray, 'getterProp', { get: function() { return 2; }});
+                 Object.defineProperty(trickyArray, 'setterProp', { set: function(x) {}});
+                 Object.defineProperty(trickyArray, 'getterSetterProp', { get: function() { return 3; }, set: function(x) {}});
+                 trickyArray.callableProp = function() {};
+                 trickyArray.nonXrayableProp = new WeakMap();
+                 ${symbolProps}
+                 trickyArray;`);
 
     // Test indexed access.
     trickyArray.wrappedJSObject[9] = "some indexed property";
     is(trickyArray[9], "some indexed property", "indexed properties work correctly over Xrays");
     is(trickyArray.length, 10, "Length works correctly over Xrays");
     checkThrows(function() { "use strict"; delete trickyArray.length; }, /config/, "Can't delete non-configurable 'length' property");
     delete trickyArray[9];
     is(trickyArray[9], undefined, "Delete works correctly over Xrays");
@@ -361,37 +393,47 @@ function testArray() {
     is(trickyArray.length, 0, "Setting length works over Xray");
     is(trickyArray[11], undefined, "Setting length truncates over Xray");
     Object.defineProperty(trickyArray, 'length', { configurable: false, enumerable: false, writable: false, value: 0 });
     trickyArray[1] = "hi";
     is(trickyArray.length, 0, "Length remains non-writable");
     is(trickyArray[1], undefined, "Frozen length forbids new properties");
 
     testTrickyObject(trickyArray);
-}
+  }
 
-// Parts of this function are kind of specific to testing Object, but we factor
-// it out so that we can re-use the trickyObject stuff on Arrays.
-function testTrickyObject(trickyObject) {
+  // Parts of this function are kind of specific to testing Object, but we factor
+  // it out so that we can re-use the trickyObject stuff on Arrays.
+  function testTrickyObject(trickyObject) {
 
     // Make sure it looks right under the hood.
     is(trickyObject.wrappedJSObject.getterProp, 2, "Underlying object has getter");
     is(Cu.unwaiveXrays(trickyObject.wrappedJSObject.xoProp), top.location, "Underlying object has xo property");
 
     // Test getOwnPropertyNames.
     var expectedNames = ['objectProp', 'primitiveProp'];
     if (trickyObject instanceof iwin.Array)
       expectedNames.push('length');
     is(Object.getOwnPropertyNames(trickyObject).sort().toSource(),
        expectedNames.sort().toSource(), "getOwnPropertyNames should be filtered correctly");
+    if (jsHasSymbols) {
+      var expectedSymbols = [Symbol.for("registrySymbolProp"), uniqueSymbol];
+      is(Object.getOwnPropertySymbols(trickyObject).map(uneval).sort().toSource(),
+         expectedSymbols.map(uneval).sort().toSource(),
+         "getOwnPropertySymbols should be filtered correctly");
+    }
 
     // Test that cloning uses the Xray view.
     var cloned = Cu.cloneInto(trickyObject, this);
     is(Object.getOwnPropertyNames(cloned).sort().toSource(),
        expectedNames.sort().toSource(), "structured clone should use the Xray view");
+    if (jsHasSymbols) {
+      is(Object.getOwnPropertySymbols(cloned).map(uneval).sort().toSource(),
+         "[]", "structured cloning doesn't clone symbol-keyed properties yet");
+    }
 
     // Test iteration and in-place modification. Beware of 'expando', which is the property
     // we placed on the xray proto.
     var propCount = 0;
     for (let prop in trickyObject) {
       if (prop == 'primitiveProp')
         trickyObject[prop] = trickyObject[prop] - 10;
       if (prop != 'expando')
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -453,31 +453,24 @@ JSXrayTraits::resolveOwnProperty(JSConte
     // Handle the 'name' property for error prototypes.
     if (IsErrorObjectKey(key) && id == GetRTIdByIndex(cx, XPCJSRuntime::IDX_NAME)) {
         RootedId className(cx);
         ProtoKeyToId(cx, key, &className);
         FillPropertyDescriptor(desc, wrapper, 0, UndefinedValue());
         return JS_IdToValue(cx, className, desc.value());
     }
 
-    // Compute the property name we're looking for. Indexed array properties
-    // are handled above. We'll handle well-known symbols when we start
-    // supporting Symbol.iterator in bug 918828.
-    if (!JSID_IS_STRING(id))
-        return true;
-    Rooted<JSFlatString*> str(cx, JSID_TO_FLAT_STRING(id));
-
     // Grab the JSClass. We require all Xrayable classes to have a ClassSpec.
     const js::Class *clasp = js::GetObjectClass(target);
     MOZ_ASSERT(clasp->spec.defined());
 
-    // Scan through the functions.
+    // Scan through the functions. Indexed array properties are handled above.
     const JSFunctionSpec *fsMatch = nullptr;
     for (const JSFunctionSpec *fs = clasp->spec.prototypeFunctions; fs && fs->name; ++fs) {
-        if (JS_FlatStringEqualsAscii(str, fs->name)) {
+        if (PropertySpecNameEqualsId(fs->name, id)) {
             fsMatch = fs;
             break;
         }
     }
     if (fsMatch) {
         // Generate an Xrayed version of the method.
         RootedFunction fun(cx);
         if (fsMatch->selfHostedName) {
@@ -496,17 +489,17 @@ JSXrayTraits::resolveOwnProperty(JSConte
         RootedObject funObj(cx, JS_GetFunctionObject(fun));
         return JS_DefinePropertyById(cx, holder, id, funObj, 0) &&
                JS_GetPropertyDescriptorById(cx, holder, id, desc);
     }
 
     // Scan through the properties.
     const JSPropertySpec *psMatch = nullptr;
     for (const JSPropertySpec *ps = clasp->spec.prototypeProperties; ps && ps->name; ++ps) {
-        if (JS_FlatStringEqualsAscii(str, ps->name)) {
+        if (PropertySpecNameEqualsId(ps->name, id)) {
             psMatch = ps;
             break;
         }
     }
     if (psMatch) {
         desc.value().setUndefined();
         // Note that this is also kind of an abuse of JSPROP_NATIVE_ACCESSORS.
         // See bug 992977.
@@ -627,16 +620,25 @@ JSXrayTraits::defineProperty(JSContext *
         }
         *defined = true;
         return true;
     }
 
     return true;
 }
 
+static bool
+MaybeAppend(jsid id, unsigned flags, AutoIdVector &props)
+{
+    MOZ_ASSERT(!(flags & JSITER_SYMBOLSONLY));
+    if (!(flags & JSITER_SYMBOLS) && JSID_IS_SYMBOL(id))
+        return true;
+    return props.append(id);
+}
+
 bool
 JSXrayTraits::enumerateNames(JSContext *cx, HandleObject wrapper, unsigned flags,
                              AutoIdVector &props)
 {
     RootedObject target(cx, getTargetObject(wrapper));
     RootedObject holder(cx, ensureHolder(cx, wrapper));
     if (!holder)
         return false;
@@ -706,36 +708,37 @@ JSXrayTraits::enumerateNames(JSContext *
     // For Error protoypes, add the 'name' property.
     if (IsErrorObjectKey(key) && !props.append(GetRTIdByIndex(cx, XPCJSRuntime::IDX_NAME)))
         return false;
 
     // Grab the JSClass. We require all Xrayable classes to have a ClassSpec.
     const js::Class *clasp = js::GetObjectClass(target);
     MOZ_ASSERT(clasp->spec.defined());
 
-    // Intern all the strings, and pass theme to the caller.
+    // Convert the method and property names to jsids and pass them to the caller.
     for (const JSFunctionSpec *fs = clasp->spec.prototypeFunctions; fs && fs->name; ++fs) {
-        RootedString str(cx, JS_InternString(cx, fs->name));
-        if (!str)
+        jsid id;
+        if (!PropertySpecNameToPermanentId(cx, fs->name, &id))
             return false;
-        if (!props.append(INTERNED_STRING_TO_JSID(cx, str)))
+        if (!MaybeAppend(id, flags, props))
             return false;
     }
     for (const JSPropertySpec *ps = clasp->spec.prototypeProperties; ps && ps->name; ++ps) {
         // We have code to Xray self-hosted accessors. But at present, there don't appear
         // to be any self-hosted accessors anywhere in SpiderMonkey, let alone in on an
         // Xrayable class, so we can't test it. Assert against it to make sure that we get
         // test coverage in test_XrayToJS.xul when the time comes.
         MOZ_ASSERT(ps->flags & JSPROP_NATIVE_ACCESSORS,
                    "Self-hosted accessor added to Xrayable class - ping the XPConnect "
                    "module owner about adding test coverage");
-        RootedString str(cx, JS_InternString(cx, ps->name));
-        if (!str)
+
+        jsid id;
+        if (!PropertySpecNameToPermanentId(cx, ps->name, &id))
             return false;
-        if (!props.append(INTERNED_STRING_TO_JSID(cx, str)))
+        if (!MaybeAppend(id, flags, props))
             return false;
     }
 
     return true;
 }
 
 JSObject*
 JSXrayTraits::createHolder(JSContext *cx, JSObject *wrapper)
@@ -1973,17 +1976,17 @@ XrayWrapper<Base, Traits>::definePropert
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::ownPropertyKeys(JSContext *cx, HandleObject wrapper,
                                            AutoIdVector &props) const
 {
     assertEnteredPolicy(cx, wrapper, JSID_VOID, BaseProxyHandler::ENUMERATE);
-    return enumerate(cx, wrapper, JSITER_OWNONLY | JSITER_HIDDEN, props);
+    return enumerate(cx, wrapper, JSITER_OWNONLY | JSITER_HIDDEN | JSITER_SYMBOLS, props);
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::delete_(JSContext *cx, HandleObject wrapper,
                                    HandleId id, bool *bp) const
 {
     assertEnteredPolicy(cx, wrapper, id, BaseProxyHandler::SET);