Bug 855136 - Don't use call/construct slots for direct proxies (r=luke)
authorBill McCloskey <wmccloskey@mozilla.com>
Tue, 26 Mar 2013 17:53:00 -0700
changeset 126582 b2818d4dccfe5d47ceffcaea6e06f3da4444aba2
parent 126581 3d7ab7be4d31ddd6c107668a4a4cc0f260e1af67
child 126583 c1d4ca637c6107a5a56b38ff2ca0a0993869da06
push id24488
push userryanvm@gmail.com
push dateFri, 29 Mar 2013 00:54:52 +0000
treeherdermozilla-central@8aeabe064932 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs855136
milestone22.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 855136 - Don't use call/construct slots for direct proxies (r=luke)
js/src/jscompartment.cpp
js/src/jsgc.cpp
js/src/jsproxy.cpp
js/src/jsproxy.h
js/src/jswrapper.cpp
js/xpconnect/src/XPCComponents.cpp
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -480,22 +480,16 @@ JSCompartment::markCrossCompartmentWrapp
 
             /*
              * We have a cross-compartment wrapper. Its private pointer may
              * point into the compartment being collected, so we should mark it.
              */
             Value referent = GetProxyPrivate(wrapper);
             MarkValueRoot(trc, &referent, "cross-compartment wrapper");
             JS_ASSERT(referent == GetProxyPrivate(wrapper));
-
-            if (IsFunctionProxy(wrapper)) {
-                Value call = GetProxyCall(wrapper);
-                MarkValueRoot(trc, &call, "cross-compartment wrapper");
-                JS_ASSERT(call == GetProxyCall(wrapper));
-            }
         }
     }
 }
 
 void
 JSCompartment::mark(JSTracer *trc)
 {
 #ifdef JS_ION
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3255,21 +3255,16 @@ JSCompartment::findOutgoingEdges(Compone
              * Add edge for debugger object wrappers, to ensure (in conjuction
              * with call to Debugger::findCompartmentEdges below) that debugger
              * and debuggee objects are always swept in the same group.
              */
             JS::Zone *w = other->tenuredZone();
             if (w->isGCMarking())
                 finder.addEdgeTo(w);
         }
-
-#ifdef DEBUG
-        JSObject *wrapper = &e.front().value.toObject();
-        JS_ASSERT_IF(IsFunctionProxy(wrapper), &GetProxyCall(wrapper).toObject() == other);
-#endif
     }
 
     Debugger::findCompartmentEdges(zone(), finder);
 }
 
 void
 Zone::findOutgoingEdges(ComponentFinder<JS::Zone> &finder)
 {
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -322,60 +322,41 @@ BaseProxyHandler::preventExtensions(JSCo
     JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CANT_CHANGE_EXTENSIBILITY);
     return false;
 }
 
 bool
 BaseProxyHandler::call(JSContext *cx, HandleObject proxy, unsigned argc,
                        Value *vp)
 {
-    assertEnteredPolicy(cx, proxy, JSID_VOID);
-    AutoValueRooter rval(cx);
-    RootedValue call(cx, GetCall(proxy));
-    JSBool ok = Invoke(cx, vp[1], call, argc, JS_ARGV(cx, vp), rval.addr());
-    if (ok)
-        JS_SET_RVAL(cx, vp, rval.value());
-    return ok;
+    JS_NOT_REACHED("callable proxies should implement call trap");
+    return false;
 }
 
 bool
 BaseProxyHandler::construct(JSContext *cx, HandleObject proxy, unsigned argc,
                             Value *argv, MutableHandleValue rval)
 {
-    assertEnteredPolicy(cx, proxy, JSID_VOID);
-    RootedValue fval(cx, GetConstruct(proxy));
-    if (fval.isUndefined())
-        fval = GetCall(proxy);
-    return InvokeConstructor(cx, fval, argc, argv, rval.address());
+    JS_NOT_REACHED("callable proxies should implement construct trap");
+    return false;
 }
 
 JSString *
 BaseProxyHandler::obj_toString(JSContext *cx, HandleObject proxy)
 {
     return JS_NewStringCopyZ(cx, IsFunctionProxy(proxy)
                                  ? "[object Function]"
                                  : "[object Object]");
 }
 
 JSString *
 BaseProxyHandler::fun_toString(JSContext *cx, HandleObject proxy, unsigned indent)
 {
-    assertEnteredPolicy(cx, proxy, JSID_VOID);
-    Value fval = GetCall(proxy);
-    if (IsFunctionProxy(proxy) &&
-        (fval.isPrimitive() || !fval.toObject().isFunction())) {
-        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
-                             JSMSG_INCOMPATIBLE_PROTO,
-                             js_Function_str, js_toString_str,
-                             "object");
-        return NULL;
-    }
-    RootedObject obj(cx, &fval.toObject());
-    return fun_toStringHelper(cx, obj, indent);
-
+    JS_NOT_REACHED("callable proxies should implement fun_toString trap");
+    return false;
 }
 
 bool
 BaseProxyHandler::regexp_toShared(JSContext *cx, HandleObject proxy,
                                   RegExpGuard *g)
 {
     JS_NOT_REACHED("This should have been a wrapped regexp");
     return false;
@@ -507,16 +488,38 @@ DirectProxyHandler::enumerate(JSContext 
 {
     assertEnteredPolicy(cx, proxy, JSID_VOID);
     JS_ASSERT(!hasPrototype()); // Should never be called if there's a prototype.
     RootedObject target(cx, GetProxyTargetObject(proxy));
     return GetPropertyNames(cx, target, 0, &props);
 }
 
 bool
+DirectProxyHandler::call(JSContext *cx, HandleObject proxy, unsigned argc,
+                         Value *vp)
+{
+    assertEnteredPolicy(cx, proxy, JSID_VOID);
+    AutoValueRooter rval(cx);
+    RootedValue target(cx, GetProxyPrivate(proxy));
+    JSBool ok = Invoke(cx, vp[1], target, argc, JS_ARGV(cx, vp), rval.addr());
+    if (ok)
+        JS_SET_RVAL(cx, vp, rval.value());
+    return ok;
+}
+
+bool
+DirectProxyHandler::construct(JSContext *cx, HandleObject proxy, unsigned argc,
+                              Value *argv, MutableHandleValue rval)
+{
+    assertEnteredPolicy(cx, proxy, JSID_VOID);
+    RootedValue target(cx, GetProxyPrivate(proxy));
+    return InvokeConstructor(cx, target, argc, argv, rval.address());
+}
+
+bool
 DirectProxyHandler::nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl,
                                CallArgs args)
 {
     args.setThis(ObjectValue(*GetProxyTargetObject(&args.thisv().toObject())));
     if (!test(args.thisv())) {
         ReportIncompatible(cx, args);
         return false;
     }
@@ -807,18 +810,22 @@ class ScriptedIndirectProxyHandler : pub
                      MutableHandleValue vp) MOZ_OVERRIDE;
     virtual bool set(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id,
                      bool strict, MutableHandleValue vp) MOZ_OVERRIDE;
     virtual bool keys(JSContext *cx, HandleObject proxy, AutoIdVector &props) MOZ_OVERRIDE;
     virtual bool iterate(JSContext *cx, HandleObject proxy, unsigned flags,
                          MutableHandleValue vp) MOZ_OVERRIDE;
 
     /* Spidermonkey extensions. */
+    virtual bool call(JSContext *cx, HandleObject proxy, unsigned argc, Value *vp) MOZ_OVERRIDE;
+    virtual bool construct(JSContext *cx, HandleObject proxy, unsigned argc,
+                           Value *argv, MutableHandleValue rval) MOZ_OVERRIDE;
     virtual bool nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl,
                             CallArgs args) MOZ_OVERRIDE;
+    virtual JSString *fun_toString(JSContext *cx, HandleObject proxy, unsigned indent) MOZ_OVERRIDE;
     virtual bool defaultValue(JSContext *cx, HandleObject obj, JSType hint,
                               MutableHandleValue vp) MOZ_OVERRIDE;
 
     static ScriptedIndirectProxyHandler singleton;
 };
 
 static int sScriptedIndirectProxyHandlerFamily = 0;
 
@@ -1007,22 +1014,63 @@ ScriptedIndirectProxyHandler::iterate(JS
         return false;
     if (!js_IsCallable(value))
         return BaseProxyHandler::iterate(cx, proxy, flags, vp);
     return Trap(cx, handler, value, 0, NULL, vp) &&
            ReturnedValueMustNotBePrimitive(cx, proxy, cx->names().iterate, vp);
 }
 
 bool
+ScriptedIndirectProxyHandler::call(JSContext *cx, HandleObject proxy, unsigned argc,
+                                   Value *vp)
+{
+    assertEnteredPolicy(cx, proxy, JSID_VOID);
+    AutoValueRooter rval(cx);
+    RootedValue call(cx, GetCall(proxy));
+    JSBool ok = Invoke(cx, vp[1], call, argc, JS_ARGV(cx, vp), rval.addr());
+    if (ok)
+        JS_SET_RVAL(cx, vp, rval.value());
+    return ok;
+}
+
+bool
+ScriptedIndirectProxyHandler::construct(JSContext *cx, HandleObject proxy, unsigned argc,
+                                        Value *argv, MutableHandleValue rval)
+{
+    assertEnteredPolicy(cx, proxy, JSID_VOID);
+    RootedValue fval(cx, GetConstruct(proxy));
+    if (fval.isUndefined())
+        fval = GetCall(proxy);
+    return InvokeConstructor(cx, fval, argc, argv, rval.address());
+}
+
+bool
 ScriptedIndirectProxyHandler::nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl,
                                          CallArgs args)
 {
     return BaseProxyHandler::nativeCall(cx, test, impl, args);
 }
 
+JSString *
+ScriptedIndirectProxyHandler::fun_toString(JSContext *cx, HandleObject proxy, unsigned indent)
+{
+    assertEnteredPolicy(cx, proxy, JSID_VOID);
+    Value fval = GetCall(proxy);
+    if (IsFunctionProxy(proxy) &&
+        (fval.isPrimitive() || !fval.toObject().isFunction())) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
+                             JSMSG_INCOMPATIBLE_PROTO,
+                             js_Function_str, js_toString_str,
+                             "object");
+        return NULL;
+    }
+    RootedObject obj(cx, &fval.toObject());
+    return fun_toStringHelper(cx, obj, indent);
+}
+
 bool
 ScriptedIndirectProxyHandler::defaultValue(JSContext *cx, HandleObject proxy, JSType hint,
                                            MutableHandleValue vp)
 {
     /*
      * This function is only here to prevent bug 757063. It will be removed when
      * the direct proxy refactor is complete.
      */
@@ -3257,19 +3305,19 @@ NewProxyObject(JSContext *cx, BaseProxyH
 
 JS_FRIEND_API(JSObject *)
 js::NewProxyObject(JSContext *cx, BaseProxyHandler *handler, const Value &priv_, JSObject *proto_,
                    JSObject *parent_, ProxyCallable callable)
 {
     return NewProxyObject(cx, handler, priv_, TaggedProto(proto_), parent_, callable);
 }
 
-JS_FRIEND_API(JSObject *)
-js::NewProxyObject(JSContext *cx, BaseProxyHandler *handler, const Value &priv_, JSObject *proto_,
-                   JSObject *parent_, JSObject *call, JSObject *construct)
+static JSObject *
+NewProxyObject(JSContext *cx, BaseProxyHandler *handler, const Value &priv_, JSObject *proto_,
+               JSObject *parent_, JSObject *call, JSObject *construct)
 {
     JS_ASSERT_IF(construct, cx->compartment == construct->compartment());
     JS_ASSERT_IF(call && cx->compartment != call->compartment(), priv_ == ObjectValue(*call));
 
     JSObject *proxy = NewProxyObject(cx, handler, priv_, TaggedProto(proto_), parent_,
                                      call || construct ? ProxyIsCallable : ProxyNotCallable);
     if (!proxy)
         return NULL;
--- a/js/src/jsproxy.h
+++ b/js/src/jsproxy.h
@@ -186,16 +186,19 @@ public:
                      HandleId id, bool strict, MutableHandleValue vp) MOZ_OVERRIDE;
     virtual bool keys(JSContext *cx, HandleObject proxy,
                       AutoIdVector &props) MOZ_OVERRIDE;
     virtual bool iterate(JSContext *cx, HandleObject proxy, unsigned flags,
                          MutableHandleValue vp) MOZ_OVERRIDE;
 
     /* Spidermonkey extensions. */
     virtual bool isExtensible(JSObject *proxy) MOZ_OVERRIDE;
+    virtual bool call(JSContext *cx, HandleObject proxy, unsigned argc, Value *vp) MOZ_OVERRIDE;
+    virtual bool construct(JSContext *cx, HandleObject proxy, unsigned argc,
+                           Value *argv, MutableHandleValue rval) MOZ_OVERRIDE;
     virtual bool nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl,
                             CallArgs args) MOZ_OVERRIDE;
     virtual bool hasInstance(JSContext *cx, HandleObject proxy, MutableHandleValue v,
                              bool *bp) MOZ_OVERRIDE;
     virtual bool objectClassIs(HandleObject obj, ESClassValue classValue,
                                JSContext *cx) MOZ_OVERRIDE;
     virtual JSString *obj_toString(JSContext *cx, HandleObject proxy) MOZ_OVERRIDE;
     virtual JSString *fun_toString(JSContext *cx, HandleObject proxy,
@@ -312,23 +315,16 @@ GetProxyPrivate(RawObject obj)
 inline JSObject *
 GetProxyTargetObject(RawObject obj)
 {
     JS_ASSERT(IsProxy(obj));
     return GetProxyPrivate(obj).toObjectOrNull();
 }
 
 inline const Value &
-GetProxyCall(RawObject obj)
-{
-    JS_ASSERT(IsFunctionProxy(obj));
-    return GetReservedSlot(obj, JSSLOT_PROXY_CALL);
-}
-
-inline const Value &
 GetProxyExtra(RawObject obj, size_t n)
 {
     JS_ASSERT(IsProxy(obj));
     return GetReservedSlot(obj, JSSLOT_PROXY_EXTRA + n);
 }
 
 inline void
 SetProxyHandler(RawObject obj, BaseProxyHandler *handler)
@@ -349,20 +345,16 @@ enum ProxyCallable {
     ProxyNotCallable = false,
     ProxyIsCallable = true
 };
 
 JS_FRIEND_API(JSObject *)
 NewProxyObject(JSContext *cx, BaseProxyHandler *handler, const Value &priv,
                JSObject *proto, JSObject *parent, ProxyCallable callable = ProxyNotCallable);
 
-JS_FRIEND_API(JSObject *)
-NewProxyObject(JSContext *cx, BaseProxyHandler *handler, const Value &priv,
-               JSObject *proto, JSObject *parent, JSObject *call, JSObject *construct);
-
 JSObject *
 RenewProxyObject(JSContext *cx, JSObject *obj, BaseProxyHandler *handler, Value priv);
 
 class JS_FRIEND_API(AutoEnterPolicy)
 {
   public:
     typedef BaseProxyHandler::Action Action;
     AutoEnterPolicy(JSContext *cx, BaseProxyHandler *handler,
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -41,17 +41,17 @@ JSObject *
 Wrapper::New(JSContext *cx, JSObject *obj, JSObject *proto, JSObject *parent,
              Wrapper *handler)
 {
     JS_ASSERT(parent);
 
     AutoMarkInDeadZone amd(cx->zone());
 
     return NewProxyObject(cx, handler, ObjectValue(*obj), proto, parent,
-                          obj->isCallable() ? obj : NULL, NULL);
+                          obj->isCallable() ? ProxyIsCallable : ProxyNotCallable);
 }
 
 JSObject *
 Wrapper::Renew(JSContext *cx, JSObject *existing, JSObject *obj, Wrapper *handler)
 {
     JS_ASSERT(!obj->isCallable());
     return RenewProxyObject(cx, existing, handler, ObjectValue(*obj));
 }
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -3052,18 +3052,17 @@ xpc::IsSandboxPrototypeProxy(JSObject *o
     return js::IsProxy(obj) &&
            js::GetProxyHandler(obj) == &xpc::sandboxProxyHandler;
 }
 
 bool
 xpc::SandboxCallableProxyHandler::call(JSContext *cx, JS::Handle<JSObject*> proxy,
                                        unsigned argc, Value *vp)
 {
-    // We forward the call to our underlying callable. The callable to forward
-    // to can be gotten via GetProxyCall.
+    // We forward the call to our underlying callable.
 
     // The parent of our proxy is the SandboxProxyHandler proxy
     JSObject *sandboxProxy = JS_GetParent(proxy);
     MOZ_ASSERT(js::IsProxy(sandboxProxy) &&
                js::GetProxyHandler(sandboxProxy) == &xpc::sandboxProxyHandler);
 
     // The parent of the sandboxProxy is the sandbox global, and the
     // target object is the original proto.
@@ -3101,17 +3100,17 @@ xpc::SandboxCallableProxyHandler::call(J
     // remap |this|.
     JS::Value thisVal =
       WrapperFactory::IsXrayWrapper(sandboxProxy) ? JS_THIS(cx, vp)
                                                   : JS_THIS_VALUE(cx, vp);
     if (thisVal == ObjectValue(*sandboxGlobal)) {
         thisVal = ObjectValue(*js::GetProxyTargetObject(sandboxProxy));
     }
 
-    return JS::Call(cx, thisVal, js::GetProxyCall(proxy), argc,
+    return JS::Call(cx, thisVal, js::GetProxyPrivate(proxy), argc,
                     JS_ARGV(cx, vp), vp);
 }
 
 xpc::SandboxCallableProxyHandler xpc::sandboxCallableProxyHandler;
 
 // Wrap a callable such that if we're called with oldThisObj as the
 // "this" we will instead call it with newThisObj as the this.
 static JSObject*
@@ -3120,21 +3119,19 @@ WrapCallable(JSContext *cx, JSObject *ca
     MOZ_ASSERT(JS_ObjectIsCallable(cx, callable));
     // Our proxy is wrapping the callable.  So we need to use the
     // callable as the private.  We use the given sandboxProtoProxy as
     // the parent, and our call() hook depends on that.
     MOZ_ASSERT(js::IsProxy(sandboxProtoProxy) &&
                js::GetProxyHandler(sandboxProtoProxy) ==
                  &xpc::sandboxProxyHandler);
 
-    // We need to pass the given callable in as the "call" and
-    // "construct" so we get a function proxy.
     return js::NewProxyObject(cx, &xpc::sandboxCallableProxyHandler,
                               ObjectValue(*callable), nullptr,
-                              sandboxProtoProxy, callable, callable);
+                              sandboxProtoProxy, js::ProxyIsCallable);
 }
 
 template<typename Op>
 bool BindPropertyOp(JSContext *cx, Op& op, PropertyDescriptor *desc, jsid id,
                     unsigned attrFlag, JSObject *sandboxProtoProxy)
 {
     if (!op) {
         return true;