Bug 768537 part 1. Update parser support for dictionaries to spec changes. r=jlebar

There are several parts here:

1)  Enforce the requirement that dictionary arguments not followed by a required argument are optional.
2)  Make dictionaries no longer be distinguishable from nullable types.
3)  Disallow dictionaries or unions containing dictionaries inside a nullable type.
4)  Force optional dictionaries to have a default value of null so that codegen doesn't have to worry about dealing with
    optional arguments that have no default value in the IDL but need to be treated as if they were null.

content/base/src/nsXMLHttpRequest.h

@@ -181,7 +181,7 @@ public:
   static already_AddRefed<nsXMLHttpRequest>
   Constructor(JSContext* aCx,
               nsISupports* aGlobal,
-              const mozilla::dom::Nullable<mozilla::dom::MozXMLHttpRequestParameters>& aParams,
+              const mozilla::dom::MozXMLHttpRequestParameters& aParams,
               ErrorResult& aRv)
   {
     nsCOMPtr<nsPIDOMWindow> window = do_QueryInterface(aGlobal);
@@ -193,10 +193,7 @@ public:
 
     nsRefPtr<nsXMLHttpRequest> req = new nsXMLHttpRequest();
     req->Construct(principal->GetPrincipal(), window);
-    if (!aParams.IsNull()) {
-      const mozilla::dom::MozXMLHttpRequestParameters& params = aParams.Value();
-      req->InitParameters(params.mozAnon, params.mozSystem);
-    }
+    req->InitParameters(aParams.mozAnon, aParams.mozSystem);
     return req.forget();
   }
 

dom/bindings/parser/WebIDL.py

@@ -707,11 +707,9 @@ class IDLDictionary(IDLObjectWithScope):
 
         for member in self.members:
             member.resolve(self)
-            if not member.type.isComplete():
-                type = member.type.complete(scope)
-                assert not isinstance(type, IDLUnresolvedType)
-                assert not isinstance(type.name, IDLUnresolvedIdentifier)
-                member.type = type
+            if not member.isComplete():
+                member.complete(scope)
+                assert member.type.isComplete()
 
         # Members of a dictionary are sorted in lexicographic order
         self.members.sort(cmp=cmp, key=lambda x: x.identifier.name)
@@ -1019,10 +1017,23 @@ class IDLNullableType(IDLType):
 
     def complete(self, scope):
         self.inner = self.inner.complete(scope)
-        if self.inner.isUnion() and self.inner.hasNullableType:
+        if self.inner.isUnion():
+            if self.inner.hasNullableType:
+                raise WebIDLError("The inner type of a nullable type must not "
+                                  "be a union type that itself has a nullable "
+                                  "type as a member type", [self.location])
+            # Check for dictionaries in the union
+            for memberType in self.inner.flatMemberTypes:
+                if memberType.isDictionary():
+                    raise WebIDLError("The inner type of a nullable type must "
+                                      "not be a union type containing a "
+                                      "dictionary type",
+                                      [self.location, memberType.location])
+                    
+        if self.inner.isDictionary():
             raise WebIDLError("The inner type of a nullable type must not be a "
-                              "union type that itself has a nullable type as a "
-                              "member type", [self.location])
+                              "dictionary type", [self.location])
+
         self.name = self.inner.name
         return self
 
@@ -1030,7 +1041,8 @@ class IDLNullableType(IDLType):
         return self.inner.unroll()
 
     def isDistinguishableFrom(self, other):
-        if other.nullable() or (other.isUnion() and other.hasNullableType):
+        if (other.nullable() or (other.isUnion() and other.hasNullableType) or
+            other.isDictionary()):
             # Can't tell which type null should become
             return False
         return self.inner.isDistinguishableFrom(other)
@@ -1405,8 +1417,9 @@ class IDLWrapperType(IDLType):
         if other.isPrimitive() or other.isString() or other.isEnum() or other.isDate():
             return True
         if self.isDictionary():
-            return (other.isNonCallbackInterface() or other.isSequence() or
-                    other.isArray())
+            return (not other.nullable() and
+                    (other.isNonCallbackInterface() or other.isSequence() or
+                     other.isArray()))
 
         assert self.isInterface()
         # XXXbz need to check that the interfaces can't be implemented
@@ -1726,7 +1739,9 @@ class IDLNullValue(IDLObject):
         self.value = None
 
     def coerceToType(self, type, location):
-        if not isinstance(type, IDLNullableType) and not (type.isUnion() and type.hasNullableType):
+        if (not isinstance(type, IDLNullableType) and
+            not (type.isUnion() and type.hasNullableType) and
+            not type.isDictionary()):
             raise WebIDLError("Cannot coerce null value to type %s." % type,
                               [location])
 
@@ -1862,14 +1877,11 @@ class IDLArgument(IDLObjectWithIdentifier):
         assert isinstance(type, IDLType)
         self.type = type
 
-        if defaultValue:
-            defaultValue = defaultValue.coerceToType(type, location)
-            assert defaultValue
-
         self.optional = optional
         self.defaultValue = defaultValue
         self.variadic = variadic
         self.dictionaryMember = dictionaryMember
+        self._isComplete = False
 
         assert not variadic or optional
 
@@ -1886,6 +1898,33 @@ class IDLArgument(IDLObjectWithIdentifier):
         # But actually, we can't handle this at all, so far.
         assert len(attrs) == 0
 
+    def isComplete(self):
+        return self._isComplete
+
+    def complete(self, scope):
+        if self._isComplete:
+            return
+
+        self._isComplete = True
+
+        if not self.type.isComplete():
+            type = self.type.complete(scope)
+            assert not isinstance(type, IDLUnresolvedType)
+            assert not isinstance(type.name, IDLUnresolvedIdentifier)
+            self.type = type
+
+        if self.type.isDictionary() and self.optional and not self.defaultValue:
+            # Default optional dictionaries to null, for simplicity,
+            # so the codegen doesn't have to special-case this.
+            self.defaultValue = IDLNullValue(self.location)
+
+        # Now do the coercing thing; this needs to happen after the
+        # above creation of a default value.
+        if self.defaultValue:
+            self.defaultValue = self.defaultValue.coerceToType(self.type,
+                                                               self.location)
+            assert self.defaultValue
+
 class IDLCallbackType(IDLType, IDLObjectWithScope):
     def __init__(self, location, parentScope, identifier, returnType, arguments):
         assert isinstance(returnType, IDLType)
@@ -2038,34 +2077,6 @@ class IDLMethod(IDLInterfaceMember, IDLScope):
             assert len(overload.arguments) == 0
             assert overload.returnType == BuiltinTypes[IDLBuiltinType.Types.domstring]
 
-        inOptionalArguments = False
-        variadicArgument = None
-        sawOptionalWithNoDefault = False
-
-        assert len(self._overloads) == 1
-        arguments = self._overloads[0].arguments
-
-        for argument in arguments:
-            # Only the last argument can be variadic
-            if variadicArgument:
-                raise WebIDLError("Variadic argument is not last argument",
-                                  [variadicArgument.location])
-            # Once we see an optional argument, there can't be any non-optional
-            # arguments.
-            if inOptionalArguments and not argument.optional:
-                raise WebIDLError("Non-optional argument after optional arguments",
-                                  [argument.location])
-            # Once we see an argument with no default value, there can
-            # be no more default values.
-            if sawOptionalWithNoDefault and argument.defaultValue:
-                raise WebIDLError("Argument with default value after optional "
-                                  "arguments with no default values",
-                                  [argument.location])
-            inOptionalArguments = argument.optional
-            if argument.variadic:
-                variadicArgument = argument
-            sawOptionalWithNoDefault = argument.optional and not argument.defaultValue
-
     def isStatic(self):
         return self._static
 
@@ -2148,15 +2159,49 @@ class IDLMethod(IDLInterfaceMember, IDLScope):
 
     def finish(self, scope):
         for overload in self._overloads:
-            for argument in overload.arguments:
-                if argument.type.isComplete():
+            inOptionalArguments = False
+            variadicArgument = None
+            sawOptionalWithNoDefault = False
+
+            arguments = overload.arguments
+            for (idx, argument) in enumerate(arguments):
+                if argument.isComplete():
                     continue
 
-                type = argument.type.complete(scope)
+                argument.complete(scope)
+                assert argument.type.isComplete()
 
-                assert not isinstance(type, IDLUnresolvedType)
-                assert not isinstance(type.name, IDLUnresolvedIdentifier)
-                argument.type = type
+                if argument.type.isDictionary():
+                    # Dictionaries at the end of the list or followed by
+                    # optional arguments must be optional.
+                    if (not argument.optional and
+                        (idx == len(arguments) - 1 or arguments[idx+1].optional)):
+                        raise WebIDLError("Dictionary argument not followed by "
+                                          "a required argument must be "
+                                          "optional", [argument.location])
+
+                # Only the last argument can be variadic
+                if variadicArgument:
+                    raise WebIDLError("Variadic argument is not last argument",
+                                      [variadicArgument.location])
+                # Once we see an optional argument, there can't be any non-optional
+                # arguments.
+                if inOptionalArguments and not argument.optional:
+                    raise WebIDLError("Non-optional argument after optional "
+                                      "arguments",
+                                      [argument.location])
+                # Once we see an argument with no default value, there can
+                # be no more default values.
+                if sawOptionalWithNoDefault and argument.defaultValue:
+                    raise WebIDLError("Argument with default value after "
+                                      "optional arguments with no default "
+                                      "values",
+                                      [argument.location])
+                inOptionalArguments = argument.optional
+                if argument.variadic:
+                    variadicArgument = argument
+                sawOptionalWithNoDefault = (argument.optional and
+                                            not argument.defaultValue)
 
             returnType = overload.returnType
             if returnType.isComplete():

dom/bindings/parser/tests/test_dictionary.py

@@ -121,3 +121,78 @@ def WebIDLTest(parser, harness):
         threw = True
 
     harness.ok(threw, "Should not allow [TreatUndefinedAs] on dictionary members");
+
+    parser = parser.reset()
+    threw = False
+    try:
+        parser.parse("""
+            dictionary A {
+            };
+            interface X {
+              void doFoo(A arg);
+            };
+        """)
+        results = parser.finish()
+    except:
+        threw = True
+
+    harness.ok(threw, "Trailing dictionary arg must be optional")
+
+    parser = parser.reset()
+    threw = False
+    try:
+        parser.parse("""
+            dictionary A {
+            };
+            interface X {
+              void doFoo(A arg1, optional long arg2);
+            };
+        """)
+        results = parser.finish()
+    except:
+        threw = True
+
+    harness.ok(threw, "Dictionary arg followed by optional arg must be optional")
+
+    parser = parser.reset()
+    parser.parse("""
+            dictionary A {
+            };
+            interface X {
+              void doFoo(A arg1, long arg2);
+            };
+        """)
+    results = parser.finish()
+    harness.ok(True, "Dictionary arg followed by required arg can be required")
+
+    parser = parser.reset()
+    threw = False
+    try:
+        parser.parse("""
+            dictionary A {
+            };
+            interface X {
+              void doFoo(optional A? arg1);
+            };
+        """)
+        results = parser.finish()
+    except:
+        threw = True
+
+    harness.ok(threw, "Dictionary arg must not be nullable")
+
+    parser = parser.reset()
+    threw = False
+    try:
+        parser.parse("""
+            dictionary A {
+            };
+            interface X {
+              void doFoo((A or long)? arg1);
+            };
+        """)
+        results = parser.finish()
+    except:
+        threw = True
+
+    harness.ok(threw, "Dictionary arg must not be in a nullable union")

dom/bindings/parser/tests/test_distinguishability.py

@@ -9,7 +9,7 @@ def WebIDLTest(parser, harness):
       };
       interface Bar {
         // Bit of a pain to get things that have dictionary types
-        void passDict(Dict arg);
+        void passDict(optional Dict arg);
         void passFoo(Foo arg);
         void passNullableUnion((object? or DOMString) arg);
         void passNullable(Foo? arg);

dom/bindings/test/TestBindingHeader.h

@@ -346,9 +346,6 @@ public:
 
   // Dictionary tests
   void PassDictionary(const Dict&, ErrorResult&);
-  void PassOptionalDictionary(const Optional<Dict>&, ErrorResult&);
-  void PassNullableDictionary(const Nullable<Dict>&, ErrorResult&);
-  void PassOptionalNullableDictionary(const Optional<Nullable<Dict> >&, ErrorResult&);
   void PassOtherDictionary(const GrandparentDict&, ErrorResult&);
   void PassSequenceOfDictionaries(const Sequence<Dict>&, ErrorResult&);
 

dom/bindings/test/TestCodeGen.webidl

@@ -261,11 +261,8 @@ interface TestInterface {
   readonly attribute byte attributeGetterRenamedFrom;
   attribute byte attributeRenamedFrom;
 
-  void passDictionary(Dict x);
-  void passOptionalDictionary(optional Dict x);
-  void passNullableDictionary(Dict? x);
-  void passOptionalNullableDictionary(optional Dict? x);
-  void passOtherDictionary(GrandparentDict x);
+  void passDictionary(optional Dict x);
+  void passOtherDictionary(optional GrandparentDict x);
   void passSequenceOfDictionaries(sequence<Dict> x);
 };
 

dom/webidl/XMLHttpRequest.webidl

@@ -51,7 +51,7 @@ dictionary MozXMLHttpRequestParameters
   boolean mozSystem = false;
 };
 
-[Constructor(optional MozXMLHttpRequestParameters? params = null)]
+[Constructor(optional MozXMLHttpRequestParameters params)]
 interface XMLHttpRequest : XMLHttpRequestEventTarget {
   // event handler
   [TreatNonCallableAsNull, GetterInfallible=MainThread]

dom/workers/XMLHttpRequest.cpp

@@ -1466,7 +1466,7 @@ XMLHttpRequest::_finalize(JSFreeOp* aFop)
 XMLHttpRequest*
 XMLHttpRequest::Constructor(JSContext* aCx,
                             JSObject* aGlobal,
-                            const Nullable<MozXMLHttpRequestParametersWorkers>& aParams,
+                            const MozXMLHttpRequestParametersWorkers& aParams,
                             ErrorResult& aRv)
 {
   WorkerPrivate* workerPrivate = GetWorkerPrivateFromContext(aCx);

dom/workers/XMLHttpRequest.h

@@ -72,7 +72,7 @@ public:
 
   static XMLHttpRequest*
   Constructor(JSContext* aCx, JSObject* aGlobal,
-              const Nullable<MozXMLHttpRequestParametersWorkers>& aParams,
+              const MozXMLHttpRequestParametersWorkers& aParams,
               ErrorResult& aRv);
   void
   Unpin();