Bug 832299 - Handlify JSCompartment::wrap. r=terrence
authorTom Schuster <evilpies@gmail.com>
Fri, 08 Feb 2013 20:16:34 +0100
changeset 121362 2ed6ca2ee354
parent 121361 c1ee454506f6
child 121363 cee6c0592d6e
push id22701
push userevilpies@gmail.com
push dateFri, 08 Feb 2013 19:17:15 +0000
treeherdermozilla-inbound@2ed6ca2ee354 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs832299
milestone21.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 832299 - Handlify JSCompartment::wrap. r=terrence
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jscntxt.cpp
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jsfun.cpp
js/src/jsobj.cpp
js/src/jswrapper.cpp
js/src/shell/js.cpp
js/src/shell/jsheaptools.cpp
js/src/vm/Debugger.cpp
js/src/vm/ObjectImpl.cpp
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -1504,17 +1504,20 @@ JS_WrapObject(JSContext *cx, JSObject **
     return cx->compartment->wrap(cx, objp);
 }
 
 JS_PUBLIC_API(JSBool)
 JS_WrapValue(JSContext *cx, jsval *vp)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
-    return cx->compartment->wrap(cx, vp);
+    RootedValue value(cx, *vp);
+    bool ok = cx->compartment->wrap(cx, &value);
+    *vp = value.get();
+    return ok;
 }
 
 JS_PUBLIC_API(JSBool)
 JS_WrapId(JSContext *cx, jsid *idp)
 {
   AssertHeapIsIdle(cx);
   CHECK_REQUEST(cx);
   return cx->compartment->wrapId(cx, idp);
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -688,16 +688,20 @@ class CallReceiver
         JS_ASSERT(!usedRval_);
         return JS::HandleValue::fromMarkedLocation(&argv_[-2]);
     }
 
     JS::HandleValue thisv() const {
         return JS::HandleValue::fromMarkedLocation(&argv_[-1]);
     }
 
+    JS::MutableHandleValue mutableThisv() const {
+        return JS::MutableHandleValue::fromMarkedLocation(&argv_[-1]);
+    }
+
     JS::MutableHandleValue rval() const {
         setUsedRval();
         return JS::MutableHandleValue::fromMarkedLocation(&argv_[-2]);
     }
 
     Value *spAfterCall() const {
         setUsedRval();
         return argv_ - 1;
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -1269,20 +1269,20 @@ JSContext::getDefaultLocale()
 /*
  * Since this function is only called in the context of a pending exception,
  * the caller must subsequently take an error path. If wrapping fails, it will
  * set a new (uncatchable) exception to be used in place of the original.
  */
 void
 JSContext::wrapPendingException()
 {
-    Value v = getPendingException();
+    RootedValue value(this, getPendingException());
     clearPendingException();
-    if (compartment->wrap(this, &v))
-        setPendingException(v);
+    if (compartment->wrap(this, &value))
+        setPendingException(value);
 }
 
 
 void
 JSContext::enterGenerator(JSGenerator *gen)
 {
     JS_ASSERT(!gen->prevGenerator);
     gen->prevGenerator = innermostGenerator_;
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -205,28 +205,28 @@ JSCompartment::ensureIonCompartmentExist
         return false;
     }
 
     return true;
 }
 #endif
 
 static bool
-WrapForSameCompartment(JSContext *cx, HandleObject obj, Value *vp)
+WrapForSameCompartment(JSContext *cx, HandleObject obj, MutableHandleValue vp)
 {
     JS_ASSERT(cx->compartment == obj->compartment());
     if (!cx->runtime->sameCompartmentWrapObjectCallback) {
-        vp->setObject(*obj);
+        vp.setObject(*obj);
         return true;
     }
 
     JSObject *wrapped = cx->runtime->sameCompartmentWrapObjectCallback(cx, obj);
     if (!wrapped)
         return false;
-    vp->setObject(*wrapped);
+    vp.setObject(*wrapped);
     return true;
 }
 
 bool
 JSCompartment::putWrapper(const CrossCompartmentKey &wrapped, const js::Value &wrapper)
 {
     JS_ASSERT(wrapped.wrapped);
     JS_ASSERT(!IsPoisonedPtr(wrapped.wrapped));
@@ -235,44 +235,43 @@ JSCompartment::putWrapper(const CrossCom
     JS_ASSERT_IF(wrapped.kind == CrossCompartmentKey::StringWrapper, wrapper.isString());
     JS_ASSERT_IF(wrapped.kind != CrossCompartmentKey::StringWrapper, wrapper.isObject());
     // todo: uncomment when bug 815999 is fixed:
     // JS_ASSERT(!wrapped.wrapped->isMarked(gc::GRAY));
     return crossCompartmentWrappers.put(wrapped, wrapper);
 }
 
 bool
-JSCompartment::wrap(JSContext *cx, Value *vp, JSObject *existingArg)
+JSCompartment::wrap(JSContext *cx, MutableHandleValue vp, HandleObject existingArg)
 {
-    RootedObject existing(cx, existingArg);
     JS_ASSERT(cx->compartment == this);
-    JS_ASSERT_IF(existing, existing->compartment() == cx->compartment);
-    JS_ASSERT_IF(existing, vp->isObject());
-    JS_ASSERT_IF(existing, IsDeadProxyObject(existing));
+    JS_ASSERT_IF(existingArg, existingArg->compartment() == cx->compartment);
+    JS_ASSERT_IF(existingArg, vp.isObject());
+    JS_ASSERT_IF(existingArg, IsDeadProxyObject(existingArg));
 
     unsigned flags = 0;
 
     JS_CHECK_CHROME_RECURSION(cx, return false);
 
 #ifdef DEBUG
     struct AutoDisableProxyCheck {
         JSRuntime *runtime;
         AutoDisableProxyCheck(JSRuntime *rt) : runtime(rt) {
             runtime->gcDisableStrictProxyCheckingCount++;
         }
         ~AutoDisableProxyCheck() { runtime->gcDisableStrictProxyCheckingCount--; }
     } adpc(rt);
 #endif
 
     /* Only GC things have to be wrapped or copied. */
-    if (!vp->isMarkable())
+    if (!vp.isMarkable())
         return true;
 
-    if (vp->isString()) {
-        JSString *str = vp->toString();
+    if (vp.isString()) {
+        RootedString str(cx, vp.toString());
 
         /* If the string is already in this compartment, we are done. */
         if (str->zone() == zone())
             return true;
 
         /* If the string is an atom, we don't have to copy. */
         if (str->isAtom()) {
             JS_ASSERT(str->zone() == cx->runtime->atomsCompartment->zone());
@@ -285,77 +284,74 @@ JSCompartment::wrap(JSContext *cx, Value
      * object, but in that case a wrapped global object would have a NULL
      * parent without being a proper global object (JSCLASS_IS_GLOBAL). Instead,
      * we parent all wrappers to the global object in their home compartment.
      * This loses us some transparency, and is generally very cheesy.
      */
     HandleObject global = cx->global();
 
     /* Unwrap incoming objects. */
-    if (vp->isObject()) {
-        RootedObject obj(cx, &vp->toObject());
+    if (vp.isObject()) {
+        RootedObject obj(cx, &vp.toObject());
 
         if (obj->compartment() == this)
             return WrapForSameCompartment(cx, obj, vp);
 
         /* Translate StopIteration singleton. */
-        if (obj->isStopIteration()) {
-            RootedValue vvp(cx, *vp);
-            bool result = js_FindClassObject(cx, JSProto_StopIteration, &vvp);
-            *vp = vvp;
-            return result;
-        }
+        if (obj->isStopIteration())
+            return js_FindClassObject(cx, JSProto_StopIteration, vp);
 
         /* Unwrap the object, but don't unwrap outer windows. */
-        obj = UnwrapObject(&vp->toObject(), /* stopAtOuter = */ true, &flags);
+        obj = UnwrapObject(obj, /* stopAtOuter = */ true, &flags);
 
         if (obj->compartment() == this)
             return WrapForSameCompartment(cx, obj, vp);
 
         if (cx->runtime->preWrapObjectCallback) {
             obj = cx->runtime->preWrapObjectCallback(cx, global, obj, flags);
             if (!obj)
                 return false;
         }
 
         if (obj->compartment() == this)
             return WrapForSameCompartment(cx, obj, vp);
-        vp->setObject(*obj);
+        vp.setObject(*obj);
 
 #ifdef DEBUG
         {
             JSObject *outer = GetOuterObject(cx, obj);
             JS_ASSERT(outer && outer == obj);
         }
 #endif
     }
 
-    RootedValue key(cx, *vp);
+    RootedValue key(cx, vp);
 
     /* If we already have a wrapper for this value, use it. */
     if (WrapperMap::Ptr p = crossCompartmentWrappers.lookup(key)) {
-        *vp = p->value;
-        if (vp->isObject()) {
-            RootedObject obj(cx, &vp->toObject());
+        vp.set(p->value);
+        if (vp.isObject()) {
+            RootedObject obj(cx, &vp.toObject());
             JS_ASSERT(obj->isCrossCompartmentWrapper());
             JS_ASSERT(obj->getParent() == global);
         }
         return true;
     }
 
-    if (vp->isString()) {
-        RootedValue orig(cx, *vp);
-        Rooted<JSStableString *> str(cx, vp->toString()->ensureStable(cx));
+    if (vp.isString()) {
+        Rooted<JSStableString *> str(cx, vp.toString()->ensureStable(cx));
         if (!str)
             return false;
+
         RootedString wrapped(cx, js_NewStringCopyN<CanGC>(cx, str->chars().get(), str->length()));
         if (!wrapped)
             return false;
-        vp->setString(wrapped);
-        if (!putWrapper(orig, *vp))
+
+        vp.setString(wrapped);
+        if (!putWrapper(key, vp))
             return false;
 
         if (str->zone()->isGCMarking()) {
             /*
              * All string wrappers are dropped when collection starts, but we
              * just created a new one.  Mark the wrapped string to stop it being
              * finalized, because if it was then the pointer in this
              * compartment's wrapper map would be left dangling.
@@ -363,19 +359,19 @@ JSCompartment::wrap(JSContext *cx, Value
             JSString *tmp = str;
             MarkStringUnbarriered(&rt->gcMarker, &tmp, "wrapped string");
             JS_ASSERT(tmp == str);
         }
 
         return true;
     }
 
-    RootedObject obj(cx, &vp->toObject());
-
-    JSObject *proto = Proxy::LazyProto;
+    RootedObject proto(cx, Proxy::LazyProto);
+    RootedObject obj(cx, &vp.toObject());
+    RootedObject existing(cx, existingArg);
     if (existing) {
         /* Is it possible to reuse |existing|? */
         if (!existing->getTaggedProto().isLazy() ||
             existing->getClass() != &ObjectProxyClass ||
             existing->getParent() != global ||
             obj->isCallable())
         {
             existing = NULL;
@@ -391,99 +387,109 @@ JSCompartment::wrap(JSContext *cx, Value
     wrapper = cx->runtime->wrapObjectCallback(cx, existing, obj, proto, global, flags);
     if (!wrapper)
         return false;
 
     // We maintain the invariant that the key in the cross-compartment wrapper
     // map is always directly wrapped by the value.
     JS_ASSERT(Wrapper::wrappedObject(wrapper) == &key.get().toObject());
 
-    vp->setObject(*wrapper);
-
-    if (!putWrapper(key, *vp))
-        return false;
-
-    return true;
+    vp.setObject(*wrapper);
+    return putWrapper(key, vp);
 }
 
 bool
 JSCompartment::wrap(JSContext *cx, JSString **strp)
 {
     RootedValue value(cx, StringValue(*strp));
-    if (!wrap(cx, value.address()))
+    if (!wrap(cx, &value))
         return false;
     *strp = value.get().toString();
     return true;
 }
 
 bool
 JSCompartment::wrap(JSContext *cx, HeapPtrString *strp)
 {
     RootedValue value(cx, StringValue(*strp));
-    if (!wrap(cx, value.address()))
+    if (!wrap(cx, &value))
         return false;
     *strp = value.get().toString();
     return true;
 }
 
 bool
-JSCompartment::wrap(JSContext *cx, JSObject **objp, JSObject *existing)
+JSCompartment::wrap(JSContext *cx, JSObject **objp, JSObject *existingArg)
 {
     if (!*objp)
         return true;
     RootedValue value(cx, ObjectValue(**objp));
-    if (!wrap(cx, value.address(), existing))
+    RootedObject existing(cx, existingArg);
+    if (!wrap(cx, &value, existing))
         return false;
     *objp = &value.get().toObject();
     return true;
 }
 
 bool
 JSCompartment::wrapId(JSContext *cx, jsid *idp)
 {
     if (JSID_IS_INT(*idp))
         return true;
     RootedValue value(cx, IdToValue(*idp));
-    if (!wrap(cx, value.address()))
+    if (!wrap(cx, &value))
         return false;
     RootedId id(cx);
     if (!ValueToId<CanGC>(cx, value.get(), &id))
         return false;
 
     *idp = id;
     return true;
 }
 
 bool
 JSCompartment::wrap(JSContext *cx, PropertyOp *propp)
 {
-    Value v = CastAsObjectJsval(*propp);
-    if (!wrap(cx, &v))
+    RootedValue value(cx, CastAsObjectJsval(*propp));
+    if (!wrap(cx, &value))
         return false;
-    *propp = CastAsPropertyOp(v.toObjectOrNull());
+    *propp = CastAsPropertyOp(value.toObjectOrNull());
     return true;
 }
 
 bool
 JSCompartment::wrap(JSContext *cx, StrictPropertyOp *propp)
 {
-    Value v = CastAsObjectJsval(*propp);
-    if (!wrap(cx, &v))
+    RootedValue value(cx, CastAsObjectJsval(*propp));
+    if (!wrap(cx, &value))
         return false;
-    *propp = CastAsStrictPropertyOp(v.toObjectOrNull());
+    *propp = CastAsStrictPropertyOp(value.toObjectOrNull());
     return true;
 }
 
 bool
 JSCompartment::wrap(JSContext *cx, PropertyDescriptor *desc)
 {
-    return wrap(cx, &desc->obj) &&
-           (!(desc->attrs & JSPROP_GETTER) || wrap(cx, &desc->getter)) &&
-           (!(desc->attrs & JSPROP_SETTER) || wrap(cx, &desc->setter)) &&
-           wrap(cx, &desc->value);
+    if (!wrap(cx, &desc->obj))
+        return false;
+
+    if (desc->attrs & JSPROP_GETTER) {
+        if (!wrap(cx, &desc->getter))
+            return false;
+    }
+    if (desc->attrs & JSPROP_SETTER) {
+        if (!wrap(cx, &desc->setter))
+            return false;
+    }
+
+    RootedValue value(cx, desc->value);
+    if (!wrap(cx, &value))
+        return false;
+    desc->value = value.get();
+    return true;
 }
 
 bool
 JSCompartment::wrap(JSContext *cx, AutoIdVector &props)
 {
     jsid *vector = props.begin();
     int length = props.length();
     for (size_t n = 0; n < size_t(length); ++n) {
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -433,17 +433,17 @@ struct JSCompartment : private JS::shado
     JSCompartment(JSRuntime *rt);
     ~JSCompartment();
 
     bool init(JSContext *cx);
 
     /* Mark cross-compartment wrappers. */
     void markCrossCompartmentWrappers(JSTracer *trc);
 
-    bool wrap(JSContext *cx, js::Value *vp, JSObject *existing = NULL);
+    bool wrap(JSContext *cx, JS::MutableHandleValue vp, JS::HandleObject existing = JS::NullPtr());
     bool wrap(JSContext *cx, JSString **strp);
     bool wrap(JSContext *cx, js::HeapPtrString *strp);
     bool wrap(JSContext *cx, JSObject **objp, JSObject *existing = NULL);
     bool wrapId(JSContext *cx, jsid *idp);
     bool wrap(JSContext *cx, js::PropertyOp *op);
     bool wrap(JSContext *cx, js::StrictPropertyOp *op);
     bool wrap(JSContext *cx, js::PropertyDescriptor *desc);
     bool wrap(JSContext *cx, js::AutoIdVector &props);
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -171,17 +171,17 @@ fun_getProperty(JSContext *cx, HandleObj
     if (JSID_IS_ATOM(id, cx->names().caller)) {
         ++iter;
         if (iter.done() || !iter.isFunctionFrame()) {
             JS_ASSERT(vp.isNull());
             return true;
         }
 
         vp.set(iter.calleev());
-        if (!cx->compartment->wrap(cx, vp.address()))
+        if (!cx->compartment->wrap(cx, vp))
             return false;
 
         /*
          * Censor the caller if we don't have full access to it.
          */
         JSObject &caller = vp.toObject();
         if (caller.isWrapper() && !Wrapper::wrapperHandler(&caller)->isSafeToUnwrap()) {
             vp.setNull();
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1656,17 +1656,17 @@ JS_CopyPropertiesFrom(JSContext *cx, JSO
         PropertyOp getter = shape->getter();
         StrictPropertyOp setter = shape->setter();
         AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter);
         if ((attrs & JSPROP_GETTER) && !cx->compartment->wrap(cx, &getter))
             return false;
         if ((attrs & JSPROP_SETTER) && !cx->compartment->wrap(cx, &setter))
             return false;
         v = shape->hasSlot() ? obj->getSlot(shape->slot()) : UndefinedValue();
-        if (!cx->compartment->wrap(cx, v.address()))
+        if (!cx->compartment->wrap(cx, &v))
             return false;
         id = shape->propid();
         if (!JSObject::defineGeneric(cx, target, id, v, getter, setter, attrs))
             return false;
     }
     return true;
 }
 
@@ -1681,18 +1681,19 @@ CopySlots(JSContext *cx, HandleObject fr
         (Wrapper::wrapperHandler(from)->flags() &
          Wrapper::CROSS_COMPARTMENT)) {
         to->setSlot(0, from->getSlot(0));
         to->setSlot(1, from->getSlot(1));
         n = 2;
     }
 
     size_t span = JSCLASS_RESERVED_SLOTS(from->getClass());
+    RootedValue v(cx);
     for (; n < span; ++n) {
-        Value v = from->getSlot(n);
+        v = from->getSlot(n);
         if (!cx->compartment->wrap(cx, &v))
             return false;
         to->setSlot(n, v);
     }
     return true;
 }
 
 JSObject *
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -470,38 +470,51 @@ CrossCompartmentWrapper::hasOwn(JSContex
     PIERCE(cx, wrapper,
            cx->compartment->wrapId(cx, &id),
            Wrapper::hasOwn(cx, wrapper, id, bp),
            NOTHING);
 }
 
 bool
 CrossCompartmentWrapper::get(JSContext *cx, JSObject *wrapperArg, JSObject *receiverArg,
-                             jsid idArg, Value *vp)
+                             jsid idArg, Value *vpArg)
 {
     RootedObject wrapper(cx, wrapperArg);
     RootedObject receiver(cx, receiverArg);
     RootedId id(cx, idArg);
-    PIERCE(cx, wrapper,
-           cx->compartment->wrap(cx, receiver.address()) && cx->compartment->wrapId(cx, id.address()),
-           Wrapper::get(cx, wrapper, receiver, id, vp),
-           cx->compartment->wrap(cx, vp));
+    RootedValue vp(cx, *vpArg);
+
+    {
+        AutoCompartment call(cx, wrappedObject(wrapper));
+        if (!cx->compartment->wrap(cx, receiver.address()) ||
+            !cx->compartment->wrapId(cx, id.address()))
+        {
+            return false;
+        }
+
+        if (!Wrapper::get(cx, wrapper, receiver, id, vp.address()))
+            return false;
+    }
+
+    bool ok = cx->compartment->wrap(cx, &vp);
+    *vpArg = vp.get();
+    return ok;
 }
 
 bool
 CrossCompartmentWrapper::set(JSContext *cx, JSObject *wrapper_, JSObject *receiver_, jsid id_,
-                             bool strict, Value *vp)
+                             bool strict, Value *valueArg)
 {
     RootedObject wrapper(cx, wrapper_), receiver(cx, receiver_);
     RootedId id(cx, id_);
-    RootedValue value(cx, *vp);
+    RootedValue value(cx, *valueArg);
     PIERCE(cx, wrapper,
            cx->compartment->wrap(cx, receiver.address()) &&
            cx->compartment->wrapId(cx, id.address()) &&
-           cx->compartment->wrap(cx, value.address()),
+           cx->compartment->wrap(cx, &value),
            Wrapper::set(cx, wrapper, receiver, id, strict, value.address()),
            NOTHING);
 }
 
 bool
 CrossCompartmentWrapper::keys(JSContext *cx, JSObject *wrapper, AutoIdVector &props)
 {
     PIERCE(cx, wrapper,
@@ -532,19 +545,19 @@ struct AutoCloseIterator
     void clear() { obj = NULL; }
 
   private:
     JSContext *cx;
     RootedObject obj;
 };
 
 static bool
-Reify(JSContext *cx, JSCompartment *origin, Value *vp)
+Reify(JSContext *cx, JSCompartment *origin, MutableHandleValue vp)
 {
-    Rooted<PropertyIteratorObject*> iterObj(cx, &vp->toObject().asPropertyIterator());
+    Rooted<PropertyIteratorObject*> iterObj(cx, &vp.toObject().asPropertyIterator());
     NativeIterator *ni = iterObj->getNativeIterator();
 
     AutoCloseIterator close(cx, iterObj);
 
     /* Wrap the iteratee. */
     RootedObject obj(cx, ni->obj);
     if (!origin->wrap(cx, obj.address()))
         return false;
@@ -569,102 +582,120 @@ Reify(JSContext *cx, JSCompartment *orig
                 return false;
         }
     }
 
     close.clear();
     if (!CloseIterator(cx, iterObj))
         return false;
 
-    RootedValue value(cx, *vp);
-
     if (isKeyIter) {
-        if (!VectorToKeyIterator(cx, obj, ni->flags, keys, &value))
+        if (!VectorToKeyIterator(cx, obj, ni->flags, keys, vp))
             return false;
     } else {
-        if (!VectorToValueIterator(cx, obj, ni->flags, keys, &value))
+        if (!VectorToValueIterator(cx, obj, ni->flags, keys, vp))
             return false;
     }
-
-    *vp = value;
     return true;
 }
 
 bool
-CrossCompartmentWrapper::iterate(JSContext *cx, JSObject *wrapper, unsigned flags, Value *vp)
+CrossCompartmentWrapper::iterate(JSContext *cx, JSObject *wrapperArg, unsigned flags, Value *vpArg)
 {
-    PIERCE(cx, wrapper,
-           NOTHING,
-           Wrapper::iterate(cx, wrapper, flags, vp),
-           CanReify(vp) ? Reify(cx, cx->compartment, vp) : cx->compartment->wrap(cx, vp));
+    RootedObject wrapper(cx, wrapperArg);
+    RootedValue vp(cx, *vpArg);
+
+    {
+        AutoCompartment call(cx, wrappedObject(wrapper));
+        if (!Wrapper::iterate(cx, wrapper, flags, vp.address()))
+            return false;
+    }
+
+    bool ok = CanReify(vp.address())
+              ? Reify(cx, cx->compartment, &vp)
+              : cx->compartment->wrap(cx, &vp);
+    *vpArg = vp.get();
+    return ok;
 }
 
 bool
-CrossCompartmentWrapper::call(JSContext *cx, JSObject *wrapper_, unsigned argc, Value *vp)
+CrossCompartmentWrapper::call(JSContext *cx, JSObject *wrapperArg, unsigned argc, Value *vp)
 {
-    RootedObject wrapper(cx, wrapper_);
-    JSObject *wrapped = wrappedObject(wrapper);
+    RootedObject wrapper(cx, wrapperArg);
+    RootedObject wrapped(cx, wrappedObject(wrapper));
+
+    CallArgs args = CallArgsFromVp(argc, vp);
     {
         AutoCompartment call(cx, wrapped);
 
-        vp[0] = ObjectValue(*wrapped);
-        if (!cx->compartment->wrap(cx, &vp[1]))
+        args.setCallee(ObjectValue(*wrapped));
+        if (!cx->compartment->wrap(cx, args.mutableThisv()))
             return false;
-        Value *argv = JS_ARGV(cx, vp);
-        for (size_t n = 0; n < argc; ++n) {
-            if (!cx->compartment->wrap(cx, &argv[n]))
+
+        for (size_t n = 0; n < args.length(); ++n) {
+            if (!cx->compartment->wrap(cx, args.handleAt(n)))
                 return false;
         }
+
         if (!Wrapper::call(cx, wrapper, argc, vp))
             return false;
     }
-    return cx->compartment->wrap(cx, vp);
+
+    return cx->compartment->wrap(cx, args.rval());
 }
 
 bool
-CrossCompartmentWrapper::construct(JSContext *cx, JSObject *wrapper_, unsigned argc, Value *argv,
-                                   Value *rval)
+CrossCompartmentWrapper::construct(JSContext *cx, JSObject *wrapperArg, unsigned argc, Value *argv,
+                                   Value *rvalArg)
 {
-    RootedObject wrapper(cx, wrapper_);
+    RootedObject wrapper(cx, wrapperArg);
     JSObject *wrapped = wrappedObject(wrapper);
     {
         AutoCompartment call(cx, wrapped);
 
         for (size_t n = 0; n < argc; ++n) {
-            if (!cx->compartment->wrap(cx, &argv[n]))
+            RootedValue arg(cx, argv[n]);
+            if (!cx->compartment->wrap(cx, &arg))
                 return false;
+            argv[n] = arg;
         }
-        if (!Wrapper::construct(cx, wrapper, argc, argv, rval))
+        if (!Wrapper::construct(cx, wrapper, argc, argv, rvalArg))
             return false;
     }
-    return cx->compartment->wrap(cx, rval);
+    RootedValue rval(cx, *rvalArg);
+    bool ok = cx->compartment->wrap(cx, &rval);
+    *rvalArg = rval;
+    return ok;
 }
 
 bool
 CrossCompartmentWrapper::nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl,
                                     CallArgs srcArgs)
 {
-    Rooted<JSObject*> wrapper(cx, &srcArgs.thisv().toObject());
+    RootedObject wrapper(cx, &srcArgs.thisv().toObject());
     JS_ASSERT(srcArgs.thisv().isMagic(JS_IS_CONSTRUCTING) ||
               !UnwrapObject(wrapper)->isCrossCompartmentWrapper());
 
     RootedObject wrapped(cx, wrappedObject(wrapper));
     {
         AutoCompartment call(cx, wrapped);
         InvokeArgsGuard dstArgs;
         if (!cx->stack.pushInvokeArgs(cx, srcArgs.length(), &dstArgs))
             return false;
 
         Value *src = srcArgs.base();
         Value *srcend = srcArgs.array() + srcArgs.length();
         Value *dst = dstArgs.base();
+
+        RootedValue source(cx);
         for (; src < srcend; ++src, ++dst) {
-            *dst = *src;
-            if (!cx->compartment->wrap(cx, dst))
+            source = *src;
+            if (!cx->compartment->wrap(cx, &source))
                 return false;
+            *dst = source.get();
 
             // Handle |this| specially. When we rewrap on the other side of the
             // membrane, we might apply a same-compartment security wrapper that
             // will stymie this whole process. If that happens, unwrap the wrapper.
             // This logic can go away when same-compartment security wrappers go away.
             if ((src == srcArgs.base() + 1) && dst->isObject()) {
                 JSObject *thisObj = &dst->toObject();
                 if (thisObj->isWrapper() &&
@@ -677,24 +708,24 @@ CrossCompartmentWrapper::nativeCall(JSCo
         }
 
         if (!CallNonGenericMethod(cx, test, impl, dstArgs))
             return false;
 
         srcArgs.rval().set(dstArgs.rval());
         dstArgs.pop();
     }
-    return cx->compartment->wrap(cx, srcArgs.rval().address());
+    return cx->compartment->wrap(cx, srcArgs.rval());
 }
 
 bool
 CrossCompartmentWrapper::hasInstance(JSContext *cx, HandleObject wrapper, MutableHandleValue v, bool *bp)
 {
     AutoCompartment call(cx, wrappedObject(wrapper));
-    if (!cx->compartment->wrap(cx, v.address()))
+    if (!cx->compartment->wrap(cx, v))
         return false;
     return Wrapper::hasInstance(cx, wrapper, v, bp);
 }
 
 JSString *
 CrossCompartmentWrapper::obj_toString(JSContext *cx, JSObject *wrapper)
 {
     JSString *str = NULL;
@@ -727,21 +758,25 @@ CrossCompartmentWrapper::fun_toString(JS
 bool
 CrossCompartmentWrapper::regexp_toShared(JSContext *cx, JSObject *wrapper, RegExpGuard *g)
 {
     AutoCompartment call(cx, wrappedObject(wrapper));
     return Wrapper::regexp_toShared(cx, wrapper, g);
 }
 
 bool
-CrossCompartmentWrapper::defaultValue(JSContext *cx, JSObject *wrapper, JSType hint, Value *vp)
+CrossCompartmentWrapper::defaultValue(JSContext *cx, JSObject *wrapper, JSType hint, Value *vpArg)
 {
-    if (!Wrapper::defaultValue(cx, wrapper, hint, vp))
+    if (!Wrapper::defaultValue(cx, wrapper, hint, vpArg))
         return false;
-    return cx->compartment->wrap(cx, vp);
+
+    RootedValue vp(cx, *vpArg);
+    bool ok = cx->compartment->wrap(cx, &vp);
+    *vpArg = vp;
+    return ok;
 }
 
 bool
 CrossCompartmentWrapper::getPrototypeOf(JSContext *cx, JSObject *proxy, JSObject **protop)
 {
     assertSameCompartment(cx, proxy);
 
     if (!proxy->getTaggedProto().isLazy()) {
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -2532,17 +2532,17 @@ EvalInContext(JSContext *cx, unsigned ar
         JS_SET_RVAL(cx, vp, OBJECT_TO_JSVAL(sobj));
         return true;
     }
 
     RootedScript script(cx);
     unsigned lineno;
 
     JS_DescribeScriptedCaller(cx, script.address(), &lineno);
-    jsval rval;
+    RootedValue rval(cx);
     {
         Maybe<JSAutoCompartment> ac;
         unsigned flags;
         JSObject *unwrapped = UnwrapObject(sobj, true, &flags);
         if (flags & Wrapper::CROSS_COMPARTMENT) {
             sobj = unwrapped;
             ac.construct(cx, sobj);
         }
@@ -2552,17 +2552,17 @@ EvalInContext(JSContext *cx, unsigned ar
             return false;
         if (!(sobj->getClass()->flags & JSCLASS_IS_GLOBAL)) {
             JS_ReportError(cx, "Invalid scope argument to evalcx");
             return false;
         }
         if (!JS_EvaluateUCScript(cx, sobj, src, srclen,
                                  script->filename,
                                  lineno,
-                                 &rval)) {
+                                 rval.address())) {
             return false;
         }
     }
 
     if (!cx->compartment->wrap(cx, &rval))
         return false;
 
     JS_SET_RVAL(cx, vp, rval);
--- a/js/src/shell/jsheaptools.cpp
+++ b/js/src/shell/jsheaptools.cpp
@@ -476,21 +476,21 @@ ReferenceFinder::Path::computeName(JSCon
         }
     }
     JS_ASSERT(next + 1 == path + size);
 
     return path;
 }
 
 bool
-ReferenceFinder::addReferrer(jsval referrer_, Path *path)
+ReferenceFinder::addReferrer(jsval referrerArg, Path *path)
 {
-    Rooted<jsval> referrer(context, referrer_);
+    RootedValue referrer(context, referrerArg);
 
-    if (!context->compartment->wrap(context, referrer.address()))
+    if (!context->compartment->wrap(context, &referrer))
         return false;
 
     ScopedJSFreePtr<char> pathName(path->computeName(context));
     if (!pathName)
         return false;
 
     /* Find the property of the results object named |pathName|. */
     RootedValue valRoot(context);
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -704,17 +704,17 @@ Debugger::wrapDebuggeeValue(JSContext *c
                     objects.remove(obj);
                     js_ReportOutOfMemory(cx);
                     return false;
                 }
             }
 
             vp.setObject(*dobj);
         }
-    } else if (!cx->compartment->wrap(cx, vp.address())) {
+    } else if (!cx->compartment->wrap(cx, vp)) {
         vp.setUndefined();
         return false;
     }
 
     return true;
 }
 
 bool
@@ -879,23 +879,24 @@ Debugger::parseResumptionValue(Maybe<Aut
     if (!okResumption) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_DEBUG_BAD_RESUMPTION);
         return handleUncaughtException(ac, vp, callHook);
     }
 
     RootedValue v(cx, *vp);
     if (!js_NativeGet(cx, obj, obj, shape, 0, &v) || !unwrapDebuggeeValue(cx, &v))
         return handleUncaughtException(ac, v.address(), callHook);
-    *vp = v;
 
     ac.destroy();
-    if (!cx->compartment->wrap(cx, vp)) {
+    if (!cx->compartment->wrap(cx, &v)) {
         vp->setUndefined();
         return JSTRAP_ERROR;
     }
+    *vp = v;
+
     return shape->propid() == returnId ? JSTRAP_RETURN : JSTRAP_THROW;
 }
 
 bool
 CallMethodIfPresent(JSContext *cx, HandleObject obj, const char *name, int argc, Value *argv,
                     Value *rval)
 {
     rval->setUndefined();
@@ -3775,17 +3776,17 @@ DebuggerGenericEval(JSContext *cx, const
         /* TODO - This should probably be a Call object, like ES5 strict eval. */
         env = NewObjectWithGivenProto(cx, &ObjectClass, NULL, env);
         if (!env)
             return false;
         RootedId id(cx);
         for (size_t i = 0; i < keys.length(); i++) {
             id = keys[i];
             MutableHandleValue val = values.handleAt(i);
-            if (!cx->compartment->wrap(cx, val.address()) ||
+            if (!cx->compartment->wrap(cx, val) ||
                 !DefineNativeProperty(cx, env, id, val, NULL, NULL, 0, 0, 0))
             {
                 return false;
             }
         }
     }
 
     /* Run the code and produce the completion value. */
@@ -4187,17 +4188,17 @@ DebuggerObject_getOwnPropertyNames(JSCon
          jsid id = keys[i];
          if (JSID_IS_INT(id)) {
              JSString *str = Int32ToString<CanGC>(cx, JSID_TO_INT(id));
              if (!str)
                  return false;
              vals[i].setString(str);
          } else if (JSID_IS_ATOM(id)) {
              vals[i].setString(JSID_TO_STRING(id));
-             if (!cx->compartment->wrap(cx, &vals[i]))
+             if (!cx->compartment->wrap(cx, vals.handleAt(i)))
                  return false;
          } else {
              vals[i].setObject(*JSID_TO_OBJECT(id));
              if (!dbg->wrapDebuggeeValue(cx, vals.handleAt(i)))
                  return false;
          }
     }
 
@@ -4312,21 +4313,21 @@ DebuggerObject_defineProperties(JSContex
 /*
  * This does a non-strict delete, as a matter of API design. The case where the
  * property is non-configurable isn't necessarily exceptional here.
  */
 static JSBool
 DebuggerObject_deleteProperty(JSContext *cx, unsigned argc, Value *vp)
 {
     THIS_DEBUGOBJECT_OWNER_REFERENT(cx, argc, vp, "deleteProperty", args, dbg, obj);
-    RootedValue nameArg(cx, argc > 0 ? args[0] : UndefinedValue());
+    RootedValue nameArg(cx, args.get(0));
 
     Maybe<AutoCompartment> ac;
     ac.construct(cx, obj);
-    if (!cx->compartment->wrap(cx, nameArg.address()))
+    if (!cx->compartment->wrap(cx, &nameArg))
         return false;
 
     ErrorCopier ec(ac, dbg->toJSObject());
     return JSObject::deleteByValue(cx, obj, nameArg, args.rval(), false);
 }
 
 enum SealHelperOp { Seal, Freeze, PreventExtensions };
 
@@ -4471,21 +4472,25 @@ ApplyOrCall(JSContext *cx, unsigned argc
     }
 
     /*
      * Enter the debuggee compartment and rewrap all input value for that compartment.
      * (Rewrapping always takes place in the destination compartment.)
      */
     Maybe<AutoCompartment> ac;
     ac.construct(cx, obj);
-    if (!cx->compartment->wrap(cx, calleev.address()) || !cx->compartment->wrap(cx, thisv.address()))
+    if (!cx->compartment->wrap(cx, &calleev) || !cx->compartment->wrap(cx, &thisv))
         return false;
+
+    RootedValue arg(cx);
     for (unsigned i = 0; i < callArgc; i++) {
-        if (!cx->compartment->wrap(cx, &callArgv[i]))
-            return false;
+        arg = callArgv[i];
+        if (!cx->compartment->wrap(cx, &arg))
+             return false;
+        callArgv[i] = arg;
     }
 
     /*
      * Call the function. Use receiveCompletionValue to return to the debugger
      * compartment and populate args.rval().
      */
     Value rval;
     bool ok = Invoke(cx, thisv, calleev, callArgc, callArgv, &rval);
@@ -4513,17 +4518,17 @@ DebuggerObject_makeDebuggeeValue(JSConte
     RootedValue arg0(cx, args[0]);
 
     /* Non-objects are already debuggee values. */
     if (arg0.isObject()) {
         // Enter this Debugger.Object's referent's compartment, and wrap the
         // argument as appropriate for references from there.
         {
             AutoCompartment ac(cx, referent);
-            if (!cx->compartment->wrap(cx, arg0.address()))
+            if (!cx->compartment->wrap(cx, &arg0))
                 return false;
         }
 
         // Back in the debugger's compartment, produce a new Debugger.Object
         // instance referring to the wrapped argument.
         if (!dbg->wrapDebuggeeValue(cx, &arg0))
             return false;
     }
@@ -4903,17 +4908,17 @@ DebuggerEnv_setVariable(JSContext *cx, u
 
     RootedValue v(cx, args[1]);
     if (!dbg->unwrapDebuggeeValue(cx, &v))
         return false;
 
     {
         Maybe<AutoCompartment> ac;
         ac.construct(cx, env);
-        if (!cx->compartment->wrapId(cx, id.address()) || !cx->compartment->wrap(cx, v.address()))
+        if (!cx->compartment->wrapId(cx, id.address()) || !cx->compartment->wrap(cx, &v))
             return false;
 
         /* This can trigger setters. */
         ErrorCopier ec(ac, dbg->toJSObject());
 
         /* Make sure the environment actually has the specified binding. */
         bool has;
         if (!JSObject::hasProperty(cx, env, id, &has))
--- a/js/src/vm/ObjectImpl.cpp
+++ b/js/src/vm/ObjectImpl.cpp
@@ -133,22 +133,26 @@ PropDesc::wrapInto(JSContext *cx, Handle
 
     JSCompartment *comp = cx->compartment;
 
     *wrappedId = id;
     if (!comp->wrapId(cx, wrappedId))
         return false;
 
     *desc = *this;
-    if (!comp->wrap(cx, &desc->value_))
+    RootedValue value(cx, desc->value_);
+    RootedValue get(cx, desc->get_);
+    RootedValue set(cx, desc->set_);
+
+    if (!comp->wrap(cx, &value) || !comp->wrap(cx, &get) || !comp->wrap(cx, &set))
         return false;
-    if (!comp->wrap(cx, &desc->get_))
-        return false;
-    if (!comp->wrap(cx, &desc->set_))
-        return false;
+
+    desc->value_ = value;
+    desc->get_ = get;
+    desc->set_ = set;
     return !obj->isProxy() || desc->makeObject(cx);
 }
 
 static ObjectElements emptyElementsHeader(0, 0);
 
 /* Objects with no elements share one empty set of elements. */
 HeapSlot *js::emptyObjectElements =
     reinterpret_cast<HeapSlot *>(uintptr_t(&emptyElementsHeader) + sizeof(ObjectElements));