Bug 1036777 - Stop relying on useAllocator for 'in' string classes in ConvertIndependentParam. r=neil
authorBobby Holley <bobbyholley@gmail.com>
Fri, 11 Jul 2014 19:21:22 -0700
changeset 193719 9f6e47d8296d2df67a2ef4e5b5ee5717712d004c
parent 193718 42c54f0e2bf8af2029edb115dfa7c51416de4a9d
child 193720 18783d1e2a77351c3a5a86ab32d88c8e82364c99
push id27128
push usercbook@mozilla.com
push dateMon, 14 Jul 2014 12:35:14 +0000
treeherdermozilla-central@415c2a4dc778 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersneil
bugs1036777
milestone33.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 1036777 - Stop relying on useAllocator for 'in' string classes in ConvertIndependentParam. r=neil
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCWrappedNative.cpp
xpcom/reflect/xptinfo/xptinfo.h
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -1492,19 +1492,16 @@ XPCJSRuntime::NewShortLivedString()
 
     // All our internal string wrappers are used, allocate a new string.
     return new nsString();
 }
 
 void
 XPCJSRuntime::DeleteShortLivedString(nsString *string)
 {
-    if (string == &EmptyString() || string == &NullString())
-        return;
-
     for (uint32_t i = 0; i < XPCCCX_STRING_CACHE_SIZE; ++i) {
         if (!mScratchStrings[i].empty() &&
             mScratchStrings[i].addr() == string) {
             // One of our internal strings is no longer in use, mark
             // it as such and free its data.
             mScratchStrings[i].destroy();
             return;
         }
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1651,18 +1651,18 @@ class MOZ_STACK_CLASS CallMethodHelper
 
     MOZ_ALWAYS_INLINE bool ConvertIndependentParams(bool* foundDependentParam);
     MOZ_ALWAYS_INLINE bool ConvertIndependentParam(uint8_t i);
     MOZ_ALWAYS_INLINE bool ConvertDependentParams();
     MOZ_ALWAYS_INLINE bool ConvertDependentParam(uint8_t i);
 
     MOZ_ALWAYS_INLINE void CleanupParam(nsXPTCMiniVariant& param, nsXPTType& type);
 
-    MOZ_ALWAYS_INLINE bool HandleDipperParam(nsXPTCVariant* dp,
-                                             const nsXPTParamInfo& paramInfo);
+    MOZ_ALWAYS_INLINE bool AllocateStringClass(nsXPTCVariant* dp,
+                                               const nsXPTParamInfo& paramInfo);
 
     MOZ_ALWAYS_INLINE nsresult Invoke();
 
 public:
 
     CallMethodHelper(XPCCallContext& ccx)
         : mCallContext(ccx)
         , mInvokeResult(NS_ERROR_UNEXPECTED)
@@ -2091,19 +2091,28 @@ CallMethodHelper::ConvertIndependentPara
 {
     const nsXPTParamInfo& paramInfo = mMethodInfo->GetParam(i);
     const nsXPTType& type = paramInfo.GetType();
     uint8_t type_tag = type.TagPart();
     nsXPTCVariant* dp = GetDispatchParam(i);
     dp->type = type;
     MOZ_ASSERT(!paramInfo.IsShared(), "[shared] implies [noscript]!");
 
-    // Handle dipper types separately.
-    if (paramInfo.IsDipper())
-        return HandleDipperParam(dp, paramInfo);
+    // String classes are always "in" - those that are marked "out" are converted
+    // by the XPIDL compiler to "in+dipper". See the note above IsDipper() in
+    // xptinfo.h.
+    //
+    // Also note that the fact that we bail out early for dipper parameters means
+    // that "inout" dipper parameters don't work - see bug 687612.
+    if (paramInfo.IsStringClass()) {
+        if (!AllocateStringClass(dp, paramInfo))
+            return false;
+        if (paramInfo.IsDipper())
+            return true;
+    }
 
     // Specify the correct storage/calling semantics.
     if (paramInfo.IsIndirect())
         dp->SetIndirect();
 
     // The JSVal proper is always stored within the 'val' union and passed
     // indirectly, regardless of in/out-ness.
     if (type_tag == nsXPTType::T_JSVAL) {
@@ -2151,17 +2160,17 @@ CallMethodHelper::ConvertIndependentPara
     if (type_tag == nsXPTType::T_INTERFACE &&
         NS_FAILED(mIFaceInfo->GetIIDForParamNoAlloc(mVTableIndex, &paramInfo,
                                                     &param_iid))) {
         ThrowBadParam(NS_ERROR_XPC_CANT_GET_PARAM_IFACE_INFO, i, mCallContext);
         return false;
     }
 
     nsresult err;
-    if (!XPCConvert::JSData2Native(&dp->val, src, type, true, &param_iid, &err)) {
+    if (!XPCConvert::JSData2Native(&dp->val, src, type, !paramInfo.IsStringClass(), &param_iid, &err)) {
         ThrowBadParam(err, i, mCallContext);
         return false;
     }
 
     return true;
 }
 
 bool
@@ -2312,65 +2321,39 @@ CallMethodHelper::CleanupParam(nsXPTCMin
         case nsXPTType::T_ASTRING:
         case nsXPTType::T_DOMSTRING:
             nsXPConnect::GetRuntimeInstance()->DeleteShortLivedString((nsString*)param.val.p);
             break;
         case nsXPTType::T_UTF8STRING:
         case nsXPTType::T_CSTRING:
             {
                 nsCString* rs = (nsCString*)param.val.p;
-                if (rs != &EmptyCString() && rs != &NullCString())
-                    delete rs;
+                delete rs;
             }
             break;
         default:
             MOZ_ASSERT(!type.IsArithmetic(), "Cleanup requested on unexpected type.");
             nsMemory::Free(param.val.p);
             break;
     }
 }
 
-// Handle parameters with dipper types.
-//
-// Dipper types are one of the more inscrutable aspects of xpidl. In a
-// nutshell, dippers are empty container objects, created and passed by
-// the caller, and filled by the callee. The callee receives a
-// fully-formed object, and thus does not have to construct anything. But
-// the object is functionally empty, and the callee is responsible for
-// putting something useful inside of it.
-//
-// XPIDL decides which types to make dippers. The list of these types
-// is given in the isDipperType() function in typelib.py, and is currently
-// limited to 4 string types.
-//
-// When a dipper type is declared as an 'out' parameter, xpidl internally
-// converts it to an 'in', and sets the XPT_PD_DIPPER flag on it. For this
-// reason, dipper types are sometimes referred to as 'out parameters
-// masquerading as in'. The burden of maintaining this illusion falls mostly
-// on XPConnect - we create the empty containers, and harvest the results
-// after the call.
-//
-// This method creates these empty containers.
 bool
-CallMethodHelper::HandleDipperParam(nsXPTCVariant* dp,
-                                    const nsXPTParamInfo& paramInfo)
+CallMethodHelper::AllocateStringClass(nsXPTCVariant* dp,
+                                      const nsXPTParamInfo& paramInfo)
 {
     // Get something we can make comparisons with.
     uint8_t type_tag = paramInfo.GetType().TagPart();
 
-    // Dippers always have the 'in' and 'dipper' flags set. Never 'out'.
-    MOZ_ASSERT(!paramInfo.IsOut(), "Dipper has unexpected flags.");
-
-    // xpidl.h specifies that dipper types will be used in exactly four
-    // cases, all strings. Verify that here.
+    // There should be 4 cases, all strings. Verify that here.
     MOZ_ASSERT(type_tag == nsXPTType::T_ASTRING ||
                type_tag == nsXPTType::T_DOMSTRING ||
                type_tag == nsXPTType::T_UTF8STRING ||
                type_tag == nsXPTType::T_CSTRING,
-               "Unexpected dipper type!");
+               "Unexpected string class type!");
 
     // ASTRING and DOMSTRING are very similar, and both use nsString.
     // UTF8_STRING and CSTRING are also quite similar, and both use nsCString.
     if (type_tag == nsXPTType::T_ASTRING || type_tag == nsXPTType::T_DOMSTRING)
         dp->val.p = nsXPConnect::GetRuntimeInstance()->NewShortLivedString();
     else
         dp->val.p = new nsCString();
 
--- a/xpcom/reflect/xptinfo/xptinfo.h
+++ b/xpcom/reflect/xptinfo/xptinfo.h
@@ -127,20 +127,50 @@ public:
     nsXPTParamInfo(const XPTParamDescriptor& desc)
         {*(XPTParamDescriptor*)this = desc;}
 
 
     bool IsIn()  const    {return 0 != (XPT_PD_IS_IN(flags));}
     bool IsOut() const    {return 0 != (XPT_PD_IS_OUT(flags));}
     bool IsRetval() const {return 0 != (XPT_PD_IS_RETVAL(flags));}
     bool IsShared() const {return 0 != (XPT_PD_IS_SHARED(flags));}
+
+    // Dipper types are one of the more inscrutable aspects of xpidl. In a
+    // nutshell, dippers are empty container objects, created and passed by
+    // the caller, and filled by the callee. The callee receives a fully-
+    // formed object, and thus does not have to construct anything. But
+    // the object is functionally empty, and the callee is responsible for
+    // putting something useful inside of it.
+    //
+    // XPIDL decides which types to make dippers. The list of these types
+    // is given in the isDipperType() function in typelib.py, and is currently
+    // limited to 4 string types.
+    //
+    // When a dipper type is declared as an 'out' parameter, xpidl internally
+    // converts it to an 'in', and sets the XPT_PD_DIPPER flag on it. For this
+    // reason, dipper types are sometimes referred to as 'out parameters
+    // masquerading as in'. The burden of maintaining this illusion falls mostly
+    // on XPConnect, which creates the empty containers, and harvest the results
+    // after the call.
     bool IsDipper() const {return 0 != (XPT_PD_IS_DIPPER(flags));}
     bool IsOptional() const {return 0 != (XPT_PD_IS_OPTIONAL(flags));}
     const nsXPTType GetType() const {return type.prefix;}
 
+    bool IsStringClass() const {
+      switch (GetType().TagPart()) {
+        case nsXPTType::T_ASTRING:
+        case nsXPTType::T_DOMSTRING:
+        case nsXPTType::T_UTF8STRING:
+        case nsXPTType::T_CSTRING:
+          return true;
+        default:
+          return false;
+      }
+    }
+
     // Whether this parameter is passed indirectly on the stack. This mainly
     // applies to out/inout params, but we use it unconditionally for certain
     // types.
     bool IsIndirect() const {return IsOut() ||
                                GetType().TagPart() == nsXPTType::T_JSVAL;}
 
     // NOTE: other activities on types are done via methods on nsIInterfaceInfo