Bug 845666. Add support for sequences containing other sequences (or dictionaries containing sequences) to WebIDL. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Sat, 02 Mar 2013 01:07:43 -0500
changeset 123549 2f35f1c8d0f9eb4099bdb388a9354b906aea01d3
parent 123548 a41e3c011069bc21ad0708f323a619eb4d9d7832
child 123550 3ae8720e2a118e1b02ce744581c37983ee41bcc8
push id23909
push userbzbarsky@mozilla.com
push dateSat, 02 Mar 2013 06:10:14 +0000
treeherdermozilla-inbound@2f35f1c8d0f9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs845666
milestone22.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 845666. Add support for sequences containing other sequences (or dictionaries containing sequences) to WebIDL. r=peterv This keeps simple sequence arguments and variadics using an auto array, while everything else uses a non-auto array.
dom/bindings/BindingDeclarations.h
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/test/TestBindingHeader.h
dom/bindings/test/TestCodeGen.webidl
dom/bindings/test/TestExampleGen.webidl
--- a/dom/bindings/BindingDeclarations.h
+++ b/dom/bindings/BindingDeclarations.h
@@ -309,25 +309,25 @@ private:
   // Forbid copy-construction and assignment
   Optional(const Optional& other) MOZ_DELETE;
   const Optional &operator=(const Optional &other) MOZ_DELETE;
 
   bool mPassed;
   const nsAString* mStr;
 };
 
-// Class for representing sequences in arguments.  We use an auto array that can
-// hold 16 elements, to avoid having to allocate in common cases.  This needs to
-// be fallible because web content controls the length of the array, and can
-// easily try to create very large lengths.
+// Class for representing sequences in arguments.  We use a non-auto array
+// because that allows us to use sequences of sequences and the like.  This
+// needs to be fallible because web content controls the length of the array,
+// and can easily try to create very large lengths.
 template<typename T>
-class Sequence : public AutoFallibleTArray<T, 16>
+class Sequence : public FallibleTArray<T>
 {
 public:
-  Sequence() : AutoFallibleTArray<T, 16>()
+  Sequence() : FallibleTArray<T>()
   {}
 };
 
 class RootedJSValue
 {
 public:
   RootedJSValue()
     : mCx(nullptr)
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1553,16 +1553,30 @@ public:
     const T& Value() const {
       return *storage.addr();
     }
     void Destroy() {
       storage.addr()->~T();
     }
 };
 
+// Class for simple sequence arguments, only used internally by codegen.
+template<typename T>
+class AutoSequence : public AutoFallibleTArray<T, 16>
+{
+public:
+  AutoSequence() : AutoFallibleTArray<T, 16>()
+  {}
+
+  // Allow converting to const sequences as needed
+  operator const Sequence<T>&() const {
+    return *reinterpret_cast<const Sequence<T>*>(this);
+  }
+};
+
 inline bool
 IdEquals(jsid id, const char* string)
 {
   return JSID_IS_STRING(id) &&
          JS_FlatStringEqualsAscii(JSID_TO_FLAT_STRING(id), string);
 }
 
 inline bool
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -2093,37 +2093,16 @@ 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) {
 ${codeOnFailure}
 }
 ${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
-
 # If this function is modified, modify CGNativeMember.getArg and
 # CGNativeMember.getRetvalInfo accordingly.  The latter cares about the decltype
 # and holdertype we end up using.
 def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
                                     isDefinitelyObject=False,
                                     isMember=False,
                                     isOptional=False,
                                     invalidEnumValueFatal=True,
@@ -2149,19 +2128,17 @@ def getJSToNativeConversionTemplate(type
     exceptionCode must end up doing a return, and every return from this
     function must happen via exceptionCode if exceptionCode is not None.
 
     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.  Any caller
-    passing true needs to ensure that it is handled correctly in
-    typeIsSequenceOrHasSequenceMember.
+    our jsval being rooted or outliving us in any way.
 
     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
     false).
 
@@ -2321,77 +2298,84 @@ def getJSToNativeConversionTemplate(type
 
         nullable = type.nullable();
         # Be very careful not to change "type": we need it later
         if nullable:
             elementType = type.inner.inner
         else:
             elementType = type.inner
 
-        # 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))
+        # We want to use auto arrays if we can, but we have to be careful with
+        # reallocation behavior for arrays.  In particular, if we use auto
+        # arrays for sequences and have a sequence of elements which are
+        # themselves sequences 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 typically map WebIDL sequences to our Sequence
+        # type, which is in fact memmovable.  The one exception is when we're
+        # passing in a sequence directly as an argument without any sort of
+        # optional or nullable complexity going on.  In that situation, we can
+        # use an AutoSequence instead.  We have to keep using Sequence in the
+        # nullable and optional cases because we don't want to leak the
+        # AutoSequence type to consumers, which would be unavoidable with
+        # Nullable<AutoSequence> or Optional<AutoSequence>.
+        if isMember or isOptional or nullable:
+            sequenceClass = "Sequence"
+        else:
+            sequenceClass = "AutoSequence"
 
         (elementTemplate, elementDeclType,
          elementHolderType, dealWithOptional) = getJSToNativeConversionTemplate(
             elementType, descriptorProvider, isMember=True,
             exceptionCode=exceptionCode, lenientFloatCode=lenientFloatCode)
         if dealWithOptional:
             raise TypeError("Shouldn't have optional things in sequences")
         if elementHolderType is not None:
             raise TypeError("Shouldn't need holders for sequences")
 
-        typeName = CGWrapper(elementDeclType, pre="Sequence< ", post=" >")
+        typeName = CGWrapper(elementDeclType,
+                             pre=("%s< " % sequenceClass), post=" >")
+        sequenceType = typeName.define()
         if nullable:
             typeName = CGWrapper(typeName, pre="Nullable< ", post=" >")
             arrayRef = "${declName}.Value()"
         else:
             arrayRef = "${declName}"
-        # If we're optional, the const will come from the Optional
+        # If we're optional or a member, the const will come from the Optional
+        # or whatever we're a member of.
         mutableTypeName = typeName
-        if not isOptional:
+        if not isOptional and not isMember:
             typeName = CGWrapper(typeName, pre="const ")
 
         # NOTE: Keep this in sync with variadic conversions as needed
         templateBody = ("""JSObject* seq = &${val}.toObject();\n
 if (!IsArrayLike(cx, seq)) {
 %s
 }
 uint32_t length;
 // JS_GetArrayLength actually works on all objects
 if (!JS_GetArrayLength(cx, seq, &length)) {
 %s
 }
-Sequence< %s > &arr = const_cast< Sequence< %s >& >(%s);
+%s &arr = const_cast< %s& >(%s);
 if (!arr.SetCapacity(length)) {
   JS_ReportOutOfMemory(cx);
 %s
 }
 for (uint32_t i = 0; i < length; ++i) {
   jsval temp;
   if (!JS_GetElement(cx, seq, i, &temp)) {
 %s
   }
   %s& slot = *arr.AppendElement();
 """ % (CGIndenter(CGGeneric(notSequence)).define(),
        exceptionCodeIndented.define(),
-       elementDeclType.define(),
-       elementDeclType.define(),
+       sequenceType,
+       sequenceType,
        arrayRef,
        exceptionCodeIndented.define(),
        CGIndenter(exceptionCodeIndented).define(),
        elementDeclType.define()))
 
         templateBody += CGIndenter(CGGeneric(
                 string.Template(elementTemplate).substitute(
                     {
@@ -3322,17 +3306,17 @@ class CGArgumentConverter(CGThing):
         (elementTemplate, elementDeclType,
          elementHolderType, dealWithOptional) = typeConversion
         if dealWithOptional:
             raise TypeError("Shouldn't have optional things in variadics")
         if elementHolderType is not None:
             raise TypeError("Shouldn't need holders for variadics")
 
         replacer = dict(self.argcAndIndex, **self.replacementVariables)
-        replacer["seqType"] = CGWrapper(elementDeclType, pre="Sequence< ", post=" >").define()
+        replacer["seqType"] = CGWrapper(elementDeclType, pre="AutoSequence< ", post=" >").define()
         replacer["elemType"] = elementDeclType.define()
 
         # NOTE: Keep this in sync with sequence conversions as needed
         variadicConversion = string.Template("""const ${seqType} ${declName};
 if (${argc} > ${index}) {
   ${seqType}& arr = const_cast< ${seqType}& >(${declName});
   if (!arr.SetCapacity(${argc} - ${index})) {
     JS_ReportOutOfMemory(cx);
--- a/dom/bindings/test/TestBindingHeader.h
+++ b/dom/bindings/test/TestBindingHeader.h
@@ -332,16 +332,18 @@ public:
   void PassNullableExternalInterfaceSequence(const Sequence<nsRefPtr<TestExternalInterface> >&);
 
   void ReceiveStringSequence(nsTArray<nsString>&);
   void PassStringSequence(const Sequence<nsString>&);
 
   void ReceiveAnySequence(JSContext*, nsTArray<JS::Value>&);
   void ReceiveNullableAnySequence(JSContext*, Nullable<nsTArray<JS::Value> >);
 
+  void PassSequenceOfSequences(const Sequence< Sequence<int32_t> >&);
+
   // Typed array types
   void PassArrayBuffer(ArrayBuffer&);
   void PassNullableArrayBuffer(ArrayBuffer*);
   void PassOptionalArrayBuffer(const Optional<ArrayBuffer>&);
   void PassOptionalNullableArrayBuffer(const Optional<ArrayBuffer*>&);
   void PassOptionalNullableArrayBufferWithDefaultValue(ArrayBuffer*);
   void PassArrayBufferView(ArrayBufferView&);
   void PassInt8Array(Int8Array&);
--- a/dom/bindings/test/TestCodeGen.webidl
+++ b/dom/bindings/test/TestCodeGen.webidl
@@ -311,16 +311,18 @@ interface TestInterface {
   void passNullableExternalInterfaceSequence(sequence<TestExternalInterface?> arg);
 
   sequence<DOMString> receiveStringSequence();
   void passStringSequence(sequence<DOMString> arg);
 
   sequence<any> receiveAnySequence();
   sequence<any>? receiveNullableAnySequence();
 
+  void passSequenceOfSequences(sequence<sequence<long>> arg);
+
   // Typed array types
   void passArrayBuffer(ArrayBuffer arg);
   void passNullableArrayBuffer(ArrayBuffer? arg);
   void passOptionalArrayBuffer(optional ArrayBuffer arg);
   void passOptionalNullableArrayBuffer(optional ArrayBuffer? arg);
   void passOptionalNullableArrayBufferWithDefaultValue(optional ArrayBuffer? arg= null);
   void passArrayBufferView(ArrayBufferView arg);
   void passInt8Array(Int8Array arg);
--- a/dom/bindings/test/TestExampleGen.webidl
+++ b/dom/bindings/test/TestExampleGen.webidl
@@ -227,16 +227,18 @@ interface TestExampleInterface {
   void passNullableExternalInterfaceSequence(sequence<TestExternalInterface?> arg);
 
   sequence<DOMString> receiveStringSequence();
   void passStringSequence(sequence<DOMString> arg);
 
   sequence<any> receiveAnySequence();
   sequence<any>? receiveNullableAnySequence();
 
+  void passSequenceOfSequences(sequence<sequence<long>> arg);
+
   // Typed array types
   void passArrayBuffer(ArrayBuffer arg);
   void passNullableArrayBuffer(ArrayBuffer? arg);
   void passOptionalArrayBuffer(optional ArrayBuffer arg);
   void passOptionalNullableArrayBuffer(optional ArrayBuffer? arg);
   void passOptionalNullableArrayBufferWithDefaultValue(optional ArrayBuffer? arg= null);
   void passArrayBufferView(ArrayBufferView arg);
   void passInt8Array(Int8Array arg);