Bug 645416, part 24 - Modify proxy tests to add testing for symbol-keyed properties. r=efaust.

This patch also updates legacy direct proxies to cope with symbols. Uniform
behavior seems like the easiest thing to carry forward.

--HG--
extra : rebase_source : 5e5251c942e879a4440d7c0524343cf6fc744c7e

js/src/jit-test/tests/proxy/testDirectProxyDefineProperty2.js

@@ -4,12 +4,12 @@
  * as the third argument.
  */
 var target = {};
-var called = false;
+var log = [];
 var handler = {
-    defineProperty: function (target1, name, desc1) {
+    defineProperty: function (target1, key, desc1) {
         assertEq(this, handler);
         assertEq(target1, target);
-        assertEq(name, 'foo');
+        log.push(key);
         assertEq(desc1 == desc, false);
         assertEq(desc1.value, 'bar');
         assertEq(desc1.writable, true);
@@ -27,7 +27,10 @@ var desc = {
 
 var p = new Proxy(target, handler);
 Object.defineProperty(p, 'foo', desc);
-assertEq(called, true);
+Object.defineProperty(p, Symbol.for('quux'), desc);
+assertEq(log.length, 2);
+assertEq(log[0], 'foo');
+assertEq(log[1], Symbol.for('quux'));
 assertEq(Object.isExtensible(target), true);
 assertEq(Object.isExtensible(p), true);
 Object.preventExtensions(target);

js/src/jit-test/tests/proxy/testDirectProxyGet1.js

@@ -6,3 +6,8 @@ assertEq(Proxy({
 assertEq(Proxy({
     foo: 'bar'
 }, {})['foo'], 'bar');
+
+var s = Symbol.for("moon");
+var obj = {};
+obj[s] = "dust";
+assertEq(Proxy(obj, {})[s], "dust");

js/src/jit-test/tests/proxy/testDirectProxyGet2.js

@@ -4,16 +4,18 @@
  * as the third argument
  */
 var target = {};
-var called = false;
-var handler = {
-    get: function (target1, name, receiver) {
-        assertEq(this, handler);
-        assertEq(target1, target);
-        assertEq(name, 'foo');
-        assertEq(receiver, proxy);
-        called = true;
-    }
-};
-var proxy = new Proxy(target, handler);
-proxy['foo'];
-assertEq(called, true);
+for (var key of ['foo', Symbol.iterator]) {
+    var called = false;
+    var handler = {
+        get: function (target1, name, receiver) {
+            assertEq(this, handler);
+            assertEq(target1, target);
+            assertEq(name, key);
+            assertEq(receiver, proxy);
+            called = true;
+        }
+    };
+    var proxy = new Proxy(target, handler);
+    assertEq(proxy[key], undefined);
+    assertEq(called, true);
+}

js/src/jit-test/tests/proxy/testDirectProxyGet5.js

@@ -14,3 +14,10 @@ assertEq(new Proxy({
         return undefined;
     }
 }).foo, undefined);
+
+var obj = {};
+var s1 = Symbol("moon"), s2 = Symbol("sun");
+obj[s1] = "wrong";
+assertEq(new Proxy(obj, {
+    get: () => s2
+})[s1], s2);

js/src/jit-test/tests/proxy/testDirectProxyHas1.js

@@ -11,3 +11,4 @@ var proxy = Proxy(Object.create(Object.create(null, {
 assertEq('foo' in proxy, true);
 assertEq('bar' in proxy, true);
 assertEq('baz' in proxy, false);
+assertEq(Symbol() in proxy, false);

js/src/jit-test/tests/proxy/testDirectProxyHas2.js

@@ -3,14 +3,16 @@
  * argument, and the name of the property as the second argument
  */
 var target = {};
-var called = false;
-var handler = {
-    has: function (target1, name) {
-        assertEq(this, handler);
-        assertEq(target1, target);
-        assertEq(name, 'foo');
-        called = true;
-    }
-};
-'foo' in new Proxy(target, handler);
-assertEq(called, true);
+for (var key of ['foo', Symbol('bar')]) {
+    var called = false;
+    var handler = {
+        has: function (target1, name) {
+            assertEq(this, handler);
+            assertEq(target1, target);
+            assertEq(name, key);
+            called = true;
+        }
+    };
+    key in new Proxy(target, handler);
+    assertEq(called, true);
+}

js/src/jit-test/tests/proxy/testDirectProxyHas6.js

@@ -4,9 +4,10 @@
  */
 var target = {};
 Object.preventExtensions(target);
-assertEq(
-    'foo' in new Proxy(target, {
-        has: function (target, name) {
-            return false;
-        }
-    }), false);
+var p = new Proxy(target, {
+    has: function (target, name) {
+        return false;
+    }
+});
+assertEq('foo' in p, false);
+assertEq(Symbol.iterator in p, false);

js/src/jit-test/tests/proxy/testDirectProxyHasOwnProperty.js

@@ -1,15 +1,23 @@
 // Forward to the target if the trap is not defined
-var proxy = Proxy(Object.create(Object.create(null, {
+var proto = Object.create(null, {
     'foo': {
         configurable: true
     }
-}), {
+});
+var descs = {
     'bar': {
         configurable: true
     }
-}), {});
+};
+descs[Symbol.for("quux")] = {configurable: true};
+var target = Object.create(proto, descs);
+var proxy = Proxy(target, {});
 assertEq(({}).hasOwnProperty.call(proxy, 'foo'), false);
 assertEq(({}).hasOwnProperty.call(proxy, 'bar'), true);
+assertEq(({}).hasOwnProperty.call(proxy, 'quux'), false);
+assertEq(({}).hasOwnProperty.call(proxy, Symbol('quux')), false);
+assertEq(({}).hasOwnProperty.call(proxy, 'Symbol(quux)'), false);
+assertEq(({}).hasOwnProperty.call(proxy, Symbol.for('quux')), true);
 
 // Make sure only the getOwnPropertyDescriptor trap is called, and not the has
 // trap.

js/src/jit-test/tests/proxy/testDirectProxySet1.js

@@ -2,5 +2,12 @@
 var target = {
     foo: 'bar'
 };
-Proxy(target, {})['foo'] = 'baz';
+Proxy(target, {}).foo = 'baz';
 assertEq(target.foo, 'baz');
+Proxy(target, {})['foo'] = 'buz';
+assertEq(target.foo, 'buz');
+
+var sym = Symbol.for('quux');
+Proxy(target, {})[sym] = sym;
+assertEq(target[sym], sym);
+

js/src/jit-test/tests/proxy/testDirectProxySet2.js

@@ -4,17 +4,19 @@
  * third argument, and the receiver as the fourth argument
  */
 var target = {};
-var called = false;
-var handler = {
-    set: function (target1, name, val, receiver) {
-        assertEq(this, handler);
-        assertEq(target1, target);
-        assertEq(name, 'foo');
-        assertEq(val, 'baz');
-        assertEq(receiver, proxy);
-        called = true;
-    }
-};
-var proxy = new Proxy(target, handler);
-proxy['foo'] = 'baz';
-assertEq(called, true);
+for (var key of ['foo', Symbol.for('quux')]) {
+    var called = false;
+    var handler = {
+        set: function (target1, name, val, receiver) {
+            assertEq(this, handler);
+            assertEq(target1, target);
+            assertEq(name, key);
+            assertEq(val, 'baz');
+            assertEq(receiver, proxy);
+            called = true;
+        }
+    };
+    var proxy = new Proxy(target, handler);
+    proxy[key] = 'baz';
+    assertEq(called, true);
+}

js/src/jit-test/tests/proxy/testDirectProxySet3.js

@@ -1,16 +1,18 @@
 load(libdir + "asserts.js");
 
 // Throw a TypeError if the trap sets a non-writable, non-configurable property
-var target = {};
-Object.defineProperty(target, 'foo', {
-    value: 'bar',
-    writable: false,
-    configurable: false
-});
-assertThrowsInstanceOf(function () {
-    new Proxy(target, {
-        set: function (target, name, val, receiver) {
-            return true;
-        }
-    })['foo'] = 'baz';
-}, TypeError);
+for (var key of ['foo', Symbol.for('quux')]) {
+    var target = {};
+    Object.defineProperty(target, key, {
+        value: 'bar',
+        writable: false,
+        configurable: false
+    });
+    assertThrowsInstanceOf(function () {
+        new Proxy(target, {
+            set: function (target, name, val, receiver) {
+                return true;
+            }
+        })[key] = 'baz';
+    }, TypeError);
+}

js/src/jsproxy.cpp

@@ -647,14 +647,24 @@ Trap(JSContext *cx, HandleObject handler, HandleValue fval, unsigned argc, Value
     return Invoke(cx, ObjectValue(*handler), fval, argc, argv, rval);
 }
 
+static bool
+IdToExposableValue(JSContext *cx, HandleId id, MutableHandleValue value)
+{
+    value.set(IdToValue(id)); // Re-use out-param to avoid Rooted overhead.
+    if (value.isSymbol())
+        return true;
+    JSString *name = ToString<CanGC>(cx, value);
+    if (!name)
+        return false;
+    value.setString(name);
+    return true;
+}
+
 static bool
 Trap1(JSContext *cx, HandleObject handler, HandleValue fval, HandleId id, MutableHandleValue rval)
 {
-    rval.set(IdToValue(id)); // Re-use out-param to avoid Rooted overhead.
-    JSString *str = ToString<CanGC>(cx, rval);
-    if (!str)
+    if (!IdToExposableValue(cx, id, rval)) // Re-use out-param to avoid Rooted overhead.
         return false;
-    rval.setString(str);
     return Trap(cx, handler, fval, 1, rval.address(), rval);
 }
 
@@ -663,11 +673,8 @@ Trap2(JSContext *cx, HandleObject handler, HandleValue fval, HandleId id, Value
       MutableHandleValue rval)
 {
     RootedValue v(cx, v_);
-    rval.set(IdToValue(id)); // Re-use out-param to avoid Rooted overhead.
-    JSString *str = ToString<CanGC>(cx, rval);
-    if (!str)
+    if (!IdToExposableValue(cx, id, rval)) // Re-use out-param to avoid Rooted overhead.
         return false;
-    rval.setString(str);
     JS::AutoValueArray<2> argv(cx);
     argv[0].set(rval);
     argv[1].set(v);
@@ -939,14 +946,12 @@ ScriptedIndirectProxyHandler::get(JSContext *cx, HandleObject proxy, HandleObjec
                                   HandleId id, MutableHandleValue vp)
 {
     RootedObject handler(cx, GetIndirectProxyHandlerObject(proxy));
-    RootedValue idv(cx, IdToValue(id));
-    JSString *str = ToString<CanGC>(cx, idv);
-    if (!str)
+    RootedValue idv(cx);
+    if (!IdToExposableValue(cx, id, &idv))
         return false;
-    RootedValue value(cx, StringValue(str));
     JS::AutoValueArray<2> argv(cx);
     argv[0].setObjectOrNull(receiver);
-    argv[1].set(value);
+    argv[1].set(idv);
     RootedValue fval(cx);
     if (!GetDerivedTrap(cx, handler, cx->names().get, &fval))
         return false;
@@ -960,21 +965,19 @@ ScriptedIndirectProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObjec
                                   HandleId id, bool strict, MutableHandleValue vp)
 {
     RootedObject handler(cx, GetIndirectProxyHandlerObject(proxy));
-    RootedValue idv(cx, IdToValue(id));
-    JSString *str = ToString<CanGC>(cx, idv);
-    if (!str)
+    RootedValue idv(cx);
+    if (!IdToExposableValue(cx, id, &idv))
         return false;
-    RootedValue value(cx, StringValue(str));
     JS::AutoValueArray<3> argv(cx);
     argv[0].setObjectOrNull(receiver);
-    argv[1].set(value);
+    argv[1].set(idv);
     argv[2].set(vp);
     RootedValue fval(cx);
     if (!GetDerivedTrap(cx, handler, cx->names().set, &fval))
         return false;
     if (!IsCallable(fval))
         return BaseProxyHandler::set(cx, proxy, receiver, id, strict, vp);
-    return Trap(cx, handler, fval, 3, argv.begin(), &value);
+    return Trap(cx, handler, fval, 3, argv.begin(), &idv);
 }
 
 bool
@@ -1238,17 +1241,6 @@ HasOwn(JSContext *cx, HandleObject obj, HandleId id, bool *bp)
     return true;
 }
 
-static bool
-IdToExposableValue(JSContext *cx, HandleId id, MutableHandleValue value)
-{
-    value.set(IdToValue(id)); // Re-use out-param to avoid Rooted overhead.
-    JSString *name = ToString<CanGC>(cx, value);
-    if (!name)
-        return false;
-    value.set(StringValue(name));
-    return true;
-}
-
 // Get the [[ProxyHandler]] of a scripted direct proxy.
 //
 // NB: This *must* stay synched with proxy().