Bug 683802 - Pass useAllocator=false only for wrappedjs dipper params. Everything else can allocate. r=mrbkap
☠☠ backed out by 91b82bc49470 ☠ ☠
authorBobby Holley <bobbyholley@gmail.com>
Fri, 23 Sep 2011 14:50:29 -0700
changeset 77475 3bfef7f630dc04b2c642d13d177c9b73bc8d962d
parent 77474 30472afa1c400403b0dc8cc44a09d882fc1fb240
child 77476 6e370a64b8d12176576645c9e39988e66e838b16
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersmrbkap
bugs683802
milestone9.0a1
Bug 683802 - Pass useAllocator=false only for wrappedjs dipper params. Everything else can allocate. r=mrbkap We only have one good reason for the useAllocator distinction: when C++ calls JS with a dipper parameter (ie, a string), the caller has already created the relevant nsAString or nsCString, so we shouldn't allocate another one. All other uses are superfluous or irrelevant, so we can get rid of them.
js/src/xpconnect/src/xpcconvert.cpp
js/src/xpconnect/src/xpcprivate.h
js/src/xpconnect/src/xpcvariant.cpp
js/src/xpconnect/src/xpcwrappedjsclass.cpp
js/src/xpconnect/src/xpcwrappednative.cpp
--- a/js/src/xpconnect/src/xpcconvert.cpp
+++ b/js/src/xpconnect/src/xpcconvert.cpp
@@ -684,18 +684,16 @@ XPCConvert::JSData2Native(XPCCallContext
         switch(type.TagPart())
         {
         case nsXPTType::T_VOID:
             XPC_LOG_ERROR(("XPCConvert::JSData2Native : void* params not supported"));
             NS_ERROR("void* params not supported");
             return JS_FALSE;
         case nsXPTType::T_IID:
         {
-            NS_ASSERTION(useAllocator,"trying to convert a JSID to nsID without allocator : this would leak");
-
             JSObject* obj;
             const nsID* pid=nsnull;
 
             if(JSVAL_IS_VOID(s) || JSVAL_IS_NULL(s))
             {
                 if(type.IsReference())
                 {
                     if(pErr)
@@ -815,23 +813,16 @@ XPCConvert::JSData2Native(XPCCallContext
                 else
                     ws->Assign(chars, length);
             }
             return JS_TRUE;
         }
 
         case nsXPTType::T_CHAR_STR:
         {
-            NS_ASSERTION(useAllocator,"cannot convert a JSString to char * without allocator");
-            if(!useAllocator)
-            {
-                NS_ERROR("bad type");
-                return JS_FALSE;
-            }
-            
             if(JSVAL_IS_VOID(s) || JSVAL_IS_NULL(s))
             {
                 if(type.IsReference())
                 {
                     if(pErr)
                         *pErr = NS_ERROR_XPC_BAD_CONVERT_JS_NULL_REF;
                     return JS_FALSE;
                 }
@@ -892,41 +883,30 @@ XPCConvert::JSData2Native(XPCCallContext
                 *((jschar**)d) = nsnull;
                 return JS_TRUE;
             }
 
             if(!(str = JS_ValueToString(cx, s)))
             {
                 return JS_FALSE;
             }
-            if(useAllocator)
+            if(!(chars = JS_GetStringCharsZ(cx, str)))
             {
-                if(!(chars = JS_GetStringCharsZ(cx, str)))
-                {
-                    return JS_FALSE;
-                }
-                int len = JS_GetStringLength(str);
-                int byte_len = (len+1)*sizeof(jschar);
-                if(!(*((void**)d) = nsMemory::Alloc(byte_len)))
-                {
-                    // XXX should report error
-                    return JS_FALSE;
-                }
-                jschar* destchars = *((jschar**)d);
-                memcpy(destchars, chars, byte_len);
-                destchars[len] = 0;
+                return JS_FALSE;
             }
-            else
+            int len = JS_GetStringLength(str);
+            int byte_len = (len+1)*sizeof(jschar);
+            if(!(*((void**)d) = nsMemory::Alloc(byte_len)))
             {
-                if(!(chars = JS_GetStringCharsZ(cx, str)))
-                {
-                    return JS_FALSE;
-                }
-                *((const jschar**)d) = chars;
+                // XXX should report error
+                return JS_FALSE;
             }
+            jschar* destchars = *((jschar**)d);
+            memcpy(destchars, chars, byte_len);
+            destchars[len] = 0;
 
             return JS_TRUE;
         }
 
         case nsXPTType::T_UTF8STRING:            
         {
             const jschar* chars;
             PRUint32 length;
@@ -1934,18 +1914,17 @@ failure:
 
 #undef POPULATE
 }
 
 // static
 JSBool
 XPCConvert::JSArray2Native(XPCCallContext& ccx, void** d, jsval s,
                            JSUint32 count, JSUint32 capacity,
-                           const nsXPTType& type,
-                           JSBool useAllocator, const nsID* iid,
+                           const nsXPTType& type, const nsID* iid,
                            uintN* pErr)
 {
     NS_PRECONDITION(d, "bad param");
 
     JSContext* cx = ccx.GetJSContext();
 
     // No Action, FRee memory, RElease object
     enum CleanupMode {na, fr, re};
@@ -2015,17 +1994,17 @@ XPCConvert::JSArray2Native(XPCCallContex
             if(pErr)                                                         \
                 *pErr = NS_ERROR_OUT_OF_MEMORY;                              \
             goto failure;                                                    \
         }                                                                    \
         for(initedCount = 0; initedCount < count; initedCount++)             \
         {                                                                    \
             if(!JS_GetElement(cx, jsarray, initedCount, &current) ||         \
                !JSData2Native(ccx, ((_t*)array)+initedCount, current, type,  \
-                              useAllocator, iid, pErr))                      \
+                              JS_TRUE, iid, pErr))                           \
                 goto failure;                                                \
         }                                                                    \
     PR_END_MACRO
 
 
     // XXX check IsPtr - esp. to handle array of nsID (as opposed to nsID*)
 
     // XXX make extra space at end of char* and wchar* and null termintate
@@ -2072,17 +2051,17 @@ failure:
         {
             nsISupports** a = (nsISupports**) array;
             for(PRUint32 i = 0; i < initedCount; i++)
             {
                 nsISupports* p = a[i];
                 NS_IF_RELEASE(p);
             }
         }
-        else if(cleanupMode == fr && useAllocator)
+        else if(cleanupMode == fr)
         {
             void** a = (void**) array;
             for(PRUint32 i = 0; i < initedCount; i++)
             {
                 void* p = a[i];
                 if(p) nsMemory::Free(p);
             }
         }
@@ -2144,17 +2123,16 @@ XPCConvert::NativeStringWithSize2JS(JSCo
     return JS_TRUE;
 }
 
 // static
 JSBool
 XPCConvert::JSStringWithSize2Native(XPCCallContext& ccx, void* d, jsval s,
                                     JSUint32 count, JSUint32 capacity,
                                     const nsXPTType& type,
-                                    JSBool useAllocator,
                                     uintN* pErr)
 {
     NS_PRECONDITION(!JSVAL_IS_NULL(s), "bad param");
     NS_PRECONDITION(d, "bad param");
 
     JSContext* cx = ccx.GetJSContext();
 
     JSUint32 len;
@@ -2173,23 +2151,16 @@ XPCConvert::JSStringWithSize2Native(XPCC
     {
         XPC_LOG_ERROR(("XPCConvert::JSStringWithSize2Native : unsupported type"));
         return JS_FALSE;
     }
     switch(type.TagPart())
     {
         case nsXPTType::T_PSTRING_SIZE_IS:
         {
-            NS_ASSERTION(useAllocator,"cannot convert a JSString to char * without allocator");
-            if(!useAllocator)
-            {
-                XPC_LOG_ERROR(("XPCConvert::JSStringWithSize2Native : unsupported type"));
-                return JS_FALSE;
-            }
-
             if(JSVAL_IS_VOID(s) || JSVAL_IS_NULL(s))
             {
                 if(0 != count)
                 {
                     if(pErr)
                         *pErr = NS_ERROR_XPC_NOT_ENOUGH_CHARS_IN_STRING;
                     return JS_FALSE;
                 }
@@ -2263,17 +2234,17 @@ XPCConvert::JSStringWithSize2Native(XPCC
                 }
                 if(type.IsReference())
                 {
                     if(pErr)
                         *pErr = NS_ERROR_XPC_BAD_CONVERT_JS_NULL_REF;
                     return JS_FALSE;
                 }
 
-                if(useAllocator && 0 != capacity)
+                if(0 != capacity)
                 {
                     len = (capacity + 1) * sizeof(jschar);
                     if(!(*((void**)d) = nsMemory::Alloc(len)))
                         return JS_FALSE;
                     return JS_TRUE;
                 }
 
                 // else ...
@@ -2291,39 +2262,28 @@ XPCConvert::JSStringWithSize2Native(XPCC
             {
                 if(pErr)
                     *pErr = NS_ERROR_XPC_NOT_ENOUGH_CHARS_IN_STRING;
                 return JS_FALSE;
             }
             if(len < capacity)
                 len = capacity;
 
-            if(useAllocator)
+            if(!(chars = JS_GetStringCharsZ(cx, str)))
             {
-                if(!(chars = JS_GetStringCharsZ(cx, str)))
-                {
-                    return JS_FALSE;
-                }
-                JSUint32 alloc_len = (len + 1) * sizeof(jschar);
-                if(!(*((void**)d) = nsMemory::Alloc(alloc_len)))
-                {
-                    // XXX should report error
-                    return JS_FALSE;
-                }
-                memcpy(*((jschar**)d), chars, alloc_len);
-                (*((jschar**)d))[count] = 0;
+                return JS_FALSE;
             }
-            else
+            JSUint32 alloc_len = (len + 1) * sizeof(jschar);
+            if(!(*((void**)d) = nsMemory::Alloc(alloc_len)))
             {
-                if(!(chars = JS_GetStringCharsZ(cx, str)))
-                {
-                    return JS_FALSE;
-                }
-                *((const jschar**)d) = chars;
+                // XXX should report error
+                return JS_FALSE;
             }
+            memcpy(*((jschar**)d), chars, alloc_len);
+            (*((jschar**)d))[count] = 0;
 
             return JS_TRUE;
         }
         default:
             XPC_LOG_ERROR(("XPCConvert::JSStringWithSize2Native : unsupported type"));
             return JS_FALSE;
     }
 }
--- a/js/src/xpconnect/src/xpcprivate.h
+++ b/js/src/xpconnect/src/xpcprivate.h
@@ -3296,31 +3296,28 @@ public:
      */    
     static JSBool NativeArray2JS(XPCLazyCallContext& ccx,
                                  jsval* d, const void** s,
                                  const nsXPTType& type, const nsID* iid,
                                  JSUint32 count, nsresult* pErr);
 
     static JSBool JSArray2Native(XPCCallContext& ccx, void** d, jsval s,
                                  JSUint32 count, JSUint32 capacity,
-                                 const nsXPTType& type,
-                                 JSBool useAllocator, const nsID* iid,
+                                 const nsXPTType& type, const nsID* iid,
                                  uintN* pErr);
 
     static JSBool NativeStringWithSize2JS(JSContext* cx,
                                           jsval* d, const void* s,
                                           const nsXPTType& type,
                                           JSUint32 count,
                                           nsresult* pErr);
 
     static JSBool JSStringWithSize2Native(XPCCallContext& ccx, void* d, jsval s,
                                           JSUint32 count, JSUint32 capacity,
-                                          const nsXPTType& type,
-                                          JSBool useAllocator,
-                                          uintN* pErr);
+                                          const nsXPTType& type, uintN* pErr);
 
     static nsresult JSValToXPCException(XPCCallContext& ccx,
                                         jsval s,
                                         const char* ifaceName,
                                         const char* methodName,
                                         nsIException** exception);
 
     static nsresult JSErrorToXPCException(XPCCallContext& ccx,
--- a/js/src/xpconnect/src/xpcvariant.cpp
+++ b/js/src/xpconnect/src/xpcvariant.cpp
@@ -383,18 +383,17 @@ JSBool XPCVariant::InitializeData(XPCCal
         nsXPTType type;
         nsID id;
 
         if(!XPCArrayHomogenizer::GetTypeForArray(ccx, jsobj, len, &type, &id))
             return JS_FALSE; 
 
         if(!XPCConvert::JSArray2Native(ccx, &mData.u.array.mArrayValue, 
                                        val, len, len,
-                                       type, type.IsPointer(),
-                                       &id, nsnull))
+                                       type, &id, nsnull))
             return JS_FALSE;
 
         mData.mType = nsIDataType::VTYPE_ARRAY;
         if(type.IsInterfacePointer())
             mData.u.array.mArrayInterfaceID = id;
         mData.u.array.mArrayCount = len;
         mData.u.array.mArrayType = type.TagPart();
         
--- a/js/src/xpconnect/src/xpcwrappedjsclass.cpp
+++ b/js/src/xpconnect/src/xpcwrappedjsclass.cpp
@@ -384,17 +384,20 @@ GetNamedPropertyAsVariantRaw(XPCCallCont
                              jsid aName, 
                              nsIVariant** aResult,
                              nsresult* pErr)
 {
     nsXPTType type = nsXPTType((uint8)(TD_INTERFACE_TYPE | XPT_TDP_POINTER));
     jsval val;
 
     return JS_GetPropertyById(ccx, aJSObj, aName, &val) &&
-           XPCConvert::JSData2Native(ccx, aResult, val, type, JS_FALSE, 
+           // Note that this always takes the T_INTERFACE path through
+           // JSData2Native, so the value passed for useAllocator
+           // doesn't really matter. We pass true for consistency.
+           XPCConvert::JSData2Native(ccx, aResult, val, type, JS_TRUE,
                                      &NS_GET_IID(nsIVariant), pErr);
 }
 
 // static
 nsresult
 nsXPCWrappedJSClass::GetNamedPropertyAsVariant(XPCCallContext& ccx, 
                                                JSObject* aJSObj,
                                                jsval aName, 
@@ -1716,17 +1719,16 @@ pre_call_clean_up:
         if(type.IsDependent())
         {
             foundDependentParam = JS_TRUE;
             continue;
         }
 
         jsval val;
         uint8 type_tag = type.TagPart();
-        JSBool useAllocator = JS_FALSE;
         nsXPTCMiniVariant* pv;
 
         if(param.IsDipper())
             pv = (nsXPTCMiniVariant*) &nativeParams[i].val.p;
         else
             pv = (nsXPTCMiniVariant*) nativeParams[i].val.p;
 
         if(param.IsRetval())
@@ -1741,21 +1743,19 @@ pre_call_clean_up:
 
         if(type_tag == nsXPTType::T_INTERFACE)
         {
             if(NS_FAILED(GetInterfaceInfo()->
                             GetIIDForParamNoAlloc(methodIndex, &param,
                                                   &param_iid)))
                 break;
         }
-        else if(type.IsPointer() && !param.IsDipper())
-            useAllocator = JS_TRUE;
 
         if(!XPCConvert::JSData2Native(ccx, &pv->val, val, type,
-                                      useAllocator, &param_iid, nsnull))
+                                      !param.IsDipper(), &param_iid, nsnull))
             break;
     }
 
     // if any params were dependent, then we must iterate again to convert them.
     if(foundDependentParam && i == paramCount)
     {
         for(i = 0; i < paramCount; i++)
         {
@@ -1765,17 +1765,16 @@ pre_call_clean_up:
 
             const nsXPTType& type = param.GetType();
             if(!type.IsDependent())
                 continue;
 
             jsval val;
             nsXPTCMiniVariant* pv;
             nsXPTType datum_type;
-            JSBool useAllocator = JS_FALSE;
             JSUint32 array_count;
             PRBool isArray = type.IsArray();
             PRBool isSizedString = isArray ?
                     JS_FALSE :
                     type.TagPart() == nsXPTType::T_PSTRING_SIZE_IS ||
                     type.TagPart() == nsXPTType::T_PWSTRING_SIZE_IS;
 
             pv = (nsXPTCMiniVariant*) nativeParams[i].val.p;
@@ -1800,50 +1799,46 @@ pre_call_clean_up:
 
             if(datum_type.IsInterfacePointer())
             {
                if(!GetInterfaceTypeFromParam(cx, info, param, methodIndex,
                                              datum_type, nativeParams,
                                              &param_iid))
                    break;
             }
-            else if(type.IsPointer())
-                useAllocator = JS_TRUE;
 
             if(isArray || isSizedString)
             {
                 if(!GetArraySizeFromParam(cx, info, param, methodIndex,
                                           i, GET_LENGTH, nativeParams,
                                           &array_count))
                     break;
             }
 
             if(isArray)
             {
                 if(array_count &&
                    !XPCConvert::JSArray2Native(ccx, (void**)&pv->val, val,
                                                array_count, array_count,
-                                               datum_type,
-                                               useAllocator, &param_iid,
+                                               datum_type, &param_iid,
                                                nsnull))
                     break;
             }
             else if(isSizedString)
             {
                 if(!XPCConvert::JSStringWithSize2Native(ccx,
                                                    (void*)&pv->val, val,
                                                    array_count, array_count,
-                                                   datum_type, useAllocator,
-                                                   nsnull))
+                                                   datum_type, nsnull))
                     break;
             }
             else
             {
                 if(!XPCConvert::JSData2Native(ccx, &pv->val, val, type,
-                                              useAllocator, &param_iid,
+                                              JS_TRUE, &param_iid,
                                               nsnull))
                     break;
             }
         }
     }
 
     if(i != paramCount)
     {
--- a/js/src/xpconnect/src/xpcwrappednative.cpp
+++ b/js/src/xpconnect/src/xpcwrappednative.cpp
@@ -2796,17 +2796,16 @@ CallMethodHelper::ConvertIndependentPara
     }
 
     return JS_TRUE;
 }
 
 JSBool
 CallMethodHelper::ConvertIndependentParam(uint8 i)
 {
-    JSBool useAllocator = JS_FALSE;
     const nsXPTParamInfo& paramInfo = mMethodInfo->GetParam(i);
     const nsXPTType& type = paramInfo.GetType();
     uint8 type_tag = type.TagPart();
     nsXPTCVariant* dp = GetDispatchParam(i);
     dp->type = type;
     NS_ABORT_IF_FALSE(!paramInfo.IsShared(), "[shared] implies [noscript]!");
 
     // Handle dipper types separately.
@@ -2838,54 +2837,45 @@ CallMethodHelper::ConvertIndependentPara
         dp->SetValNeedsCleanup();
     }
 
     if(paramInfo.IsOut())
     {
         if(type.IsPointer() &&
            type_tag != nsXPTType::T_INTERFACE)
         {
-            useAllocator = JS_TRUE;
             dp->SetValNeedsCleanup();
         }
 
         if(!paramInfo.IsIn())
             return JS_TRUE;
     }
     else
     {
         if(type.IsPointer())
         {
             switch(type_tag)
             {
             case nsXPTType::T_IID:
                 dp->SetValNeedsCleanup();
-                useAllocator = JS_TRUE;
                 break;
             case nsXPTType::T_CHAR_STR:
                 dp->SetValNeedsCleanup();
-                useAllocator = JS_TRUE;
                 break;
             case nsXPTType::T_ASTRING:
                 // Fall through to the T_DOMSTRING case
 
             case nsXPTType::T_DOMSTRING:
-
-                // Is an 'in' DOMString. Set 'useAllocator' to indicate
-                // that JSData2Native should allocate a new
-                // nsAString.
                 dp->SetValNeedsCleanup();
-                useAllocator = JS_TRUE;
                 break;
 
             case nsXPTType::T_UTF8STRING:
                 // Fall through to the C string case for now...
             case nsXPTType::T_CSTRING:
                 dp->SetValNeedsCleanup();
-                useAllocator = JS_TRUE;
                 break;
             }
         }
 
         // Do this *after* the above because in the case where we have a
         // "T_DOMSTRING && IsDipper()" then arg might be null since this
         // is really an 'out' param masquerading as an 'in' param.
         NS_ASSERTION(i < mArgc || paramInfo.IsOptional(),
@@ -2904,18 +2894,17 @@ CallMethodHelper::ConvertIndependentPara
                                                    &param_iid)))
     {
         ThrowBadParam(NS_ERROR_XPC_CANT_GET_PARAM_IFACE_INFO, i, mCallContext);
         return JS_FALSE;
     }
 
     uintN err;
     if(!XPCConvert::JSData2Native(mCallContext, &dp->val, src, type,
-                                  useAllocator, &param_iid, &err))
-    {
+                                  JS_TRUE, &param_iid, &err)) {
         ThrowBadParam(err, i, mCallContext);
         return JS_FALSE;
     }
 
     return JS_TRUE;
 }
 
 JSBool
@@ -2928,17 +2917,16 @@ CallMethodHelper::ConvertDependentParams
         const nsXPTType& type = paramInfo.GetType();
 
         if(!type.IsDependent())
             continue;
 
         nsXPTType datum_type;
         JSUint32 array_count;
         JSUint32 array_capacity;
-        JSBool useAllocator = JS_FALSE;
         PRBool isArray = type.IsArray();
 
         PRBool isSizedString = isArray ?
             JS_FALSE :
             type.TagPart() == nsXPTType::T_PSTRING_SIZE_IS ||
             type.TagPart() == nsXPTType::T_PWSTRING_SIZE_IS;
 
         nsXPTCVariant* dp = GetDispatchParam(i);
@@ -2971,17 +2959,16 @@ CallMethodHelper::ConvertDependentParams
             return JS_FALSE;
 
         if(paramInfo.IsOut())
         {
             if(datum_type.IsPointer() &&
                !datum_type.IsInterfacePointer() &&
                isArray)
             {
-                useAllocator = JS_TRUE;
                 dp->SetValNeedsCleanup();
             }
 
             if(!paramInfo.IsIn())
                 continue;
         }
         else
         {
@@ -2990,17 +2977,16 @@ CallMethodHelper::ConvertDependentParams
             src = i < mArgc ? mArgv[i] : JSVAL_NULL;
 
             if((datum_type.IsPointer() &&
                 (datum_type.TagPart() == nsXPTType::T_IID ||
                  datum_type.TagPart() == nsXPTType::T_PSTRING_SIZE_IS) ||
                  datum_type.TagPart() == nsXPTType::T_PWSTRING_SIZE_IS) ||
                (isArray && datum_type.TagPart() == nsXPTType::T_CHAR_STR))
             {
-                useAllocator = JS_TRUE;
                 dp->SetValNeedsCleanup();
             }
         }
 
         nsID param_iid;
         if(datum_type.IsInterfacePointer() &&
            !GetInterfaceTypeFromParam(i, datum_type, &param_iid))
             return JS_FALSE;
@@ -3014,43 +3000,40 @@ CallMethodHelper::ConvertDependentParams
                 return JS_FALSE;
 
             if(isArray)
             {
                 if(array_count &&
                    !XPCConvert::JSArray2Native(mCallContext, (void**)&dp->val, src,
                                                array_count, array_capacity,
                                                datum_type,
-                                               useAllocator,
                                                &param_iid, &err))
                 {
                     // XXX need exception scheme for arrays to indicate bad element
                     ThrowBadParam(err, i, mCallContext);
                     return JS_FALSE;
                 }
             }
             else // if(isSizedString)
             {
                 if(!XPCConvert::JSStringWithSize2Native(mCallContext,
                                                         (void*)&dp->val,
                                                         src,
                                                         array_count, array_capacity,
-                                                        datum_type, useAllocator,
-                                                        &err))
+                                                        datum_type, &err))
                 {
                     ThrowBadParam(err, i, mCallContext);
                     return JS_FALSE;
                 }
             }
         }
         else
         {
             if(!XPCConvert::JSData2Native(mCallContext, &dp->val, src, type,
-                                          useAllocator, &param_iid,
-                                          &err))
+                                          JS_TRUE, &param_iid, &err))
             {
                 ThrowBadParam(err, i, mCallContext);
                 return JS_FALSE;
             }
         }
     }
 
     return JS_TRUE;