From 203898c941e4ac2c3a3a2f247ea3924c60858633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 12 May 2016 15:39:54 +0200 Subject: [PATCH 1/6] codegen: Use the non-mangled name in set_dictionary_property Fixes #11152 --- components/script/dom/bindings/codegen/CodegenRust.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 3c0593d6e58..bfdea910159 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -5301,7 +5301,7 @@ class CGDictionary(CGThing): insertion = ("let mut %s = RootedValue::new(cx, UndefinedValue());\n" "self.%s.to_jsval(cx, %s.handle_mut());\n" "set_dictionary_property(cx, obj.handle(), \"%s\", %s.handle()).unwrap();" - % (name, name, name, name, name)) + % (name, name, name, member.identifier.name, name)) return CGGeneric("%s\n" % insertion) memberInits = CGList([memberInit(m) for m in self.memberInfo]) From f893a2eaacee70b1044cea88dd184cd05dda31fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 12 May 2016 18:09:56 +0200 Subject: [PATCH 2/6] bindings: Add test for keywords in dictionaries --- components/script/dom/testbinding.rs | 69 ++++++++++++++++++- .../script/dom/webidls/TestBinding.webidl | 4 ++ tests/wpt/mozilla/meta/MANIFEST.json | 6 ++ .../meta/mozilla/binding_keyword.html.ini | 3 + .../tests/mozilla/binding_keyword.html | 16 +++++ 5 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 tests/wpt/mozilla/meta/mozilla/binding_keyword.html.ini create mode 100644 tests/wpt/mozilla/tests/mozilla/binding_keyword.html diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index f8b45599b18..e7d4d1a9cff 100644 --- a/components/script/dom/testbinding.rs +++ b/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,71 @@ impl TestBindingMethods for TestBinding { Some(UnsignedLongOrBoolean::UnsignedLong(0u32)) } fn ReceiveNullableSequence(&self) -> Option> { 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, + } + } + + fn TypeKeywordIsSuccess(&self, arg: &TestDictionary) -> bool { + arg.type_.as_ref().map(|s| s == "success").unwrap_or(false) + } fn PassBoolean(&self, _: bool) {} fn PassByte(&self, _: i8) {} diff --git a/components/script/dom/webidls/TestBinding.webidl b/components/script/dom/webidls/TestBinding.webidl index d0bfbbfef2a..e9d94509473 100644 --- a/components/script/dom/webidls/TestBinding.webidl +++ b/components/script/dom/webidls/TestBinding.webidl @@ -30,6 +30,8 @@ dictionary TestDictionary { object objectValue; TestDictionaryDefaults dict; sequence seqDict; + // Reserved rust keyword + DOMString type; }; dictionary TestDictionaryDefaults { @@ -196,6 +198,8 @@ interface TestBinding { (sequence or boolean)? receiveNullableUnion4(); (unsigned long or boolean)? receiveNullableUnion5(); sequence? receiveNullableSequence(); + TestDictionary receiveTestDictionaryWithSuccessOnKeyword(); + boolean typeKeywordIsSuccess(TestDictionary arg); void passBoolean(boolean arg); void passByte(byte arg); diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index d81335ad928..1b3f5967e4b 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/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", diff --git a/tests/wpt/mozilla/meta/mozilla/binding_keyword.html.ini b/tests/wpt/mozilla/meta/mozilla/binding_keyword.html.ini new file mode 100644 index 00000000000..4b7bda760f0 --- /dev/null +++ b/tests/wpt/mozilla/meta/mozilla/binding_keyword.html.ini @@ -0,0 +1,3 @@ +[binding_keyword.html] + type: testharness + prefs: [dom.testbinding.enabled:true] diff --git a/tests/wpt/mozilla/tests/mozilla/binding_keyword.html b/tests/wpt/mozilla/tests/mozilla/binding_keyword.html new file mode 100644 index 00000000000..2faa1ccc8f3 --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/binding_keyword.html @@ -0,0 +1,16 @@ + + +Test for conversions from and to dictionary properties with keywords + + + From 92ba0b9c39a1eaf7c0546fea385b96b67d78c95c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 12 May 2016 20:31:48 +0200 Subject: [PATCH 3/6] codegen: Don't unconditionally set non-required dictionary attributes --- .../dom/bindings/codegen/CodegenRust.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index bfdea910159..3fc5bde830b 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -5295,14 +5295,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" - "self.%s.to_jsval(cx, %s.handle_mut());\n" - "set_dictionary_property(cx, obj.handle(), \"%s\", %s.handle()).unwrap();" - % (name, name, name, member.identifier.name, name)) - return CGGeneric("%s\n" % insertion) + if member.optional and not member.defaultValue: + insertion = CGIfWrapper("let Some(ref %s) = self.%s" % (name, name), + varInsert(name, member.identifier.name)) + else: + 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]) From 093f5c01e44d6063d65c518089d046895ff7cc9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 12 May 2016 21:08:35 +0200 Subject: [PATCH 4/6] codegen: add tests for non-nullable non-required values --- components/script/dom/testbinding.rs | 8 ++++++-- components/script/dom/webidls/TestBinding.webidl | 7 ++++++- tests/wpt/mozilla/tests/mozilla/binding_keyword.html | 7 ++++--- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index e7d4d1a9cff..6dfe66a9d75 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -347,11 +347,15 @@ impl TestBindingMethods for TestBinding { unsignedLongValue: None, unsignedShortValue: None, usvstringValue: None, + nonRequiredNullable: None, + nonRequiredNullable2: Some(None), // null } } - fn TypeKeywordIsSuccess(&self, arg: &TestDictionary) -> bool { - arg.type_.as_ref().map(|s| s == "success").unwrap_or(false) + 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) {} diff --git a/components/script/dom/webidls/TestBinding.webidl b/components/script/dom/webidls/TestBinding.webidl index e9d94509473..6706ccfdaa9 100644 --- a/components/script/dom/webidls/TestBinding.webidl +++ b/components/script/dom/webidls/TestBinding.webidl @@ -32,6 +32,11 @@ dictionary TestDictionary { sequence 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 { @@ -199,7 +204,7 @@ interface TestBinding { (unsigned long or boolean)? receiveNullableUnion5(); sequence? receiveNullableSequence(); TestDictionary receiveTestDictionaryWithSuccessOnKeyword(); - boolean typeKeywordIsSuccess(TestDictionary arg); + boolean dictMatchesPassedValues(TestDictionary arg); void passBoolean(boolean arg); void passByte(byte arg); diff --git a/tests/wpt/mozilla/tests/mozilla/binding_keyword.html b/tests/wpt/mozilla/tests/mozilla/binding_keyword.html index 2faa1ccc8f3..818d2aa2947 100644 --- a/tests/wpt/mozilla/tests/mozilla/binding_keyword.html +++ b/tests/wpt/mozilla/tests/mozilla/binding_keyword.html @@ -9,8 +9,9 @@ test(function() { var dict = t.receiveTestDictionaryWithSuccessOnKeyword(); assert_equals(dict.type, "success"); + assert_equals(dict.nonRequiredNullable, undefined); + assert_equals(dict.nonRequiredNullable2, null); - var is_success = t.typeKeywordIsSuccess(dict); - assert_true(is_success); -}, "Conversion of dictionary elements with rust keywords works") + assert_true(t.dictMatchesPassedValues(dict)); +}, "Conversion of dictionary elements with rust keywords, and null non-required nullable properties works") From e50d4b762441aacb2327e9f81888f2d08f5bf624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 13 May 2016 10:11:30 +0200 Subject: [PATCH 5/6] codegen: Throw on an invalid enum value when appropiate --- .../dom/bindings/codegen/CodegenRust.py | 17 +++++---- .../FileAPI/file/File-constructor.html.ini | 1 + .../DOMParser-parseFromString-xml.html.ini | 36 +++++++++++++++++-- .../Document-defaultView.html.ini | 6 ++-- .../DOMParser-parseFromString-html.html | 6 ++++ 5 files changed, 56 insertions(+), 10 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 3fc5bde830b..883aeb7c8d6 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/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): + return CGGeneric( + failureCode or + ('throw_type_error(cx, "%s is not a valid enum value."); %s' + % (firstCap(sourceDescription), exceptionCode))) + def onFailureNotCallable(failureCode): - return CGWrapper( - CGGeneric( - failureCode or - ('throw_type_error(cx, \"%s is not callable.\");\n' - '%s' % (firstCap(sourceDescription), exceptionCode)))) + return CGGeneric( + failureCode or + ('throw_type_error(cx, \"%s is not callable.\");\n' + '%s' % (firstCap(sourceDescription), exceptionCode))) # A helper function for handling null default values. Checks that the # default value, if it exists, is null. @@ -868,7 +873,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, "yet") enum = type.inner.identifier.name if invalidEnumValueFatal: - handleInvalidEnumValueCode = exceptionCode + handleInvalidEnumValueCode = onFailureInvalidEnumValue(failureCode).define() else: handleInvalidEnumValueCode = "return true;" diff --git a/tests/wpt/metadata/FileAPI/file/File-constructor.html.ini b/tests/wpt/metadata/FileAPI/file/File-constructor.html.ini index a0c942de8ce..7087a621261 100644 --- a/tests/wpt/metadata/FileAPI/file/File-constructor.html.ini +++ b/tests/wpt/metadata/FileAPI/file/File-constructor.html.ini @@ -11,3 +11,4 @@ [Using special character in fileName] expected: FAIL + diff --git a/tests/wpt/metadata/domparsing/DOMParser-parseFromString-xml.html.ini b/tests/wpt/metadata/domparsing/DOMParser-parseFromString-xml.html.ini index 66597d94001..c5f9f1fdb9b 100644 --- a/tests/wpt/metadata/domparsing/DOMParser-parseFromString-xml.html.ini +++ b/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 diff --git a/tests/wpt/metadata/html/browsers/the-window-object/Document-defaultView.html.ini b/tests/wpt/metadata/html/browsers/the-window-object/Document-defaultView.html.ini index 9ba1c81088c..6ba3fcf80dd 100644 --- a/tests/wpt/metadata/html/browsers/the-window-object/Document-defaultView.html.ini +++ b/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 diff --git a/tests/wpt/web-platform-tests/domparsing/DOMParser-parseFromString-html.html b/tests/wpt/web-platform-tests/domparsing/DOMParser-parseFromString-html.html index fcaa44a391b..96c4a02333c 100644 --- a/tests/wpt/web-platform-tests/domparsing/DOMParser-parseFromString-html.html +++ b/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") From 91101f6226033a663ea8e4bb215f72ab8d206da7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 13 May 2016 10:36:41 +0200 Subject: [PATCH 6/6] codegen: Throw a more descriptive invalid enum message --- .../script/dom/bindings/codegen/CodegenRust.py | 12 ++++++------ components/script/dom/bindings/utils.rs | 7 ++++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 883aeb7c8d6..076b7bd642f 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -662,11 +662,11 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, '%s' % (firstCap(sourceDescription), exceptionCode))), post="\n") - def onFailureInvalidEnumValue(failureCode): + def onFailureInvalidEnumValue(failureCode, passedVarName): return CGGeneric( failureCode or - ('throw_type_error(cx, "%s is not a valid enum value."); %s' - % (firstCap(sourceDescription), exceptionCode))) + ('throw_type_error(cx, &format!("\'{}\' is not a valid enum value for enumeration \'%s\'.", %s)); %s' + % (type.name, passedVarName, exceptionCode))) def onFailureNotCallable(failureCode): return CGGeneric( @@ -873,15 +873,15 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, "yet") enum = type.inner.identifier.name if invalidEnumValueFatal: - handleInvalidEnumValueCode = onFailureInvalidEnumValue(failureCode).define() + 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(Some(index)) => {\n" + " Ok((None, search)) => { %(handleInvalidEnumValueCode)s },\n" + " Ok((Some(index), _)) => {\n" " //XXXjdm need some range checks up in here.\n" " mem::transmute(index)\n" " },\n" diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs index 9b9a729fe0a..cf1cf37a8b9 100644 --- a/components/script/dom/bindings/utils.rs +++ b/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 /// 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, ()> { + -> Result<(Option, 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