Bug 896398 - GC: Fix some reported rooting hazards in XPConnect r=bholley
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 23 Jul 2013 10:58:28 +0100
changeset 151870 e324bd4c78f7d495c05404c7895b1eef94215c39
parent 151869 fe2ed5eff8e2955336542b9c87d16c6f97801a96
child 151871 9e8ba1191aef8838385a461d4e4e591066d14bd0
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs896398
milestone25.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 896398 - GC: Fix some reported rooting hazards in XPConnect r=bholley
js/xpconnect/src/XPCConvert.cpp
js/xpconnect/src/XPCWrappedJS.cpp
js/xpconnect/src/XPCWrappedJSClass.cpp
js/xpconnect/src/XPCWrapper.cpp
js/xpconnect/src/XPCWrapper.h
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/xpconnect/src/XPCConvert.cpp
+++ b/js/xpconnect/src/XPCConvert.cpp
@@ -850,18 +850,19 @@ XPCConvert::NativeInterface2JSObject(jsv
     } else {
         flat = nullptr;
     }
 
     // Don't double wrap CPOWs. This is a temporary measure for compatibility
     // with objects that don't provide necessary QIs (such as objects under
     // the new DOM bindings). We expect the other side of the CPOW to have
     // the appropriate wrappers in place.
-    if (JSObject *cpow = UnwrapNativeCPOW(aHelper.Object())) {
-        if (!JS_WrapObject(cx, &cpow))
+    RootedObject cpow(cx, UnwrapNativeCPOW(aHelper.Object()));
+    if (cpow) {
+        if (!JS_WrapObject(cx, cpow.address()))
             return false;
         *d = OBJECT_TO_JSVAL(cpow);
         return true;
     }
 
     // We can't simply construct a slim wrapper. Go ahead and create an
     // XPCWrappedNative for this object. At this point, |flat| could be
     // non-null, meaning that either we already have a wrapped native from
@@ -1108,23 +1109,22 @@ public:
 
 private:
     JSContext * const mContext;
     RootedValue tvr;
 };
 
 // static
 nsresult
-XPCConvert::JSValToXPCException(jsval sArg,
+XPCConvert::JSValToXPCException(MutableHandleValue s,
                                 const char* ifaceName,
                                 const char* methodName,
                                 nsIException** exceptn)
 {
     AutoJSContext cx;
-    RootedValue s(cx, sArg);
     AutoExceptionRestorer aer(cx, s);
 
     if (!JSVAL_IS_PRIMITIVE(s)) {
         // we have a JSObject
         RootedObject obj(cx, JSVAL_TO_OBJECT(s));
 
         if (!obj) {
             NS_ERROR("when is an object not an object?");
@@ -1570,17 +1570,17 @@ XPCConvert::JSTypedArray2Native(void** d
     if (pErr)
         *pErr = NS_OK;
 
     return true;
 }
 
 // static
 JSBool
-XPCConvert::JSArray2Native(void** d, JS::Value s,
+XPCConvert::JSArray2Native(void** d, HandleValue s,
                            uint32_t count, const nsXPTType& type,
                            const nsID* iid, nsresult* pErr)
 {
     NS_ABORT_IF_FALSE(d, "bad param");
 
     AutoJSContext cx;
 
     // XXX add support for getting chars from strings
@@ -1754,17 +1754,17 @@ XPCConvert::NativeStringWithSize2JS(jsva
             XPC_LOG_ERROR(("XPCConvert::NativeStringWithSize2JS : unsupported type"));
             return false;
     }
     return true;
 }
 
 // static
 JSBool
-XPCConvert::JSStringWithSize2Native(void* d, jsval s,
+XPCConvert::JSStringWithSize2Native(void* d, HandleValue s,
                                     uint32_t count, const nsXPTType& type,
                                     nsresult* pErr)
 {
     NS_PRECONDITION(!JSVAL_IS_NULL(s), "bad param");
     NS_PRECONDITION(d, "bad param");
 
     AutoJSContext cx;
     uint32_t len;
--- a/js/xpconnect/src/XPCWrappedJS.cpp
+++ b/js/xpconnect/src/XPCWrappedJS.cpp
@@ -270,27 +270,26 @@ CheckMainThreadOnly(nsXPCWrappedJS *aWra
 
     aWrapper->SetIsMainThreadOnly();
 
     return true;
 }
 
 // static
 nsresult
-nsXPCWrappedJS::GetNewOrUsed(JSObject* aJSObj,
+nsXPCWrappedJS::GetNewOrUsed(JS::HandleObject jsObj,
                              REFNSIID aIID,
                              nsISupports* aOuter,
                              nsXPCWrappedJS** wrapperResult)
 {
     // Do a release-mode assert against accessing nsXPCWrappedJS off-main-thread.
     if (!MOZ_LIKELY(NS_IsMainThread() || NS_IsCycleCollectorThread()))
         MOZ_CRASH();
 
     AutoJSContext cx;
-    JS::RootedObject jsObj(cx, aJSObj);
     JSObject2WrappedJSMap* map;
     nsXPCWrappedJS* root = nullptr;
     nsXPCWrappedJS* wrapper = nullptr;
     nsXPCWrappedJSClass* clazz = nullptr;
     XPCJSRuntime* rt = nsXPConnect::GetRuntimeInstance();
     JSBool release_root = false;
 
     map = rt->GetWrappedJSMap();
--- a/js/xpconnect/src/XPCWrappedJSClass.cpp
+++ b/js/xpconnect/src/XPCWrappedJSClass.cpp
@@ -955,23 +955,23 @@ nsXPCWrappedJSClass::CheckForException(X
     xpcc->GetException(getter_AddRefs(xpc_exception));
     if (xpc_exception)
         xpcc->SetException(nullptr);
 
     // get this right away in case we do something below to cause JS code
     // to run on this JSContext
     nsresult pending_result = xpcc->GetPendingResult();
 
-    jsval js_exception;
-    JSBool is_js_exception = JS_GetPendingException(cx, &js_exception);
+    RootedValue js_exception(cx);
+    JSBool is_js_exception = JS_GetPendingException(cx, js_exception.address());
 
     /* JS might throw an expection whether the reporter was called or not */
     if (is_js_exception) {
         if (!xpc_exception)
-            XPCConvert::JSValToXPCException(js_exception, anInterfaceName,
+            XPCConvert::JSValToXPCException(&js_exception, anInterfaceName,
                                             aPropertyName,
                                             getter_AddRefs(xpc_exception));
 
         /* cleanup and set failed even if we can't build an exception */
         if (!xpc_exception) {
             XPCJSRuntime::Get()->SetPendingException(nullptr); // XXX necessary?
         }
     }
--- a/js/xpconnect/src/XPCWrapper.cpp
+++ b/js/xpconnect/src/XPCWrapper.cpp
@@ -60,17 +60,17 @@ XrayWrapperConstructor(JSContext *cx, un
     return true;
   }
 
   *vp = JS::ObjectValue(*js::UncheckedUnwrap(&v.toObject()));
   return JS_WrapValue(cx, vp);
 }
 // static
 bool
-AttachNewConstructorObject(JSContext *aCx, JSObject *aGlobalObject)
+AttachNewConstructorObject(JSContext *aCx, JS::HandleObject aGlobalObject)
 {
   // Pushing a JSContext calls ActivateDebugger which calls this function, so
   // we can't use an AutoJSContext here until JSD is gone.
   JSAutoCompartment ac(aCx, aGlobalObject);
   JSFunction *xpcnativewrapper =
     JS_DefineFunction(aCx, aGlobalObject, "XPCNativeWrapper",
                       XrayWrapperConstructor, 1,
                       JSPROP_READONLY | JSPROP_PERMANENT | JSFUN_STUB_GSOPS | JSFUN_CONSTRUCTOR);
--- a/js/xpconnect/src/XPCWrapper.h
+++ b/js/xpconnect/src/XPCWrapper.h
@@ -18,17 +18,17 @@ namespace XPCNativeWrapper {
 // XPCNativeScriptableFlags corresponding with a flag, returns 'true'
 // if the flag is set.
 // XXX Convert to using GetFlags() and not a macro.
 #define NATIVE_HAS_FLAG(_wn, _flag)                                           \
   ((_wn)->GetScriptableInfo() &&                                              \
    (_wn)->GetScriptableInfo()->GetFlags()._flag())
 
 bool
-AttachNewConstructorObject(JSContext *aCx, JSObject *aGlobalObject);
+AttachNewConstructorObject(JSContext *aCx, JS::HandleObject aGlobalObject);
 
 } // namespace XPCNativeWrapper
 
 // This namespace wraps some common functionality between the three existing
 // wrappers. Its main purpose is to allow XPCCrossOriginWrapper to act both
 // as an XPCSafeJSObjectWrapper and as an XPCNativeWrapper when required to
 // do so (the decision is based on the principals of the wrapper and wrapped
 // objects).
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -590,19 +590,19 @@ nsXPConnect::WrapNative(JSContext * aJSC
                         nsIXPConnectJSObjectHolder **aHolder)
 {
     NS_ASSERTION(aHolder, "bad param");
     NS_ASSERTION(aJSContext, "bad param");
     NS_ASSERTION(aScopeArg, "bad param");
     NS_ASSERTION(aCOMObj, "bad param");
 
     RootedObject aScope(aJSContext, aScopeArg);
-    jsval v;
+    RootedValue v(aJSContext);
     return NativeInterface2JSObject(aScope, aCOMObj, nullptr, &aIID,
-                                    false, &v, aHolder);
+                                    false, v.address(), aHolder);
 }
 
 /* void wrapNativeToJSVal (in JSContextPtr aJSContext, in JSObjectPtr aScope, in nsISupports aCOMObj, in nsIIDPtr aIID, out jsval aVal, out nsIXPConnectJSObjectHolder aHolder); */
 NS_IMETHODIMP
 nsXPConnect::WrapNativeToJSVal(JSContext * aJSContext,
                                JSObject * aScopeArg,
                                nsISupports *aCOMObj,
                                nsWrapperCache *aCache,
@@ -1384,18 +1384,18 @@ DeferredRelease(nsISupports *obj)
 }
 
 NS_EXPORT_(bool)
 Base64Encode(JSContext *cx, JS::Value val, JS::Value *out)
 {
     MOZ_ASSERT(cx);
     MOZ_ASSERT(out);
 
-    JS::Value root = val;
-    xpc_qsACString encodedString(cx, root, &root, xpc_qsACString::eNull,
+    JS::RootedValue root(cx, val);
+    xpc_qsACString encodedString(cx, root, root.address(), xpc_qsACString::eNull,
                                  xpc_qsACString::eStringify);
     if (!encodedString.IsValid())
         return false;
 
     nsAutoCString result;
     if (NS_FAILED(mozilla::Base64Encode(encodedString, result))) {
         JS_ReportError(cx, "Failed to encode base64 data!");
         return false;
@@ -1410,18 +1410,18 @@ Base64Encode(JSContext *cx, JS::Value va
 }
 
 NS_EXPORT_(bool)
 Base64Decode(JSContext *cx, JS::Value val, JS::Value *out)
 {
     MOZ_ASSERT(cx);
     MOZ_ASSERT(out);
 
-    JS::Value root = val;
-    xpc_qsACString encodedString(cx, root, &root, xpc_qsACString::eNull,
+    JS::RootedValue root(cx, val);
+    xpc_qsACString encodedString(cx, root, root.address(), xpc_qsACString::eNull,
                                  xpc_qsACString::eNull);
     if (!encodedString.IsValid())
         return false;
 
     nsAutoCString result;
     if (NS_FAILED(mozilla::Base64Decode(encodedString, result))) {
         JS_ReportError(cx, "Failed to decode base64 string!");
         return false;
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -2730,17 +2730,17 @@ public:
 
     /*
     * This is rarely called directly. Instead one usually calls
     * XPCConvert::JSObject2NativeInterface which will handles cases where the
     * JS object is already a wrapped native or a DOM object.
     */
 
     static nsresult
-    GetNewOrUsed(JSObject* aJSObj,
+    GetNewOrUsed(JS::HandleObject aJSObj,
                  REFNSIID aIID,
                  nsISupports* aOuter,
                  nsXPCWrappedJS** wrapper);
 
     nsISomeInterface* GetXPTCStub() { return mXPTCStub; }
 
     /**
      * This getter does not change the color of the JSObject meaning that the
@@ -2859,17 +2859,16 @@ private:
 class XPCConvert
 {
 public:
     static JSBool IsMethodReflectable(const XPTMethodDescriptor& info);
 
     /**
      * Convert a native object into a jsval.
      *
-     * @param ccx the context for the whole procedure
      * @param d [out] the resulting jsval
      * @param s the native object we're working with
      * @param type the type of object that s is
      * @param iid the interface of s that we want
      * @param scope the default scope to put on the new JSObject's parent
      *        chain
      * @param pErr [out] relevant error code, if any.
      */
@@ -2881,17 +2880,16 @@ public:
     static JSBool JSData2Native(void* d, JS::HandleValue s,
                                 const nsXPTType& type,
                                 JSBool useAllocator, const nsID* iid,
                                 nsresult* pErr);
 
     /**
      * Convert a native nsISupports into a JSObject.
      *
-     * @param ccx the context for the whole procedure
      * @param dest [out] the resulting JSObject
      * @param src the native object we're working with
      * @param iid the interface of src that we want (may be null)
      * @param Interface the interface of src that we want
      * @param cache the wrapper cache for src (may be null, in which case src
      *              will be QI'ed to get the cache)
      * @param allowNativeWrapper if true, this method may wrap the resulting
      *        JSObject in an XPCNativeWrapper and return that, as needed.
@@ -2926,36 +2924,36 @@ public:
      * @param count the number of items in the array
      * @param scope the default scope to put on the new JSObjects' parent chain
      * @param pErr [out] relevant error code, if any.
      */
     static JSBool NativeArray2JS(jsval* d, const void** s,
                                  const nsXPTType& type, const nsID* iid,
                                  uint32_t count, nsresult* pErr);
 
-    static JSBool JSArray2Native(void** d, jsval s,
+    static JSBool JSArray2Native(void** d, JS::HandleValue s,
                                  uint32_t count, const nsXPTType& type,
                                  const nsID* iid, nsresult* pErr);
 
     static JSBool JSTypedArray2Native(void** d,
                                       JSObject* jsarray,
                                       uint32_t count,
                                       const nsXPTType& type,
                                       nsresult* pErr);
 
     static JSBool NativeStringWithSize2JS(jsval* d, const void* s,
                                           const nsXPTType& type,
                                           uint32_t count,
                                           nsresult* pErr);
 
-    static JSBool JSStringWithSize2Native(void* d, jsval s,
+    static JSBool JSStringWithSize2Native(void* d, JS::HandleValue s,
                                           uint32_t count, const nsXPTType& type,
                                           nsresult* pErr);
 
-    static nsresult JSValToXPCException(jsval s,
+    static nsresult JSValToXPCException(JS::MutableHandleValue s,
                                         const char* ifaceName,
                                         const char* methodName,
                                         nsIException** exception);
 
     static nsresult JSErrorToXPCException(const char* message,
                                           const char* ifaceName,
                                           const char* methodName,
                                           const JSErrorReport* report,