Auto merge of #11155 - emilio:codegen-dict-keyword, r=Ms2ger

codegen: Fix dictionary handling and semantics

Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data:
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #11152 (github issue number if applicable).

Either:
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.

Fixes #11152

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11155)
<!-- Reviewable:end -->

components/script/dom/bindings/codegen/CodegenRust.py

@@ -662,12 +662,17 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
                  '%s' % (firstCap(sourceDescription), exceptionCode))),
             post="\n")
 
+    def onFailureInvalidEnumValue(failureCode, passedVarName):
+        return CGGeneric(
+            failureCode or
+            ('throw_type_error(cx, &format!("\'{}\' is not a valid enum value for enumeration \'%s\'.", %s)); %s'
+             % (type.name, passedVarName, exceptionCode)))
+
     def onFailureNotCallable(failureCode):
-        return CGWrapper(
+        return CGGeneric(
-            CGGeneric(
             failureCode or
             ('throw_type_error(cx, \"%s is not callable.\");\n'
-                 '%s' % (firstCap(sourceDescription), exceptionCode))))
+             '%s' % (firstCap(sourceDescription), exceptionCode)))
 
     # A helper function for handling null default values. Checks that the
     # default value, if it exists, is null.
@@ -868,15 +873,15 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
                             "yet")
         enum = type.inner.identifier.name
         if invalidEnumValueFatal:
-            handleInvalidEnumValueCode = exceptionCode
+            handleInvalidEnumValueCode = onFailureInvalidEnumValue(failureCode, 'search').define()
         else:
             handleInvalidEnumValueCode = "return true;"
 
         template = (
             "match find_enum_string_index(cx, ${val}, %(values)s) {\n"
             "    Err(_) => { %(exceptionCode)s },\n"
-            "    Ok(None) => { %(handleInvalidEnumValueCode)s },\n"
+            "    Ok((None, search)) => { %(handleInvalidEnumValueCode)s },\n"
-            "    Ok(Some(index)) => {\n"
+            "    Ok((Some(index), _)) => {\n"
             "        //XXXjdm need some range checks up in here.\n"
             "        mem::transmute(index)\n"
             "    },\n"
@@ -5295,14 +5300,23 @@ class CGDictionary(CGThing):
             conversion = self.getMemberConversion(memberInfo, member.type)
             return CGGeneric("%s: %s,\n" % (name, conversion.define()))
 
+        def varInsert(varName, dictionaryName):
+            insertion = ("let mut %s_js = RootedValue::new(cx, UndefinedValue());\n"
+                         "%s.to_jsval(cx, %s_js.handle_mut());\n"
+                         "set_dictionary_property(cx, obj.handle(), \"%s\", %s_js.handle()).unwrap();"
+                         % (varName, varName, varName, dictionaryName, varName))
+            return CGGeneric(insertion)
+
         def memberInsert(memberInfo):
             member, _ = memberInfo
             name = self.makeMemberName(member.identifier.name)
-            insertion = ("let mut %s = RootedValue::new(cx, UndefinedValue());\n"
+            if member.optional and not member.defaultValue:
-                         "self.%s.to_jsval(cx, %s.handle_mut());\n"
+                insertion = CGIfWrapper("let Some(ref %s) = self.%s" % (name, name),
-                         "set_dictionary_property(cx, obj.handle(), \"%s\", %s.handle()).unwrap();"
+                                        varInsert(name, member.identifier.name))
-                         % (name, name, name, name, name))
+            else:
-            return CGGeneric("%s\n" % insertion)
+                insertion = CGGeneric("let %s = &self.%s;\n%s" %
+                                      (name, name, varInsert(name, member.identifier.name).define()))
+            return CGGeneric("%s\n" % insertion.define())
 
         memberInits = CGList([memberInit(m) for m in self.memberInfo])
         memberInserts = CGList([memberInsert(m) for m in self.memberInfo])

components/script/dom/bindings/utils.rs

@@ -39,6 +39,7 @@ use std::ptr;
 use std::slice;
 use util::non_geckolib::jsstring_to_str;
 use util::prefs;
+use util::str::DOMString;
 
 /// Proxy handler for a WindowProxy.
 pub struct WindowProxyHandler(pub *const libc::c_void);
@@ -182,18 +183,18 @@ pub fn get_array_index_from_id(_cx: *mut JSContext, id: HandleId) -> Option<u32>
 
 /// Find the index of a string given by `v` in `values`.
 /// Returns `Err(())` on JSAPI failure (there is a pending exception), and
-/// `Ok(None)` if there was no matching string.
+/// `Ok((None, value))` if there was no matching string.
 pub unsafe fn find_enum_string_index(cx: *mut JSContext,
                                      v: HandleValue,
                                      values: &[&'static str])
-                                     -> Result<Option<usize>, ()> {
+                                     -> Result<(Option<usize>, DOMString), ()> {
     let jsstr = ToString(cx, v);
     if jsstr.is_null() {
         return Err(());
     }
 
     let search = jsstring_to_str(cx, jsstr);
-    Ok(values.iter().position(|value| search == *value))
+    Ok((values.iter().position(|value| search == *value), search))
 }
 
 /// Returns wether `obj` is a platform object

components/script/dom/testbinding.rs

@@ -6,7 +6,9 @@
 
 use dom::bindings::codegen::Bindings::EventListenerBinding::EventListener;
 use dom::bindings::codegen::Bindings::FunctionBinding::Function;
-use dom::bindings::codegen::Bindings::TestBindingBinding::{self, TestBindingMethods, TestEnum};
+use dom::bindings::codegen::Bindings::TestBindingBinding;
+use dom::bindings::codegen::Bindings::TestBindingBinding::{TestBindingMethods, TestDictionary};
+use dom::bindings::codegen::Bindings::TestBindingBinding::{TestDictionaryDefaults, TestEnum};
 use dom::bindings::codegen::UnionTypes::{BlobOrBoolean, BlobOrBlobSequence};
 use dom::bindings::codegen::UnionTypes::{BlobOrString, BlobOrUnsignedLong, EventOrString};
 use dom::bindings::codegen::UnionTypes::{EventOrUSVString, HTMLElementOrLong};
@@ -286,6 +288,75 @@ impl TestBindingMethods for TestBinding {
         Some(UnsignedLongOrBoolean::UnsignedLong(0u32))
     }
     fn ReceiveNullableSequence(&self) -> Option<Vec<i32>> { Some(vec![1]) }
+    fn ReceiveTestDictionaryWithSuccessOnKeyword(&self) -> TestDictionary {
+        TestDictionary {
+            anyValue: NullValue(),
+            booleanValue: None,
+            byteValue: None,
+            dict: TestDictionaryDefaults {
+                UnrestrictedDoubleValue: 0.0,
+                anyValue: NullValue(),
+                booleanValue: false,
+                byteValue: 0,
+                doubleValue: Finite::new(1.0).unwrap(),
+                enumValue: TestEnum::Foo,
+                floatValue: Finite::new(1.0).unwrap(),
+                longLongValue: 54,
+                longValue: 12,
+                nullableBooleanValue: None,
+                nullableByteValue: None,
+                nullableDoubleValue: None,
+                nullableFloatValue: None,
+                nullableLongLongValue: None,
+                nullableLongValue: None,
+                nullableObjectValue: ptr::null_mut(),
+                nullableOctetValue: None,
+                nullableShortValue: None,
+                nullableStringValue: None,
+                nullableUnrestrictedDoubleValue: None,
+                nullableUnrestrictedFloatValue: None,
+                nullableUnsignedLongLongValue: None,
+                nullableUnsignedLongValue: None,
+                nullableUnsignedShortValue: None,
+                nullableUsvstringValue: None,
+                octetValue: 0,
+                shortValue: 0,
+                stringValue: DOMString::new(),
+                unrestrictedFloatValue: 0.0,
+                unsignedLongLongValue: 0,
+                unsignedLongValue: 0,
+                unsignedShortValue: 0,
+                usvstringValue: USVString("".to_owned()),
+            },
+            doubleValue: None,
+            enumValue: None,
+            floatValue: None,
+            interfaceValue: None,
+            longLongValue: None,
+            longValue: None,
+            objectValue: None,
+            octetValue: None,
+            requiredValue: true,
+            seqDict: None,
+            shortValue: None,
+            stringValue: None,
+            type_: Some(DOMString::from("success")),
+            unrestrictedDoubleValue: None,
+            unrestrictedFloatValue: None,
+            unsignedLongLongValue: None,
+            unsignedLongValue: None,
+            unsignedShortValue: None,
+            usvstringValue: None,
+            nonRequiredNullable: None,
+            nonRequiredNullable2: Some(None), // null
+        }
+    }
+
+    fn DictMatchesPassedValues(&self, arg: &TestDictionary) -> bool {
+        arg.type_.as_ref().map(|s| s == "success").unwrap_or(false) &&
+            arg.nonRequiredNullable.is_none() &&
+            arg.nonRequiredNullable2 == Some(None)
+    }
 
     fn PassBoolean(&self, _: bool) {}
     fn PassByte(&self, _: i8) {}

components/script/dom/webidls/TestBinding.webidl

@@ -30,6 +30,13 @@ dictionary TestDictionary {
   object objectValue;
   TestDictionaryDefaults dict;
   sequence<TestDictionaryDefaults> seqDict;
+  // Reserved rust keyword
+  DOMString type;
+  // These are used to test bidirectional conversion
+  // and differentiation of non-required and nullable types
+  // in dictionaries.
+  DOMString? nonRequiredNullable;
+  DOMString? nonRequiredNullable2;
 };
 
 dictionary TestDictionaryDefaults {
@@ -196,6 +203,8 @@ interface TestBinding {
   (sequence<long> or boolean)? receiveNullableUnion4();
   (unsigned long or boolean)? receiveNullableUnion5();
   sequence<long>? receiveNullableSequence();
+  TestDictionary receiveTestDictionaryWithSuccessOnKeyword();
+  boolean dictMatchesPassedValues(TestDictionary arg);
 
   void passBoolean(boolean arg);
   void passByte(byte arg);

tests/wpt/metadata/FileAPI/file/File-constructor.html.ini

@@ -11,3 +11,4 @@
 
   [Using special character in fileName]
     expected: FAIL
+

tests/wpt/metadata/domparsing/DOMParser-parseFromString-xml.html.ini

@@ -1,9 +1,41 @@
 [DOMParser-parseFromString-xml.html]
   type: testharness
-  expected: TIMEOUT
   [Should return an error document for XML wellformedness errors in type text/xml]
     expected: FAIL
 
   [Should parse correctly in type application/xml]
-    expected: TIMEOUT
+    expected: FAIL
+
+  [XMLDocument interface for correctly parsed document with type application/xml]
+    expected: FAIL
+
+  [Should return an error document for XML wellformedness errors in type application/xml]
+    expected: FAIL
+
+  [XMLDocument interface for incorrectly parsed document with type application/xml]
+    expected: FAIL
+
+  [Should parse correctly in type application/xhtml+xml]
+    expected: FAIL
+
+  [XMLDocument interface for correctly parsed document with type application/xhtml+xml]
+    expected: FAIL
+
+  [Should return an error document for XML wellformedness errors in type application/xhtml+xml]
+    expected: FAIL
+
+  [XMLDocument interface for incorrectly parsed document with type application/xhtml+xml]
+    expected: FAIL
+
+  [Should parse correctly in type image/svg+xml]
+    expected: FAIL
+
+  [XMLDocument interface for correctly parsed document with type image/svg+xml]
+    expected: FAIL
+
+  [Should return an error document for XML wellformedness errors in type image/svg+xml]
+    expected: FAIL
+
+  [XMLDocument interface for incorrectly parsed document with type image/svg+xml]
+    expected: FAIL
 

tests/wpt/metadata/html/browsers/the-window-object/Document-defaultView.html.ini

@@ -1,6 +1,5 @@
 [Document-defaultView.html]
   type: testharness
-  expected: TIMEOUT
   [Document created with the Document constructor]
     expected: FAIL
 
@@ -11,5 +10,8 @@
     expected: FAIL
 
   [Document created with XML DOMParser]
-    expected: TIMEOUT
+    expected: FAIL
+
+  [Document created with HTML DOMParser]
+    expected: FAIL
 

tests/wpt/mozilla/meta/MANIFEST.json

@@ -6064,6 +6064,12 @@
             "url": "/_mozilla/mozilla/bad_cert_detected.html"
           }
         ],
+        "mozilla/binding_keyword.html": [
+          {
+            "path": "mozilla/binding_keyword.html",
+            "url": "/_mozilla/mozilla/binding_keyword.html"
+          }
+        ],
         "mozilla/blob.html": [
           {
             "path": "mozilla/blob.html",

tests/wpt/mozilla/meta/mozilla/binding_keyword.html.ini

@@ -0,0 +1,3 @@
+[binding_keyword.html]
+  type: testharness
+  prefs: [dom.testbinding.enabled:true]

tests/wpt/mozilla/tests/mozilla/binding_keyword.html

@@ -0,0 +1,17 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Test for conversions from and to dictionary properties with keywords</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+test(function() {
+  var t = new TestBinding();
+
+  var dict = t.receiveTestDictionaryWithSuccessOnKeyword();
+  assert_equals(dict.type, "success");
+  assert_equals(dict.nonRequiredNullable, undefined);
+  assert_equals(dict.nonRequiredNullable2, null);
+
+  assert_true(t.dictMatchesPassedValues(dict));
+}, "Conversion of dictionary elements with rust keywords, and null non-required nullable properties works")
+</script>

tests/wpt/web-platform-tests/domparsing/DOMParser-parseFromString-html.html

@@ -61,4 +61,10 @@ test(function() {
     assert_equals(htmldoc.documentElement.localName, "html");
     assert_equals(htmldoc.documentElement.namespaceURI, "http://www.w3.org/1999/xhtml");
 }, "DOMParser parses HTML tag soup with no problems");
+
+test(function() {
+    assert_throws(new TypeError(), function() {
+        new DOMParser().parseFromString("", "text/foo-this-is-invalid");
+    })
+}, "DOMParser throws on an invalid enum value")
 </script>