Skip to content

Commit

Permalink
Use RootedTraceableBox for dictionaries.
Browse files Browse the repository at this point in the history
  • Loading branch information
Ms2ger committed Feb 16, 2017
1 parent f1605ab commit f7e2f0e
Show file tree
Hide file tree
Showing 11 changed files with 75 additions and 24 deletions.
53 changes: 50 additions & 3 deletions components/script/dom/bindings/codegen/CodegenRust.py
Expand Up @@ -19,6 +19,7 @@
IDLBuiltinType,
IDLNullValue,
IDLNullableType,
IDLObject,
IDLType,
IDLInterfaceMember,
IDLUndefinedValue,
Expand Down Expand Up @@ -1090,6 +1091,12 @@ def wrapObjectTemplate(templateBody, nullValue, isDefinitelyObject, type,
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"
Expand All @@ -1098,7 +1105,7 @@ def wrapObjectTemplate(templateBody, nullValue, isDefinitelyObject, type,
" _ => { %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
Expand Down Expand Up @@ -3147,7 +3154,7 @@ def __init__(self, errorResult, arguments, argsPre, returnType,
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))

Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -6163,6 +6171,45 @@ def define(self):
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)
Expand All @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion components/script/dom/customevent.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -51,7 +52,7 @@ impl CustomEvent {
#[allow(unsafe_code)]
pub fn Constructor(global: &GlobalScope,
type_: DOMString,
init: &CustomEventBinding::CustomEventInit)
init: RootedTraceableBox<CustomEventBinding::CustomEventInit>)
-> Fallible<Root<CustomEvent>> {
Ok(CustomEvent::new(global,
Atom::from(type_),
Expand Down
9 changes: 4 additions & 5 deletions components/script/dom/errorevent.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -72,7 +73,8 @@ impl ErrorEvent {

pub fn Constructor(global: &GlobalScope,
type_: DOMString,
init: &ErrorEventBinding::ErrorEventInit) -> Fallible<Root<ErrorEvent>>{
init: RootedTraceableBox<ErrorEventBinding::ErrorEventInit>)
-> Fallible<Root<ErrorEvent>>{
let msg = match init.message.as_ref() {
Some(message) => message.clone(),
None => DOMString::new(),
Expand All @@ -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_),
Expand All @@ -103,7 +102,7 @@ impl ErrorEvent {
file_name,
line_num,
col_num,
error.handle());
init.error.handle());
Ok(event)
}

Expand Down
6 changes: 3 additions & 3 deletions components/script/dom/extendablemessageevent.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -47,15 +48,14 @@ impl ExtendableMessageEvent {

pub fn Constructor(worker: &ServiceWorkerGlobalScope,
type_: DOMString,
init: &ExtendableMessageEventBinding::ExtendableMessageEventInit)
init: RootedTraceableBox<ExtendableMessageEventBinding::ExtendableMessageEventInit>)
-> Fallible<Root<ExtendableMessageEvent>> {
let global = worker.upcast::<GlobalScope>();
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)
Expand Down
8 changes: 3 additions & 5 deletions components/script/dom/messageevent.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -60,16 +61,13 @@ impl MessageEvent {

pub fn Constructor(global: &GlobalScope,
type_: DOMString,
init: &MessageEventBinding::MessageEventInit)
init: RootedTraceableBox<MessageEventBinding::MessageEventInit>)
-> Fallible<Root<MessageEvent>> {
// 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)
Expand Down
3 changes: 2 additions & 1 deletion components/script/dom/popstateevent.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -55,7 +56,7 @@ impl PopStateEvent {

pub fn Constructor(window: &Window,
type_: DOMString,
init: &PopStateEventBinding::PopStateEventInit)
init: RootedTraceableBox<PopStateEventBinding::PopStateEventInit>)
-> Fallible<Root<PopStateEvent>> {
Ok(PopStateEvent::new(window,
Atom::from(type_),
Expand Down
5 changes: 3 additions & 2 deletions components/script/dom/request.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -80,7 +81,7 @@ impl Request {
// https://fetch.spec.whatwg.org/#dom-request
pub fn Constructor(global: &GlobalScope,
input: RequestInfo,
init: &RequestInit)
init: RootedTraceableBox<RequestInit>)
-> Fallible<Root<Request>> {
// Step 1
let temporary_request: NetTraitsRequest;
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion components/script/dom/testbinding.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -402,7 +403,7 @@ impl TestBindingMethods for TestBinding {
}
}

fn DictMatchesPassedValues(&self, arg: &TestDictionary) -> bool {
fn DictMatchesPassedValues(&self, arg: RootedTraceableBox<TestDictionary>) -> bool {
arg.type_.as_ref().map(|s| s == "success").unwrap_or(false) &&
arg.nonRequiredNullable.is_none() &&
arg.nonRequiredNullable2 == Some(None)
Expand Down
3 changes: 2 additions & 1 deletion components/script/dom/window.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Promise> {
fn Fetch(&self, input: RequestOrUSVString, init: RootedTraceableBox<RequestInit>) -> Rc<Promise> {
fetch::Fetch(&self.upcast(), input, init)
}

Expand Down
3 changes: 2 additions & 1 deletion components/script/dom/workerglobalscope.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Promise> {
fn Fetch(&self, input: RequestOrUSVString, init: RootedTraceableBox<RequestInit>) -> Rc<Promise> {
fetch::Fetch(self.upcast(), input, init)
}
}
Expand Down
3 changes: 2 additions & 1 deletion components/script/fetch.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Promise> {
pub fn Fetch(global: &GlobalScope, input: RequestInfo, init: RootedTraceableBox<RequestInit>) -> Rc<Promise> {
let core_resource_thread = global.core_resource_thread();

// Step 1
Expand Down

0 comments on commit f7e2f0e

Please sign in to comment.