Bug 861887 - GC: Rooting in XrayWrapper.cpp - part 2 r=bholley
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 24 Apr 2013 10:45:41 +0100
changeset 129735 da6a56be2e050d677edc2cd15cfdca8be7d1be86
parent 129734 5e96a99f7c5dacc73d017d61e08e877ff9082fbd
child 129736 c307cb8bffec6ff6389cc64cfef648edb33617ec
push id24587
push userryanvm@gmail.com
push dateWed, 24 Apr 2013 20:17:47 +0000
treeherdermozilla-central@07e17dd7813b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs861887
milestone23.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 861887 - GC: Rooting in XrayWrapper.cpp - part 2 r=bholley
js/xpconnect/wrappers/XrayWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.h
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -61,19 +61,19 @@ GetXrayType(JSObject *obj)
     if (IS_WRAPPER_CLASS(clasp) || clasp->ext.innerObject) {
         NS_ASSERTION(clasp->ext.innerObject || IS_WN_WRAPPER_OBJECT(obj),
                      "We forgot to Morph a slim wrapper!");
         return XrayForWrappedNative;
     }
     return NotXray;
 }
 
-ResolvingId::ResolvingId(JSObject *wrapper, jsid id)
-    : mId(id),
-    mHolder(getHolderObject(wrapper)),
+ResolvingId::ResolvingId(JSContext *cx, JSObject *wrapper, jsid id)
+  : mId(cx, id),
+    mHolder(cx, getHolderObject(wrapper)),
     mPrev(getResolvingId(mHolder)),
     mXrayShadowing(false)
 {
     js::SetReservedSlot(mHolder, JSSLOT_RESOLVING, js::PrivateValue(this));
 }
 
 ResolvingId::~ResolvingId()
 {
@@ -115,20 +115,20 @@ ResolvingId::getHolderObject(JSObject *w
 }
 
 ResolvingId *
 ResolvingId::getResolvingIdFromWrapper(JSObject *wrapper)
 {
     return getResolvingId(getHolderObject(wrapper));
 }
 
-class ResolvingIdDummy
+class MOZ_STACK_CLASS ResolvingIdDummy
 {
 public:
-    ResolvingIdDummy(JSObject *wrapper, jsid id)
+    ResolvingIdDummy(JSContext *cx, JSObject *wrapper, jsid id)
     {
     }
 };
 
 class XrayTraits
 {
 public:
     static JSObject* getTargetObject(JSObject *wrapper) {
@@ -189,18 +189,18 @@ public:
 
     virtual bool resolveNativeProperty(JSContext *cx, HandleObject wrapper,
                                        HandleObject holder, HandleId id,
                                        JSPropertyDescriptor *desc, unsigned flags);
     virtual bool resolveOwnProperty(JSContext *cx, Wrapper &jsWrapper, HandleObject wrapper,
                                     HandleObject holder, HandleId id,
                                     JSPropertyDescriptor *desc, unsigned flags);
     static bool defineProperty(JSContext *cx, HandleObject wrapper, HandleId id,
-                               PropertyDescriptor *desc, PropertyDescriptor &existingDesc,
-                               bool *defined);
+                               PropertyDescriptor *desc,
+                               Handle<PropertyDescriptor> existingDesc, bool *defined);
     static bool enumerateNames(JSContext *cx, HandleObject wrapper, unsigned flags,
                                AutoIdVector &props);
     static bool call(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args);
     static bool construct(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args);
 
     static bool isResolving(JSContext *cx, JSObject *holder, jsid id);
 
     static bool resolveDOMCollectionProperty(JSContext *cx, HandleObject wrapper,
@@ -233,18 +233,18 @@ public:
 
     virtual bool resolveNativeProperty(JSContext *cx, HandleObject wrapper,
                                        HandleObject holder, HandleId id,
                                        JSPropertyDescriptor *desc, unsigned flags);
     virtual bool resolveOwnProperty(JSContext *cx, Wrapper &jsWrapper, HandleObject wrapper,
                                     HandleObject holder, HandleId id,
                                     JSPropertyDescriptor *desc, unsigned flags);
     static bool defineProperty(JSContext *cx, HandleObject wrapper, HandleId id,
-                               PropertyDescriptor *desc, PropertyDescriptor &existingDesc,
-                               bool *defined);
+                               PropertyDescriptor *desc,
+                               Handle<PropertyDescriptor> existingDesc, bool *defined);
     static bool enumerateNames(JSContext *cx, HandleObject wrapper, unsigned flags,
                                AutoIdVector &props);
     static bool call(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args);
     static bool construct(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args);
 
     static bool isResolving(JSContext *cx, JSObject *holder, jsid id)
     {
         return false;
@@ -1057,18 +1057,17 @@ XPCWrappedNativeXrayTraits::resolveOwnPr
     // lookups are performed in a certain order, but we can probably live with
     // that until XPCWN Xrays go away with the new DOM bindings.
     return JS_GetPropertyDescriptorById(cx, holder, id, 0, desc);
 }
 
 bool
 XPCWrappedNativeXrayTraits::defineProperty(JSContext *cx, HandleObject wrapper, HandleId id,
                                            PropertyDescriptor *desc,
-                                           PropertyDescriptor &existingDesc,
-                                           bool *defined)
+                                           Handle<PropertyDescriptor> existingDesc, bool *defined)
 {
     *defined = false;
     JSObject *holder = singleton.ensureHolder(cx, wrapper);
     if (isResolving(cx, holder, id)) {
         if (!(desc->attrs & (JSPROP_GETTER | JSPROP_SETTER))) {
             if (!desc->getter)
                 desc->getter = holder_get;
             if (!desc->setter)
@@ -1212,23 +1211,23 @@ DOMXrayTraits::resolveOwnProperty(JSCont
     NS_ASSERTION(!desc->obj || desc->obj == wrapper,
                  "What did we resolve this on?");
 
     return true;
 }
 
 bool
 DOMXrayTraits::defineProperty(JSContext *cx, HandleObject wrapper, HandleId id,
-                              PropertyDescriptor *desc, PropertyDescriptor &existingDesc,
-                              bool *defined)
+                              PropertyDescriptor *desc,
+                              Handle<PropertyDescriptor> existingDesc, bool *defined)
 {
-    if (!existingDesc.obj)
+    if (!existingDesc.obj())
         return true;
 
-    JSObject *obj= getTargetObject(wrapper);
+    JSObject *obj = getTargetObject(wrapper);
     if (!js::IsProxy(obj))
         return true;
 
     *defined = true;
     return js::GetProxyHandler(obj)->defineProperty(cx, wrapper, id, desc);
 }
 
 bool
@@ -1240,18 +1239,18 @@ DOMXrayTraits::enumerateNames(JSContext 
 }
 
 bool
 DOMXrayTraits::call(JSContext *cx, HandleObject wrapper, const JS::CallArgs &args)
 {
     RootedObject obj(cx, getTargetObject(wrapper));
     {
         JSAutoCompartment ac(cx, obj);
-        Value thisv = args.thisv();
-        if (!JS_WrapValue(cx, &thisv))
+        RootedValue thisv(cx, args.thisv());
+        if (!JS_WrapValue(cx, thisv.address()))
             return false;
         args.setThis(thisv);
         for (size_t i = 0; i < args.length(); ++i) {
             if (!JS_WrapValue(cx, &args[i]))
                 return false;
         }
         if (!Call(cx, thisv, obj, args.length(), args.array(), args.rval().address()))
             return false;
@@ -1361,17 +1360,17 @@ HasNativeProperty(JSContext *cx, HandleO
     NS_ENSURE_TRUE(holder, false);
     *hasProp = false;
     JSPropertyDescriptor desc;
     Wrapper *handler = Wrapper::wrapperHandler(wrapper);
 
     // Try resolveOwnProperty.
     Maybe<ResolvingId> resolvingId;
     if (traits == &XPCWrappedNativeXrayTraits::singleton)
-        resolvingId.construct(wrapper, id);
+        resolvingId.construct(cx, wrapper, id);
     if (!traits->resolveOwnProperty(cx, *handler, wrapper, holder, id, &desc, 0))
         return false;
     if (desc.obj) {
         *hasProp = true;
         return true;
     }
 
     // Try the holder.
@@ -1478,17 +1477,17 @@ XrayWrapper<Base, Traits>::isExtensible(
     // even if that script freezes the reflector, we don't want to make that
     // visible to the caller. DOM reflectors are always extensible by default,
     // so we can just return true here.
     return true;
 }
 
 template <typename Base, typename Traits>
 bool
-XrayWrapper<Base, Traits>::preventExtensions(JSContext *cx, JS::Handle<JSObject*> wrapper)
+XrayWrapper<Base, Traits>::preventExtensions(JSContext *cx, HandleObject wrapper)
 {
     // See above.
     JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CANT_CHANGE_EXTENSIBILITY);
     return false;
 }
 
 template <typename Base, typename Traits>
 bool
@@ -1497,17 +1496,17 @@ XrayWrapper<Base, Traits>::getPropertyDe
 {
     assertEnteredPolicy(cx, wrapper, id);
     RootedObject holder(cx, Traits::singleton.ensureHolder(cx, wrapper));
     if (Traits::isResolving(cx, holder, id)) {
         desc->obj = NULL;
         return true;
     }
 
-    typename Traits::ResolvingIdImpl resolving(wrapper, id);
+    typename Traits::ResolvingIdImpl resolving(cx, wrapper, id);
 
     if (XrayUtils::NeedsWaive(cx, wrapper, id)) {
         RootedObject waived(cx, WrapperFactory::WaiveXray(cx, wrapper));
         if (!waived || !JS_WrapObject(cx, waived.address()))
             return false;
         return JS_GetPropertyDescriptorById(cx, waived, id, flags, desc);
     }
 
@@ -1647,17 +1646,17 @@ XrayWrapper<Base, Traits>::getOwnPropert
 {
     assertEnteredPolicy(cx, wrapper, id);
     RootedObject holder(cx, Traits::singleton.ensureHolder(cx, wrapper));
     if (Traits::isResolving(cx, holder, id)) {
         desc->obj = NULL;
         return true;
     }
 
-    typename Traits::ResolvingIdImpl resolving(wrapper, id);
+    typename Traits::ResolvingIdImpl resolving(cx, wrapper, id);
 
     // NB: Nothing we do here acts on the wrapped native itself, so we don't
     // enter our policy.
 
     if (XrayUtils::NeedsWaive(cx, wrapper, id)) {
         RootedObject waived(cx, WrapperFactory::WaiveXray(cx, wrapper));
         if (!waived || !JS_WrapObject(cx, waived.address()))
             return false;
@@ -1686,21 +1685,21 @@ XrayWrapper<Base, Traits>::definePropert
         if (!waived || !JS_WrapObject(cx, waived.address()))
             return false;
         return JS_DefinePropertyById(cx, waived, id, desc->value, desc->getter, desc->setter,
                                      desc->attrs);
     }
 
     // NB: We still need JSRESOLVE_ASSIGNING here for the time being, because it
     // tells things like nodelists whether they should create the property or not.
-    PropertyDescriptor existing_desc;
-    if (!getOwnPropertyDescriptor(cx, wrapper, id, &existing_desc, JSRESOLVE_ASSIGNING))
+    Rooted<PropertyDescriptor> existing_desc(cx);
+    if (!getOwnPropertyDescriptor(cx, wrapper, id, existing_desc.address(), JSRESOLVE_ASSIGNING))
         return false;
 
-    if (existing_desc.obj && (existing_desc.attrs & JSPROP_PERMANENT))
+    if (existing_desc.object() && (existing_desc.get().attrs & JSPROP_PERMANENT))
         return true; // silently ignore attempt to overwrite native property
 
     bool defined = false;
     if (!Traits::defineProperty(cx, wrapper, id, desc, existing_desc, &defined))
         return false;
     if (defined)
         return true;
 
@@ -1711,23 +1710,23 @@ XrayWrapper<Base, Traits>::definePropert
 
     // Grab the relevant expando object.
     RootedObject expandoObject(cx, Traits::singleton.ensureExpandoObject(cx, wrapper,
                                                                          target));
     if (!expandoObject)
         return false;
 
     // Wrap the property descriptor for the target compartment.
-    PropertyDescriptor wrappedDesc = *desc;
-    if (!JS_WrapPropertyDescriptor(cx, &wrappedDesc))
+    Rooted<PropertyDescriptor> wrappedDesc(cx, *desc);
+    if (!JS_WrapPropertyDescriptor(cx, wrappedDesc.address()))
         return false;
 
-    return JS_DefinePropertyById(cx, expandoObject, id, wrappedDesc.value,
-                                 wrappedDesc.getter, wrappedDesc.setter,
-                                 wrappedDesc.attrs);
+    return JS_DefinePropertyById(cx, expandoObject, id, wrappedDesc.value(),
+                                 wrappedDesc.getter(), wrappedDesc.setter(),
+                                 wrappedDesc.get().attrs);
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::getOwnPropertyNames(JSContext *cx, HandleObject wrapper,
                                                AutoIdVector &props)
 {
     assertEnteredPolicy(cx, wrapper, JSID_VOID);
--- a/js/xpconnect/wrappers/XrayWrapper.h
+++ b/js/xpconnect/wrappers/XrayWrapper.h
@@ -162,31 +162,31 @@ public:
                       const JS::CallArgs &args) MOZ_OVERRIDE;
 };
 
 extern SandboxCallableProxyHandler sandboxCallableProxyHandler;
 
 class AutoSetWrapperNotShadowing;
 class XPCWrappedNativeXrayTraits;
 
-class ResolvingId {
+class MOZ_STACK_CLASS ResolvingId {
 public:
-    ResolvingId(JSObject *wrapper, jsid id);
+    ResolvingId(JSContext *cx, JSObject *wrapper, jsid id);
     ~ResolvingId();
 
     bool isXrayShadowing(jsid id);
     bool isResolving(jsid id);
     static ResolvingId* getResolvingId(JSObject *holder);
     static JSObject* getHolderObject(JSObject *wrapper);
     static ResolvingId *getResolvingIdFromWrapper(JSObject *wrapper);
 
 private:
     friend class AutoSetWrapperNotShadowing;
     friend class XPCWrappedNativeXrayTraits;
 
-    jsid mId;
-    JSObject *mHolder;
+    JS::RootedId mId;
+    JS::RootedObject mHolder;
     ResolvingId *mPrev;
     bool mXrayShadowing;
 };
 
 }