Bug 692342 - Reorder in/out/inout handling in XPCWrappedNative parameter conversion. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Fri, 25 Nov 2011 17:09:07 -0800
changeset 82432 206075dcdf40dd80588fa6676b692e278ca3f86b
parent 82431 47047b832f461be251f969588e920fb278a3dca4
child 82433 74a834deb8a22ab5b9d9bd5c25983b7381b7cf39
push id519
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 00:38:35 +0000
treeherdermozilla-beta@788ea1ef610b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs692342
milestone11.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 692342 - Reorder in/out/inout handling in XPCWrappedNative parameter conversion. r=mrbkap Apologies for the copy paste here. Fixing that will require some more serious re-architecting.
js/xpconnect/src/XPCWrappedNative.cpp
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -2633,38 +2633,49 @@ CallMethodHelper::ConvertIndependentPara
     // Handle dipper types separately.
     if (paramInfo.IsDipper())
         return HandleDipperParam(dp, paramInfo);
 
     // Specify the correct storage/calling semantics.
     if (paramInfo.IsIndirect())
         dp->SetIndirect();
 
-    jsval src;
-
-    if (!GetOutParamSource(i, &src))
-        return JS_FALSE;
-
     // The JSVal proper is always stored within the 'val' union and passed
     // indirectly, regardless of in/out-ness.
     if (type_tag == nsXPTType::T_JSVAL) {
         // Root the value.
         dp->val.j = JSVAL_VOID;
         if (!JS_AddValueRoot(mCallContext, &dp->val.j))
             return JS_FALSE;
     }
 
     // Flag cleanup for anything that isn't self-contained.
     if (!type.IsArithmetic())
         dp->SetValNeedsCleanup();
 
-    if (paramInfo.IsOut()) {
-        if(!paramInfo.IsIn())
-            return JS_TRUE;
-    } else {
+    // Even if there's nothing to convert, we still need to examine the
+    // JSObject container for out-params. If it's null or otherwise invalid,
+    // we want to know before the call, rather than after.
+    //
+    // This is a no-op for 'in' params.
+    jsval src;
+    if (!GetOutParamSource(i, &src))
+        return JS_FALSE;
+
+    // All that's left to do is value conversion. Bail early if we don't need
+    // to do that.
+    if (!paramInfo.IsIn())
+        return JS_TRUE;
+
+    // We're definitely some variety of 'in' now, so there's something to
+    // convert. The source value for conversion depends on whether we're
+    // dealing with an 'in' or an 'inout' parameter. 'inout' was handled above,
+    // so all that's left is 'in'.
+    if (!paramInfo.IsOut()) {
+        // Handle the 'in' case.
         NS_ASSERTION(i < mArgc || paramInfo.IsOptional(),
                      "Expected either enough arguments or an optional argument");
         if (i < mArgc)
             src = mArgv[i];
         else if (type_tag == nsXPTType::T_JSVAL)
             src = JSVAL_VOID;
         else
             src = JSVAL_NULL;
@@ -2736,25 +2747,36 @@ CallMethodHelper::ConvertDependentParam(
     } else {
         datum_type = type;
     }
 
     if (datum_type.IsInterfacePointer()) {
         dp->SetValNeedsCleanup();
     }
 
+    // Even if there's nothing to convert, we still need to examine the
+    // JSObject container for out-params. If it's null or otherwise invalid,
+    // we want to know before the call, rather than after.
+    //
+    // This is a no-op for 'in' params.
     jsval src;
-
     if (!GetOutParamSource(i, &src))
         return JS_FALSE;
 
-    if (paramInfo.IsOut()) {
-        if (!paramInfo.IsIn())
-            return JS_TRUE;
-    } else {
+    // All that's left to do is value conversion. Bail early if we don't need
+    // to do that.
+    if (!paramInfo.IsIn())
+        return JS_TRUE;
+
+    // We're definitely some variety of 'in' now, so there's something to
+    // convert. The source value for conversion depends on whether we're
+    // dealing with an 'in' or an 'inout' parameter. 'inout' was handled above,
+    // so all that's left is 'in'.
+    if (!paramInfo.IsOut()) {
+        // Handle the 'in' case.
         NS_ASSERTION(i < mArgc || paramInfo.IsOptional(),
                      "Expected either enough arguments or an optional argument");
         src = i < mArgc ? mArgv[i] : JSVAL_NULL;
 
         if (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)) {