Bug 877281 part 4. Eliminate uses of ${valHandle} in binding conversions and make ${val} be a handle. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 07 Jun 2013 22:45:45 -0400
changeset 134430 20f763a69b1f4dd9ae86ef6c91936968494dbb50
parent 134429 98ec2d7ed87f37e95eed4f6a9e8274a930885163
child 134431 f047d17cdb7830f6990ee157a6e0efcecfeddd0f
push id29211
push userbzbarsky@mozilla.com
push dateSat, 08 Jun 2013 02:50:12 +0000
treeherdermozilla-inbound@d27cb123e9de [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs877281
milestone24.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 877281 part 4. Eliminate uses of ${valHandle} in binding conversions and make ${val} be a handle. r=peterv
dom/bindings/Codegen.py
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -2421,18 +2421,17 @@ class JSToNativeConversionInfo():
     """
     def __init__(self, template, declType=None, holderType=None,
                  dealWithOptional=False, declArgs=None,
                  holderArgs=None):
         """
         template: A string representing the conversion code.  This will have
                   template substitution performed on it as follows:
 
-          ${val} replaced by an expression for the JS::Value in question
-          ${valHandle} is a handle to the JS::Value in question
+          ${val} is a handle to the JS::Value in question
           ${mutableVal} is a mutable handle to the JS::Value in question
           ${holderName} replaced by the holder's name, if any
           ${declName} replaced by the declaration's name
           ${haveValue} replaced by an expression that evaluates to a boolean
                        for whether we have a JS::Value.  Only used when
                        defaultValue is not None or when True is passed for
                        checkForValue to instantiateJSToNativeConversion.
 
@@ -2452,22 +2451,22 @@ class JSToNativeConversionInfo():
                           both declType and holderType to be wrapped in
                           Optional<>, with ${declName} and ${holderName}
                           adjusted to point to the Value() of the Optional, and
                           Construct() calls to be made on the Optional<>s as
                           needed.
 
         declArgs: If not None, the arguments to pass to the ${declName}
                   constructor.  These will have template substitution performed
-                  on them so you can use things like ${valHandle}.  This is a
+                  on them so you can use things like ${val}.  This is a
                   single string, not a list of strings.
 
         holderArgs: If not None, the arguments to pass to the ${holderName}
                     constructor.  These will have template substitution
-                    performed on them so you can use things like ${valHandle}.
+                    performed on them so you can use things like ${val}.
                     This is a single string, not a list of strings.
 
         ${declName} must be in scope before the code from 'template' is entered.
 
         If holderType is not None then ${holderName} must be in scope before
         the code from 'template' is entered.
         """
         assert isinstance(template, str)
@@ -2654,17 +2653,17 @@ def getJSToNativeConversionInfo(type, de
                 declType = CGGeneric("JS::Handle<JSObject*>")
             else:
                 declType = CGGeneric("JS::Rooted<JSObject*>")
         else:
             assert (isMember == "Sequence" or isMember == "Variadic" or
                     isMember == "Dictionary")
             # We'll get traced by the sequence or dictionary tracer
             declType = CGGeneric("JSObject*")
-        templateBody = "${declName} = &${valHandle}.toObject();"
+        templateBody = "${declName} = &${val}.toObject();"
         setToNullCode = "${declName} = nullptr;"
         template = wrapObjectTemplate(templateBody, type, setToNullCode,
                                       failureCode)
         return JSToNativeConversionInfo(template, declType=declType,
                                         dealWithOptional=isOptional,
                                         declArgs="cx")
 
     assert not (isEnforceRange and isClamp) # These are mutually exclusive
@@ -2752,18 +2751,17 @@ for (uint32_t i = 0; i < length; ++i) {
        arrayRef,
        exceptionCodeIndented.define(),
        CGIndenter(exceptionCodeIndented).define(),
        elementInfo.declType.define()))
 
         templateBody += CGIndenter(CGGeneric(
                 string.Template(elementInfo.template).substitute(
                     {
-                        "val" : "temp",
-                        "valHandle": "temp",
+                        "val": "temp",
                         "mutableVal": "&temp",
                         "declName" : "slot",
                         # We only need holderName here to handle isExternal()
                         # interfaces, which use an internal holder for the
                         # conversion even when forceOwningType ends up true.
                         "holderName": "tempHolder",
                         }
                     ))).define()
@@ -2806,28 +2804,28 @@ for (uint32_t i = 0; i < length; ++i) {
         interfaceMemberTypes = filter(lambda t: t.isNonCallbackInterface(), memberTypes)
         if len(interfaceMemberTypes) > 0:
             interfaceObject = []
             for memberType in interfaceMemberTypes:
                 if type.isGeckoInterface():
                     name = memberType.inner.identifier.name
                 else:
                     name = memberType.name
-                interfaceObject.append(CGGeneric("(failed = !%s.TrySetTo%s(cx, ${valHandle}, ${mutableVal}, tryNext)) || !tryNext" % (unionArgumentObj, name)))
+                interfaceObject.append(CGGeneric("(failed = !%s.TrySetTo%s(cx, ${val}, ${mutableVal}, tryNext)) || !tryNext" % (unionArgumentObj, name)))
                 names.append(name)
             interfaceObject = CGWrapper(CGList(interfaceObject, " ||\n"), pre="done = ", post=";\n", reindent=True)
         else:
             interfaceObject = None
 
         arrayObjectMemberTypes = filter(lambda t: t.isArray() or t.isSequence(), memberTypes)
         if len(arrayObjectMemberTypes) > 0:
             assert len(arrayObjectMemberTypes) == 1
             memberType = arrayObjectMemberTypes[0]
             name = memberType.name
-            arrayObject = CGGeneric("done = (failed = !%s.TrySetTo%s(cx, ${valHandle}, ${mutableVal}, tryNext)) || !tryNext;" % (unionArgumentObj, name))
+            arrayObject = CGGeneric("done = (failed = !%s.TrySetTo%s(cx, ${val}, ${mutableVal}, tryNext)) || !tryNext;" % (unionArgumentObj, name))
             arrayObject = CGIfWrapper(arrayObject, "IsArrayLike(cx, argObj)")
             names.append(name)
         else:
             arrayObject = None
 
         dateObjectMemberTypes = filter(lambda t: t.isDate(), memberTypes)
         if len(dateObjectMemberTypes) > 0:
             assert len(dateObjectMemberTypes) == 1
@@ -2840,17 +2838,17 @@ for (uint32_t i = 0; i < length; ++i) {
         else:
             dateObject = None
 
         callbackMemberTypes = filter(lambda t: t.isCallback() or t.isCallbackInterface(), memberTypes)
         if len(callbackMemberTypes) > 0:
             assert len(callbackMemberTypes) == 1
             memberType = callbackMemberTypes[0]
             name = memberType.name
-            callbackObject = CGGeneric("done = (failed = !%s.TrySetTo%s(cx, ${valHandle}, ${mutableVal}, tryNext)) || !tryNext;" % (unionArgumentObj, name))
+            callbackObject = CGGeneric("done = (failed = !%s.TrySetTo%s(cx, ${val}, ${mutableVal}, tryNext)) || !tryNext;" % (unionArgumentObj, name))
             names.append(name)
         else:
             callbackObject = None
 
         dictionaryMemberTypes = filter(lambda t: t.isDictionary(), memberTypes)
         if len(dictionaryMemberTypes) > 0:
             raise TypeError("No support for unwrapping dictionaries as member "
                             "of a union")
@@ -2884,32 +2882,32 @@ for (uint32_t i = 0; i < length; ++i) {
                 if templateBody:
                     templateBody = CGIfWrapper(templateBody, "!done")
                 templateBody = CGList([interfaceObject, templateBody], "\n")
             else:
                 templateBody = CGList([templateBody, object], "\n")
 
             if any([arrayObject, dateObject, callbackObject, dictionaryObject,
                     object]):
-                templateBody.prepend(CGGeneric("JS::Rooted<JSObject*> argObj(cx, &${valHandle}.toObject());"))
-            templateBody = CGIfWrapper(templateBody, "${valHandle}.isObject()")
+                templateBody.prepend(CGGeneric("JS::Rooted<JSObject*> argObj(cx, &${val}.toObject());"))
+            templateBody = CGIfWrapper(templateBody, "${val}.isObject()")
         else:
             templateBody = CGGeneric()
 
         otherMemberTypes = filter(lambda t: t.isString() or t.isEnum(),
                                   memberTypes)
         otherMemberTypes.extend(t for t in memberTypes if t.isPrimitive())
         if len(otherMemberTypes) > 0:
             assert len(otherMemberTypes) == 1
             memberType = otherMemberTypes[0]
             if memberType.isEnum():
                 name = memberType.inner.identifier.name
             else:
                 name = memberType.name
-            other = CGGeneric("done = (failed = !%s.TrySetTo%s(cx, ${valHandle}, ${mutableVal}, tryNext)) || !tryNext;" % (unionArgumentObj, name))
+            other = CGGeneric("done = (failed = !%s.TrySetTo%s(cx, ${val}, ${mutableVal}, tryNext)) || !tryNext;" % (unionArgumentObj, name))
             names.append(name)
             if hasObjectTypes:
                 other = CGWrapper(CGIndenter(other), "{\n", post="\n}")
                 if object:
                     join = " else "
                 else:
                     other = CGWrapper(other, pre="if (!done) ")
                     join = "\n"
@@ -3063,17 +3061,17 @@ for (uint32_t i = 0; i < length; ++i) {
             callbackConversion = str(CallbackObjectUnwrapper(
                     descriptor,
                     "callbackObj",
                     "${declName}",
                     exceptionCode,
                     codeOnFailure=failureCode))
             templateBody += (
                 "{ // Scope for callbackObj\n"
-                "  JS::Rooted<JSObject*> callbackObj(cx, &${valHandle}.toObject());\n" +
+                "  JS::Rooted<JSObject*> callbackObj(cx, &${val}.toObject());\n" +
                 CGIndenter(CGGeneric(callbackConversion)).define() +
                 "\n}")
         elif not descriptor.skipGen and not descriptor.interface.isConsequential() and not descriptor.interface.isExternal():
             if failureCode is not None:
                 templateBody += str(CastableObjectUnwrapper(
                         descriptor,
                         "&${val}.toObject()",
                         "${declName}",
@@ -3209,17 +3207,17 @@ for (uint32_t i = 0; i < length; ++i) {
             # specified otherwise.
             if treatUndefinedAs == "Default":
                 treatUndefinedAs = "Null"
         nullBehavior = treatAs[treatNullAs]
         undefinedBehavior = treatAs[treatUndefinedAs]
 
         def getConversionCode(varName):
             conversionCode = (
-                "if (!ConvertJSValueToString(cx, ${valHandle}, ${mutableVal}, %s, %s, %s)) {\n"
+                "if (!ConvertJSValueToString(cx, ${val}, ${mutableVal}, %s, %s, %s)) {\n"
                 "%s\n"
                 "}" % (nullBehavior, undefinedBehavior, varName,
                        exceptionCodeIndented.define()))
             if defaultValue is None:
                 return conversionCode
 
             if isinstance(defaultValue, IDLNullValue):
                 assert(type.nullable())
@@ -3325,17 +3323,17 @@ for (uint32_t i = 0; i < length; ++i) {
         assert not type.treatNonCallableAsNull() or type.nullable()
 
         if descriptorProvider.workers:
             if isMember:
                 raise NoSuchDescriptorError("Can't handle member callbacks in "
                                             "workers; need to sort out rooting"
                                             "issues")
             declType = CGGeneric("JS::Rooted<JSObject*>")
-            conversion = "  ${declName} = &${valHandle}.toObject();\n"
+            conversion = "  ${declName} = &${val}.toObject();\n"
             declArgs = "cx"
         else:
             name = type.unroll().identifier.name
             if type.nullable():
                 declType = CGGeneric("nsRefPtr<%s>" % name);
             else:
                 declType = CGGeneric("OwningNonNull<%s>" % name)
             conversion = (
@@ -3420,27 +3418,27 @@ for (uint32_t i = 0; i < length; ++i) {
         # We do manual default value handling here, because we
         # actually do want a jsval, and we only handle null anyway
         # NOTE: if isNullOrUndefined or isDefinitelyObject are true,
         # we know we have a value, so we don't have to worry about the
         # default value.
         if (not isNullOrUndefined and not isDefinitelyObject and
             defaultValue is not None):
             assert(isinstance(defaultValue, IDLNullValue))
-            val = "(${haveValue}) ? ${valHandle} : JS::NullHandleValue"
-        else:
-            val = "${valHandle}"
+            val = "(${haveValue}) ? ${val} : JS::NullHandleValue"
+        else:
+            val = "${val}"
 
         if failureCode is not None:
             assert isDefinitelyObject
             # Check that the value we have can in fact be converted to
             # a dictionary, and return failureCode if not.
             template = CGIfWrapper(
                 CGGeneric(failureCode),
-                "!IsObjectValueConvertibleToDictionary(cx, ${valHandle})").define() + "\n\n"
+                "!IsObjectValueConvertibleToDictionary(cx, ${val})").define() + "\n\n"
         else:
             template = ""
 
         template += ("if (!${declName}.Init(cx, %s)) {\n"
                      "%s\n"
                      "}" % (val, exceptionCodeIndented.define()))
 
         # Dictionary arguments that might contain traceable things need to get
@@ -3507,27 +3505,27 @@ for (uint32_t i = 0; i < length; ++i) {
         writeLoc = "${declName}.SetValue()"
         readLoc = "${declName}.Value()"
         nullCondition = "${val}.isNullOrUndefined()"
         if defaultValue is not None and isinstance(defaultValue, IDLNullValue):
             nullCondition = "!(${haveValue}) || " + nullCondition
         template = (
             "if (%s) {\n"
             "  ${declName}.SetNull();\n"
-            "} else if (!ValueToPrimitive<%s, %s>(cx, ${valHandle}, &%s)) {\n"
+            "} else if (!ValueToPrimitive<%s, %s>(cx, ${val}, &%s)) {\n"
             "%s\n"
             "}" % (nullCondition, typeName, conversionBehavior,
                    writeLoc, exceptionCodeIndented.define()))
     else:
         assert(defaultValue is None or
                not isinstance(defaultValue, IDLNullValue))
         writeLoc = "${declName}"
         readLoc = writeLoc
         template = (
-            "if (!ValueToPrimitive<%s, %s>(cx, ${valHandle}, &%s)) {\n"
+            "if (!ValueToPrimitive<%s, %s>(cx, ${val}, &%s)) {\n"
             "%s\n"
             "}" % (typeName, conversionBehavior, writeLoc,
                    exceptionCodeIndented.define()))
         declType = CGGeneric(typeName)
 
     if type.isFloat() and not type.isUnrestricted():
         if lenientFloatCode is not None:
             nonFiniteCode = CGIndenter(CGGeneric(lenientFloatCode)).define()
@@ -3696,17 +3694,16 @@ class CGArgumentConverter(CGThing):
         self.replacementVariables = {
             "declName" : "arg%d" % index,
             "holderName" : ("arg%d" % index) + "_holder",
             "obj" : "obj"
             }
         self.replacementVariables["val"] = string.Template(
             "args.handleAt(${index})"
             ).substitute(replacer)
-        self.replacementVariables["valHandle"] = self.replacementVariables["val"]
         self.replacementVariables["mutableVal"] = self.replacementVariables["val"]
         if argument.treatUndefinedAs == "Missing":
             haveValueCheck = "args.hasDefined(${index})"
         else:
             haveValueCheck = "${index} < args.length()"
         haveValueCheck = string.Template(haveValueCheck).substitute(replacer)
         self.replacementVariables["haveValue"] = haveValueCheck
         self.descriptorProvider = descriptorProvider
@@ -3768,17 +3765,16 @@ class CGArgumentConverter(CGThing):
     ${elemType}& slot = *${declName}.AppendElement();
 """).substitute(replacer)
 
         val = string.Template("args.handleAt(variadicArg)").substitute(replacer)
         variadicConversion += CGIndenter(CGGeneric(
                 string.Template(typeConversion.template).substitute(
                     {
                         "val" : val,
-                        "valHandle" : val,
                         "mutableVal" : val,
                         "declName" : "slot",
                         # We only need holderName here to handle isExternal()
                         # interfaces, which use an internal holder for the
                         # conversion even when forceOwningType ends up true.
                         "holderName": "tempHolder",
                         # Use the same ${obj} as for the variadic arg itself
                         "obj": replacer["obj"]
@@ -4809,17 +4805,16 @@ class CGMethodCall(CGThing):
                     getJSToNativeConversionInfo(type, descriptor,
                                                 failureCode=failureCode,
                                                 isDefinitelyObject=isDefinitelyObject,
                                                 isNullOrUndefined=isNullOrUndefined),
                     {
                         "declName" : "arg%d" % distinguishingIndex,
                         "holderName" : ("arg%d" % distinguishingIndex) + "_holder",
                         "val" : distinguishingArg,
-                        "valHandle" : distinguishingArg,
                         "mutableVal" : distinguishingArg,
                         "obj" : "obj"
                         })
                 caseBody.append(CGIndenter(testCode, indent));
                 # If we got this far, we know we unwrapped to the right
                 # C++ type, so just do the call.  Start conversion with
                 # distinguishingIndex + 1, since we already converted
                 # distinguishingIndex.
@@ -5721,17 +5716,16 @@ return true;"""
                            "{\n"
                            "  mUnion.mValue.mObject.SetValue(cx, obj);\n"
                            "  mUnion.mType = mUnion.eObject;\n"
                            "}")
     else:
         jsConversion = string.Template(conversionInfo.template).substitute(
             {
                 "val": "value",
-                "valHandle": "value",
                 "mutableVal": "pvalue",
                 "declName": "SetAs" + name + "()",
                 "holderName": "m" + name + "Holder",
                 }
             )
         jsConversion = CGWrapper(CGGeneric(jsConversion),
                                  post="\n"
                                       "return true;")
@@ -6579,18 +6573,17 @@ class CGProxySpecialOperation(CGPerSigna
             # arguments[0] is the index or name of the item that we're setting.
             argument = arguments[1]
             info = getJSToNativeConversionInfo(argument.type, descriptor,
                                                treatNullAs=argument.treatNullAs,
                                                treatUndefinedAs=argument.treatUndefinedAs)
             templateValues = {
                 "declName": argument.identifier.name,
                 "holderName": argument.identifier.name + "_holder",
-                "val": "desc->value",
-                "valHandle" : "JS::Handle<JS::Value>::fromMarkedLocation(&desc->value)",
+                "val" : "JS::Handle<JS::Value>::fromMarkedLocation(&desc->value)",
                 "mutableVal" : "JS::MutableHandle<JS::Value>::fromMarkedLocation(&desc->value)",
                 "obj": "obj"
             }
             self.cgRoot.prepend(instantiateJSToNativeConversion(info, templateValues))
         elif operation.isGetter() or operation.isDeleter():
             self.cgRoot.prepend(CGGeneric("bool found;"))
 
     def getArguments(self):
@@ -7787,17 +7780,16 @@ class CGDictionary(CGThing):
         declType = conversionInfo.declType
         if conversionInfo.dealWithOptional:
             declType = CGTemplatedType("Optional", declType)
         return declType.define()
 
     def getMemberConversion(self, memberInfo):
         (member, conversionInfo) = memberInfo
         replacements = { "val": "temp",
-                         "valHandle": "temp",
                          "mutableVal": "&temp",
                          "declName": self.makeMemberName(member.identifier.name),
                          # We need a holder name for external interfaces, but
                          # it's scoped down to the conversion so we can just use
                          # anything we want.
                          "holderName": "holder" }
         # We can't handle having a holderType here
         assert conversionInfo.holderType is None
@@ -9408,17 +9400,16 @@ class CallbackMember(CGNativeMember):
             "${argvDecl}"
             "${convertArgs}"
             "${doCall}"
             "${returnResult}").substitute(replacements)
 
     def getResultConversion(self):
         replacements = {
             "val": "rval",
-            "valHandle": "rval",
             "mutableVal": "&rval",
             "holderName" : "rvalHolder",
             "declName" : "rvalDecl",
             # We actually want to pass in a null scope object here, because
             # wrapping things into our current compartment (that of mCallback)
             # is what we want.
             "obj": "nullptr"
             }