Bug 926012 - Part 2: Allow __proto__ sets on proxies. (r=Waldo)
authorEric Faust <efaustbmo@gmail.com>
Fri, 13 Dec 2013 12:01:30 -0800
changeset 160401 8ba79063973d486a5f32d7f8bb67a22523399705
parent 160400 8e34b4680c170160c8b4a9e581ee40ce49e28e5b
child 160402 ae50af3377666ac4f78992828e5ea38590fa2b29
push id37587
push userefaustbmo@gmail.com
push dateFri, 13 Dec 2013 20:01:47 +0000
treeherdermozilla-inbound@ae50af337766 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs926012
milestone29.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 926012 - Part 2: Allow __proto__ sets on proxies. (r=Waldo)
js/src/jsobjinlines.h
js/src/jsproxy.cpp
js/src/jsproxy.h
js/src/tests/ecma_5/extensions/proxy-__proto__.js
js/src/vm/GlobalObject.cpp
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -408,16 +408,22 @@ JSObject::getProto(JSContext *cx, js::Ha
         protop.set(obj->js::ObjectImpl::getProto());
         return true;
     }
 }
 
 /* static */ inline bool
 JSObject::setProto(JSContext *cx, JS::HandleObject obj, JS::HandleObject proto, bool *succeeded)
 {
+    /* Proxies live in their own little world. */
+    if (obj->getTaggedProto().isLazy()) {
+        JS_ASSERT(obj->is<js::ProxyObject>());
+        return js::Proxy::setPrototypeOf(cx, obj, proto, succeeded);
+    }
+
     /* ES6 9.1.2 step 5 forbids changing [[Prototype]] if not [[Extensible]]. */
     bool extensible;
     if (!JSObject::isExtensible(cx, obj, &extensible))
         return false;
     if (!extensible) {
         *succeeded = false;
         return true;
     }
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -340,19 +340,27 @@ JSObject *
 BaseProxyHandler::weakmapKeyDelegate(JSObject *proxy)
 {
     return nullptr;
 }
 
 bool
 BaseProxyHandler::getPrototypeOf(JSContext *cx, HandleObject proxy, MutableHandleObject protop)
 {
-    // The default implementation here just uses proto of the proxy object.
-    protop.set(proxy->getTaggedProto().toObjectOrNull());
-    return true;
+    MOZ_ASSUME_UNREACHABLE("Must override getPrototypeOf with lazy prototype.");
+}
+
+bool
+BaseProxyHandler::setPrototypeOf(JSContext *cx, HandleObject, HandleObject, bool *)
+{
+    // Disallow sets of protos on proxies with lazy protos, but no hook.
+    // This keeps us away from the footgun of having the first proto set opt
+    // you out of having dynamic protos altogether.
+    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_SETPROTOTYPEOF_FAIL);
+    return false;
 }
 
 bool
 BaseProxyHandler::watch(JSContext *cx, HandleObject proxy, HandleId id, HandleObject callable)
 {
     JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_WATCH,
                          proxy->getClass()->name);
     return false;
@@ -2300,17 +2308,17 @@ ScriptedDirectProxyHandler::construct(JS
     return Invoke(cx, thisValue, trap, ArrayLength(constructArgv), constructArgv, args.rval());
 }
 
 ScriptedDirectProxyHandler ScriptedDirectProxyHandler::singleton;
 
 #define INVOKE_ON_PROTOTYPE(cx, handler, proxy, protoCall)                   \
     JS_BEGIN_MACRO                                                           \
         RootedObject proto(cx);                                              \
-        if (!handler->getPrototypeOf(cx, proxy, &proto))                     \
+        if (!JSObject::getProto(cx, proxy, &proto))                          \
             return false;                                                    \
         if (!proto)                                                          \
             return true;                                                     \
         assertSameCompartment(cx, proxy, proto);                             \
         return protoCall;                                                    \
     JS_END_MACRO                                                             \
 
 bool
@@ -2538,17 +2546,19 @@ Proxy::set(JSContext *cx, HandleObject p
     if (handler->hasPrototype()) {
         // If we're using a prototype, we still want to use the proxy trap unless
         // we have a non-own property with a setter.
         bool hasOwn;
         if (!handler->hasOwn(cx, proxy, id, &hasOwn))
             return false;
         if (!hasOwn) {
             RootedObject proto(cx);
-            if (!handler->getPrototypeOf(cx, proxy, &proto))
+            // Proxies might still use the normal prototype mechanism, rather than
+            // a hook, so query the engine proper
+            if (!JSObject::getProto(cx, proxy, &proto))
                 return false;
             if (proto) {
                 Rooted<PropertyDescriptor> desc(cx);
                 if (!JS_GetPropertyDescriptorById(cx, proto, id, 0, &desc))
                     return false;
                 if (desc.object() && desc.setter())
                     return JSObject::setGeneric(cx, proto, receiver, id, vp, strict);
             }
@@ -2720,24 +2730,33 @@ Proxy::regexp_toShared(JSContext *cx, Ha
 
 bool
 Proxy::defaultValue(JSContext *cx, HandleObject proxy, JSType hint, MutableHandleValue vp)
 {
     JS_CHECK_RECURSION(cx, return false);
     return proxy->as<ProxyObject>().handler()->defaultValue(cx, proxy, hint, vp);
 }
 
+JSObject * const Proxy::LazyProto = reinterpret_cast<JSObject *>(0x1);
+
 bool
 Proxy::getPrototypeOf(JSContext *cx, HandleObject proxy, MutableHandleObject proto)
 {
+    JS_ASSERT(proxy->getTaggedProto().isLazy());
     JS_CHECK_RECURSION(cx, return false);
     return proxy->as<ProxyObject>().handler()->getPrototypeOf(cx, proxy, proto);
 }
 
-JSObject * const Proxy::LazyProto = reinterpret_cast<JSObject *>(0x1);
+bool
+Proxy::setPrototypeOf(JSContext *cx, HandleObject proxy, HandleObject proto, bool *bp)
+{
+    JS_ASSERT(proxy->getTaggedProto().isLazy());
+    JS_CHECK_RECURSION(cx, return false);
+    return proxy->as<ProxyObject>().handler()->setPrototypeOf(cx, proxy, proto, bp);
+}
 
 /* static */ bool
 Proxy::watch(JSContext *cx, JS::HandleObject proxy, JS::HandleId id, JS::HandleObject callable)
 {
     JS_CHECK_RECURSION(cx, return false);
     return proxy->as<ProxyObject>().handler()->watch(cx, proxy, id, callable);
 }
 
--- a/js/src/jsproxy.h
+++ b/js/src/jsproxy.h
@@ -42,16 +42,32 @@ class JS_FRIEND_API(Wrapper);
  * proxy has the notion of a target, however.
  *
  * Proxy traps are grouped into fundamental and derived traps. Every proxy has
  * to at least provide implementations for the fundamental traps, but the
  * derived traps can be implemented in terms of the fundamental ones
  * BaseProxyHandler provides implementations of the derived traps in terms of
  * the (pure virtual) fundamental traps.
  *
+ * In addition to the normal traps, there are two models for proxy prototype
+ * chains. First, proxies may opt to use the standard prototype mechanism used
+ * throughout the engine. To do so, simply pass a prototype to NewProxyObject()
+ * at creation time. All prototype accesses will then "just work" to treat the
+ * proxy as a "normal" object. Alternatively, if instead the proxy wishes to
+ * implement more complicated prototype semantics (if, for example, it wants to
+ * delegate the prototype lookup to a wrapped object), it may pass Proxy::LazyProto
+ * as the prototype at create time and opt in to the trapped prototype system,
+ * which guarantees that their trap will be called on any and every prototype
+ * chain access of the object.
+ *
+ * This system is implemented with two traps: {get,set}PrototypeOf. The default
+ * implementation of setPrototypeOf throws a TypeError. Since it is not possible
+ * to create an object without a sense of prototype chain, handler implementors
+ * must provide a getPrototypeOf trap if opting in to the dynamic prototype system.
+ *
  * To minimize code duplication, a set of abstract proxy handler classes is
  * provided, from which other handlers may inherit. These abstract classes
  * are organized in the following hierarchy:
  *
  * BaseProxyHandler
  * |
  * DirectProxyHandler
  * |
@@ -162,16 +178,17 @@ class JS_FRIEND_API(BaseProxyHandler)
     virtual bool hasInstance(JSContext *cx, HandleObject proxy, MutableHandleValue v, bool *bp);
     virtual bool objectClassIs(HandleObject obj, ESClassValue classValue, JSContext *cx);
     virtual const char *className(JSContext *cx, HandleObject proxy);
     virtual JSString *fun_toString(JSContext *cx, HandleObject proxy, unsigned indent);
     virtual bool regexp_toShared(JSContext *cx, HandleObject proxy, RegExpGuard *g);
     virtual bool defaultValue(JSContext *cx, HandleObject obj, JSType hint, MutableHandleValue vp);
     virtual void finalize(JSFreeOp *fop, JSObject *proxy);
     virtual bool getPrototypeOf(JSContext *cx, HandleObject proxy, MutableHandleObject protop);
+    virtual bool setPrototypeOf(JSContext *cx, HandleObject proxy, HandleObject proto, bool *bp);
 
     // These two hooks must be overridden, or not overridden, in tandem -- no
     // overriding just one!
     virtual bool watch(JSContext *cx, JS::HandleObject proxy, JS::HandleId id,
                        JS::HandleObject callable);
     virtual bool unwatch(JSContext *cx, JS::HandleObject proxy, JS::HandleId id);
 
     virtual bool slice(JSContext *cx, HandleObject proxy, uint32_t begin, uint32_t end,
@@ -289,16 +306,17 @@ class Proxy
     static bool nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl, CallArgs args);
     static bool hasInstance(JSContext *cx, HandleObject proxy, MutableHandleValue v, bool *bp);
     static bool objectClassIs(HandleObject obj, ESClassValue classValue, JSContext *cx);
     static const char *className(JSContext *cx, HandleObject proxy);
     static JSString *fun_toString(JSContext *cx, HandleObject proxy, unsigned indent);
     static bool regexp_toShared(JSContext *cx, HandleObject proxy, RegExpGuard *g);
     static bool defaultValue(JSContext *cx, HandleObject obj, JSType hint, MutableHandleValue vp);
     static bool getPrototypeOf(JSContext *cx, HandleObject proxy, MutableHandleObject protop);
+    static bool setPrototypeOf(JSContext *cx, HandleObject proxy, HandleObject proto, bool *bp);
 
     static bool watch(JSContext *cx, HandleObject proxy, HandleId id, HandleObject callable);
     static bool unwatch(JSContext *cx, HandleObject proxy, HandleId id);
 
     static bool slice(JSContext *cx, HandleObject obj, uint32_t begin, uint32_t end,
                       HandleObject result);
 
     /* IC entry path for handling __noSuchMethod__ on access. */
--- a/js/src/tests/ecma_5/extensions/proxy-__proto__.js
+++ b/js/src/tests/ecma_5/extensions/proxy-__proto__.js
@@ -29,30 +29,21 @@ function testProxy(creator, args, proto)
 
   var pobj = Proxy[creator].apply(Proxy, args);
 
   // Check [[Prototype]] before attempted mutation
   assertEq(Object.getPrototypeOf(pobj), proto);
   assertEq(protoGetter.call(pobj), proto);
 
   // Attempt [[Prototype]] mutation
-  try
-  {
-    protoSetter.call(pobj);
-    throw new Error("should throw trying to mutate a proxy's [[Prototype]]");
-  }
-  catch (e)
-  {
-    assertEq(e instanceof TypeError, true,
-             "expected TypeError, instead got: " + e);
-  }
+  protoSetter.call(pobj, null);
 
   // Check [[Prototype]] after attempted mutation
-  assertEq(Object.getPrototypeOf(pobj), proto);
-  assertEq(protoGetter.call(pobj), proto);
+  assertEq(Object.getPrototypeOf(pobj), null);
+  assertEq(protoGetter.call(pobj), null);
 }
 
 // Proxy object with non-null [[Prototype]]
 var nonNullProto = { toString: function() { return "non-null prototype"; } };
 var nonNullHandler = { toString: function() { return "non-null handler"; } };
 testProxy("create", [nonNullHandler, nonNullProto], nonNullProto);
 
 // Proxy object with null [[Prototype]]
--- a/js/src/vm/GlobalObject.cpp
+++ b/js/src/vm/GlobalObject.cpp
@@ -63,25 +63,25 @@ static bool
 ThrowTypeError(JSContext *cx, unsigned argc, Value *vp)
 {
     JS_ReportErrorFlagsAndNumber(cx, JSREPORT_ERROR, js_GetErrorMessage, nullptr,
                                  JSMSG_THROW_TYPE_ERROR);
     return false;
 }
 
 static bool
-TestProtoGetterThis(HandleValue v)
+TestProtoThis(HandleValue v)
 {
     return !v.isNullOrUndefined();
 }
 
 static bool
 ProtoGetterImpl(JSContext *cx, CallArgs args)
 {
-    JS_ASSERT(TestProtoGetterThis(args.thisv()));
+    JS_ASSERT(TestProtoThis(args.thisv()));
 
     HandleValue thisv = args.thisv();
     if (thisv.isPrimitive() && !BoxNonStrictThis(cx, args))
         return false;
 
     unsigned dummy;
     RootedObject obj(cx, &args.thisv().toObject());
     RootedId nid(cx, NameToId(cx->names().proto));
@@ -92,66 +92,50 @@ ProtoGetterImpl(JSContext *cx, CallArgs 
     args.rval().set(v);
     return true;
 }
 
 static bool
 ProtoGetter(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
-    return CallNonGenericMethod(cx, TestProtoGetterThis, ProtoGetterImpl, args);
+    return CallNonGenericMethod(cx, TestProtoThis, ProtoGetterImpl, args);
 }
 
 namespace js {
 size_t sSetProtoCalled = 0;
 } // namespace js
 
 static bool
-TestProtoSetterThis(HandleValue v)
-{
-    if (v.isNullOrUndefined())
-        return false;
-
-    /* These will work as if on a boxed primitive; dumb, but whatever. */
-    if (!v.isObject())
-        return true;
-
-    /* Otherwise, only accept non-proxies. */
-    return !v.toObject().is<ProxyObject>();
-}
-
-static bool
 ProtoSetterImpl(JSContext *cx, CallArgs args)
 {
-    JS_ASSERT(TestProtoSetterThis(args.thisv()));
+    JS_ASSERT(TestProtoThis(args.thisv()));
 
     HandleValue thisv = args.thisv();
     if (thisv.isPrimitive()) {
         JS_ASSERT(!thisv.isNullOrUndefined());
 
         // Mutating a boxed primitive's [[Prototype]] has no side effects.
         args.rval().setUndefined();
         return true;
     }
 
     if (!cx->runningWithTrustedPrincipals())
         ++sSetProtoCalled;
 
     Rooted<JSObject*> obj(cx, &args.thisv().toObject());
 
     /*
-     * Disallow mutating the [[Prototype]] of a proxy that wasn't simply
-     * wrapping some other object.  Also disallow it on ArrayBuffer objects,
-     * which due to their complicated delegate-object shenanigans can't easily
+     * Disallow mutating the [[Prototype]] on ArrayBuffer objects, which
+     * due to their complicated delegate-object shenanigans can't easily
      * have a mutable [[Prototype]].
      */
-    if (obj->is<ProxyObject>() || obj->is<ArrayBufferObject>()) {
+    if (obj->is<ArrayBufferObject>()) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_INCOMPATIBLE_PROTO,
-                             "Object", "__proto__ setter",
-                             obj->is<ProxyObject>() ? "Proxy" : "ArrayBuffer");
+                             "Object", "__proto__ setter", "ArrayBuffer");
         return false;
     }
 
     /* Do nothing if __proto__ isn't being set to an object or null. */
     if (args.length() == 0 || !args[0].isObjectOrNull()) {
         args.rval().setUndefined();
         return true;
     }
@@ -176,17 +160,17 @@ ProtoSetterImpl(JSContext *cx, CallArgs 
     args.rval().setUndefined();
     return true;
 }
 
 static bool
 ProtoSetter(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
-    return CallNonGenericMethod(cx, TestProtoSetterThis, ProtoSetterImpl, args);
+    return CallNonGenericMethod(cx, TestProtoThis, ProtoSetterImpl, args);
 }
 
 JSObject *
 GlobalObject::initFunctionAndObjectClasses(JSContext *cx)
 {
     Rooted<GlobalObject*> self(cx, this);
 
     JS_ASSERT(!cx->runtime()->isAtomsCompartment(cx->compartment()));