Bug 869526 - GC: Fix more rooting hazards in xpconnect r=bholley
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 09 May 2013 10:39:21 +0100
changeset 138125 83459f51f50095e2eaa864d8e355904f0cc7aa1c
parent 138124 22dffe8154e556c658bb58e01bec587870e573cf
child 138126 0c5f4ae8b572600b27596b7894856e81ec5b81c6
push id3752
push userlsblakk@mozilla.com
push dateMon, 13 May 2013 17:21:10 +0000
treeherdermozilla-aurora@1580544aef0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs869526
milestone23.0a1
Bug 869526 - GC: Fix more rooting hazards in xpconnect r=bholley
js/xpconnect/src/XPCComponents.cpp
js/xpconnect/src/XPCQuickStubs.cpp
js/xpconnect/src/XPCVariant.cpp
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -1412,20 +1412,22 @@ nsXPCComponents_Results::NewEnumerate(ns
             return NS_OK;
     }
 }
 
 
 /* bool newResolve (in nsIXPConnectWrappedNative wrapper, in JSContextPtr cx, in JSObjectPtr obj, in jsval id, in uint32_t flags, out JSObjectPtr objp); */
 NS_IMETHODIMP
 nsXPCComponents_Results::NewResolve(nsIXPConnectWrappedNative *wrapper,
-                                    JSContext * cx, JSObject * obj,
-                                    jsid id, uint32_t flags,
+                                    JSContext *cx, JSObject *objArg,
+                                    jsid idArg, uint32_t flags,
                                     JSObject * *objp, bool *_retval)
 {
+    RootedObject obj(cx, objArg);
+    RootedId id(cx, idArg);
     JSAutoByteString name;
 
     if (JSID_IS_STRING(id) && name.encodeLatin1(cx, JSID_TO_STRING(id))) {
         const char* rv_name;
         void* iter = nullptr;
         nsresult rv;
         while (nsXPCException::IterateNSResults(&rv, &rv_name, nullptr, &iter)) {
             if (!strcmp(name.ptr(), rv_name)) {
@@ -4952,24 +4954,24 @@ ContentComponentsGetterOp(JSContext *cx,
     return true;
 }
 
 // static
 JSBool
 nsXPCComponents::AttachComponentsObject(XPCCallContext& ccx,
                                         XPCWrappedNativeScope* aScope)
 {
-    JSObject *components = aScope->GetComponentsJSObject(ccx);
+    RootedObject components(ccx, aScope->GetComponentsJSObject(ccx));
     if (!components)
         return false;
 
-    JSObject *global = aScope->GetGlobalJSObject();
+    RootedObject global(ccx, aScope->GetGlobalJSObject());
     MOZ_ASSERT(js::IsObjectInContextCompartment(global, ccx));
 
-    jsid id = ccx.GetRuntime()->GetStringID(XPCJSRuntime::IDX_COMPONENTS);
+    RootedId id(ccx, ccx.GetRuntime()->GetStringID(XPCJSRuntime::IDX_COMPONENTS));
     JSPropertyOp getter = AccessCheck::isChrome(global) ? nullptr
                                                         : &ContentComponentsGetterOp;
     return JS_DefinePropertyById(ccx, global, id, js::ObjectValue(*components),
                                  getter, nullptr, JSPROP_PERMANENT | JSPROP_READONLY);
 }
 
 /* void lookupMethod (); */
 NS_IMETHODIMP
--- a/js/xpconnect/src/XPCQuickStubs.cpp
+++ b/js/xpconnect/src/XPCQuickStubs.cpp
@@ -208,21 +208,21 @@ GetMemberInfo(JSObject *obj, jsid member
             }
         }
     }
 }
 
 static void
 GetMethodInfo(JSContext *cx, jsval *vp, const char **ifaceNamep, jsid *memberIdp)
 {
-    JSObject *funobj = JSVAL_TO_OBJECT(JS_CALLEE(cx, vp));
+    RootedObject funobj(cx, JSVAL_TO_OBJECT(JS_CALLEE(cx, vp)));
     NS_ASSERTION(JS_ObjectIsFunction(cx, funobj),
                  "JSNative callee should be Function object");
-    JSString *str = JS_GetFunctionId(JS_GetObjectFunction(funobj));
-    jsid methodId = str ? INTERNED_STRING_TO_JSID(cx, str) : JSID_VOID;
+    RootedString str(cx, JS_GetFunctionId(JS_GetObjectFunction(funobj)));
+    RootedId methodId(cx, str ? INTERNED_STRING_TO_JSID(cx, str) : JSID_VOID);
     GetMemberInfo(JSVAL_TO_OBJECT(vp[1]), methodId, ifaceNamep);
     *memberIdp = methodId;
 }
 
 static bool
 ThrowCallFailed(JSContext *cx, nsresult rv,
                 const char *ifaceName, HandleId memberId, const char *memberName)
 {
@@ -356,18 +356,18 @@ ThrowBadArg(JSContext *cx, nsresult rv, 
     if (sz)
         JS_smprintf_free(sz);
 }
 
 void
 xpc_qsThrowBadArg(JSContext *cx, nsresult rv, jsval *vp, unsigned paramnum)
 {
     const char *ifaceName;
-    jsid memberId;
-    GetMethodInfo(cx, vp, &ifaceName, &memberId);
+    RootedId memberId(cx);
+    GetMethodInfo(cx, vp, &ifaceName, memberId.address());
     ThrowBadArg(cx, rv, ifaceName, memberId, NULL, paramnum);
 }
 
 void
 xpc_qsThrowBadArgWithCcx(XPCCallContext &ccx, nsresult rv, unsigned paramnum)
 {
     XPCThrower::ThrowBadParam(rv, paramnum, ccx);
 }
@@ -376,18 +376,19 @@ void
 xpc_qsThrowBadArgWithDetails(JSContext *cx, nsresult rv, unsigned paramnum,
                              const char *ifaceName, const char *memberName)
 {
     ThrowBadArg(cx, rv, ifaceName, JSID_VOID, memberName, paramnum);
 }
 
 void
 xpc_qsThrowBadSetterValue(JSContext *cx, nsresult rv,
-                          JSObject *obj, jsid propId)
+                          JSObject *obj, jsid propIdArg)
 {
+    RootedId propId(cx, propIdArg);
     const char *ifaceName;
     GetMemberInfo(obj, propId, &ifaceName);
     ThrowBadArg(cx, rv, ifaceName, propId, NULL, 0);
 }
 
 void
 xpc_qsThrowBadSetterValue(JSContext *cx, nsresult rv,
                           JSObject *objArg, const char* propName)
--- a/js/xpconnect/src/XPCVariant.cpp
+++ b/js/xpconnect/src/XPCVariant.cpp
@@ -411,151 +411,178 @@ XPCVariant::VariantDataToJS(XPCLazyCallC
 
     // We just fall through to the code below and let it do what it does.
 
     // The nsIVariant is not a XPCVariant (or we act like it isn't).
     // So we extract the data and do the Right Thing.
 
     // We ASSUME that the variant implementation can do these conversions...
 
-    nsXPTCVariant xpctvar;
     nsID iid;
-    nsAutoString astring;
-    nsAutoCString cString;
-    nsUTF8String utf8String;
-    uint32_t size;
-    xpctvar.flags = 0;
-    JSBool success;
 
     NS_ABORT_IF_FALSE(js::IsObjectInContextCompartment(lccx.GetScopeForNewJSObjects(), cx),
                       "bad scope for new JSObjects");
 
     switch (type) {
         case nsIDataType::VTYPE_INT8:
         case nsIDataType::VTYPE_INT16:
         case nsIDataType::VTYPE_INT32:
         case nsIDataType::VTYPE_INT64:
         case nsIDataType::VTYPE_UINT8:
         case nsIDataType::VTYPE_UINT16:
         case nsIDataType::VTYPE_UINT32:
         case nsIDataType::VTYPE_UINT64:
         case nsIDataType::VTYPE_FLOAT:
         case nsIDataType::VTYPE_DOUBLE:
         {
-            // Easy. Handle inline.
-            if (NS_FAILED(variant->GetAsDouble(&xpctvar.val.d)))
+            double d;
+            if (NS_FAILED(variant->GetAsDouble(&d)))
                 return false;
-            *pJSVal = JS_NumberValue(xpctvar.val.d);
+            *pJSVal = JS_NumberValue(d);
             return true;
         }
         case nsIDataType::VTYPE_BOOL:
         {
-            // Easy. Handle inline.
-            if (NS_FAILED(variant->GetAsBool(&xpctvar.val.b)))
+            bool b;
+            if (NS_FAILED(variant->GetAsBool(&b)))
                 return false;
-            *pJSVal = BOOLEAN_TO_JSVAL(xpctvar.val.b);
+            *pJSVal = BOOLEAN_TO_JSVAL(b);
             return true;
         }
         case nsIDataType::VTYPE_CHAR:
-            if (NS_FAILED(variant->GetAsChar(&xpctvar.val.c)))
+        {
+            char c;
+            if (NS_FAILED(variant->GetAsChar(&c)))
                 return false;
-            xpctvar.type = (uint8_t)TD_CHAR;
-            break;
+            return XPCConvert::NativeData2JS(lccx, pJSVal, (const void*)&c, TD_CHAR, &iid, pErr);
+        }
         case nsIDataType::VTYPE_WCHAR:
-            if (NS_FAILED(variant->GetAsWChar(&xpctvar.val.wc)))
+        {
+            PRUnichar wc;
+            if (NS_FAILED(variant->GetAsWChar(&wc)))
                 return false;
-            xpctvar.type = (uint8_t)TD_WCHAR;
-            break;
+            return XPCConvert::NativeData2JS(lccx, pJSVal, (const void*)&wc, TD_WCHAR, &iid, pErr);
+        }
         case nsIDataType::VTYPE_ID:
+        {
             if (NS_FAILED(variant->GetAsID(&iid)))
                 return false;
-            xpctvar.type = (uint8_t)TD_PNSIID;
-            xpctvar.val.p = &iid;
-            break;
+            nsID *v = &iid;
+            return XPCConvert::NativeData2JS(lccx, pJSVal, (const void*)&v, TD_PNSIID, &iid, pErr);
+        }
         case nsIDataType::VTYPE_ASTRING:
+        {
+            nsAutoString astring;
             if (NS_FAILED(variant->GetAsAString(astring)))
                 return false;
-            xpctvar.type = (uint8_t)TD_ASTRING;
-            xpctvar.val.p = &astring;
-            break;
+            nsAutoString *v = &astring;
+            return XPCConvert::NativeData2JS(lccx, pJSVal, (const void*)&v, TD_ASTRING, &iid, pErr);
+        }
         case nsIDataType::VTYPE_DOMSTRING:
+        {
+            nsAutoString astring;
             if (NS_FAILED(variant->GetAsAString(astring)))
                 return false;
-            xpctvar.type = (uint8_t)TD_DOMSTRING;
-            xpctvar.val.p = &astring;
-            break;
+            nsAutoString *v = &astring;
+            return XPCConvert::NativeData2JS(lccx, pJSVal, (const void*)&v,
+                                             TD_DOMSTRING, &iid, pErr);
+        }
         case nsIDataType::VTYPE_CSTRING:
+        {
+            nsAutoCString cString;
             if (NS_FAILED(variant->GetAsACString(cString)))
                 return false;
-            xpctvar.type = (uint8_t)TD_CSTRING;
-            xpctvar.val.p = &cString;
-            break;
+            nsAutoCString *v = &cString;
+            return XPCConvert::NativeData2JS(lccx, pJSVal, (const void*)&v,
+                                             TD_CSTRING, &iid, pErr);
+        }
         case nsIDataType::VTYPE_UTF8STRING:
+        {
+            nsUTF8String utf8String;
             if (NS_FAILED(variant->GetAsAUTF8String(utf8String)))
                 return false;
-            xpctvar.type = (uint8_t)TD_UTF8STRING;
-            xpctvar.val.p = &utf8String;
-            break;
+            nsUTF8String *v = &utf8String;
+            return XPCConvert::NativeData2JS(lccx, pJSVal, (const void*)&v,
+                                             TD_UTF8STRING, &iid, pErr);
+        }
         case nsIDataType::VTYPE_CHAR_STR:
-            if (NS_FAILED(variant->GetAsString((char**)&xpctvar.val.p)))
+        {
+            char *pc;
+            if (NS_FAILED(variant->GetAsString(&pc)))
                 return false;
-            xpctvar.type = (uint8_t)TD_PSTRING;
-            xpctvar.SetValNeedsCleanup();
-            break;
+            bool success = XPCConvert::NativeData2JS(lccx, pJSVal, (const void*)&pc,
+                                                     TD_PSTRING, &iid, pErr);
+            nsMemory::Free(pc);
+            return success;
+        }
         case nsIDataType::VTYPE_STRING_SIZE_IS:
-            if (NS_FAILED(variant->GetAsStringWithSize(&size,
-                                                       (char**)&xpctvar.val.p)))
+        {
+            char *pc;
+            uint32_t size;
+            if (NS_FAILED(variant->GetAsStringWithSize(&size, &pc)))
                 return false;
-            xpctvar.type = (uint8_t)TD_PSTRING_SIZE_IS;
-            xpctvar.SetValNeedsCleanup();
-            break;
+            bool success = XPCConvert::NativeStringWithSize2JS(cx, pJSVal, (const void*)&pc,
+                                                               TD_PSTRING_SIZE_IS, size, pErr);
+            nsMemory::Free(pc);
+            return success;
+        }
         case nsIDataType::VTYPE_WCHAR_STR:
-            if (NS_FAILED(variant->GetAsWString((PRUnichar**)&xpctvar.val.p)))
+        {
+            PRUnichar *pwc;
+            if (NS_FAILED(variant->GetAsWString(&pwc)))
                 return false;
-            xpctvar.type = (uint8_t)TD_PWSTRING;
-            xpctvar.SetValNeedsCleanup();
-            break;
+            bool success = XPCConvert::NativeData2JS(lccx, pJSVal, (const void*)&pwc,
+                                                     TD_PSTRING, &iid, pErr);
+            nsMemory::Free(pwc);
+            return success;
+        }
         case nsIDataType::VTYPE_WSTRING_SIZE_IS:
-            if (NS_FAILED(variant->GetAsWStringWithSize(&size,
-                                                        (PRUnichar**)&xpctvar.val.p)))
+        {
+            PRUnichar *pwc;
+            uint32_t size;
+            if (NS_FAILED(variant->GetAsWStringWithSize(&size, &pwc)))
                 return false;
-            xpctvar.type = (uint8_t)TD_PWSTRING_SIZE_IS;
-            xpctvar.SetValNeedsCleanup();
-            break;
+            bool success = XPCConvert::NativeStringWithSize2JS(cx, pJSVal, (const void*)&pwc,
+                                                               TD_PWSTRING_SIZE_IS, size, pErr);
+            nsMemory::Free(pwc);
+            return success;
+        }
         case nsIDataType::VTYPE_INTERFACE:
         case nsIDataType::VTYPE_INTERFACE_IS:
         {
+            nsISupports *pi;
             nsID* piid;
-            if (NS_FAILED(variant->GetAsInterface(&piid, &xpctvar.val.p)))
+            if (NS_FAILED(variant->GetAsInterface(&piid, (void **)&pi)))
                 return false;
 
             iid = *piid;
             nsMemory::Free((char*)piid);
 
-            xpctvar.type = (uint8_t)TD_INTERFACE_IS_TYPE;
-            if (xpctvar.val.p)
-                xpctvar.SetValNeedsCleanup();
-            break;
+            bool success = XPCConvert::NativeData2JS(lccx, pJSVal, (const void*)&pi,
+                                                     TD_INTERFACE_IS_TYPE, &iid, pErr);
+            if (pi)
+                pi->Release();
+            return success;
         }
         case nsIDataType::VTYPE_ARRAY:
         {
             nsDiscriminatedUnion du;
             nsVariant::Initialize(&du);
             nsresult rv;
 
             rv = variant->GetAsArray(&du.u.array.mArrayType,
                                      &du.u.array.mArrayInterfaceID,
                                      &du.u.array.mArrayCount,
                                      &du.u.array.mArrayValue);
             if (NS_FAILED(rv))
                 return false;
 
             // must exit via VARIANT_DONE from here on...
             du.mType = nsIDataType::VTYPE_ARRAY;
-            success = false;
+            bool success = false;
 
             nsXPTType conversionType;
             uint16_t elementType = du.u.array.mArrayType;
             const nsID* pid = nullptr;
 
             switch (elementType) {
                 case nsIDataType::VTYPE_INT8:
                 case nsIDataType::VTYPE_INT16:
@@ -628,42 +655,16 @@ VARIANT_DONE:
             return true;
         case nsIDataType::VTYPE_EMPTY:
             *pJSVal = JSVAL_NULL;
             return true;
         default:
             NS_ERROR("bad type in variant!");
             return false;
     }
-
-    // If we are here then we need to convert the data in the xpctvar.
-
-    if (xpctvar.type.TagPart() == TD_PSTRING_SIZE_IS ||
-        xpctvar.type.TagPart() == TD_PWSTRING_SIZE_IS) {
-        success = XPCConvert::NativeStringWithSize2JS(cx, pJSVal,
-                                                      (const void*)&xpctvar.val,
-                                                      xpctvar.type,
-                                                      size, pErr);
-    } else {
-        success = XPCConvert::NativeData2JS(lccx, pJSVal,
-                                            (const void*)&xpctvar.val,
-                                            xpctvar.type,
-                                            &iid, pErr);
-    }
-
-    // We may have done something in the above code that requires cleanup.
-    if (xpctvar.DoesValNeedCleanup()) {
-        if (type == nsIDataType::VTYPE_INTERFACE ||
-            type == nsIDataType::VTYPE_INTERFACE_IS)
-            ((nsISupports*)xpctvar.val.p)->Release();
-        else
-            nsMemory::Free((char*)xpctvar.val.p);
-    }
-
-    return success;
 }
 
 /***************************************************************************/
 /***************************************************************************/
 // XXX These default implementations need to be improved to allow for
 // some more interesting conversions.
 
 
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -2053,17 +2053,17 @@ XPCWrappedNative::InitTearOffJSObject(XP
     return true;
 }
 
 JSObject*
 XPCWrappedNative::GetSameCompartmentSecurityWrapper(JSContext *cx)
 {
     // Grab the current state of affairs.
     RootedObject flat(cx, GetFlatJSObject());
-    JSObject *wrapper = GetWrapper();
+    RootedObject wrapper(cx, GetWrapper());
 
     // If we already have a wrapper, it must be what we want.
     if (wrapper)
         return wrapper;
 
     // Chrome callers don't need same-compartment security wrappers.
     JSCompartment *cxCompartment = js::GetContextCompartment(cx);
     MOZ_ASSERT(cxCompartment == js::GetObjectCompartment(flat));
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -2026,25 +2026,25 @@ xpc_ActivateDebugMode()
     nsXPConnect::GetXPConnect()->SetDebugModeWhenPossible(true, true);
     nsXPConnect::CheckForDebugMode(rt->GetJSRuntime());
 }
 
 /* virtual */
 JSContext*
 nsXPConnect::GetCurrentJSContext()
 {
-    JSContext *cx = XPCJSRuntime::Get()->GetJSContextStack()->Peek();
+    JSContext *cx = GetRuntime()->GetJSContextStack()->Peek();
     return xpc_UnmarkGrayContext(cx);
 }
 
 /* virtual */
 JSContext*
 nsXPConnect::GetSafeJSContext()
 {
-    return XPCJSRuntime::Get()->GetJSContextStack()->GetSafeJSContext();
+    return GetRuntime()->GetJSContextStack()->GetSafeJSContext();
 }
 
 namespace xpc {
 namespace danger {
 
 NS_EXPORT_(bool)
 PushJSContext(JSContext *aCx)
 {
@@ -2368,24 +2368,24 @@ nsXPConnect::SetDebugModeWhenPossible(bo
     if (!mode && allowSyncDisable)
         CheckForDebugMode(mRuntime->GetJSRuntime());
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsXPConnect::GetTelemetryValue(JSContext *cx, jsval *rval)
 {
-    JSObject *obj = JS_NewObject(cx, NULL, NULL, NULL);
+    RootedObject obj(cx, JS_NewObject(cx, NULL, NULL, NULL));
     if (!obj)
         return NS_ERROR_OUT_OF_MEMORY;
 
     unsigned attrs = JSPROP_ENUMERATE | JSPROP_READONLY | JSPROP_PERMANENT;
 
     size_t i = JS_SetProtoCalled(cx);
-    jsval v = DOUBLE_TO_JSVAL(i);
+    RootedValue v(cx, DOUBLE_TO_JSVAL(i));
     if (!JS_DefineProperty(cx, obj, "setProto", v, NULL, NULL, attrs))
         return NS_ERROR_OUT_OF_MEMORY;
 
     i = JS_GetCustomIteratorCount(cx);
     v = DOUBLE_TO_JSVAL(i);
     if (!JS_DefineProperty(cx, obj, "customIter", v, NULL, NULL, attrs))
         return NS_ERROR_OUT_OF_MEMORY;
 
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -278,17 +278,17 @@ WrapperFactory::PrepareForWrapping(JSCon
         newwn->SetSet(unionSet);
     }
 
     return DoubleWrap(cx, obj, flags);
 }
 
 #ifdef DEBUG
 static void
-DEBUG_CheckUnwrapSafety(JSObject *obj, js::Wrapper *handler,
+DEBUG_CheckUnwrapSafety(HandleObject obj, js::Wrapper *handler,
                         JSCompartment *origin, JSCompartment *target)
 {
     if (AccessCheck::isChrome(target) || xpc::IsUniversalXPConnectEnabled(target)) {
         // If the caller is chrome (or effectively so), unwrap should always be allowed.
         MOZ_ASSERT(handler->isSafeToUnwrap());
     } else if (WrapperFactory::IsComponentsObject(obj)) {
         // The Components object that is restricted regardless of origin.
         MOZ_ASSERT(!handler->isSafeToUnwrap());