From 2a50427b1e9e8cde9d4bacfcc3e7d5c526f299d7 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Wed, 15 Feb 2017 18:21:19 +0100 Subject: [PATCH 1/8] Update js. --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index a1e018113e8..98e41e8712e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1303,7 +1303,7 @@ dependencies = [ [[package]] name = "js" version = "0.1.4" -source = "git+https://github.com/servo/rust-mozjs#b391ec674babe4a3955f562635ea936180c7eea3" +source = "git+https://github.com/servo/rust-mozjs#93e59ef1263e451143d0ed431f1aa564ea101ab8" dependencies = [ "cmake 0.1.20 (registry+https://github.com/rust-lang/crates.io-index)", "heapsize 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", From 7d24cd7752f8b4c030525612fb0fc3496d5fa169 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 14 Feb 2017 13:41:06 +0100 Subject: [PATCH 2/8] Pass isMember to getJSToNativeConversionInfo for unions. Also includes a documentation update for isMember. --- components/script/dom/bindings/codegen/CodegenRust.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index b1f188a7b45..bd8232b2a75 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -581,11 +581,9 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=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. + isMember is `False`, "Dictionary", "Union" or "Variadic", and affects + whether this function returns code suitable for an on-stack rooted binding + or suitable for storing in an appropriate larger structure. invalidEnumValueFatal controls whether an invalid enum value conversion attempt will throw (if true) or simply return without doing anything (if @@ -4059,7 +4057,8 @@ def getUnionTypeTemplateVars(type, descriptorProvider): info = getJSToNativeConversionInfo( type, descriptorProvider, failureCode="return Ok(None);", exceptionCode='return Err(());', - isDefinitelyObject=True) + isDefinitelyObject=True, + isMember="Union") template = info.template jsConversion = string.Template(template).substitute({ From 5eaa19bdd4ae221b2db7b547d6dfae0003511735 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 14 Feb 2017 13:59:07 +0100 Subject: [PATCH 3/8] Share a little less code between the branches for conversion to any. --- .../dom/bindings/codegen/CodegenRust.py | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index bd8232b2a75..37cab821d92 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -1032,8 +1032,6 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type.isAny(): assert not isEnforceRange and not isClamp - declType = "" - default = "" if isMember == "Dictionary": # TODO: Need to properly root dictionaries # https://github.com/servo/servo/issues/6381 @@ -1047,17 +1045,18 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, default = "UndefinedValue()" else: raise TypeError("Can't handle non-null, non-undefined default value here") - else: - declType = CGGeneric("HandleValue") + return handleOptional("${val}", declType, default) - if defaultValue is None: - default = None - elif isinstance(defaultValue, IDLNullValue): - default = "HandleValue::null()" - elif isinstance(defaultValue, IDLUndefinedValue): - default = "HandleValue::undefined()" - else: - raise TypeError("Can't handle non-null, non-undefined default value here") + declType = CGGeneric("HandleValue") + + if defaultValue is None: + default = None + elif isinstance(defaultValue, IDLNullValue): + default = "HandleValue::null()" + elif isinstance(defaultValue, IDLUndefinedValue): + default = "HandleValue::undefined()" + else: + raise TypeError("Can't handle non-null, non-undefined default value here") return handleOptional("${val}", declType, default) From 8ce9ca624367c8e5737b8673548b230b69f4558b Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 14 Feb 2017 14:32:49 +0100 Subject: [PATCH 4/8] Use Heap for dictionary and union members. --- .../dom/bindings/codegen/CodegenRust.py | 29 ++++++++++++------- components/script/dom/bindings/iterable.rs | 6 ++-- components/script/dom/customevent.rs | 2 +- components/script/dom/errorevent.rs | 2 +- .../script/dom/extendablemessageevent.rs | 2 +- components/script/dom/filereader.rs | 2 +- components/script/dom/messageevent.rs | 2 +- components/script/dom/popstateevent.rs | 3 +- components/script/dom/request.rs | 6 ++-- components/script/dom/testbinding.rs | 8 ++--- 10 files changed, 35 insertions(+), 27 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 37cab821d92..c47cf185bf2 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -1031,21 +1031,22 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if type.isAny(): assert not isEnforceRange and not isClamp + assert isMember != "Union" if isMember == "Dictionary": # TODO: Need to properly root dictionaries # https://github.com/servo/servo/issues/6381 - declType = CGGeneric("JSVal") + declType = CGGeneric("Heap") if defaultValue is None: default = None elif isinstance(defaultValue, IDLNullValue): - default = "NullValue()" + default = "Heap::new(NullValue())" elif isinstance(defaultValue, IDLUndefinedValue): - default = "UndefinedValue()" + default = "Heap::new(UndefinedValue())" else: raise TypeError("Can't handle non-null, non-undefined default value here") - return handleOptional("${val}", declType, default) + return handleOptional("Heap::new(${val}.get())", declType, default) declType = CGGeneric("HandleValue") @@ -1065,13 +1066,22 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, # TODO: Need to root somehow # https://github.com/servo/servo/issues/6382 - declType = CGGeneric("*mut JSObject") + default = "ptr::null_mut()" templateBody = wrapObjectTemplate("${val}.get().to_object()", - "ptr::null_mut()", + default, isDefinitelyObject, type, failureCode) + 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 + declType = CGGeneric("*mut JSObject") + return handleOptional(templateBody, declType, - handleDefaultNull("ptr::null_mut()")) + handleDefaultNull(default)) if type.isDictionary(): # There are no nullable dictionaries @@ -2230,6 +2240,7 @@ def UnionTypes(descriptors, dictionaries, callbacks, typedefs, config): 'dom::types::*', 'js::error::throw_type_error', 'js::jsapi::HandleValue', + 'js::jsapi::Heap', 'js::jsapi::JSContext', 'js::jsapi::JSObject', 'js::jsapi::MutableHandleValue', @@ -4049,7 +4060,7 @@ def getUnionTypeTemplateVars(type, descriptorProvider): typeName = builtinNames[type.tag()] elif type.isObject(): name = type.name - typeName = "*mut JSObject" + typeName = "Heap<*mut JSObject>" else: raise TypeError("Can't handle %s in unions yet" % type) @@ -5993,8 +6004,6 @@ class CGDictionary(CGThing): default = info.default replacements = {"val": "rval.handle()"} conversion = string.Template(templateBody).substitute(replacements) - if memberType.isAny(): - conversion = "%s.get()" % conversion assert (member.defaultValue is None) == (default is None) if not member.optional: diff --git a/components/script/dom/bindings/iterable.rs b/components/script/dom/bindings/iterable.rs index 08439d16517..f10bd320b85 100644 --- a/components/script/dom/bindings/iterable.rs +++ b/components/script/dom/bindings/iterable.rs @@ -15,7 +15,7 @@ use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object}; use dom::bindings::trace::JSTraceable; use dom::globalscope::GlobalScope; use js::conversions::ToJSValConvertible; -use js::jsapi::{HandleValue, JSContext, JSObject, MutableHandleObject}; +use js::jsapi::{HandleValue, Heap, JSContext, JSObject, MutableHandleObject}; use js::jsval::UndefinedValue; use std::cell::Cell; use std::ptr; @@ -116,7 +116,7 @@ fn dict_return(cx: *mut JSContext, value: HandleValue) -> Fallible<()> { let mut dict = unsafe { IterableKeyOrValueResult::empty(cx) }; dict.done = done; - dict.value = value.get(); + dict.value.set(value.get()); rooted!(in(cx) let mut dict_value = UndefinedValue()); unsafe { dict.to_jsval(cx, dict_value.handle_mut()); @@ -131,7 +131,7 @@ 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![key.get(), value.get()]); + dict.value = Some(vec![Heap::new(key.get()), Heap::new(value.get())]); rooted!(in(cx) let mut dict_value = UndefinedValue()); unsafe { dict.to_jsval(cx, dict_value.handle_mut()); diff --git a/components/script/dom/customevent.rs b/components/script/dom/customevent.rs index f4346280cb7..942253c30dc 100644 --- a/components/script/dom/customevent.rs +++ b/components/script/dom/customevent.rs @@ -57,7 +57,7 @@ impl CustomEvent { Atom::from(type_), init.parent.bubbles, init.parent.cancelable, - unsafe { HandleValue::from_marked_location(&init.detail) })) + init.detail.handle())) } fn init_custom_event(&self, diff --git a/components/script/dom/errorevent.rs b/components/script/dom/errorevent.rs index 3739d5732f3..a89a7b7e527 100644 --- a/components/script/dom/errorevent.rs +++ b/components/script/dom/errorevent.rs @@ -93,7 +93,7 @@ impl ErrorEvent { // Dictionaries need to be rooted // https://github.com/servo/servo/issues/6381 - rooted!(in(global.get_cx()) let error = init.error); + rooted!(in(global.get_cx()) let error = init.error.get()); let event = ErrorEvent::new( global, Atom::from(type_), diff --git a/components/script/dom/extendablemessageevent.rs b/components/script/dom/extendablemessageevent.rs index a7ee5f797af..b070007f831 100644 --- a/components/script/dom/extendablemessageevent.rs +++ b/components/script/dom/extendablemessageevent.rs @@ -50,7 +50,7 @@ impl ExtendableMessageEvent { init: &ExtendableMessageEventBinding::ExtendableMessageEventInit) -> Fallible> { let global = worker.upcast::(); - rooted!(in(global.get_cx()) let data = init.data); + rooted!(in(global.get_cx()) let data = init.data.get()); let ev = ExtendableMessageEvent::new(global, Atom::from(type_), init.parent.parent.bubbles, diff --git a/components/script/dom/filereader.rs b/components/script/dom/filereader.rs index f97f14480c3..0c303010656 100644 --- a/components/script/dom/filereader.rs +++ b/components/script/dom/filereader.rs @@ -344,7 +344,7 @@ impl FileReaderMethods for FileReader { FileReaderResult::String(ref string) => StringOrObject::String(string.clone()), FileReaderResult::ArrayBuffer(ref arr_buffer) => { - StringOrObject::Object((*arr_buffer.ptr.get()).to_object()) + StringOrObject::Object(Heap::new((*arr_buffer.ptr.get()).to_object())) } }) } diff --git a/components/script/dom/messageevent.rs b/components/script/dom/messageevent.rs index 5b36b14f6ce..b066cf810fe 100644 --- a/components/script/dom/messageevent.rs +++ b/components/script/dom/messageevent.rs @@ -64,7 +64,7 @@ impl MessageEvent { -> Fallible> { // Dictionaries need to be rooted // https://github.com/servo/servo/issues/6381 - rooted!(in(global.get_cx()) let data = init.data); + rooted!(in(global.get_cx()) let data = init.data.get()); let ev = MessageEvent::new(global, Atom::from(type_), init.parent.bubbles, diff --git a/components/script/dom/popstateevent.rs b/components/script/dom/popstateevent.rs index fde05e30394..7db2eea8fe6 100644 --- a/components/script/dom/popstateevent.rs +++ b/components/script/dom/popstateevent.rs @@ -53,7 +53,6 @@ impl PopStateEvent { ev } - #[allow(unsafe_code)] pub fn Constructor(window: &Window, type_: DOMString, init: &PopStateEventBinding::PopStateEventInit) @@ -62,7 +61,7 @@ impl PopStateEvent { Atom::from(type_), init.parent.bubbles, init.parent.cancelable, - unsafe { HandleValue::from_marked_location(&init.state) })) + init.state.handle())) } } diff --git a/components/script/dom/request.rs b/components/script/dom/request.rs index 2b26d5e1e8c..bf4914f8255 100644 --- a/components/script/dom/request.rs +++ b/components/script/dom/request.rs @@ -139,12 +139,12 @@ impl Request { // TODO: `environment settings object` is not implemented in Servo yet. // Step 10 - if !init.window.is_undefined() && !init.window.is_null() { + if !init.window.handle().is_null_or_undefined() { return Err(Error::Type("Window is present and is not null".to_string())) } // Step 11 - if !init.window.is_undefined() { + if !init.window.handle().is_undefined() { window = Window::NoWindow; } @@ -179,7 +179,7 @@ impl Request { init.redirect.is_some() || init.referrer.is_some() || init.referrerPolicy.is_some() || - !init.window.is_undefined() { + !init.window.handle().is_undefined() { // Step 13.1 if request.mode == NetTraitsRequestMode::Navigate { return Err(Error::Type( diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index 4726b82d900..677d5c611db 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -33,7 +33,7 @@ use dom::globalscope::GlobalScope; use dom::promise::Promise; use dom::promisenativehandler::{PromiseNativeHandler, Callback}; use dom::url::URL; -use js::jsapi::{HandleObject, HandleValue, JSContext, JSObject, JSAutoCompartment}; +use js::jsapi::{HandleObject, HandleValue, Heap, JSContext, JSObject, JSAutoCompartment}; use js::jsapi::{JS_NewPlainObject, JS_NewUint8ClampedArray}; use js::jsval::{JSVal, NullValue}; use script_traits::MsDuration; @@ -338,12 +338,12 @@ impl TestBindingMethods for TestBinding { fn ReceiveNullableSequence(&self) -> Option> { Some(vec![1]) } fn ReceiveTestDictionaryWithSuccessOnKeyword(&self) -> TestDictionary { TestDictionary { - anyValue: NullValue(), + anyValue: Heap::new(NullValue()), booleanValue: None, byteValue: None, dict: TestDictionaryDefaults { UnrestrictedDoubleValue: 0.0, - anyValue: NullValue(), + anyValue: Heap::new(NullValue()), booleanValue: false, bytestringValue: ByteString::new(vec![]), byteValue: 0, @@ -359,7 +359,7 @@ impl TestBindingMethods for TestBinding { nullableFloatValue: None, nullableLongLongValue: None, nullableLongValue: None, - nullableObjectValue: ptr::null_mut(), + nullableObjectValue: Heap::new(ptr::null_mut()), nullableOctetValue: None, nullableShortValue: None, nullableStringValue: None, From 3613e8f231a06142abb726a28bcd8847e768708c Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 14 Feb 2017 14:42:35 +0100 Subject: [PATCH 5/8] Implement JSTraceable for more types. --- components/script/dom/bindings/codegen/CodegenRust.py | 2 ++ components/script/dom/bindings/js.rs | 6 ++++++ components/script/dom/bindings/mozmap.rs | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index c47cf185bf2..444d52d3ae6 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -4103,6 +4103,7 @@ class CGUnionStruct(CGThing): % (self.type, v["name"]) for v in templateVars ] return ("""\ +#[derive(JSTraceable)] pub enum %s { %s } @@ -5881,6 +5882,7 @@ class CGDictionary(CGThing): for m in self.memberInfo] return (string.Template( + "#[derive(JSTraceable)]\n" "pub struct ${selfName} {\n" + "${inheritance}" + "\n".join(memberDecls) + "\n" + diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index bdd2b81ce37..e63177e3bd8 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -642,3 +642,9 @@ impl Drop for Root { } } } + +unsafe impl JSTraceable for Root { + unsafe fn trace(&self, _: *mut JSTracer) { + // Already traced. + } +} diff --git a/components/script/dom/bindings/mozmap.rs b/components/script/dom/bindings/mozmap.rs index 5ef102539b7..abfedf02caf 100644 --- a/components/script/dom/bindings/mozmap.rs +++ b/components/script/dom/bindings/mozmap.rs @@ -23,7 +23,7 @@ use std::collections::HashMap; use std::ops::Deref; /// The `MozMap` (open-ended dictionary) type. -#[derive(Clone)] +#[derive(Clone, JSTraceable)] pub struct MozMap { map: HashMap, } From 8c8eb41cdf56feb3b03d3b47cf0a13024a2690d9 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 14 Feb 2017 16:45:32 +0100 Subject: [PATCH 6/8] Use from_jsval for dictionaries. --- components/script/dom/bindings/codegen/CodegenRust.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 444d52d3ae6..2d188d41104 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -1090,13 +1090,13 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, typeName = "%s::%s" % (CGDictionary.makeModuleName(type.inner), CGDictionary.makeDictionaryName(type.inner)) declType = CGGeneric(typeName) - template = ("match %s::new(cx, ${val}) {\n" + template = ("match FromJSValConvertible::from_jsval(cx, ${val}, ()) {\n" " Ok(ConversionResult::Success(dictionary)) => dictionary,\n" " Ok(ConversionResult::Failure(error)) => {\n" "%s\n" " }\n" " _ => { %s },\n" - "}" % (typeName, indent(failOrPropagate, 8), exceptionCode)) + "}" % (indent(failOrPropagate, 8), exceptionCode)) return handleOptional(template, declType, handleDefaultNull("%s::empty(cx)" % typeName)) From f1605ab149032adb20aec667d7660a4e433824e8 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 14 Feb 2017 16:49:25 +0100 Subject: [PATCH 7/8] Introduce RootedTraceableBox. --- components/script/dom/bindings/conversions.rs | 17 ++++++ components/script/dom/bindings/trace.rs | 55 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs index 747573cec05..0cbb49bea3e 100644 --- a/components/script/dom/bindings/conversions.rs +++ b/components/script/dom/bindings/conversions.rs @@ -37,6 +37,7 @@ use dom::bindings::js::Root; use dom::bindings::num::Finite; use dom::bindings::reflector::{DomObject, Reflector}; use dom::bindings::str::{ByteString, DOMString, USVString}; +use dom::bindings::trace::{JSTraceable, RootedTraceableBox}; use dom::bindings::utils::DOMClass; use js; pub use js::conversions::{FromJSValConvertible, ToJSValConvertible, ConversionResult}; @@ -117,6 +118,22 @@ 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), + } + }) + } +} + /// Convert `id` to a `DOMString`, assuming it is string-valued. /// /// Handling of invalid UTF-16 in strings depends on the relevant option. diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 763ccb9b22b..aeddcdabe4b 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -653,6 +653,61 @@ impl<'a, T: JSTraceable + 'static> Drop for RootedTraceable<'a, T> { } } +/// Roots any JSTraceable thing +/// +/// If you have a valid DomObject, use Root. +/// 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. +pub struct RootedTraceableBox { + ptr: *mut T, +} + +unsafe impl JSTraceable for RootedTraceableBox { + unsafe fn trace(&self, tracer: *mut JSTracer) { + (*self.ptr).trace(tracer); + } +} + +impl RootedTraceableBox { + /// Root a JSTraceable thing for the life of this RootedTraceable + pub fn new(traceable: T) -> RootedTraceableBox { + let traceable = Box::into_raw(box traceable); + unsafe { + RootedTraceableSet::add(traceable); + } + RootedTraceableBox { + ptr: traceable, + } + } +} + +impl Deref for RootedTraceableBox { + type Target = T; + fn deref(&self) -> &T { + unsafe { + &*self.ptr + } + } +} + +impl DerefMut for RootedTraceableBox { + fn deref_mut(&mut self) -> &mut T { + unsafe { + &mut *self.ptr + } + } +} + +impl Drop for RootedTraceableBox { + fn drop(&mut self) { + unsafe { + RootedTraceableSet::remove(self.ptr); + let _ = Box::from_raw(self.ptr); + } + } +} + /// A vector of items to be rooted with `RootedVec`. /// Guaranteed to be empty when not rooted. /// Usage: `rooted_vec!(let mut v);` or if you have an From f7e2f0e641967ba78a0b6b057aec760f9a9ca519 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 14 Feb 2017 16:45:36 +0100 Subject: [PATCH 8/8] Use RootedTraceableBox for dictionaries. --- .../dom/bindings/codegen/CodegenRust.py | 53 +++++++++++++++++-- components/script/dom/customevent.rs | 3 +- components/script/dom/errorevent.rs | 9 ++-- .../script/dom/extendablemessageevent.rs | 6 +-- components/script/dom/messageevent.rs | 8 ++- components/script/dom/popstateevent.rs | 3 +- components/script/dom/request.rs | 5 +- components/script/dom/testbinding.rs | 3 +- components/script/dom/window.rs | 3 +- components/script/dom/workerglobalscope.rs | 3 +- components/script/fetch.rs | 3 +- 11 files changed, 75 insertions(+), 24 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 2d188d41104..e4448ff834c 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -19,6 +19,7 @@ from WebIDL import ( IDLBuiltinType, IDLNullValue, IDLNullableType, + IDLObject, IDLType, IDLInterfaceMember, IDLUndefinedValue, @@ -1090,6 +1091,12 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, typeName = "%s::%s" % (CGDictionary.makeModuleName(type.inner), CGDictionary.makeDictionaryName(type.inner)) declType = CGGeneric(typeName) + empty = "%s::empty(cx)" % typeName + + if isMember != "Dictionary" and 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" " Ok(ConversionResult::Failure(error)) => {\n" @@ -1098,7 +1105,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, " _ => { %s },\n" "}" % (indent(failOrPropagate, 8), exceptionCode)) - return handleOptional(template, declType, handleDefaultNull("%s::empty(cx)" % typeName)) + return handleOptional(template, declType, handleDefaultNull(empty)) if type.isVoid(): # This one only happens for return values, and its easy: Just @@ -3147,7 +3154,7 @@ class CGCallGenerator(CGThing): args = CGList([CGGeneric(arg) for arg in argsPre], ", ") for (a, name) in arguments: # XXXjdm Perhaps we should pass all nontrivial types by borrowed pointer - if a.type.isDictionary(): + if a.type.isDictionary() and not type_needs_tracing(a.type): name = "&" + name args.append(CGGeneric(name)) @@ -5577,6 +5584,7 @@ def generate_imports(config, cgthings, descriptors, callbacks=None, dictionaries 'dom::bindings::utils::trace_global', 'dom::bindings::trace::JSTraceable', 'dom::bindings::trace::RootedTraceable', + 'dom::bindings::trace::RootedTraceableBox', 'dom::bindings::callback::CallSetup', 'dom::bindings::callback::CallbackContainer', 'dom::bindings::callback::CallbackInterface', @@ -6163,6 +6171,45 @@ class CGBindingRoot(CGThing): return stripTrailingWhitespace(self.root.define()) +def type_needs_tracing(t): + assert isinstance(t, IDLObject), (t, type(t)) + + if t.isType(): + if isinstance(t, IDLWrapperType): + return type_needs_tracing(t.inner) + + if t.nullable(): + return type_needs_tracing(t.inner) + + if t.isAny(): + return True + + if t.isObject(): + return True + + if t.isSequence(): + return type_needs_tracing(t.inner) + + return False + + if t.isDictionary(): + if t.parent and type_needs_tracing(t.parent): + return True + + if any(type_needs_tracing(member.type) for member in t.members): + return True + + return False + + if t.isInterface(): + return False + + if t.isEnum(): + return False + + assert False, (t, type(t)) + + def argument_type(descriptorProvider, ty, optional=False, defaultValue=None, variadic=False): info = getJSToNativeConversionInfo( ty, descriptorProvider, isArgument=True) @@ -6176,7 +6223,7 @@ def argument_type(descriptorProvider, ty, optional=False, defaultValue=None, var elif optional and not defaultValue: declType = CGWrapper(declType, pre="Option<", post=">") - if ty.isDictionary(): + if ty.isDictionary() and not type_needs_tracing(ty): declType = CGWrapper(declType, pre="&") return declType.define() diff --git a/components/script/dom/customevent.rs b/components/script/dom/customevent.rs index 942253c30dc..f11f2643dad 100644 --- a/components/script/dom/customevent.rs +++ b/components/script/dom/customevent.rs @@ -10,6 +10,7 @@ use dom::bindings::inheritance::Castable; use dom::bindings::js::{MutHeapJSVal, Root}; use dom::bindings::reflector::reflect_dom_object; use dom::bindings::str::DOMString; +use dom::bindings::trace::RootedTraceableBox; use dom::event::Event; use dom::globalscope::GlobalScope; use js::jsapi::{HandleValue, JSContext}; @@ -51,7 +52,7 @@ impl CustomEvent { #[allow(unsafe_code)] pub fn Constructor(global: &GlobalScope, type_: DOMString, - init: &CustomEventBinding::CustomEventInit) + init: RootedTraceableBox) -> Fallible> { Ok(CustomEvent::new(global, Atom::from(type_), diff --git a/components/script/dom/errorevent.rs b/components/script/dom/errorevent.rs index a89a7b7e527..37197b6125a 100644 --- a/components/script/dom/errorevent.rs +++ b/components/script/dom/errorevent.rs @@ -11,6 +11,7 @@ use dom::bindings::inheritance::Castable; use dom::bindings::js::{MutHeapJSVal, Root}; use dom::bindings::reflector::reflect_dom_object; use dom::bindings::str::DOMString; +use dom::bindings::trace::RootedTraceableBox; use dom::event::{Event, EventBubbles, EventCancelable}; use dom::globalscope::GlobalScope; use js::jsapi::{HandleValue, JSContext}; @@ -72,7 +73,8 @@ impl ErrorEvent { pub fn Constructor(global: &GlobalScope, type_: DOMString, - init: &ErrorEventBinding::ErrorEventInit) -> Fallible>{ + init: RootedTraceableBox) + -> Fallible>{ let msg = match init.message.as_ref() { Some(message) => message.clone(), None => DOMString::new(), @@ -91,9 +93,6 @@ impl ErrorEvent { let cancelable = EventCancelable::from(init.parent.cancelable); - // Dictionaries need to be rooted - // https://github.com/servo/servo/issues/6381 - rooted!(in(global.get_cx()) let error = init.error.get()); let event = ErrorEvent::new( global, Atom::from(type_), @@ -103,7 +102,7 @@ impl ErrorEvent { file_name, line_num, col_num, - error.handle()); + init.error.handle()); Ok(event) } diff --git a/components/script/dom/extendablemessageevent.rs b/components/script/dom/extendablemessageevent.rs index b070007f831..dd0360cb408 100644 --- a/components/script/dom/extendablemessageevent.rs +++ b/components/script/dom/extendablemessageevent.rs @@ -9,6 +9,7 @@ use dom::bindings::inheritance::Castable; use dom::bindings::js::Root; use dom::bindings::reflector::reflect_dom_object; use dom::bindings::str::DOMString; +use dom::bindings::trace::RootedTraceableBox; use dom::event::Event; use dom::eventtarget::EventTarget; use dom::extendableevent::ExtendableEvent; @@ -47,15 +48,14 @@ impl ExtendableMessageEvent { pub fn Constructor(worker: &ServiceWorkerGlobalScope, type_: DOMString, - init: &ExtendableMessageEventBinding::ExtendableMessageEventInit) + init: RootedTraceableBox) -> Fallible> { let global = worker.upcast::(); - rooted!(in(global.get_cx()) let data = init.data.get()); let ev = ExtendableMessageEvent::new(global, Atom::from(type_), init.parent.parent.bubbles, init.parent.parent.cancelable, - data.handle(), + init.data.handle(), init.origin.clone().unwrap(), init.lastEventId.clone().unwrap()); Ok(ev) diff --git a/components/script/dom/messageevent.rs b/components/script/dom/messageevent.rs index b066cf810fe..3df960d0882 100644 --- a/components/script/dom/messageevent.rs +++ b/components/script/dom/messageevent.rs @@ -10,6 +10,7 @@ use dom::bindings::inheritance::Castable; use dom::bindings::js::Root; use dom::bindings::reflector::reflect_dom_object; use dom::bindings::str::DOMString; +use dom::bindings::trace::RootedTraceableBox; use dom::event::Event; use dom::eventtarget::EventTarget; use dom::globalscope::GlobalScope; @@ -60,16 +61,13 @@ impl MessageEvent { pub fn Constructor(global: &GlobalScope, type_: DOMString, - init: &MessageEventBinding::MessageEventInit) + init: RootedTraceableBox) -> Fallible> { - // Dictionaries need to be rooted - // https://github.com/servo/servo/issues/6381 - rooted!(in(global.get_cx()) let data = init.data.get()); let ev = MessageEvent::new(global, Atom::from(type_), init.parent.bubbles, init.parent.cancelable, - data.handle(), + init.data.handle(), init.origin.clone(), init.lastEventId.clone()); Ok(ev) diff --git a/components/script/dom/popstateevent.rs b/components/script/dom/popstateevent.rs index 7db2eea8fe6..1282d847d7c 100644 --- a/components/script/dom/popstateevent.rs +++ b/components/script/dom/popstateevent.rs @@ -10,6 +10,7 @@ use dom::bindings::inheritance::Castable; use dom::bindings::js::{MutHeapJSVal, Root}; use dom::bindings::reflector::reflect_dom_object; use dom::bindings::str::DOMString; +use dom::bindings::trace::RootedTraceableBox; use dom::event::Event; use dom::window::Window; use js::jsapi::{HandleValue, JSContext}; @@ -55,7 +56,7 @@ impl PopStateEvent { pub fn Constructor(window: &Window, type_: DOMString, - init: &PopStateEventBinding::PopStateEventInit) + init: RootedTraceableBox) -> Fallible> { Ok(PopStateEvent::new(window, Atom::from(type_), diff --git a/components/script/dom/request.rs b/components/script/dom/request.rs index bf4914f8255..a81d9f767b6 100644 --- a/components/script/dom/request.rs +++ b/components/script/dom/request.rs @@ -20,6 +20,7 @@ use dom::bindings::error::{Error, Fallible}; use dom::bindings::js::{MutNullableJS, Root}; use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object}; use dom::bindings::str::{ByteString, DOMString, USVString}; +use dom::bindings::trace::RootedTraceableBox; use dom::globalscope::GlobalScope; use dom::headers::{Guard, Headers}; use dom::promise::Promise; @@ -80,7 +81,7 @@ impl Request { // https://fetch.spec.whatwg.org/#dom-request pub fn Constructor(global: &GlobalScope, input: RequestInfo, - init: &RequestInit) + init: RootedTraceableBox) -> Fallible> { // Step 1 let temporary_request: NetTraitsRequest; @@ -311,7 +312,7 @@ impl Request { if let Some(possible_header) = init.headers.as_ref() { match possible_header { &HeadersInit::Headers(ref init_headers) => { - headers_copy = init_headers.clone(); + headers_copy = Root::from_ref(&*init_headers); } &HeadersInit::ByteStringSequenceSequence(ref init_sequence) => { try!(headers_copy.fill(Some( diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index 677d5c611db..1582647ca33 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -27,6 +27,7 @@ use dom::bindings::num::Finite; use dom::bindings::refcounted::TrustedPromise; use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object}; use dom::bindings::str::{ByteString, DOMString, USVString}; +use dom::bindings::trace::RootedTraceableBox; use dom::bindings::weakref::MutableWeakRef; use dom::blob::{Blob, BlobImpl}; use dom::globalscope::GlobalScope; @@ -402,7 +403,7 @@ impl TestBindingMethods for TestBinding { } } - fn DictMatchesPassedValues(&self, arg: &TestDictionary) -> bool { + fn DictMatchesPassedValues(&self, arg: RootedTraceableBox) -> bool { arg.type_.as_ref().map(|s| s == "success").unwrap_or(false) && arg.nonRequiredNullable.is_none() && arg.nonRequiredNullable2 == Some(None) diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index b68bd492fd0..d1eba0245e0 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -26,6 +26,7 @@ use dom::bindings::refcounted::Trusted; use dom::bindings::reflector::DomObject; use dom::bindings::str::DOMString; use dom::bindings::structuredclone::StructuredCloneData; +use dom::bindings::trace::RootedTraceableBox; use dom::bindings::utils::{GlobalStaticData, WindowProxyHandler}; use dom::bluetooth::BluetoothExtraPermissionData; use dom::browsingcontext::BrowsingContext; @@ -921,7 +922,7 @@ impl WindowMethods for Window { #[allow(unrooted_must_root)] // https://fetch.spec.whatwg.org/#fetch-method - fn Fetch(&self, input: RequestOrUSVString, init: &RequestInit) -> Rc { + fn Fetch(&self, input: RequestOrUSVString, init: RootedTraceableBox) -> Rc { fetch::Fetch(&self.upcast(), input, init) } diff --git a/components/script/dom/workerglobalscope.rs b/components/script/dom/workerglobalscope.rs index 16bd699da7c..03e531c5f16 100644 --- a/components/script/dom/workerglobalscope.rs +++ b/components/script/dom/workerglobalscope.rs @@ -14,6 +14,7 @@ use dom::bindings::js::{MutNullableJS, Root}; use dom::bindings::reflector::DomObject; use dom::bindings::settings_stack::AutoEntryScript; use dom::bindings::str::DOMString; +use dom::bindings::trace::RootedTraceableBox; use dom::crypto::Crypto; use dom::dedicatedworkerglobalscope::DedicatedWorkerGlobalScope; use dom::globalscope::GlobalScope; @@ -314,7 +315,7 @@ impl WorkerGlobalScopeMethods for WorkerGlobalScope { #[allow(unrooted_must_root)] // https://fetch.spec.whatwg.org/#fetch-method - fn Fetch(&self, input: RequestOrUSVString, init: &RequestInit) -> Rc { + fn Fetch(&self, input: RequestOrUSVString, init: RootedTraceableBox) -> Rc { fetch::Fetch(self.upcast(), input, init) } } diff --git a/components/script/fetch.rs b/components/script/fetch.rs index f1fd49147b7..0e7cacdfc1a 100644 --- a/components/script/fetch.rs +++ b/components/script/fetch.rs @@ -10,6 +10,7 @@ use dom::bindings::error::Error; use dom::bindings::js::Root; use dom::bindings::refcounted::{Trusted, TrustedPromise}; use dom::bindings::reflector::DomObject; +use dom::bindings::trace::RootedTraceableBox; use dom::globalscope::GlobalScope; use dom::headers::Guard; use dom::promise::Promise; @@ -68,7 +69,7 @@ fn request_init_from_request(request: NetTraitsRequest) -> NetTraitsRequestInit // https://fetch.spec.whatwg.org/#fetch-method #[allow(unrooted_must_root)] -pub fn Fetch(global: &GlobalScope, input: RequestInfo, init: &RequestInit) -> Rc { +pub fn Fetch(global: &GlobalScope, input: RequestInfo, init: RootedTraceableBox) -> Rc { let core_resource_thread = global.core_resource_thread(); // Step 1