Bug 798187 part 1. Add support for dictionary return values. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 22 Oct 2012 13:08:52 -0400
changeset 110955 085bffb67692
parent 110954 ffa2d8f5b8be
child 110956 0b6a823a8cfe
push id16833
push userbzbarsky@mozilla.com
push date2012-10-22 17:09 +0000
treeherdermozilla-inbound@0b6a823a8cfe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs798187
milestone19.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 798187 part 1. Add support for dictionary return values. r=peterv
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/test/TestBindingHeader.h
dom/bindings/test/TestCodeGen.webidl
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1037,16 +1037,25 @@ protected:
   }
 
   nsRefPtr<T> ptr;
 #ifdef DEBUG
   bool inited;
 #endif
 };
 
+// Helper for OwningNonNull
+template <class T>
+inline bool
+WrapNewBindingObject(JSContext* cx, JSObject* scope, OwningNonNull<T>& value,
+                     JS::Value* vp)
+{
+  return WrapNewBindingObject(cx, scope, &static_cast<T&>(value), vp);
+}
+
 // A struct that has the same layout as an nsDependentString but much
 // faster constructor and destructor behavior
 struct FakeDependentString {
   FakeDependentString() :
     mFlags(nsDependentString::F_TERMINATED)
   {
   }
 
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -2820,33 +2820,36 @@ if (%s.IsNull()) {
             {
                 'result' :  "%s[i]" % result,
                 'successCode': "break;",
                 'jsvalRef': "tmp",
                 'jsvalPtr': "&tmp",
                 'isCreator': isCreator
                 }
             )
-        innerTemplate = CGIndenter(CGGeneric(innerTemplate), 4).define()
+        innerTemplate = CGIndenter(CGGeneric(innerTemplate), 6).define()
         return (("""
 uint32_t length = %s.Length();
 JSObject *returnArray = JS_NewArrayObject(cx, length, NULL);
 if (!returnArray) {
   return false;
 }
-jsval tmp;
-for (uint32_t i = 0; i < length; ++i) {
-  // Control block to let us common up the JS_DefineElement calls when there
-  // are different ways to succeed at wrapping the object.
-  do {
+// Scope for 'tmp'
+{
+  jsval tmp;
+  for (uint32_t i = 0; i < length; ++i) {
+    // Control block to let us common up the JS_DefineElement calls when there
+    // are different ways to succeed at wrapping the object.
+    do {
 %s
-  } while (0);
-  if (!JS_DefineElement(cx, returnArray, i, tmp,
-                        nullptr, nullptr, JSPROP_ENUMERATE)) {
-    return false;
+    } while (0);
+    if (!JS_DefineElement(cx, returnArray, i, tmp,
+                          nullptr, nullptr, JSPROP_ENUMERATE)) {
+      return false;
+    }
   }
 }\n""" % (result, innerTemplate)) + setValue("JS::ObjectValue(*returnArray)"), False)
 
     if type.isGeckoInterface():
         descriptor = descriptorProvider.getDescriptor(type.unroll().inner.identifier.name)
         if type.nullable():
             wrappingCode = ("if (!%s) {\n" % (result) +
                             CGIndenter(CGGeneric(setValue("JSVAL_NULL"))).define() + "\n" +
@@ -2931,16 +2934,20 @@ if (!%(resultStr)s) {
         # to wrap here.
         if type.nullable():
             toValue = "JS::ObjectOrNullValue(%s)"
         else:
             toValue = "JS::ObjectValue(*%s)"
         # NB: setValue(..., True) calls JS_WrapValue(), so is fallible
         return (setValue(toValue % result, True), False)
 
+    if type.isDictionary():
+        assert not type.nullable()
+        return ("return %s.ToObject(cx, ${obj}, ${jsvalPtr});" % result, False)
+
     if not type.isPrimitive():
         raise TypeError("Need to learn to wrap %s" % type)
 
     if type.nullable():
         (recTemplate, recInfal) = getWrapTemplateForType(type.inner, descriptorProvider,
                                                          "%s.Value()" % result, successCode,
                                                          isCreator)
         return ("if (%s.IsNull()) {\n" % result +
@@ -3070,16 +3077,23 @@ def getRetvalDeclarationForType(returnTy
         # sequence argument here.
         (result, _) = getRetvalDeclarationForType(returnType.inner,
                                                   descriptorProvider,
                                                   resultAlreadyAddRefed)
         result = CGWrapper(result, pre="nsTArray< ", post=" >")
         if nullable:
             result = CGWrapper(result, pre="Nullable< ", post=" >")
         return result, True
+    if returnType.isDictionary():
+        assert not returnType.nullable()
+        result = CGGeneric(
+            CGDictionary.makeDictionaryName(returnType.unroll().inner,
+                                            descriptorProvider.workers) +
+            "Initializer")
+        return result, True
     raise TypeError("Don't know how to declare return value for %s" %
                     returnType)
 
 def isResultAlreadyAddRefed(descriptor, extendedAttributes):
     # Default to already_AddRefed on the main thread, raw pointer in workers
     return not descriptor.workers and not 'resultNotAddRefed' in extendedAttributes
 
 def needCx(returnType, arguments, extendedAttributes):
@@ -3092,17 +3106,17 @@ class CGCallGenerator(CGThing):
     A class to generate an actual call to a C++ object.  Assumes that the C++
     object is stored in a variable whose name is given by the |object| argument.
 
     errorReport should be a CGThing for an error report or None if no
     error reporting is needed.
     """
     def __init__(self, errorReport, arguments, argsPre, returnType,
                  extendedAttributes, descriptorProvider, nativeMethodName,
-                 static, object="self", declareResult=True):
+                 static, object="self"):
         CGThing.__init__(self)
 
         assert errorReport is None or isinstance(errorReport, CGThing)
 
         isFallible = errorReport is not None
 
         resultAlreadyAddRefed = isResultAlreadyAddRefed(descriptorProvider,
                                                         extendedAttributes)
@@ -3133,19 +3147,18 @@ class CGCallGenerator(CGThing):
 
         call = CGGeneric(nativeMethodName)
         if static:
             call = CGWrapper(call, pre="%s::" % descriptorProvider.nativeType)
         else: 
             call = CGWrapper(call, pre="%s->" % object)
         call = CGList([call, CGWrapper(args, pre="(", post=");")])
         if result is not None:
-            if declareResult:
-                result = CGWrapper(result, post=" result;")
-                self.cgRoot.prepend(result)
+            result = CGWrapper(result, post=" result;")
+            self.cgRoot.prepend(result)
             if not resultOutParam:
                 call = CGWrapper(call, pre="result = ")
 
         call = CGWrapper(call)
         self.cgRoot.append(call)
 
         if isFallible:
             self.cgRoot.prepend(CGGeneric("ErrorResult rv;"))
@@ -5375,16 +5388,17 @@ class CGNamespacedEnum(CGThing):
     def declare(self):
         return self.node.declare()
     def define(self):
         assert False # Only for headers.
 
 class CGDictionary(CGThing):
     def __init__(self, dictionary, descriptorProvider):
         self.dictionary = dictionary;
+        self.descriptorProvider = descriptorProvider
         self.workers = descriptorProvider.workers
         if all(CGDictionary(d, descriptorProvider).generatable for
                d in CGDictionary.getDictionaryDependencies(dictionary)):
             self.generatable = True
         else:
             self.generatable = False
             # Nothing else to do here
             return
@@ -5419,17 +5433,18 @@ class CGDictionary(CGThing):
         memberDecls = ["  %s %s;" %
                        (self.getMemberType(m), m[0].identifier.name)
                        for m in self.memberInfo]
 
         return (string.Template(
                 "struct ${selfName} ${inheritance}{\n"
                 "  ${selfName}() {}\n"
                 "  bool Init(JSContext* cx, const JS::Value& val);\n"
-                "  \n" +
+                "  bool ToObject(JSContext* cx, JSObject* parentObject, JS::Value *vp);\n"
+                "\n" +
                 ("  bool Init(const nsAString& aJSON)\n"
                  "  {\n"
                  "    mozilla::Maybe<JSAutoRequest> ar;\n"
                  "    mozilla::Maybe<JSAutoCompartment> ac;\n"
                  "    jsval json = JSVAL_VOID;\n"
                  "    JSContext* cx = ParseJSON(aJSON, ar, ac, json);\n"
                  "    NS_ENSURE_TRUE(cx, false);\n"
                  "    return Init(cx, json);\n"
@@ -5440,42 +5455,60 @@ class CGDictionary(CGThing):
                 "  // Disallow copy-construction\n"
                 "  ${selfName}(const ${selfName}&) MOZ_DELETE;\n" +
                 # NOTE: jsids are per-runtime, so don't use them in workers
                 ("  static bool InitIds(JSContext* cx);\n"
                  "  static bool initedIds;\n" if not self.workers else "") +
                 "\n".join("  static jsid " +
                           self.makeIdName(m.identifier.name) + ";" for
                           m in d.members) + "\n"
+                "};\n"
+                "struct ${selfName}Initializer : public ${selfName} {\n"
+                "  ${selfName}Initializer() {\n"
+                "    // Safe to pass a null context if we pass a null value\n"
+                "    Init(nullptr, JS::NullValue());\n"
+                "  }\n"
                 "};").substitute( { "selfName": self.makeClassName(d),
                                     "inheritance": inheritance }))
 
     def define(self):
         if not self.generatable:
             return ""
         d = self.dictionary
         if d.parent:
             initParent = ("// Per spec, we init the parent's members first\n"
                           "if (!%s::Init(cx, val)) {\n"
                           "  return false;\n"
                           "}\n" % self.makeClassName(d.parent))
+            toObjectParent = ("// Per spec, we define the parent's members first\n"
+                              "if (!%s::ToObject(cx, parentObject, vp)) {\n"
+                              "  return false;\n"
+                              "}\n" % self.makeClassName(d.parent))
+            ensureObject = "JSObject* obj = &vp->toObject();\n"
         else:
             initParent = ""
+            toObjectParent = ""
+            ensureObject = ("JSObject* obj = JS_NewObject(cx, nullptr, nullptr, nullptr);\n"
+                            "if (!obj) {\n"
+                            "  return false;\n"
+                            "}\n")
 
         memberInits = [CGIndenter(self.getMemberConversion(m)).define()
                        for m in self.memberInfo]
         idinit = [CGGeneric('!InternJSString(cx, %s, "%s")' %
                             (m.identifier.name + "_id", m.identifier.name))
                   for m in d.members]
         idinit = CGList(idinit, " ||\n")
         idinit = CGWrapper(idinit, pre="if (",
                            post=(") {\n"
                                  "  return false;\n"
                                  "}"),
                            reindent=True)
+        memberDefines = [CGIndenter(self.getMemberDefinition(m)).define()
+                         for m in self.memberInfo]
 
         return string.Template(
             # NOTE: jsids are per-runtime, so don't use them in workers
             ("bool ${selfName}::initedIds = false;\n" +
              "\n".join("jsid ${selfName}::%s = JSID_VOID;" %
                        self.makeIdName(m.identifier.name)
                        for m in d.members) + "\n"
              "\n"
@@ -5485,37 +5518,57 @@ class CGDictionary(CGThing):
              "  MOZ_ASSERT(!initedIds);\n"
              "${idInit}\n"
              "  initedIds = true;\n"
              "  return true;\n"
              "}\n"
              "\n" if not self.workers else "") +
             "bool\n"
             "${selfName}::Init(JSContext* cx, const JS::Value& val)\n"
-            "{\n" +
+            "{\n"
+            "  // Passing a null JSContext is OK only if we're initing from null,\n"
+            "  // Since in that case we will not have to do any property gets\n"
+            "  MOZ_ASSERT_IF(!cx, val.isNull());\n" +
             # NOTE: jsids are per-runtime, so don't use them in workers
-            ("  if (!initedIds && !InitIds(cx)) {\n"
+            ("  if (cx && !initedIds && !InitIds(cx)) {\n"
              "    return false;\n"
              "  }\n" if not self.workers else "") +
             "${initParent}"
             "  JSBool found;\n"
             "  JS::Value temp;\n"
             "  bool isNull = val.isNullOrUndefined();\n"
             "  if (!isNull && !val.isObject()) {\n"
             "    return ThrowErrorMessage(cx, MSG_NOT_OBJECT);\n"
             "  }\n"
             "\n"
             "${initMembers}\n"
             "  return true;\n"
+            "}\n"
+            "\n"
+            "bool\n"
+            "${selfName}::ToObject(JSContext* cx, JSObject* parentObject, JS::Value *vp)\n"
+            "{\n" +
+            # NOTE: jsids are per-runtime, so don't use them in workers
+            ("  if (!initedIds && !InitIds(cx)) {\n"
+             "    return false;\n"
+             "  }\n" if not self.workers else "") +
+            "${toObjectParent}"
+            "${ensureObject}"
+            "\n"
+            "${defineMembers}\n"
+            "  return true;\n"
             "}").substitute({
                 "selfName": self.makeClassName(d),
                 "initParent": CGIndenter(CGGeneric(initParent)).define(),
                 "initMembers": "\n\n".join(memberInits),
                 "idInit": CGIndenter(idinit).define(),
-                "isMainThread": toStringBool(not self.workers)
+                "isMainThread": toStringBool(not self.workers),
+                "toObjectParent": CGIndenter(CGGeneric(toObjectParent)).define(),
+                "ensureObject": CGIndenter(CGGeneric(ensureObject)).define(),
+                "defineMembers": "\n\n".join(memberDefines)
                 })
 
     @staticmethod
     def makeDictionaryName(dictionary, workers):
         suffix = "Workers" if workers else ""
         return dictionary.identifier.name + suffix
 
     def makeClassName(self, dictionary):
@@ -5598,16 +5651,73 @@ class CGDictionary(CGThing):
                 "}")
             conversionReplacements["convert"] = CGIndenter(
                 CGGeneric(conversionReplacements["convert"])).define()
         
         return CGGeneric(
             string.Template(conversion).substitute(conversionReplacements)
             )
 
+    def getMemberDefinition(self, memberInfo):
+        member = memberInfo[0]
+        declType = memberInfo[1][1]
+        # Use this->%s to refer to members, because we don't control
+        # the member names and want to make sure we're talking about
+        # the member, not some local that shadows the member.  Another
+        # option would be to move the guts of init to a static method
+        # which is passed an explicit reference to our dictionary
+        # object, so we couldn't screw this up even if we wanted
+        # to....
+        memberLoc = "(this->%s)" % member.identifier.name
+        if member.defaultValue:
+            memberData = memberLoc
+        else:
+            # The data is inside the Optional<>
+            memberData = "%s.Value()" % memberLoc
+
+        if self.workers:
+            propDef = (
+                'JS_DefineProperty(cx, obj, "%s", temp, nullptr, nullptr, JSPROP_ENUMERATE)' %
+                member.identifier.name)
+        else:
+            propDef = (
+                'JS_DefinePropertyById(cx, obj, %s, temp, nullptr, nullptr, JSPROP_ENUMERATE)' %
+                self.makeIdName(member.identifier.name))
+
+        innerTemplate = wrapForType(
+            member.type, self.descriptorProvider,
+            {
+                'result' : "currentValue",
+                'successCode' : ("if (!%s) {\n"
+                                 "  return false;\n"
+                                 "}" % propDef),
+                'jsvalRef': "temp",
+                'jsvalPtr': "&temp",
+                'isCreator': False,
+                'obj': "parentObject"
+                }
+            )
+        conversion = CGGeneric(innerTemplate)
+        conversion = CGWrapper(conversion,
+                               pre=("JS::Value temp;\n"
+                                    "%s& currentValue = %s;\n" %
+                                    (declType.define(), memberData)
+                                    ))
+
+        if not member.defaultValue:
+            # Only do the conversion if we have a value
+            conversion = CGIfWrapper(conversion, "%s.WasPassed()" % memberLoc)
+        else:
+            # Make sure we have a scope for our stuff
+            conversion = CGWrapper(CGIndenter(conversion),
+                                   pre=("{\n"
+                                        "  // scope for 'temp' and 'currentValue'\n"),
+                                   post="\n}")
+        return conversion
+
     @staticmethod
     def makeIdName(name):
         return name + "_id"
 
     @staticmethod
     def getDictionaryDependencies(dictionary):
         deps = set();
         if dictionary.parent:
--- a/dom/bindings/test/TestBindingHeader.h
+++ b/dom/bindings/test/TestBindingHeader.h
@@ -394,16 +394,17 @@ public:
   void MethodRenamedTo();
   void MethodRenamedTo(int8_t);
   int8_t AttributeGetterRenamedTo();
   int8_t AttributeRenamedTo();
   void SetAttributeRenamedTo(int8_t);
 
   // Dictionary tests
   void PassDictionary(const Dict&);
+  void ReceiveDictionary(Dict&);
   void PassOtherDictionary(const GrandparentDict&);
   void PassSequenceOfDictionaries(const Sequence<Dict>&);
   void PassDictionaryOrLong(const Dict&);
   void PassDictionaryOrLong(int32_t);
   void PassDictContainingDict(const DictContainingDict&);
   void PassDictContainingSequence(const DictContainingSequence&);
 
   // Typedefs
--- a/dom/bindings/test/TestCodeGen.webidl
+++ b/dom/bindings/test/TestCodeGen.webidl
@@ -306,16 +306,17 @@ interface TestInterface {
 
   // binaryNames tests
   void methodRenamedFrom();
   void methodRenamedFrom(byte argument);
   readonly attribute byte attributeGetterRenamedFrom;
   attribute byte attributeRenamedFrom;
 
   void passDictionary(optional Dict x);
+  Dict receiveDictionary();
   void passOtherDictionary(optional GrandparentDict x);
   void passSequenceOfDictionaries(sequence<Dict> x);
   void passDictionaryOrLong(optional Dict x);
   void passDictionaryOrLong(long x);
 
   void passDictContainingDict(optional DictContainingDict arg);
   void passDictContainingSequence(optional DictContainingSequence arg);