Bug 1138157 - Change ScriptedDirectProxyHandler to inherit from BaseProxyHandler. r=efaust
authorTom Schuster <evilpies@gmail.com>
Sat, 21 Mar 2015 20:30:57 +0100
changeset 263782 2cf8496afd5daf135e51bc728de67090da4be19b
parent 263781 d65328c6afe95c977f6670d55c08a8587ca487b9
child 263783 2bf4149711c980470a8081cbd71c3da10fe90069
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1138157
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 1138157 - Change ScriptedDirectProxyHandler to inherit from BaseProxyHandler. r=efaust This avoids the fallback behavior from DirectProxyHandler, which can't handle revoked proxies correctly. Test created by André Bargull and me.
js/src/proxy/DirectProxyHandler.cpp
js/src/proxy/ScriptedDirectProxyHandler.cpp
js/src/proxy/ScriptedDirectProxyHandler.h
js/src/tests/ecma_6/Proxy/revoke-as-side-effect.js
--- a/js/src/proxy/DirectProxyHandler.cpp
+++ b/js/src/proxy/DirectProxyHandler.cpp
@@ -98,22 +98,18 @@ DirectProxyHandler::nativeCall(JSContext
     return CallNativeImpl(cx, impl, args);
 }
 
 bool
 DirectProxyHandler::hasInstance(JSContext *cx, HandleObject proxy, MutableHandleValue v,
                                 bool *bp) const
 {
     assertEnteredPolicy(cx, proxy, JSID_VOID, GET);
-    bool b;
     RootedObject target(cx, proxy->as<ProxyObject>().target());
-    if (!HasInstance(cx, target, v, &b))
-        return false;
-    *bp = !!b;
-    return true;
+    return HasInstance(cx, target, v, bp);
 }
 
 bool
 DirectProxyHandler::getPrototype(JSContext *cx, HandleObject proxy, MutableHandleObject protop) const
 {
     RootedObject target(cx, proxy->as<ProxyObject>().target());
     return GetPrototype(cx, target, protop);
 }
@@ -193,30 +189,26 @@ DirectProxyHandler::weakmapKeyDelegate(J
     return UncheckedUnwrap(proxy);
 }
 
 bool
 DirectProxyHandler::has(JSContext *cx, HandleObject proxy, HandleId id, bool *bp) const
 {
     assertEnteredPolicy(cx, proxy, id, GET);
     MOZ_ASSERT(!hasPrototype()); // Should never be called if there's a prototype.
-    bool found;
     RootedObject target(cx, proxy->as<ProxyObject>().target());
-    if (!JS_HasPropertyById(cx, target, id, &found))
-        return false;
-    *bp = !!found;
-    return true;
+    return HasProperty(cx, target, id, bp);
 }
 
 bool
 DirectProxyHandler::hasOwn(JSContext *cx, HandleObject proxy, HandleId id, bool *bp) const
 {
     assertEnteredPolicy(cx, proxy, id, GET);
     RootedObject target(cx, proxy->as<ProxyObject>().target());
-    return js::HasOwnProperty(cx, target, id, bp);
+    return HasOwnProperty(cx, target, id, bp);
 }
 
 bool
 DirectProxyHandler::get(JSContext *cx, HandleObject proxy, HandleObject receiver,
                         HandleId id, MutableHandleValue vp) const
 {
     assertEnteredPolicy(cx, proxy, id, GET);
     RootedObject target(cx, proxy->as<ProxyObject>().target());
--- a/js/src/proxy/ScriptedDirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedDirectProxyHandler.cpp
@@ -269,45 +269,45 @@ ScriptedDirectProxyHandler::getPrototype
 {
     RootedObject target(cx, proxy->as<ProxyObject>().target());
     // Though handler is used elsewhere, spec mandates that both get set to null.
     if (!target) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_PROXY_REVOKED);
         return false;
     }
 
-    return DirectProxyHandler::getPrototype(cx, proxy, protop);
+    return GetPrototype(cx, target, protop);
 }
 
 bool
 ScriptedDirectProxyHandler::setPrototype(JSContext *cx, HandleObject proxy, HandleObject proto,
                                          ObjectOpResult &result) const
 {
     RootedObject target(cx, proxy->as<ProxyObject>().target());
     if (!target) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_PROXY_REVOKED);
         return false;
     }
 
-    return DirectProxyHandler::setPrototype(cx, proxy, proto, result);
+    return SetPrototype(cx, target, proto, result);
 }
 
 // Not yet part of ES6, but hopefully to be standards-tracked -- and needed to
 // handle revoked proxies in any event.
 bool
 ScriptedDirectProxyHandler::setImmutablePrototype(JSContext *cx, HandleObject proxy,
                                                   bool *succeeded) const
 {
     RootedObject target(cx, proxy->as<ProxyObject>().target());
     if (!target) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_PROXY_REVOKED);
         return false;
     }
 
-    return DirectProxyHandler::setImmutablePrototype(cx, proxy, succeeded);
+    return SetImmutablePrototype(cx, target, succeeded);
 }
 
 // ES6 draft rev 32 (2 Feb 2015) 9.5.4 Proxy.[[PreventExtensions]]()
 bool
 ScriptedDirectProxyHandler::preventExtensions(JSContext *cx, HandleObject proxy,
                                               ObjectOpResult &result) const
 {
     // Steps 1-3.
@@ -322,17 +322,17 @@ ScriptedDirectProxyHandler::preventExten
 
     // Steps 5-6.
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().preventExtensions, &trap))
         return false;
 
     // Step 7.
     if (trap.isUndefined())
-        return DirectProxyHandler::preventExtensions(cx, proxy, result);
+        return PreventExtensions(cx, target, result);
 
     // Steps 8-9.
     Value argv[] = {
         ObjectValue(*target)
     };
     RootedValue trapResult(cx);
     if (!Invoke(cx, ObjectValue(*handler), trap, ArrayLength(argv), argv, &trapResult))
         return false;
@@ -370,17 +370,17 @@ ScriptedDirectProxyHandler::isExtensible
 
     // step 4-5
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().isExtensible, &trap))
         return false;
 
     // step 6
     if (trap.isUndefined())
-        return DirectProxyHandler::isExtensible(cx, proxy, extensible);
+        return IsExtensible(cx, target, extensible);
 
     // step 7, 9
     Value argv[] = {
         ObjectValue(*target)
     };
     RootedValue trapResult(cx);
     if (!Invoke(cx, ObjectValue(*handler), trap, ArrayLength(argv), argv, &trapResult))
         return false;
@@ -445,17 +445,17 @@ ScriptedDirectProxyHandler::getOwnProper
 
     // step 5-6
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().getOwnPropertyDescriptor, &trap))
         return false;
 
     // step 7
     if (trap.isUndefined())
-        return DirectProxyHandler::getOwnPropertyDescriptor(cx, proxy, id, desc);
+        return GetOwnPropertyDescriptor(cx, target, id, desc);
 
     // step 8-9
     RootedValue propKey(cx);
     if (!IdToStringOrSymbol(cx, id, &propKey))
         return false;
 
     Value argv[] = {
         ObjectValue(*target),
@@ -565,17 +565,17 @@ ScriptedDirectProxyHandler::defineProper
 
     // steps 6-7
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().defineProperty, &trap))
         return false;
 
     // step 8
     if (trap.isUndefined())
-        return DirectProxyHandler::defineProperty(cx, proxy, id, desc, result);
+        return StandardDefineProperty(cx, target, id, desc, result);
 
     // step 9
     RootedValue descObj(cx);
     if (!FromPropertyDescriptor(cx, desc, &descObj))
         return false;
 
     // steps 10-11
     RootedValue propKey(cx);
@@ -652,17 +652,17 @@ ScriptedDirectProxyHandler::ownPropertyK
 
     // step 4-5
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().ownKeys, &trap))
         return false;
 
     // step 6
     if (trap.isUndefined())
-        return DirectProxyHandler::ownPropertyKeys(cx, proxy, props);
+        return GetPropertyKeys(cx, target, JSITER_OWNONLY | JSITER_HIDDEN | JSITER_SYMBOLS, &props);
 
     // step 7-8
     Value argv[] = {
         ObjectValue(*target)
     };
     RootedValue trapResult(cx);
     if (!Invoke(cx, ObjectValue(*handler), trap, ArrayLength(argv), argv, &trapResult))
         return false;
@@ -699,17 +699,17 @@ ScriptedDirectProxyHandler::delete_(JSCo
 
     // steps 6-7
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().deleteProperty, &trap))
         return false;
 
     // step 8
     if (trap.isUndefined())
-        return DirectProxyHandler::delete_(cx, proxy, id, result);
+        return DeleteProperty(cx, target, id, result);
 
     // steps 9-10
     RootedValue value(cx);
     if (!IdToStringOrSymbol(cx, id, &value))
         return false;
     Value argv[] = {
         ObjectValue(*target),
         value
@@ -758,17 +758,17 @@ ScriptedDirectProxyHandler::enumerate(JS
 
     // step 5-6
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().enumerate, &trap))
         return false;
 
     // step 7
     if (trap.isUndefined())
-        return DirectProxyHandler::enumerate(cx, proxy, objp);
+        return GetIterator(cx, target, 0, objp);
 
     // step 8-9
     Value argv[] = {
         ObjectOrNullValue(target)
     };
     RootedValue trapResult(cx);
     if (!Invoke(cx, ObjectValue(*handler), trap, ArrayLength(argv), argv, &trapResult))
         return false;
@@ -802,17 +802,17 @@ ScriptedDirectProxyHandler::has(JSContex
 
     // step 5-6
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().has, &trap))
         return false;
 
     // step 7
     if (trap.isUndefined())
-        return DirectProxyHandler::has(cx, proxy, id, bp);
+        return HasProperty(cx, target, id, bp);
 
     // step 8,10
     RootedValue value(cx);
     if (!IdToStringOrSymbol(cx, id, &value))
         return false;
     Value argv[] = {
         ObjectOrNullValue(target),
         value
@@ -870,17 +870,17 @@ ScriptedDirectProxyHandler::get(JSContex
 
     // step 5-6
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().get, &trap))
         return false;
 
     // step 7
     if (trap.isUndefined())
-        return DirectProxyHandler::get(cx, proxy, receiver, id, vp);
+        return GetProperty(cx, target, receiver, id, vp);
 
     // step 8-9
     RootedValue value(cx);
     if (!IdToStringOrSymbol(cx, id, &value))
         return false;
     Value argv[] = {
         ObjectOrNullValue(target),
         value,
@@ -935,17 +935,17 @@ ScriptedDirectProxyHandler::set(JSContex
     // step 5-7
     RootedObject target(cx, proxy->as<ProxyObject>().target());
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().set, &trap))
         return false;
 
     // step 8
     if (trap.isUndefined())
-        return DirectProxyHandler::set(cx, proxy, receiver, id, vp, result);
+        return SetProperty(cx, target, receiver, id, vp, result);
 
     // step 9-10
     RootedValue value(cx);
     if (!IdToStringOrSymbol(cx, id, &value))
         return false;
     Value argv[] = {
         ObjectOrNullValue(target),
         value,
@@ -1010,18 +1010,20 @@ ScriptedDirectProxyHandler::call(JSConte
         return false;
 
     // step 4-5
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().apply, &trap))
         return false;
 
     // step 6
-    if (trap.isUndefined())
-        return DirectProxyHandler::call(cx, proxy, args);
+    if (trap.isUndefined()) {
+        RootedValue targetv(cx, ObjectValue(*target));
+        return Invoke(cx, args.thisv(), targetv, args.length(), args.array(), args.rval());
+    }
 
     // step 8
     Value argv[] = {
         ObjectValue(*target),
         args.thisv(),
         ObjectValue(*argsArray)
     };
     RootedValue thisValue(cx, ObjectValue(*handler));
@@ -1051,18 +1053,20 @@ ScriptedDirectProxyHandler::construct(JS
         return false;
 
     // step 4-5
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().construct, &trap))
         return false;
 
     // step 6
-    if (trap.isUndefined())
-        return DirectProxyHandler::construct(cx, proxy, args);
+    if (trap.isUndefined()) {
+        RootedValue targetv(cx, ObjectValue(*target));
+        return InvokeConstructor(cx, targetv, args.length(), args.array(), args.rval());
+    }
 
     // step 8-9
     Value constructArgv[] = {
         ObjectValue(*target),
         ObjectValue(*argsArray)
     };
     RootedValue thisValue(cx, ObjectValue(*handler));
     if (!Invoke(cx, thisValue, trap, ArrayLength(constructArgv), constructArgv, args.rval()))
@@ -1083,23 +1087,23 @@ ScriptedDirectProxyHandler::nativeCall(J
     ReportIncompatible(cx, args);
     return false;
 }
 
 bool
 ScriptedDirectProxyHandler::hasInstance(JSContext *cx, HandleObject proxy, MutableHandleValue v,
                                         bool *bp) const
 {
-    RootedObject handler(cx, GetDirectProxyHandlerObject(proxy));
-    if (!handler) {
+    RootedObject target(cx, proxy->as<ProxyObject>().target());
+    if (!target) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_PROXY_REVOKED);
         return false;
     }
 
-    return DirectProxyHandler::hasInstance(cx, proxy, v, bp);
+    return HasInstance(cx, target, v, bp);
 }
 
 bool
 ScriptedDirectProxyHandler::objectClassIs(HandleObject proxy, ESClassValue classValue,
                                           JSContext *cx) const
 {
     // Special case IsArray. In every other instance ES wants to have exactly
     // one object type and not a proxy around it, so return false.
@@ -1116,21 +1120,21 @@ ScriptedDirectProxyHandler::objectClassI
 
     return IsArray(target, cx);
 }
 
 const char *
 ScriptedDirectProxyHandler::className(JSContext *cx, HandleObject proxy) const
 {
     // Right now the caller is not prepared to handle failures.
-    RootedObject handler(cx, GetDirectProxyHandlerObject(proxy));
-    if (!handler)
+    RootedObject target(cx, proxy->as<ProxyObject>().target());
+    if (!target)
         return BaseProxyHandler::className(cx, proxy);
 
-    return DirectProxyHandler::className(cx, proxy);
+    return GetObjectClassName(cx, target);
 }
 JSString *
 ScriptedDirectProxyHandler::fun_toString(JSContext *cx, HandleObject proxy,
                                          unsigned indent) const
 {
     JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_INCOMPATIBLE_PROTO,
                          js_Function_str, js_toString_str, "object");
     return nullptr;
--- a/js/src/proxy/ScriptedDirectProxyHandler.h
+++ b/js/src/proxy/ScriptedDirectProxyHandler.h
@@ -7,20 +7,20 @@
 #ifndef proxy_ScriptedDirectProxyHandler_h
 #define proxy_ScriptedDirectProxyHandler_h
 
 #include "js/Proxy.h"
 
 namespace js {
 
 /* Derived class for all scripted direct proxy handlers. */
-class ScriptedDirectProxyHandler : public DirectProxyHandler {
+class ScriptedDirectProxyHandler : public BaseProxyHandler {
   public:
     MOZ_CONSTEXPR ScriptedDirectProxyHandler()
-      : DirectProxyHandler(&family)
+      : BaseProxyHandler(&family)
     { }
 
     /* Standard internal methods. */
     virtual bool getOwnPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
                                           MutableHandle<JSPropertyDescriptor> desc) const override;
     virtual bool defineProperty(JSContext *cx, HandleObject proxy, HandleId id,
                                 MutableHandle<JSPropertyDescriptor> desc,
                                 ObjectOpResult &result) const override;
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Proxy/revoke-as-side-effect.js
@@ -0,0 +1,78 @@
+function createProxy(proxyTarget) {
+  var {proxy, revoke} = Proxy.revocable(proxyTarget, new Proxy({}, {
+    get(target, propertyKey, receiver) {
+      print("trap get:", propertyKey);
+      revoke();
+    }
+  }));
+  return proxy;
+}
+
+var obj;
+
+// [[GetPrototypeOf]]
+assertEq(Object.getPrototypeOf(createProxy({})), Object.prototype);
+assertEq(Object.getPrototypeOf(createProxy([])), Array.prototype);
+
+// [[SetPrototypeOf]]
+obj = {};
+Object.setPrototypeOf(createProxy(obj), Array.prototype);
+assertEq(Object.getPrototypeOf(obj), Array.prototype);
+
+// [[IsExtensible]]
+assertEq(Object.isExtensible(createProxy({})), true);
+assertEq(Object.isExtensible(createProxy(Object.preventExtensions({}))), false);
+
+// [[PreventExtensions]]
+obj = {};
+Object.preventExtensions(createProxy(obj));
+assertEq(Object.isExtensible(obj), false);
+
+// [[GetOwnProperty]]
+assertEq(Object.getOwnPropertyDescriptor(createProxy({}), "a"), undefined);
+assertEq(Object.getOwnPropertyDescriptor(createProxy({a: 5}), "a").value, 5);
+
+// [[DefineOwnProperty]]
+obj = {};
+Object.defineProperty(createProxy(obj), "a", {value: 5});
+assertEq(obj.a, 5);
+
+// [[HasProperty]]
+assertEq("a" in createProxy({}), false);
+assertEq("a" in createProxy({a: 5}), true);
+
+// [[Get]]
+assertEq(createProxy({}).a, undefined);
+assertEq(createProxy({a: 5}).a, 5);
+
+// [[Set]]
+assertThrowsInstanceOf(() => createProxy({}).a = 0, TypeError);
+assertThrowsInstanceOf(() => createProxy({a: 5}).a = 0, TypeError);
+
+// [[Delete]]
+assertEq(delete createProxy({}).a, true);
+assertEq(delete createProxy(Object.defineProperty({}, "a", {configurable: false})).a, false);
+
+// [[Enumerate]]
+for (var k in createProxy({})) {
+    // No properties in object.
+    assertEq(true, false);
+}
+for (var k in createProxy({a: 5})) {
+    // Properties in object.
+    assertEq(k, "a");
+}
+
+// [[OwnPropertyKeys]]
+assertEq(Object.getOwnPropertyNames(createProxy({})).length, 0);
+assertEq(Object.getOwnPropertyNames(createProxy({a: 5})).length, 1);
+
+// [[Call]]
+assertEq(createProxy(function() { return "ok" })(), "ok");
+
+// [[Construct]]
+// This should throw per bug 1141865.
+assertEq(new (createProxy(function(){ return obj; })), obj);
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);