Bug 942631 part 6. Fix performance regression in the imagedata getter due to using MaybeWrapValue on a non-DOM object. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 04 Dec 2013 08:02:18 -0500
changeset 173488 f96a77402a2684a9d86716cb62fe147e79d3c42c
parent 173487 fad25ed7cdef3f852e086bac57680602436db71e
child 173489 cb8aa9f8396867bf7567ca65ca4a0e7782353e61
push id3224
push userlsblakk@mozilla.com
push dateTue, 04 Feb 2014 01:06:49 +0000
treeherdermozilla-beta@60c04d0987f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs942631
milestone28.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 942631 part 6. Fix performance regression in the imagedata getter due to using MaybeWrapValue on a non-DOM object. r=peterv
dom/bindings/Codegen.py
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -4120,16 +4120,36 @@ class CGArgumentConverter(CGThing):
                         }
                     )), 4).define()
 
         variadicConversion += ("\n"
                                "  }\n"
                                "}")
         return variadicConversion
 
+def getMaybeWrapValueFuncForType(type):
+    # Callbacks might actually be DOM objects; nothing prevents a page from
+    # doing that.
+    if type.isCallback() or type.isCallbackInterface() or type.isObject():
+        if type.nullable():
+            return "MaybeWrapObjectOrNullValue"
+        return "MaybeWrapObjectValue"
+    # Spidermonkey interfaces are never DOM objects
+    if type.isSpiderMonkeyInterface():
+        if type.nullable():
+            return "MaybeWrapNonDOMObjectOrNullValue"
+        return "MaybeWrapNonDOMObjectValue"
+    if type.isAny():
+        return "MaybeWrapValue"
+
+    # For other types, just go ahead an fall back on MaybeWrapValue for now:
+    # it's always safe to do, and shouldn't be particularly slow for any of
+    # them
+    return "MaybeWrapValue"
+
 sequenceWrapLevel = 0
 
 def getWrapTemplateForType(type, descriptorProvider, result, successCode,
                            returnsNewObject, exceptionCode, typedArraysAreStructs):
     """
     Reflect a C++ value stored in "result", of IDL type "type" into JS.  The
     "successCode" is the code to run once we have successfully done the
     conversion and must guarantee that execution of the conversion template
@@ -4156,40 +4176,29 @@ def getWrapTemplateForType(type, descrip
     """
     if successCode is None:
         successCode = "return true;"
 
     # We often want exceptionCode to be indented, since it often appears in an
     # if body.
     exceptionCodeIndented = CGIndenter(CGGeneric(exceptionCode))
 
-    def setValue(value, callWrapValue=None):
+    def setValue(value, wrapAsType=None):
         """
         Returns the code to set the jsval to value.
 
-        "callWrapValue" can be set to the following values:
-          * None: no wrapping will be done.
-          * "object": will wrap using MaybeWrapObjectValue.
-          * "objectOrNull": will wrap using MaybeWrapObjectOrNullValue.
-          * "nonDOMObject": will wrap using MaybeWrapNonDOMObjectValue.
-          * "nonDOMObjectOrNull": will wrap using MaybeWrapNonDOMObjectOrNullValue
-          * "value": will wrap using MaybeWrapValue.
+        If wrapAsType is not None, then will wrap the resulting value using the
+        function that getMaybeWrapValueFuncForType(wrapAsType) returns.
+        Otherwise, no wrapping will be done.
         """
-        if not callWrapValue:
+        if wrapAsType is None:
             tail = successCode
         else:
-            methodMap = {
-                "object": "MaybeWrapObjectValue",
-                "objectOrNull": "MaybeWrapObjectOrNullValue",
-                "nonDOMObject": "MaybeWrapNonDOMObjectValue",
-                "nonDOMObjectOrNull": "MaybeWrapNonDOMObjectOrNullValue",
-                "value": "MaybeWrapValue"
-                }
             tail = (("if (!%s(cx, ${jsvalHandle})) {\n"
-                     "%s\n" % (methodMap[callWrapValue],
+                     "%s\n" % (getMaybeWrapValueFuncForType(wrapAsType),
                                exceptionCodeIndented.define())) +
                     "}\n" +
                     successCode)
         return ("${jsvalRef}.set(%s);\n" +
                 tail) % (value)
 
     def wrapAndSetPtr(wrapCall, failureCode=None):
         """
@@ -4342,58 +4351,50 @@ if (!returnArray) {
         "strings" : type.unroll().inner.identifier.name + "Values::" + ENUM_ENTRY_VARIABLE_NAME,
         "exceptionCode" : CGIndenter(exceptionCodeIndented).define() } +
         CGIndenter(CGGeneric(setValue("JS::StringValue(resultStr)"))).define() +
                       "\n}")
 
         if type.nullable():
             conversion = CGIfElseWrapper(
                 "%s.IsNull()" % result,
-                CGGeneric(setValue("JS::NullValue()", False)),
+                CGGeneric(setValue("JS::NullValue()")),
                 CGGeneric(conversion)).define()
         return conversion, False
 
     if type.isCallback() or type.isCallbackInterface():
         wrapCode = setValue(
             "JS::ObjectValue(*GetCallbackFromCallbackObject(%(result)s))",
-            "object")
+            wrapAsType=type)
         if type.nullable():
             wrapCode = (
                 "if (%(result)s) {\n" +
                 CGIndenter(CGGeneric(wrapCode)).define() + "\n"
                 "} else {\n" +
                 CGIndenter(CGGeneric(setValue("JS::NullValue()"))).define() + "\n"
                 "}")
         wrapCode = wrapCode % { "result": result }
         return wrapCode, False
 
     if type.isAny():
         # See comments in WrapNewBindingObject explaining why we need
         # to wrap here.
-        # NB: setValue(..., True) calls JS_WrapValue(), so is fallible
-        return (setValue(result, "value"), False)
+        # NB: setValue(..., type-that-is-any) calls JS_WrapValue(), so is fallible
+        return (setValue(result, wrapAsType=type), False)
 
     if (type.isObject() or (type.isSpiderMonkeyInterface() and
                             not typedArraysAreStructs)):
         # See comments in WrapNewBindingObject explaining why we need
         # to wrap here.
         if type.nullable():
             toValue = "JS::ObjectOrNullValue(%s)"
-            if type.isSpiderMonkeyInterface():
-                wrapType = "nonDOMObjectOrNull"
-            else:
-                wrapType = "objectOrNull"
         else:
             toValue = "JS::ObjectValue(*%s)"
-            if type.isSpiderMonkeyInterface():
-                wrapType = "nonDOMObject"
-            else:
-                wrapType = "object"
-        # NB: setValue(..., True) calls JS_WrapValue(), so is fallible
-        return (setValue(toValue % result, wrapType), False)
+        # NB: setValue(..., some-object-type) calls JS_WrapValue(), so is fallible
+        return (setValue(toValue % result, wrapAsType=type), False)
 
     if not (type.isUnion() or type.isPrimitive() or type.isDictionary() or
             type.isDate() or
             (type.isSpiderMonkeyInterface() and typedArraysAreStructs)):
         raise TypeError("Need to learn to wrap %s" % type)
 
     if type.nullable():
         (recTemplate, recInfal) = getWrapTemplateForType(type.inner, descriptorProvider,
@@ -4403,19 +4404,19 @@ if (!returnArray) {
         return ("if (%s.IsNull()) {\n" % result +
                 CGIndenter(CGGeneric(setValue("JSVAL_NULL"))).define() + "\n" +
                 "}\n" + recTemplate, recInfal)
 
     if type.isSpiderMonkeyInterface():
         assert typedArraysAreStructs
         # See comments in WrapNewBindingObject explaining why we need
         # to wrap here.
-        # NB: setValue(..., True) calls JS_WrapValue(), so is fallible
+        # NB: setValue(..., some-object-type) calls JS_WrapValue(), so is fallible
         return (setValue("JS::ObjectValue(*%s.Obj())" % result,
-                         "nonDOMObject"), False)
+                         wrapAsType=type), False)
 
     if type.isUnion():
         return (wrapAndSetPtr("%s.ToJSVal(cx, ${obj}, ${jsvalHandle})" % result),
                 False)
 
     if type.isDictionary():
         return (wrapAndSetPtr("%s.ToObject(cx, ${obj}, ${jsvalHandle})" % result),
                 False)
@@ -5092,24 +5093,25 @@ if (!${obj}) {
             successCode = (
                 "// Be careful here: Have to wrap the value into the\n"
                 "// compartment of reflector before storing, since we might\n"
                 "// be coming in via Xrays and the value is already in the\n"
                 "// caller compartment.\n"
                 "{ // Scope for tempVal\n"
                 "  JS::Rooted<JS::Value> tempVal(cx, args.rval());\n"
                 "  JSAutoCompartment ac(cx, reflector);\n"
-                "  if (!MaybeWrapValue(cx, &tempVal)) {\n"
+                "  if (!%s(cx, &tempVal)) {\n"
                 "    return false;\n"
                 "  }\n"
                 "  js::SetReservedSlot(reflector, %d, tempVal);\n"
                 "%s"
                 "}\n"
                 "return true;" %
-                (memberReservedSlot(self.idlNode), preserveWrapper))
+                (getMaybeWrapValueFuncForType(self.idlNode.type),
+                 memberReservedSlot(self.idlNode), preserveWrapper))
         else:
             successCode = None
 
         resultTemplateValues = { 'jsvalRef': 'args.rval()',
                                  'jsvalHandle': 'args.rval()',
                                  'returnsNewObject': returnsNewObject,
                                  'successCode': successCode,
                                  }
@@ -5963,19 +5965,20 @@ class CGSpecializedGetter(CGAbstractStat
                 "  reflector = IsDOMObject(obj) ? obj : js::UncheckedUnwrap(obj, /* stopAtOuter = */ false);\n"
                 "  {\n"
                 "    // Scope for cachedVal\n"
                 "    JS::Value cachedVal = js::GetReservedSlot(reflector, %d);\n"
                 "    if (!cachedVal.isUndefined()) {\n"
                 "      args.rval().set(cachedVal);\n"
                 "      // The cached value is in the compartment of reflector,\n"
                 "      // so wrap into the caller compartment as needed.\n"
-                "      return MaybeWrapValue(cx, args.rval());\n"
+                "      return %s(cx, args.rval());\n"
                 "    }\n"
-                "  }\n\n" % memberReservedSlot(self.attr))
+                "  }\n\n" % (memberReservedSlot(self.attr),
+                             getMaybeWrapValueFuncForType(self.attr.type)))
         else:
             prefix = ""
 
         return (prefix +
                 CGIndenter(CGGetterCall(self.attr.type, nativeName,
                                         self.descriptor, self.attr)).define())
 
     @staticmethod
@@ -8913,17 +8916,17 @@ if (""",
 
         if self.needToInitIds:
             methods.append(self.initIdsMethod())
 
         methods.append(self.initMethod())
         methods.append(self.initFromJSONMethod())
         try:
             methods.append(self.toObjectMethod())
-        except MethodNotCreatorError:
+        except MethodNotNewObjectError:
             # If we can't have a ToObject() because one of our members can only
             # be returned from [NewObject] methods, then just skip generating
             # ToObject().
             pass
         methods.append(self.traceDictionaryMethod())
 
         if CGDictionary.isDictionaryCopyConstructible(d):
             disallowCopyConstruction = False