Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a safe, mostly-sound rooting rooting strategy. #2101

Merged
merged 16 commits into from May 3, 2014
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Remove all root collections.

  • Loading branch information
jdm committed May 3, 2014
commit 7b3e6d1f2125faf598919722b72cc56197d0102c
@@ -4,7 +4,7 @@

use dom::bindings::codegen::BindingDeclarations::AttrBinding;
use dom::bindings::codegen::InheritTypes::NodeCast;
use dom::bindings::js::{JS, JSRef, Temporary, RootCollection};
use dom::bindings::js::{JS, JSRef, Temporary};
use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
use dom::element::Element;
use dom::node::Node;
@@ -65,8 +65,7 @@ impl Attr {
}

pub fn set_value(&mut self, set_type: AttrSettingType, value: DOMString) {
let roots = RootCollection::new();
let owner = self.owner.root(&roots);
let owner = self.owner.root();
let node: &JSRef<Node> = NodeCast::from_ref(&*owner);
let namespace_is_null = self.namespace == namespace::Null;

@@ -4,7 +4,7 @@

use dom::attr::Attr;
use dom::bindings::codegen::BindingDeclarations::AttrListBinding;
use dom::bindings::js::{JS, JSRef, Temporary, RootCollection};
use dom::bindings::js::{JS, JSRef, Temporary};
use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
use dom::element::Element;
use dom::window::Window;
@@ -39,13 +39,11 @@ pub trait AttrListMethods {

impl<'a> AttrListMethods for JSRef<'a, AttrList> {
fn Length(&self) -> u32 {
let roots = RootCollection::new();
self.owner.root(&roots).attrs.len() as u32
self.owner.root().attrs.len() as u32
}

fn Item(&self, index: u32) -> Option<Temporary<Attr>> {
let roots = RootCollection::new();
self.owner.root(&roots).attrs.as_slice().get(index as uint).map(|x| Temporary::new(x.clone()))
self.owner.root().attrs.as_slice().get(index as uint).map(|x| Temporary::new(x.clone()))
}

fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Temporary<Attr>> {
@@ -903,10 +903,10 @@ def instantiateJSToNativeConversionTemplate(templateTuple, replacements,
if type and 'JS<' in type:
if dealWithOptional or 'Option<' in type:
rootBody = """let ${simpleDeclName} = ${declName}.as_ref().map(|inner| {
inner.root(&roots) //second root code
inner.root() //second root code
});"""
else:
rootBody = "let ${simpleDeclName} = ${declName}.root(&roots); //third root code"
rootBody = "let ${simpleDeclName} = ${declName}.root(); //third root code"
result.append(CGGeneric(string.Template(rootBody).substitute(replacements)))
result.append(CGGeneric(""))

@@ -1799,7 +1799,7 @@ def _decorators(self):
def _returnType(self):
return (" -> %s" % self.returnType) if self.returnType != "void" else ""
def _unsafe_open(self):
return "\n unsafe {\n let roots = RootCollection::new();\n" if self.unsafe else ""
return "\n unsafe {\n" if self.unsafe else ""
def _unsafe_close(self):
return "\n }\n" if self.unsafe else ""

@@ -2247,7 +2247,7 @@ def __init__(self, errorReport, arguments, argsPre, returnType,
self.cgRoot.append(CGGeneric("result = result_fallible.unwrap();"))

if typeRetValNeedsRooting(returnType):
self.cgRoot.append(CGGeneric("let result = result.root(&roots);"))
self.cgRoot.append(CGGeneric("let result = result.root();"))

def define(self):
return self.cgRoot.define()
@@ -2498,7 +2498,7 @@ def definition_body(self):
self.descriptor, self.method),
pre=extraPre +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n").define()
" let mut this = this.root();\n").define()

class CGGenericGetter(CGAbstractBindingMethod):
"""
@@ -2552,7 +2552,7 @@ def definition_body(self):
self.descriptor, self.attr)),
pre=extraPre +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n").define()
" let mut this = this.root();\n").define()

class CGGenericSetter(CGAbstractBindingMethod):
"""
@@ -2606,7 +2606,7 @@ def definition_body(self):
self.descriptor, self.attr)),
pre=extraPre +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n").define()
" let mut this = this.root();\n").define()


class CGMemberJITInfo(CGThing):
@@ -3531,7 +3531,7 @@ def getBody(self):
" let index = index.unwrap();\n" +
" let this = UnwrapProxy(proxy);\n" +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n" +
" let mut this = this.root();\n" +
CGIndenter(CGProxyIndexedGetter(self.descriptor, templateValues)).define() + "\n" +
"}\n")

@@ -3578,7 +3578,7 @@ def getBody(self):
" let name = Some(jsid_to_str(cx, id));\n" +
" let this = UnwrapProxy(proxy);\n" +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n" +
" let mut this = this.root();\n" +
CGIndenter(CGProxyNamedGetter(self.descriptor, templateValues)).define() + "\n" +
"}\n")
else:
@@ -3623,7 +3623,7 @@ def getBody(self):
" let index = index.unwrap();\n" +
" let this = UnwrapProxy(proxy);\n" +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n" +
" let mut this = this.root();\n" +
CGIndenter(CGProxyIndexedSetter(self.descriptor)).define() +
" return 1;\n" +
"}\n")
@@ -3641,15 +3641,15 @@ def getBody(self):
" let name = Some(jsid_to_str(cx, id));\n" +
" let this = UnwrapProxy(proxy);\n" +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n" +
" let mut this = this.root();\n" +
CGIndenter(CGProxyNamedSetter(self.descriptor)).define() + "\n" +
"}\n")
elif self.descriptor.operations['NamedGetter']:
set += ("if RUST_JSID_IS_STRING(id) {\n" +
" let name = Some(jsid_to_str(cx, id));\n" +
" let this = UnwrapProxy(proxy);\n" +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n" +
" let mut this = this.root();\n" +
CGIndenter(CGProxyNamedGetter(self.descriptor)).define() +
" if (found) {\n"
" return 0;\n" +
@@ -3676,7 +3676,7 @@ def getBody(self):
" let index = index.unwrap();\n" +
" let this = UnwrapProxy(proxy);\n" +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n" +
" let mut this = this.root();\n" +
CGIndenter(CGProxyIndexedGetter(self.descriptor)).define() + "\n" +
" *bp = found as JSBool;\n" +
" return 1;\n" +
@@ -3690,7 +3690,7 @@ def getBody(self):
" let name = Some(jsid_to_str(cx, id));\n" +
" let this = UnwrapProxy(proxy);\n" +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n" +
" let mut this = this.root();\n" +
CGIndenter(CGProxyNamedGetter(self.descriptor)).define() + "\n" +
" *bp = found as JSBool;\n"
" return 1;\n"
@@ -3744,7 +3744,7 @@ def getBody(self):
" let index = index.unwrap();\n" +
" let this = UnwrapProxy(proxy);\n" +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n" +
" let mut this = this.root();\n" +
CGIndenter(CGProxyIndexedGetter(self.descriptor, templateValues)).define())
getIndexedOrExpando += """
// Even if we don't have this index, we don't forward the
@@ -3762,7 +3762,7 @@ def getBody(self):
" let name = Some(jsid_to_str(cx, id));\n" +
" let this = UnwrapProxy(proxy);\n" +
" let this = JS::from_raw(this);\n" +
" let mut this = this.root(&roots);\n" +
" let mut this = this.root();\n" +
CGIndenter(CGProxyNamedGetter(self.descriptor, templateValues)).define() +
"}\n") % (self.descriptor.concreteType)
else:
@@ -3879,9 +3879,8 @@ def definition_body(self):

def generate_code(self):
preamble = """
let roots = RootCollection::new();
let global = global_object_for_js_object(JS_CALLEE(cx, &*vp).to_object());
let global = global.root(&roots);
let global = global.root();
let obj = global.reflector().get_jsobject();
"""
nativeName = MakeNativeName(self._ctor.identifier.name)
@@ -4123,7 +4122,6 @@ def memberInit(memberInfo):
return string.Template(
"impl ${selfName} {\n"
" pub fn new(cx: *JSContext, val: JSVal) -> Result<${selfName}, ()> {\n"
" let roots = RootCollection::new();\n" # XXXjdm need to root dict members outside of Init
" let object = if val.is_null_or_undefined() {\n"
" ptr::null()\n"
" } else if val.is_object() {\n"
@@ -4323,7 +4321,7 @@ def __init__(self, config, prefix, webIDLFile):
'js::glue::{RUST_JS_NumberValue, RUST_JSID_IS_STRING}',
'dom::types::*',
'dom::bindings',
'dom::bindings::js::{JS, JSRef, RootCollection, RootedReference, Temporary, OptionalRootable}',
'dom::bindings::js::{JS, JSRef, RootedReference, Temporary, OptionalRootable}',
'dom::bindings::utils::{CreateDOMGlobal, CreateInterfaceObjects2}',
'dom::bindings::utils::{ConstantSpec, cx_for_dom_object, Default}',
'dom::bindings::utils::{dom_object_slot, DOM_OBJECT_SLOT, DOMClass}',
@@ -54,7 +54,7 @@ impl<T: Reflectable> Temporary<T> {
}

/// Root this unrooted value.
pub fn root<'a, 'b>(self, _collection: &'a RootCollection) -> Root<'a, 'b, T> {
pub fn root<'a, 'b>(self) -> Root<'a, 'b, T> {
local_data::get(StackRoots, |opt| {
let collection = opt.unwrap();
unsafe {
@@ -119,8 +119,13 @@ impl<T: Reflectable> JS<T> {
}

/// Root this JS-owned value to prevent its collection as garbage.
pub fn root<'a, 'b>(&self, collection: &'a RootCollection) -> Root<'a, 'b, T> {
collection.new_root(self)
pub fn root<'a, 'b>(&self) -> Root<'a, 'b, T> {
local_data::get(StackRoots, |opt| {
let collection = opt.unwrap();
unsafe {
(**collection).new_root(self)
}
})
}
}

@@ -214,32 +219,32 @@ impl<T: Assignable<U>, U: Reflectable> OptionalAssignable<T> for Option<JS<U>> {
}

pub trait OptionalRootable<T> {
fn root<'a, 'b>(self, roots: &'a RootCollection) -> Option<Root<'a, 'b, T>>;
fn root<'a, 'b>(self) -> Option<Root<'a, 'b, T>>;
}

impl<T: Reflectable> OptionalRootable<T> for Option<Temporary<T>> {
fn root<'a, 'b>(self, roots: &'a RootCollection) -> Option<Root<'a, 'b, T>> {
self.map(|inner| inner.root(roots))
fn root<'a, 'b>(self) -> Option<Root<'a, 'b, T>> {
self.map(|inner| inner.root())
}
}

pub trait OptionalRootedRootable<T> {
fn root<'a, 'b>(&self, roots: &'a RootCollection) -> Option<Root<'a, 'b, T>>;
fn root<'a, 'b>(&self) -> Option<Root<'a, 'b, T>>;
}

impl<T: Reflectable> OptionalRootedRootable<T> for Option<JS<T>> {
fn root<'a, 'b>(&self, roots: &'a RootCollection) -> Option<Root<'a, 'b, T>> {
self.as_ref().map(|inner| inner.root(roots))
fn root<'a, 'b>(&self) -> Option<Root<'a, 'b, T>> {
self.as_ref().map(|inner| inner.root())
}
}

pub trait ResultRootable<T,U> {
fn root<'a, 'b>(self, roots: &'a RootCollection) -> Result<Root<'a, 'b, T>, U>;
fn root<'a, 'b>(self) -> Result<Root<'a, 'b, T>, U>;
}

impl<T: Reflectable, U> ResultRootable<T, U> for Result<Temporary<T>, U> {
fn root<'a, 'b>(self, roots: &'a RootCollection) -> Result<Root<'a, 'b, T>, U> {
self.map(|inner| inner.root(roots))
fn root<'a, 'b>(self) -> Result<Root<'a, 'b, T>, U> {
self.map(|inner| inner.root())
}
}

@@ -5,7 +5,7 @@
use dom::bindings::codegen::PrototypeList;
use dom::bindings::codegen::PrototypeList::MAX_PROTO_CHAIN_LENGTH;
use dom::bindings::conversions::{FromJSValConvertible, IDLInterface};
use dom::bindings::js::{JS, JSRef, RootCollection, Temporary, Root};
use dom::bindings::js::{JS, JSRef, Temporary, Root};
use dom::bindings::trace::Untraceable;
use dom::browsercontext;
use dom::window;
@@ -606,15 +606,14 @@ pub extern fn wrap_for_same_compartment(cx: *JSContext, obj: *JSObject) -> *JSOb

pub extern fn outerize_global(_cx: *JSContext, obj: JSHandleObject) -> *JSObject {
unsafe {
let roots = RootCollection::new();
debug!("outerizing");
let obj = *obj.unnamed;
let win: Root<window::Window> =
unwrap_jsmanaged(obj,
IDLInterface::get_prototype_id(None::<window::Window>),
IDLInterface::get_prototype_depth(None::<window::Window>))
.unwrap()
.root(&roots);
.root();
win.deref().browser_context.get_ref().window_proxy()
}
}
@@ -631,8 +630,7 @@ pub fn global_object_for_js_object(obj: *JSObject) -> JS<window::Window> {
}

fn cx_for_dom_reflector(obj: *JSObject) -> *JSContext {
let roots = RootCollection::new();
let win = global_object_for_js_object(obj).root(&roots);
let win = global_object_for_js_object(obj).root();
let js_info = win.get().page().js_info();
match *js_info {
Some(ref info) => info.js_context.deref().deref().ptr,
@@ -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::js::{JS, JSRef, RootCollection, Temporary};
use dom::bindings::js::{JS, JSRef, Temporary};
use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
use dom::bindings::error::Fallible;
use dom::bindings::codegen::BindingDeclarations::BlobBinding;
@@ -51,8 +51,7 @@ impl<'a> BlobMethods for JSRef<'a, Blob> {
}

fn Slice(&self, _start: Option<i64>, _end: Option<i64>, _contentType: Option<DOMString>) -> Temporary<Blob> {
let roots = RootCollection::new();
let window = self.window.root(&roots);
let window = self.window.root();
Blob::new(&window.root_ref())
}

@@ -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::js::{JS, JSRef, Temporary, RootCollection};
use dom::bindings::js::{JS, JSRef, Temporary};
use dom::bindings::trace::Traceable;
use dom::bindings::utils::Reflectable;
use dom::document::Document;
@@ -37,8 +37,7 @@ impl BrowserContext {
}

pub fn active_window(&self) -> Temporary<Window> {
let roots = RootCollection::new();
let doc = self.active_document().root(&roots);
let doc = self.active_document().root();
Temporary::new(doc.deref().window.clone())
}

@@ -48,8 +47,7 @@ impl BrowserContext {
}

pub fn create_window_proxy(&self) -> *JSObject {
let roots = RootCollection::new();
let win = self.active_window().root(&roots);
let win = self.active_window().root();
let page = win.get().page();
let js_info = page.js_info();

@@ -4,7 +4,7 @@

use dom::bindings::codegen::InheritTypes::CommentDerived;
use dom::bindings::codegen::BindingDeclarations::CommentBinding;
use dom::bindings::js::{JS, JSRef, RootCollection, Temporary};
use dom::bindings::js::{JS, JSRef, Temporary};
use dom::bindings::error::Fallible;
use dom::characterdata::CharacterData;
use dom::document::Document;
@@ -41,8 +41,7 @@ impl Comment {
}

pub fn Constructor(owner: &JSRef<Window>, data: DOMString) -> Fallible<Temporary<Comment>> {
let roots = RootCollection::new();
let document = owner.Document().root(&roots);
let document = owner.Document().root();
Ok(Comment::new(data, &*document))
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.