From e481e8934a0c37a4b1eba19862ff732ec9bf19c9 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 26 May 2017 12:40:50 -0400 Subject: [PATCH 1/7] Don't generate union conversion functions for object variants. --- components/script/dom/bindings/codegen/CodegenRust.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 9a98848108b..208d4b1d923 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -4313,10 +4313,14 @@ class CGUnionConversionStruct(CGThing): returnType = "Result, ()>" % templateVars["typeName"] jsConversion = templateVars["jsConversion"] + # Any code to convert to Object is unused, since we're already converting + # from an Object value. + if t.name == 'Object': + return CGGeneric('') + return CGWrapper( CGIndenter(jsConversion, 4), - # TryConvertToObject is unused, but not generating it while generating others is tricky. - pre="#[allow(dead_code)] unsafe fn TryConvertTo%s(cx: *mut JSContext, value: HandleValue) -> %s {\n" + pre="unsafe fn TryConvertTo%s(cx: *mut JSContext, value: HandleValue) -> %s {\n" % (t.name, returnType), post="\n}") From da65698c5c5934220b82493b3f7bb2ab05a2e512 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 26 May 2017 12:29:31 -0400 Subject: [PATCH 2/7] Be more conservative about safety of dictionary and union values. Mark dictionaries containing GC values as must_root, and wrap them in RootedTraceableBox in automatically-generated APIs. To accommodate union variants that are now flagged as unsafe, add RootedTraceableBox to union variants that need to be rooted, rather than wrapping the entire union value. --- .../dom/bindings/codegen/CodegenRust.py | 43 +++++++++++++++---- components/script/dom/bindings/iterable.rs | 6 +-- components/script/dom/bindings/trace.rs | 1 + components/script/dom/filereader.rs | 4 +- components/script/dom/testbinding.rs | 10 ++--- 5 files changed, 47 insertions(+), 17 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 208d4b1d923..4d31da25efe 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -723,9 +723,6 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type.nullable(): declType = CGWrapper(declType, pre="Option<", post=" >") - if isMember != "Dictionary" and type_needs_tracing(type): - declType = CGTemplatedType("RootedTraceableBox", declType) - templateBody = ("match FromJSValConvertible::from_jsval(cx, ${val}, ()) {\n" " Ok(ConversionResult::Success(value)) => value,\n" " Ok(ConversionResult::Failure(error)) => {\n" @@ -1427,6 +1424,8 @@ def getRetvalDeclarationForType(returnType, descriptorProvider): nullable = returnType.nullable() dictName = returnType.inner.name if nullable else returnType.name result = CGGeneric(dictName) + if type_needs_tracing(returnType): + result = CGWrapper(result, pre="RootedTraceableBox<", post=">") if nullable: result = CGWrapper(result, pre="Option<", post=">") return result @@ -2262,6 +2261,7 @@ def UnionTypes(descriptors, dictionaries, callbacks, typedefs, config): 'dom::bindings::str::ByteString', 'dom::bindings::str::DOMString', 'dom::bindings::str::USVString', + 'dom::bindings::trace::RootedTraceableBox', 'dom::types::*', 'js::error::throw_type_error', 'js::jsapi::HandleValue', @@ -4132,15 +4132,23 @@ class CGUnionStruct(CGThing): self.type = type self.descriptorProvider = descriptorProvider + def membersNeedTracing(self): + for t in self.type.flatMemberTypes: + if type_needs_tracing(t): + return True + return False + def define(self): - templateVars = map(lambda t: getUnionTypeTemplateVars(t, self.descriptorProvider), + templateVars = map(lambda t: (getUnionTypeTemplateVars(t, self.descriptorProvider), + type_needs_tracing(t)), self.type.flatMemberTypes) enumValues = [ - " %s(%s)," % (v["name"], v["typeName"]) for v in templateVars + " %s(%s)," % (v["name"], "RootedTraceableBox<%s>" % v["typeName"] if trace else v["typeName"]) + for (v, trace) in templateVars ] enumConversions = [ " %s::%s(ref inner) => inner.to_jsval(cx, rval)," - % (self.type, v["name"]) for v in templateVars + % (self.type, v["name"]) for (v, _) in templateVars ] return ("""\ #[derive(JSTraceable)] @@ -4167,6 +4175,12 @@ class CGUnionConversionStruct(CGThing): self.type = type self.descriptorProvider = descriptorProvider + def membersNeedTracing(self): + for t in self.type.flatMemberTypes: + if type_needs_tracing(t): + return True + return False + def from_jsval(self): memberTypes = self.type.flatMemberTypes names = [] @@ -4310,7 +4324,10 @@ class CGUnionConversionStruct(CGThing): def try_method(self, t): templateVars = getUnionTypeTemplateVars(t, self.descriptorProvider) - returnType = "Result, ()>" % templateVars["typeName"] + actualType = templateVars["typeName"] + if type_needs_tracing(t): + actualType = "RootedTraceableBox<%s>" % actualType + returnType = "Result, ()>" % actualType jsConversion = templateVars["jsConversion"] # Any code to convert to Object is unused, since we're already converting @@ -6022,13 +6039,17 @@ class CGDictionary(CGThing): (self.makeMemberName(m[0].identifier.name), self.getMemberType(m)) for m in self.memberInfo] + mustRoot = "#[must_root]\n" if self.membersNeedTracing() else "" + return (string.Template( "#[derive(JSTraceable)]\n" + "${mustRoot}" + "pub struct ${selfName} {\n" + "${inheritance}" + "\n".join(memberDecls) + "\n" + "}").substitute({"selfName": self.makeClassName(d), - "inheritance": inheritance})) + "inheritance": inheritance, + "mustRoot": mustRoot})) def impl(self): d = self.dictionary @@ -6120,6 +6141,12 @@ class CGDictionary(CGThing): "insertMembers": CGIndenter(memberInserts, indentLevel=8).define(), }) + def membersNeedTracing(self): + for member, _ in self.memberInfo: + if type_needs_tracing(member.type): + return True + return False + @staticmethod def makeDictionaryName(dictionary): return dictionary.identifier.name diff --git a/components/script/dom/bindings/iterable.rs b/components/script/dom/bindings/iterable.rs index 03fac1cb27f..39c681c7d8b 100644 --- a/components/script/dom/bindings/iterable.rs +++ b/components/script/dom/bindings/iterable.rs @@ -12,7 +12,7 @@ use dom::bindings::codegen::Bindings::IterableIteratorBinding::IterableKeyOrValu use dom::bindings::error::Fallible; use dom::bindings::js::{JS, Root}; use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object}; -use dom::bindings::trace::JSTraceable; +use dom::bindings::trace::{JSTraceable, RootedTraceableBox}; use dom::globalscope::GlobalScope; use dom_struct::dom_struct; use js::conversions::ToJSValConvertible; @@ -115,7 +115,7 @@ fn dict_return(cx: *mut JSContext, result: MutableHandleObject, done: bool, value: HandleValue) -> Fallible<()> { - let mut dict = unsafe { IterableKeyOrValueResult::empty(cx) }; + let mut dict = RootedTraceableBox::new(unsafe { IterableKeyOrValueResult::empty(cx) }); dict.done = done; dict.value.set(value.get()); rooted!(in(cx) let mut dict_value = UndefinedValue()); @@ -130,7 +130,7 @@ fn key_and_value_return(cx: *mut JSContext, result: MutableHandleObject, key: HandleValue, value: HandleValue) -> Fallible<()> { - let mut dict = unsafe { IterableKeyAndValueResult::empty(cx) }; + let mut dict = RootedTraceableBox::new(unsafe { IterableKeyAndValueResult::empty(cx) }); dict.done = false; dict.value = Some(vec![Heap::new(key.get()), Heap::new(value.get())]); rooted!(in(cx) let mut dict_value = UndefinedValue()); diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index ab2b20b97c8..797ee356bd8 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -745,6 +745,7 @@ impl<'a, T: JSTraceable + 'static> Drop for RootedTraceable<'a, T> { /// If you have GC things like *mut JSObject or JSVal, use rooted!. /// If you have an arbitrary number of DomObjects to root, use rooted_vec!. /// If you know what you're doing, use this. +#[allow_unrooted_interior] pub struct RootedTraceableBox { ptr: *mut T, } diff --git a/components/script/dom/filereader.rs b/components/script/dom/filereader.rs index 3ee3c146b75..370643bfa44 100644 --- a/components/script/dom/filereader.rs +++ b/components/script/dom/filereader.rs @@ -13,6 +13,7 @@ use dom::bindings::js::{MutNullableJS, Root}; use dom::bindings::refcounted::Trusted; use dom::bindings::reflector::{DomObject, reflect_dom_object}; use dom::bindings::str::DOMString; +use dom::bindings::trace::RootedTraceableBox; use dom::blob::Blob; use dom::domexception::{DOMErrorName, DOMException}; use dom::event::{Event, EventBubbles, EventCancelable}; @@ -338,7 +339,8 @@ impl FileReaderMethods for FileReader { FileReaderResult::String(ref string) => StringOrObject::String(string.clone()), FileReaderResult::ArrayBuffer(ref arr_buffer) => { - StringOrObject::Object(Heap::new((*arr_buffer.ptr.get()).to_object())) + StringOrObject::Object(RootedTraceableBox::new( + Heap::new((*arr_buffer.ptr.get()).to_object()))) } }) } diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index 1a56a77a64f..266497da83e 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -338,8 +338,8 @@ impl TestBindingMethods for TestBinding { Some(ByteStringOrLong::ByteString(ByteString::new(vec!()))) } fn ReceiveNullableSequence(&self) -> Option> { Some(vec![1]) } - fn ReceiveTestDictionaryWithSuccessOnKeyword(&self) -> TestDictionary { - TestDictionary { + fn ReceiveTestDictionaryWithSuccessOnKeyword(&self) -> RootedTraceableBox { + RootedTraceableBox::new(TestDictionary { anyValue: Heap::new(NullValue()), booleanValue: None, byteValue: None, @@ -401,7 +401,7 @@ impl TestBindingMethods for TestBinding { usvstringValue: None, nonRequiredNullable: None, nonRequiredNullable2: Some(None), // null - } + }) } fn DictMatchesPassedValues(&self, arg: RootedTraceableBox) -> bool { @@ -436,9 +436,9 @@ impl TestBindingMethods for TestBinding { fn PassUnion6(&self, _: UnsignedLongOrBoolean) {} fn PassUnion7(&self, _: StringSequenceOrUnsignedLong) {} fn PassUnion8(&self, _: ByteStringSequenceOrLong) {} - fn PassUnion9(&self, _: RootedTraceableBox) {} + fn PassUnion9(&self, _: UnionTypes::TestDictionaryOrLong) {} #[allow(unsafe_code)] - unsafe fn PassUnion10(&self, _: *mut JSContext, _: RootedTraceableBox) {} + unsafe fn PassUnion10(&self, _: *mut JSContext, _: UnionTypes::StringOrObject) {} fn PassUnionWithTypedef(&self, _: DocumentOrTestTypedef) {} fn PassUnionWithTypedef2(&self, _: LongSequenceOrTestTypedef) {} #[allow(unsafe_code)] From b169689f32db6d497b04f3a5b173cba4acd33e93 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 26 May 2017 13:25:05 -0400 Subject: [PATCH 3/7] Store rootable dictionary members of dictionaries in RootedTraceableBox. --- .../dom/bindings/codegen/CodegenRust.py | 27 +++++++++++++------ components/script/dom/bindings/conversions.rs | 18 ++++--------- components/script/dom/bindings/iterable.rs | 6 ++--- components/script/dom/testbinding.rs | 4 +-- 4 files changed, 29 insertions(+), 26 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 4d31da25efe..5b2b987c233 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -1096,9 +1096,8 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, declType = CGGeneric(typeName) empty = "%s::empty(cx)" % typeName - if isMember != "Dictionary" and type_needs_tracing(type): + if type_needs_tracing(type): declType = CGTemplatedType("RootedTraceableBox", declType) - empty = "RootedTraceableBox::new(%s)" % empty template = ("match FromJSValConvertible::from_jsval(cx, ${val}, ()) {\n" " Ok(ConversionResult::Success(dictionary)) => dictionary,\n" @@ -6094,16 +6093,24 @@ class CGDictionary(CGThing): memberInits = CGList([memberInit(m) for m in self.memberInfo]) memberInserts = CGList([memberInsert(m) for m in self.memberInfo]) + actualType = self.makeClassName(d) + preInitial = "" + postInitial = "" + if self.membersNeedTracing(): + actualType = "RootedTraceableBox<%s>" % actualType + preInitial = "RootedTraceableBox::new(" + postInitial = ")" + return string.Template( "impl ${selfName} {\n" - " pub unsafe fn empty(cx: *mut JSContext) -> ${selfName} {\n" + " pub unsafe fn empty(cx: *mut JSContext) -> ${actualType} {\n" " match ${selfName}::new(cx, HandleValue::null()) {\n" " Ok(ConversionResult::Success(v)) => v,\n" " _ => unreachable!(),\n" " }\n" " }\n" " pub unsafe fn new(cx: *mut JSContext, val: HandleValue) \n" - " -> Result, ()> {\n" + " -> Result, ()> {\n" " let object = if val.get().is_null_or_undefined() {\n" " ptr::null_mut()\n" " } else if val.get().is_object() {\n" @@ -6113,17 +6120,18 @@ class CGDictionary(CGThing): " return Err(());\n" " };\n" " rooted!(in(cx) let object = object);\n" - " Ok(ConversionResult::Success(${selfName} {\n" + " let dictionary = ${preInitial}${selfName} {\n" "${initParent}" "${initMembers}" - " }))\n" + " }${postInitial};\n" + " Ok(ConversionResult::Success(dictionary))\n" " }\n" "}\n" "\n" - "impl FromJSValConvertible for ${selfName} {\n" + "impl FromJSValConvertible for ${actualType} {\n" " type Config = ();\n" " unsafe fn from_jsval(cx: *mut JSContext, value: HandleValue, _option: ())\n" - " -> Result, ()> {\n" + " -> Result, ()> {\n" " ${selfName}::new(cx, value)\n" " }\n" "}\n" @@ -6136,9 +6144,12 @@ class CGDictionary(CGThing): " }\n" "}\n").substitute({ "selfName": self.makeClassName(d), + "actualType": actualType, "initParent": CGIndenter(CGGeneric(initParent), indentLevel=12).define(), "initMembers": CGIndenter(memberInits, indentLevel=12).define(), "insertMembers": CGIndenter(memberInserts, indentLevel=8).define(), + "preInitial": CGGeneric(preInitial).define(), + "postInitial": CGGeneric(postInitial).define(), }) def membersNeedTracing(self): diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs index 10accbcc62b..69751f2c263 100644 --- a/components/script/dom/bindings/conversions.rs +++ b/components/script/dom/bindings/conversions.rs @@ -116,19 +116,11 @@ impl FromJSValConvertible for Root { } } -impl FromJSValConvertible for RootedTraceableBox { - type Config = T::Config; - - unsafe fn from_jsval(cx: *mut JSContext, - value: HandleValue, - config: Self::Config) - -> Result, ()> { - T::from_jsval(cx, value, config).map(|result| { - match result { - ConversionResult::Success(v) => ConversionResult::Success(RootedTraceableBox::new(v)), - ConversionResult::Failure(e) => ConversionResult::Failure(e), - } - }) +impl ToJSValConvertible for RootedTraceableBox { + #[inline] + unsafe fn to_jsval(&self, cx: *mut JSContext, rval: MutableHandleValue) { + let value = &**self; + value.to_jsval(cx, rval); } } diff --git a/components/script/dom/bindings/iterable.rs b/components/script/dom/bindings/iterable.rs index 39c681c7d8b..03fac1cb27f 100644 --- a/components/script/dom/bindings/iterable.rs +++ b/components/script/dom/bindings/iterable.rs @@ -12,7 +12,7 @@ use dom::bindings::codegen::Bindings::IterableIteratorBinding::IterableKeyOrValu use dom::bindings::error::Fallible; use dom::bindings::js::{JS, Root}; use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object}; -use dom::bindings::trace::{JSTraceable, RootedTraceableBox}; +use dom::bindings::trace::JSTraceable; use dom::globalscope::GlobalScope; use dom_struct::dom_struct; use js::conversions::ToJSValConvertible; @@ -115,7 +115,7 @@ fn dict_return(cx: *mut JSContext, result: MutableHandleObject, done: bool, value: HandleValue) -> Fallible<()> { - let mut dict = RootedTraceableBox::new(unsafe { IterableKeyOrValueResult::empty(cx) }); + let mut dict = unsafe { IterableKeyOrValueResult::empty(cx) }; dict.done = done; dict.value.set(value.get()); rooted!(in(cx) let mut dict_value = UndefinedValue()); @@ -130,7 +130,7 @@ fn key_and_value_return(cx: *mut JSContext, result: MutableHandleObject, key: HandleValue, value: HandleValue) -> Fallible<()> { - let mut dict = RootedTraceableBox::new(unsafe { IterableKeyAndValueResult::empty(cx) }); + let mut dict = unsafe { IterableKeyAndValueResult::empty(cx) }; dict.done = false; dict.value = Some(vec![Heap::new(key.get()), Heap::new(value.get())]); rooted!(in(cx) let mut dict_value = UndefinedValue()); diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index 266497da83e..1803403f72f 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -343,7 +343,7 @@ impl TestBindingMethods for TestBinding { anyValue: Heap::new(NullValue()), booleanValue: None, byteValue: None, - dict: TestDictionaryDefaults { + dict: RootedTraceableBox::new(TestDictionaryDefaults { UnrestrictedDoubleValue: 0.0, anyValue: Heap::new(NullValue()), booleanValue: false, @@ -379,7 +379,7 @@ impl TestBindingMethods for TestBinding { unsignedLongValue: 0, unsignedShortValue: 0, usvstringValue: USVString("".to_owned()), - }, + }), doubleValue: None, enumValue: None, floatValue: None, From 16166d66731d7040a91ddbed6ffd05d4fc64c97b Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 26 May 2017 13:46:13 -0400 Subject: [PATCH 4/7] Derive the Default trait for dictionaries containing GC values. --- .../dom/bindings/codegen/CodegenRust.py | 19 +++++++++++++++---- components/script/dom/bindings/num.rs | 7 +++++++ components/script/dom/bindings/str.rs | 5 +++-- components/script/dom/bindings/trace.rs | 6 ++++++ components/script/dom/event.rs | 9 +++++++++ components/script/dom/extendableevent.rs | 10 +++++++++- 6 files changed, 49 insertions(+), 7 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 5b2b987c233..aa58afe1913 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -4028,12 +4028,18 @@ impl super::%s { } } +impl Default for super::%s { + fn default() -> super::%s { + pairs[0].1 + } +} + impl ToJSValConvertible for super::%s { unsafe fn to_jsval(&self, cx: *mut JSContext, rval: MutableHandleValue) { pairs[*self as usize].0.to_jsval(cx, rval); } } - """ % (ident, pairs, ident, ident) + """ % (ident, pairs, ident, ident, ident, ident) self.cgRoot = CGList([ CGGeneric(decl), CGNamespace.build([ident + "Values"], @@ -6038,17 +6044,22 @@ class CGDictionary(CGThing): (self.makeMemberName(m[0].identifier.name), self.getMemberType(m)) for m in self.memberInfo] - mustRoot = "#[must_root]\n" if self.membersNeedTracing() else "" + derive = ["JSTraceable"] + mustRoot = "" + if self.membersNeedTracing(): + mustRoot = "#[must_root]\n" + derive += ["Default"] return (string.Template( - "#[derive(JSTraceable)]\n" + "#[derive(${derive})]\n" "${mustRoot}" + "pub struct ${selfName} {\n" + "${inheritance}" + "\n".join(memberDecls) + "\n" + "}").substitute({"selfName": self.makeClassName(d), "inheritance": inheritance, - "mustRoot": mustRoot})) + "mustRoot": mustRoot, + "derive": ', '.join(derive)})) def impl(self): d = self.dictionary diff --git a/components/script/dom/bindings/num.rs b/components/script/dom/bindings/num.rs index c7d24f4fb83..f7604cfab20 100644 --- a/components/script/dom/bindings/num.rs +++ b/components/script/dom/bindings/num.rs @@ -6,6 +6,7 @@ use heapsize::HeapSizeOf; use num_traits::Float; +use std::default::Default; use std::ops::Deref; /// Encapsulates the IDL restricted float type. @@ -45,3 +46,9 @@ impl HeapSizeOf for Finite { (**self).heap_size_of_children() } } + +impl Default for Finite { + fn default() -> Finite { + Finite::wrap(T::default()) + } +} diff --git a/components/script/dom/bindings/str.rs b/components/script/dom/bindings/str.rs index ea0b4f529b0..b665b64da8a 100644 --- a/components/script/dom/bindings/str.rs +++ b/components/script/dom/bindings/str.rs @@ -9,6 +9,7 @@ use html5ever::{LocalName, Namespace}; use servo_atoms::Atom; use std::ascii::AsciiExt; use std::borrow::{Borrow, Cow, ToOwned}; +use std::default::Default; use std::fmt; use std::hash::{Hash, Hasher}; use std::marker::PhantomData; @@ -18,7 +19,7 @@ use std::str; use std::str::{Bytes, FromStr}; /// Encapsulates the IDL `ByteString` type. -#[derive(Clone, Debug, Eq, HeapSizeOf, JSTraceable, PartialEq)] +#[derive(Clone, Debug, Default, Eq, HeapSizeOf, JSTraceable, PartialEq)] pub struct ByteString(Vec); impl ByteString { @@ -77,7 +78,7 @@ impl ops::Deref for ByteString { /// A string that is constructed from a UCS-2 buffer by replacing invalid code /// points with the replacement character. -#[derive(Clone, HeapSizeOf)] +#[derive(Clone, Default, HeapSizeOf)] pub struct USVString(pub String); diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 797ee356bd8..cdb2fd425ac 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -769,6 +769,12 @@ impl RootedTraceableBox { } } +impl Default for RootedTraceableBox { + fn default() -> RootedTraceableBox { + RootedTraceableBox::new(T::default()) + } +} + impl Deref for RootedTraceableBox { type Target = T; fn deref(&self) -> &T { diff --git a/components/script/dom/event.rs b/components/script/dom/event.rs index c9ef22bc6c1..cead9f74df9 100644 --- a/components/script/dom/event.rs +++ b/components/script/dom/event.rs @@ -522,3 +522,12 @@ fn inner_invoke(window: Option<&Window>, // Step 3. found } + +impl Default for EventBinding::EventInit { + fn default() -> EventBinding::EventInit { + EventBinding::EventInit { + bubbles: false, + cancelable: false, + } + } +} diff --git a/components/script/dom/extendableevent.rs b/components/script/dom/extendableevent.rs index 8cc88706fb1..553ed7a1fe4 100644 --- a/components/script/dom/extendableevent.rs +++ b/components/script/dom/extendableevent.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use dom::bindings::codegen::Bindings::EventBinding::EventMethods; +use dom::bindings::codegen::Bindings::EventBinding::{self, EventMethods}; use dom::bindings::codegen::Bindings::ExtendableEventBinding; use dom::bindings::error::{Error, ErrorResult, Fallible}; use dom::bindings::inheritance::Castable; @@ -67,3 +67,11 @@ impl ExtendableEvent { self.event.IsTrusted() } } + +impl Default for ExtendableEventBinding::ExtendableEventInit { + fn default() -> ExtendableEventBinding::ExtendableEventInit { + ExtendableEventBinding::ExtendableEventInit { + parent: EventBinding::EventInit::default(), + } + } +} From f5eb8445b05d892b432d769f5e036f786e223fd4 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 26 May 2017 14:27:05 -0400 Subject: [PATCH 5/7] Initialize rooted dictionaries to a stable value before setting fields. --- .../dom/bindings/codegen/CodegenRust.py | 52 +++++++++++-------- components/script/dom/bindings/js.rs | 29 ++++++++++- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index aa58afe1913..05db89f3766 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -1045,12 +1045,12 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if defaultValue is None: default = None elif isinstance(defaultValue, IDLNullValue): - default = "Heap::new(NullValue())" + default = "NullValue()" elif isinstance(defaultValue, IDLUndefinedValue): - default = "Heap::new(UndefinedValue())" + default = "UndefinedValue()" else: raise TypeError("Can't handle non-null, non-undefined default value here") - return handleOptional("Heap::new(${val}.get())", declType, default) + return handleOptional("${val}.get()", declType, default) declType = CGGeneric("HandleValue") @@ -1077,8 +1077,6 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if isMember in ("Dictionary", "Union"): declType = CGGeneric("Heap<*mut JSObject>") - templateBody = "Heap::new(%s)" % templateBody - default = "Heap::new(%s)" % default else: # TODO: Need to root somehow # https://github.com/servo/servo/issues/6382 @@ -5708,6 +5706,7 @@ def generate_imports(config, cgthings, descriptors, callbacks=None, dictionaries 'dom::bindings::iterable::Iterable', 'dom::bindings::iterable::IteratorType', 'dom::bindings::js::JS', + 'dom::bindings::js::OptionalHeapSetter', 'dom::bindings::js::Root', 'dom::bindings::js::RootedReference', 'dom::bindings::namespace::NamespaceObjectClass', @@ -6064,7 +6063,7 @@ class CGDictionary(CGThing): def impl(self): d = self.dictionary if d.parent: - initParent = ("parent: {\n" + initParent = ("{\n" " match try!(%s::%s::new(cx, val)) {\n" " ConversionResult::Success(v) => v,\n" " ConversionResult::Failure(error) => {\n" @@ -6072,16 +6071,20 @@ class CGDictionary(CGThing): " return Err(());\n" " }\n" " }\n" - "},\n" % (self.makeModuleName(d.parent), - self.makeClassName(d.parent))) + "}" % (self.makeModuleName(d.parent), + self.makeClassName(d.parent))) else: initParent = "" - def memberInit(memberInfo): + def memberInit(memberInfo, isInitial): member, _ = memberInfo name = self.makeMemberName(member.identifier.name) conversion = self.getMemberConversion(memberInfo, member.type) - return CGGeneric("%s: %s,\n" % (name, conversion.define())) + if isInitial: + return CGGeneric("%s: %s,\n" % (name, conversion.define())) + if member.type.isAny() or member.type.isObject(): + return CGGeneric("dictionary.%s.set(%s);\n" % (name, conversion.define())) + return CGGeneric("dictionary.%s = %s;\n" % (name, conversion.define())) def varInsert(varName, dictionaryName): insertion = ("rooted!(in(cx) let mut %s_js = UndefinedValue());\n" @@ -6101,16 +6104,21 @@ class CGDictionary(CGThing): (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]) - actualType = self.makeClassName(d) - preInitial = "" - postInitial = "" + selfName = self.makeClassName(d) if self.membersNeedTracing(): - actualType = "RootedTraceableBox<%s>" % actualType - preInitial = "RootedTraceableBox::new(" - postInitial = ")" + actualType = "RootedTraceableBox<%s>" % selfName + preInitial = "let mut dictionary = RootedTraceableBox::new(%s::default());\n" % selfName + initParent = initParent = ("dictionary.parent = %s;\n" % initParent) if initParent else "" + memberInits = CGList([memberInit(m, False) for m in self.memberInfo]) + postInitial = "" + else: + actualType = selfName + preInitial = "let dictionary = %s {\n" % selfName + postInitial = "};\n" + initParent = ("parent: %s,\n" % initParent) if initParent else "" + memberInits = CGList([memberInit(m, True) for m in self.memberInfo]) return string.Template( "impl ${selfName} {\n" @@ -6131,10 +6139,10 @@ class CGDictionary(CGThing): " return Err(());\n" " };\n" " rooted!(in(cx) let object = object);\n" - " let dictionary = ${preInitial}${selfName} {\n" + "${preInitial}" "${initParent}" "${initMembers}" - " }${postInitial};\n" + "${postInitial}" " Ok(ConversionResult::Success(dictionary))\n" " }\n" "}\n" @@ -6154,13 +6162,13 @@ class CGDictionary(CGThing): " rval.set(ObjectOrNullValue(obj.get()))\n" " }\n" "}\n").substitute({ - "selfName": self.makeClassName(d), + "selfName": selfName, "actualType": actualType, "initParent": CGIndenter(CGGeneric(initParent), indentLevel=12).define(), "initMembers": CGIndenter(memberInits, indentLevel=12).define(), "insertMembers": CGIndenter(memberInserts, indentLevel=8).define(), - "preInitial": CGGeneric(preInitial).define(), - "postInitial": CGGeneric(postInitial).define(), + "preInitial": CGIndenter(CGGeneric(preInitial), indentLevel=12).define(), + "postInitial": CGIndenter(CGGeneric(postInitial), indentLevel=12).define(), }) def membersNeedTracing(self): diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index 976665e4850..07dc4d74778 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -31,7 +31,8 @@ use dom::bindings::trace::JSTraceable; use dom::bindings::trace::trace_reflector; use dom::node::Node; use heapsize::HeapSizeOf; -use js::jsapi::{JSObject, JSTracer}; +use js::jsapi::{JSObject, JSTracer, Heap}; +use js::rust::GCMethods; use mitochondria::OnceCell; use script_layout_interface::TrustedNodeAddress; use script_thread::STACK_ROOTS; @@ -654,3 +655,29 @@ unsafe impl JSTraceable for Root { // Already traced. } } + +/// Helper trait for safer manipulations of Option> values. +pub trait OptionalHeapSetter { + type Value; + /// Update this optional heap value with a new value. + fn set(&mut self, v: Option); +} + +impl OptionalHeapSetter for Option> where Heap: Default { + type Value = T; + fn set(&mut self, v: Option) { + let v = match v { + None => { + *self = None; + return; + } + Some(v) => v, + }; + + if self.is_none() { + *self = Some(Heap::default()); + } + + self.as_ref().unwrap().set(v); + } +} From 77b3e911c11b2472efa4d7aea944f6aee56171c4 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 26 May 2017 10:47:09 -0400 Subject: [PATCH 6/7] Remove almost all uses of Heap::new. --- components/script/dom/bindings/codegen/CodegenRust.py | 10 +++++++--- components/script/dom/bindings/iterable.rs | 5 ++++- components/script/dom/filereader.rs | 5 +++-- components/script/dom/testbinding.rs | 6 +++--- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 05db89f3766..30d847da19f 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -6699,7 +6699,7 @@ class CallbackMember(CGNativeMember): replacements["argCount"] = self.argCountStr replacements["argvDecl"] = string.Template( "rooted_vec!(let mut argv);\n" - "argv.extend((0..${argCount}).map(|_| Heap::new(UndefinedValue())));\n" + "argv.extend((0..${argCount}).map(|_| Heap::default()));\n" ).substitute(replacements) else: # Avoid weird 0-sized arrays @@ -6774,7 +6774,11 @@ class CallbackMember(CGNativeMember): conversion = wrapForType( "argv_root.handle_mut()", result=argval, - successCode="argv[%s] = Heap::new(argv_root.get());" % jsvalIndex, + successCode=("{\n" + + "let arg = &mut argv[%s];\n" + + "*arg = Heap::default();\n" + + "arg.set(argv_root.get());\n" + + "}") % jsvalIndex, pre="rooted!(in(cx) let mut argv_root = UndefinedValue());") if arg.variadic: conversion = string.Template( @@ -6790,7 +6794,7 @@ class CallbackMember(CGNativeMember): " // This is our current trailing argument; reduce argc\n" " argc -= 1;\n" "} else {\n" - " argv[%d] = Heap::new(UndefinedValue());\n" + " argv[%d] = Heap::default();\n" "}" % (i + 1, i)) return conversion diff --git a/components/script/dom/bindings/iterable.rs b/components/script/dom/bindings/iterable.rs index 03fac1cb27f..7926ff54173 100644 --- a/components/script/dom/bindings/iterable.rs +++ b/components/script/dom/bindings/iterable.rs @@ -132,7 +132,10 @@ fn key_and_value_return(cx: *mut JSContext, value: HandleValue) -> Fallible<()> { let mut dict = unsafe { IterableKeyAndValueResult::empty(cx) }; dict.done = false; - dict.value = Some(vec![Heap::new(key.get()), Heap::new(value.get())]); + let values = vec![Heap::default(), Heap::default()]; + values[0].set(key.get()); + values[1].set(value.get()); + dict.value = Some(values); rooted!(in(cx) let mut dict_value = UndefinedValue()); unsafe { dict.to_jsval(cx, dict_value.handle_mut()); diff --git a/components/script/dom/filereader.rs b/components/script/dom/filereader.rs index 370643bfa44..355cd85bcd4 100644 --- a/components/script/dom/filereader.rs +++ b/components/script/dom/filereader.rs @@ -339,8 +339,9 @@ impl FileReaderMethods for FileReader { FileReaderResult::String(ref string) => StringOrObject::String(string.clone()), FileReaderResult::ArrayBuffer(ref arr_buffer) => { - StringOrObject::Object(RootedTraceableBox::new( - Heap::new((*arr_buffer.ptr.get()).to_object()))) + let result = RootedTraceableBox::new(Heap::default()); + result.set((*arr_buffer.ptr.get()).to_object()); + StringOrObject::Object(result) } }) } diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index 1803403f72f..22f8066d24a 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -340,12 +340,12 @@ impl TestBindingMethods for TestBinding { fn ReceiveNullableSequence(&self) -> Option> { Some(vec![1]) } fn ReceiveTestDictionaryWithSuccessOnKeyword(&self) -> RootedTraceableBox { RootedTraceableBox::new(TestDictionary { - anyValue: Heap::new(NullValue()), + anyValue: Heap::default(), booleanValue: None, byteValue: None, dict: RootedTraceableBox::new(TestDictionaryDefaults { UnrestrictedDoubleValue: 0.0, - anyValue: Heap::new(NullValue()), + anyValue: Heap::default(), booleanValue: false, bytestringValue: ByteString::new(vec![]), byteValue: 0, @@ -361,7 +361,7 @@ impl TestBindingMethods for TestBinding { nullableFloatValue: None, nullableLongLongValue: None, nullableLongValue: None, - nullableObjectValue: Heap::new(ptr::null_mut()), + nullableObjectValue: Heap::default(), nullableOctetValue: None, nullableShortValue: None, nullableStringValue: None, From 44822c364c2f1970705834fb0f95df1411089f9d Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Mon, 26 Jun 2017 07:46:19 -0400 Subject: [PATCH 7/7] Use more named string interpolation. --- .../dom/bindings/codegen/CodegenRust.py | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 30d847da19f..68ab0a33c18 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -4011,33 +4011,36 @@ pub enum %s { pairs = ",\n ".join(['("%s", super::%s::%s)' % (val, ident, getEnumValueName(val)) for val in enum.values()]) - inner = """\ + inner = string.Template("""\ use dom::bindings::conversions::ToJSValConvertible; use js::jsapi::{JSContext, MutableHandleValue}; use js::jsval::JSVal; -pub const pairs: &'static [(&'static str, super::%s)] = &[ - %s, +pub const pairs: &'static [(&'static str, super::${ident})] = &[ + ${pairs}, ]; -impl super::%s { +impl super::${ident} { pub fn as_str(&self) -> &'static str { pairs[*self as usize].0 } } -impl Default for super::%s { - fn default() -> super::%s { - pairs[0].1 - } +impl Default for super::${ident} { + fn default() -> super::${ident} { + pairs[0].1 + } } -impl ToJSValConvertible for super::%s { +impl ToJSValConvertible for super::${ident} { unsafe fn to_jsval(&self, cx: *mut JSContext, rval: MutableHandleValue) { pairs[*self as usize].0.to_jsval(cx, rval); } } - """ % (ident, pairs, ident, ident, ident, ident) + """).substitute({ + 'ident': ident, + 'pairs': pairs + }) self.cgRoot = CGList([ CGGeneric(decl), CGNamespace.build([ident + "Values"],