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 111178 085bffb67692ead726e5129573da57edfddc86aa
parent 111177 ffa2d8f5b8be06649da504d863bb6c57e2f586d6
child 111179 0b6a823a8cfeba5f2184d8d53d21297fa064c7c9
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewerspeterv
bugs798187
milestone19.0a1
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);