Bug 790975. Support sequences as dictionary members in WebIDL. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 18 Sep 2012 23:24:27 -0400
changeset 107453 17dcbe563c2b00d5bb4544138c7e4355f3cbd62f
parent 107452 0a28200d2954d5b625f47cadae71156c96131d12
child 107454 e61b7c193325f33b5a1f7238271632eb93982fc1
push id15045
push userbzbarsky@mozilla.com
push dateWed, 19 Sep 2012 03:25:45 +0000
treeherdermozilla-inbound@e61b7c193325 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
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 790975. Support sequences as dictionary members in WebIDL. r=peterv
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -1570,16 +1570,37 @@ if (NS_FAILED(rv) || !wrappedJS) {
 // Use a temp nsCOMPtr for the null-check, because ${target} might be
 // OwningNonNull, not an nsCOMPtr.
 nsCOMPtr<${nativeType}> tmp = do_QueryObject(wrappedJS.get());
 if (!tmp) {
 ${target} = tmp.forget();""").substitute(self.substitution)
+def dictionaryHasSequenceMember(dictionary):
+    return (any(typeIsSequenceOrHasSequenceMember(m.type) for m in
+                dictionary.members) or
+            (dictionary.parent and
+             dictionaryHasSequenceMember(dictionary.parent)))
+def typeIsSequenceOrHasSequenceMember(type):
+    if type.nullable():
+        type = type.inner
+    if type.isSequence():
+        return True
+    if  type.isArray():
+        elementType = type.inner
+        return typeIsSequenceOrHasSequenceMember(elementType)
+    if type.isDictionary():
+        return dictionaryHasSequenceMember(type.inner)
+    if type.isUnion():
+        return any(typeIsSequenceOrHasSequenceMember(m.type) for m in
+                   type.flatMemberTypes)
+    return False
 def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
@@ -1595,17 +1616,19 @@ def getJSToNativeConversionTemplate(type
     out of memory conditions need to throw exceptions no matter what
     failureCode is.
     If isDefinitelyObject is True, that means we know the value
     isObject() and we have no need to recheck that.
     if isMember is True, we're being converted from a property of some
     JS object, not from an actual method argument, so we can't rely on
-    our jsval being rooted or outliving us in any way.
+    our jsval being rooted or outliving us in any way.  Any caller
+    passing true needs to ensure that it is handled correctly in
+    typeIsSequenceOrHasSequenceMember.
     If isOptional is true, then we are doing conversion of an optional
     argument with no default value.
     invalidEnumValueFatal controls whether an invalid enum value conversion
     attempt will throw (if true) or simply return without doing anything (if
@@ -1716,34 +1739,41 @@ def getJSToNativeConversionTemplate(type
     assert not (isEnforceRange and isClamp) # These are mutually exclusive
     if type.isArray():
         raise TypeError("Can't handle array arguments yet")
     if type.isSequence():
         assert not isEnforceRange and not isClamp
-        if isMember:
-            # XXXbz we probably _could_ handle this; we just have to be careful
-            # with reallocation behavior for arrays.  In particular, if we have
-            # a return value that's a sequence of dictionaries of sequences,
-            # that will cause us to have an nsTArray containing objects with
-            # nsAutoTArray members, which is a recipe for badness as the
-            # outermost array is resized.
-            raise TypeError("Can't handle unrooted sequences")
         if failureCode is not None:
             raise TypeError("Can't handle sequences when failureCode is not None")
         nullable = type.nullable();
         # Be very careful not to change "type": we need it later
         if nullable:
             elementType = type.inner.inner
             elementType = type.inner
-        # We don't know anything about the object-ness of the things
-        # we wrap, so don't pass through isDefinitelyObject
+        # We have to be careful with reallocation behavior for arrays.  In
+        # particular, if we have a sequence of elements which are themselves
+        # sequences (so nsAutoTArrays) or have sequences as members, we have a
+        # problem.  In that case, resizing the outermost nsAutoTarray to the
+        # right size will memmove its elements, but nsAutoTArrays are not
+        # memmovable and hence will end up with pointers to bogus memory, which
+        # is bad.  To deal with this, we disallow sequences, arrays,
+        # dictionaries, and unions which contain sequences as sequence item
+        # types.  If WebIDL ever adds another container type, we'd have to
+        # disallow it as well.
+        if typeIsSequenceOrHasSequenceMember(elementType):
+            raise TypeError("Can't handle a sequence containing another "
+                            "sequence as an element or member of an element.  "
+                            "See the big comment explaining why.\n%s" %
+                            str(type.location))
         (elementTemplate, elementDeclType,
          elementHolderType, dealWithOptional) = getJSToNativeConversionTemplate(
             elementType, descriptorProvider, isMember=True)
         if dealWithOptional:
             raise TypeError("Shouldn't have optional things in sequences")
         if elementHolderType is not None:
             raise TypeError("Shouldn't need holders for sequences")
--- a/dom/bindings/test/TestBindingHeader.h
+++ b/dom/bindings/test/TestBindingHeader.h
@@ -392,16 +392,17 @@ public:
   // Dictionary tests
   void PassDictionary(const 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
   void ExerciseTypedefInterfaces1(TestInterface&);
   already_AddRefed<TestInterface> ExerciseTypedefInterfaces2(TestInterface*);
   void ExerciseTypedefInterfaces3(TestInterface&);
   // Miscellania
   int32_t AttrWithLenientThis();
--- a/dom/bindings/test/TestCodeGen.webidl
+++ b/dom/bindings/test/TestCodeGen.webidl
@@ -305,16 +305,17 @@ interface TestInterface {
   void passDictionary(optional Dict x);
   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);
   // EnforceRange/Clamp tests
   void dontEnforceRangeOrClamp(byte arg);
   void doEnforceRange([EnforceRange] byte arg);
   void doClamp([Clamp] byte arg);
   // Typedefs
   const myLong myLongConstant = 5;
@@ -389,16 +390,20 @@ dictionary ParentDict : GrandparentDict 
   TestInterface someInterface;
   TestExternalInterface someExternalInterface;
 dictionary DictContainingDict {
   Dict memberDict;
+dictionary DictContainingSequence {
+  sequence<long> ourSequence;
 interface TestIndexedGetterInterface {
   getter long item(unsigned long index);
   readonly attribute unsigned long length;
 interface TestNamedGetterInterface {
   getter DOMString (DOMString name);