Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-03 20:50:07 +01:00
Code Issues Projects Releases Packages Wiki Activity

Make get() and set() on MutNullableHeap use the correct types.

Browse source 
get() must always return a rooted value, because we have no way of
ensuring the value won't be invalidated. set() takes an &T because it's
convenient; there isn't any need to expose JS<T>.
This commit is contained in:
Eli Friedman 2015-10-14 17:48:28 -07:00
parent 7a08b29201
commit 57584e74c6
11 changed files with 68 additions and 67 deletions
Download patch file Download diff file Expand all files Collapse all files

2
components/script/dom/attr.rs
View file

@ -315,7 +315,7 @@ impl Attr {
} }
(old, new) => assert!(old == new) (old, new) => assert!(old == new)
} }
self.owner.set(owner.map(JS::from_ref)) self.owner.set(owner);
} }
pub fn owner(&self) -> Option<Root<Element>> { pub fn owner(&self) -> Option<Root<Element>> {

31
components/script/dom/bindings/js.rs
View file

@ -254,16 +254,6 @@ impl<T: HeapGCValue> MutNullableHeap<T> {
ptr: UnsafeCell::new(initial) ptr: UnsafeCell::new(initial)
} }
} }
/// Set this `MutNullableHeap` to the given value.
pub fn set(&self, val: Option<T>) {
unsafe { *self.ptr.get() = val; }
}
/// Retrieve a copy of the current optional inner value.
pub fn get(&self) -> Option<T> {
unsafe { ptr::read(self.ptr.get()) }
}
} }
impl<T: Reflectable> MutNullableHeap<JS<T>> { impl<T: Reflectable> MutNullableHeap<JS<T>> {
@ -273,10 +263,10 @@ impl<T: Reflectable> MutNullableHeap<JS<T>> {
where F: FnOnce() -> Root<T> where F: FnOnce() -> Root<T>
{ {
match self.get() { match self.get() {
Some(inner) => Root::from_rooted(inner), Some(inner) => inner,
None => { None => {
let inner = cb(); let inner = cb();
self.set(Some(JS::from_rooted(&inner))); self.set(Some(&inner));
inner inner
}, },
} }
@ -289,9 +279,22 @@ impl<T: Reflectable> MutNullableHeap<JS<T>> {
} }
/// Get a rooted value out of this object /// Get a rooted value out of this object
// FIXME(#6684) pub fn get(&self) -> Option<Root<T>> {
unsafe {
ptr::read(self.ptr.get()).map(|o| o.root())
}
}
/// Get a rooted value out of this object
pub fn get_rooted(&self) -> Option<Root<T>> { pub fn get_rooted(&self) -> Option<Root<T>> {
self.get().map(|o| o.root()) self.get()
}
/// Set this `MutNullableHeap` to the given value.
pub fn set(&self, val: Option<&T>) {
unsafe {
*self.ptr.get() = val.map(|p| JS::from_ref(p));
}
} }
} }

13
components/script/dom/document.rs
View file

@ -297,7 +297,7 @@ impl Document {
.filter_map(HTMLBaseElementCast::to_root) .filter_map(HTMLBaseElementCast::to_root)
.filter(|element| ElementCast::from_ref(&**element).has_attribute(&atom!("href"))) .filter(|element| ElementCast::from_ref(&**element).has_attribute(&atom!("href")))
.next(); .next();
self.base_element.set(base.map(|element| JS::from_ref(&*element))); self.base_element.set(base.as_ref().map(Root::r));
} }
pub fn quirks_mode(&self) -> QuirksMode { pub fn quirks_mode(&self) -> QuirksMode {
@ -506,7 +506,7 @@ impl Document {
/// Request that the given element receive focus once the current transaction is complete. /// Request that the given element receive focus once the current transaction is complete.
pub fn request_focus(&self, elem: &Element) { pub fn request_focus(&self, elem: &Element) {
if elem.is_focusable_area() { if elem.is_focusable_area() {
self.possibly_focused.set(Some(JS::from_ref(elem))) self.possibly_focused.set(Some(elem))
} }
} }
@ -520,7 +520,7 @@ impl Document {
node.set_focus_state(false); node.set_focus_state(false);
} }
self.focused.set(self.possibly_focused.get()); self.focused.set(self.possibly_focused.get().r());
if let Some(ref elem) = self.focused.get_rooted() { if let Some(ref elem) = self.focused.get_rooted() {
let node = NodeCast::from_ref(elem.r()); let node = NodeCast::from_ref(elem.r());
@ -852,7 +852,7 @@ impl Document {
} }
pub fn set_current_script(&self, script: Option<&HTMLScriptElement>) { pub fn set_current_script(&self, script: Option<&HTMLScriptElement>) {
self.current_script.set(script.map(JS::from_ref)); self.current_script.set(script);
} }
pub fn trigger_mozbrowser_event(&self, event: MozBrowserEvent) { pub fn trigger_mozbrowser_event(&self, event: MozBrowserEvent) {
@ -959,7 +959,7 @@ impl Document {
} }
pub fn set_current_parser(&self, script: Option<&ServoHTMLParser>) { pub fn set_current_parser(&self, script: Option<&ServoHTMLParser>) {
self.current_parser.set(script.map(JS::from_ref)); self.current_parser.set(script);
} }
pub fn get_current_parser(&self) -> Option<Root<ServoHTMLParser>> { pub fn get_current_parser(&self) -> Option<Root<ServoHTMLParser>> {
@ -1119,8 +1119,7 @@ impl Document {
let new_doc = Document::new( let new_doc = Document::new(
&*self.window(), None, doctype, None, None, &*self.window(), None, doctype, None, None,
DocumentSource::NotFromParser, DocumentLoader::new(&self.loader())); DocumentSource::NotFromParser, DocumentLoader::new(&self.loader()));
new_doc.appropriate_template_contents_owner_document.set( new_doc.appropriate_template_contents_owner_document.set(Some(&new_doc));
Some(JS::from_ref(&*new_doc)));
new_doc new_doc
}) })
} }

4
components/script/dom/event.rs
View file

@ -106,12 +106,12 @@ impl Event {
#[inline] #[inline]
pub fn set_current_target(&self, val: &EventTarget) { pub fn set_current_target(&self, val: &EventTarget) {
self.current_target.set(Some(JS::from_ref(val))); self.current_target.set(Some(val));
} }
#[inline] #[inline]
pub fn set_target(&self, val: &EventTarget) { pub fn set_target(&self, val: &EventTarget) {
self.target.set(Some(JS::from_ref(val))); self.target.set(Some(val));
} }
#[inline] #[inline]

6
components/script/dom/filereader.rs
View file

@ -116,7 +116,7 @@ impl FileReader {
let global = fr.global.root(); let global = fr.global.root();
let exception = DOMException::new(global.r(), error); let exception = DOMException::new(global.r(), error);
fr.error.set(Some(JS::from_rooted(&exception))); fr.error.set(Some(&exception));
fr.dispatch_progress_event("error".to_owned(), 0, None); fr.dispatch_progress_event("error".to_owned(), 0, None);
return_on_abort!(); return_on_abort!();
@ -292,7 +292,7 @@ impl FileReaderMethods for FileReader {
let global = self.global.root(); let global = self.global.root();
let exception = DOMException::new(global.r(), DOMErrorName::AbortError); let exception = DOMException::new(global.r(), DOMErrorName::AbortError);
self.error.set(Some(JS::from_rooted(&exception))); self.error.set(Some(&exception));
self.terminate_ongoing_reading(); self.terminate_ongoing_reading();
// Steps 5 & 6 // Steps 5 & 6
@ -346,7 +346,7 @@ impl FileReader {
if blob.IsClosed() { if blob.IsClosed() {
let global = self.global.root(); let global = self.global.root();
let exception = DOMException::new(global.r(), DOMErrorName::InvalidStateError); let exception = DOMException::new(global.r(), DOMErrorName::InvalidStateError);
self.error.set(Some(JS::from_rooted(&exception))); self.error.set(Some(&exception));
self.dispatch_progress_event("error".to_owned(), 0, None); self.dispatch_progress_event("error".to_owned(), 0, None);
return Ok(()); return Ok(());

2
components/script/dom/mouseevent.rs
View file

@ -205,6 +205,6 @@ impl MouseEventMethods for MouseEvent {
self.shift_key.set(shiftKeyArg); self.shift_key.set(shiftKeyArg);
self.meta_key.set(metaKeyArg); self.meta_key.set(metaKeyArg);
self.button.set(buttonArg); self.button.set(buttonArg);
self.related_target.set(relatedTargetArg.map(JS::from_ref)); self.related_target.set(relatedTargetArg);
} }
} }

44
components/script/dom/node.rs
View file

@ -271,32 +271,32 @@ impl Node {
match prev_sibling { match prev_sibling {
None => { None => {
assert!(Some(*before) == self.first_child.get_rooted().r()); assert!(Some(*before) == self.first_child.get_rooted().r());
self.first_child.set(Some(JS::from_ref(new_child))); self.first_child.set(Some(new_child));
}, },
Some(ref prev_sibling) => { Some(ref prev_sibling) => {
prev_sibling.next_sibling.set(Some(JS::from_ref(new_child))); prev_sibling.next_sibling.set(Some(new_child));
new_child.prev_sibling.set(Some(JS::from_ref(prev_sibling.r()))); new_child.prev_sibling.set(Some(prev_sibling.r()));
}, },
} }
before.prev_sibling.set(Some(JS::from_ref(new_child))); before.prev_sibling.set(Some(new_child));
new_child.next_sibling.set(Some(JS::from_ref(before))); new_child.next_sibling.set(Some(before));
}, },
None => { None => {
let last_child = self.GetLastChild(); let last_child = self.GetLastChild();
match last_child { match last_child {
None => self.first_child.set(Some(JS::from_ref(new_child))), None => self.first_child.set(Some(new_child)),
Some(ref last_child) => { Some(ref last_child) => {
assert!(last_child.next_sibling.get().is_none()); assert!(last_child.next_sibling.get().is_none());
last_child.r().next_sibling.set(Some(JS::from_ref(new_child))); last_child.r().next_sibling.set(Some(new_child));
new_child.prev_sibling.set(Some(JS::from_rooted(&last_child))); new_child.prev_sibling.set(Some(&last_child));
} }
} }
self.last_child.set(Some(JS::from_ref(new_child))); self.last_child.set(Some(new_child));
}, },
} }
new_child.parent_node.set(Some(JS::from_ref(self))); new_child.parent_node.set(Some(self));
let parent_in_doc = self.is_in_doc(); let parent_in_doc = self.is_in_doc();
for node in new_child.traverse_preorder() { for node in new_child.traverse_preorder() {
@ -315,19 +315,19 @@ impl Node {
let prev_sibling = child.GetPreviousSibling(); let prev_sibling = child.GetPreviousSibling();
match prev_sibling { match prev_sibling {
None => { None => {
self.first_child.set(child.next_sibling.get()); self.first_child.set(child.next_sibling.get().r());
} }
Some(ref prev_sibling) => { Some(ref prev_sibling) => {
prev_sibling.next_sibling.set(child.next_sibling.get()); prev_sibling.next_sibling.set(child.next_sibling.get().r());
} }
} }
let next_sibling = child.GetNextSibling(); let next_sibling = child.GetNextSibling();
match next_sibling { match next_sibling {
None => { None => {
self.last_child.set(child.prev_sibling.get()); self.last_child.set(child.prev_sibling.get().r());
} }
Some(ref next_sibling) => { Some(ref next_sibling) => {
next_sibling.prev_sibling.set(child.prev_sibling.get()); next_sibling.prev_sibling.set(child.prev_sibling.get().r());
} }
} }
@ -591,7 +591,7 @@ impl Node {
match self.parent_node.get() { match self.parent_node.get() {
None => return, None => return,
Some(parent) => parent, Some(parent) => parent,
}.root(); };
for sibling in parent.r().children() { for sibling in parent.r().children() {
sibling.r().set_has_dirty_siblings(true); sibling.r().set_has_dirty_siblings(true);
@ -660,7 +660,7 @@ impl Node {
pub fn is_parent_of(&self, child: &Node) -> bool { pub fn is_parent_of(&self, child: &Node) -> bool {
match child.parent_node.get() { match child.parent_node.get() {
Some(ref parent) => parent.root().r() == self, Some(ref parent) => parent.r() == self,
None => false, None => false,
} }
} }
@ -689,7 +689,7 @@ impl Node {
// Step 2. // Step 2.
let parent = match parent.get() { let parent = match parent.get() {
None => return Ok(()), None => return Ok(()),
Some(ref parent) => parent.root(), Some(parent) => parent,
}; };
// Step 3. // Step 3.
@ -702,7 +702,7 @@ impl Node {
let viable_previous_sibling = match viable_previous_sibling { let viable_previous_sibling = match viable_previous_sibling {
Some(ref viable_previous_sibling) => viable_previous_sibling.next_sibling.get(), Some(ref viable_previous_sibling) => viable_previous_sibling.next_sibling.get(),
None => parent.first_child.get(), None => parent.first_child.get(),
}.map(|s| s.root()); };
// Step 6. // Step 6.
try!(Node::pre_insert(&node, &parent, viable_previous_sibling.r())); try!(Node::pre_insert(&node, &parent, viable_previous_sibling.r()));
@ -718,7 +718,7 @@ impl Node {
// Step 2. // Step 2.
let parent = match parent.get() { let parent = match parent.get() {
None => return Ok(()), None => return Ok(()),
Some(ref parent) => parent.root(), Some(parent) => parent,
}; };
// Step 3. // Step 3.
@ -745,7 +745,7 @@ impl Node {
let doc = self.owner_doc(); let doc = self.owner_doc();
let node = try!(doc.r().node_from_nodes_and_strings(nodes)); let node = try!(doc.r().node_from_nodes_and_strings(nodes));
// Step 3. // Step 3.
parent_node.root().r().ReplaceChild(node.r(), self).map(|_| ()) parent_node.r().ReplaceChild(node.r(), self).map(|_| ())
}, },
} }
} }
@ -823,11 +823,11 @@ impl Node {
} }
pub fn owner_doc(&self) -> Root<Document> { pub fn owner_doc(&self) -> Root<Document> {
self.owner_doc.get().unwrap().root() self.owner_doc.get().unwrap()
} }
pub fn set_owner_doc(&self, document: &Document) { pub fn set_owner_doc(&self, document: &Document) {
self.owner_doc.set(Some(JS::from_ref(document))); self.owner_doc.set(Some(document));
} }
pub fn is_in_html_doc(&self) -> bool { pub fn is_in_html_doc(&self) -> bool {

27
components/script/dom/nodelist.rs
View file

@ -127,14 +127,14 @@ impl ChildrenList {
let last_index = self.last_index.get(); let last_index = self.last_index.get();
if index == last_index { if index == last_index {
// Item is last visited child, no need to update last visited. // Item is last visited child, no need to update last visited.
return Some(self.last_visited.get().unwrap().root()); return Some(self.last_visited.get().unwrap());
} }
let last_visited = if index - 1u32 == last_index { let last_visited = if index - 1u32 == last_index {
// Item is last visited's next sibling. // Item is last visited's next sibling.
self.last_visited.get().unwrap().root().GetNextSibling().unwrap() self.last_visited.get().unwrap().GetNextSibling().unwrap()
} else if last_index > 0 && index == last_index - 1u32 { } else if last_index > 0 && index == last_index - 1u32 {
// Item is last visited's previous sibling. // Item is last visited's previous sibling.
self.last_visited.get().unwrap().root().GetPreviousSibling().unwrap() self.last_visited.get().unwrap().GetPreviousSibling().unwrap()
} else if index > last_index { } else if index > last_index {
if index == len - 1u32 { if index == len - 1u32 {
// Item is parent's last child, not worth updating last visited. // Item is parent's last child, not worth updating last visited.
@ -142,7 +142,7 @@ impl ChildrenList {
} }
if index <= last_index + (len - last_index) / 2u32 { if index <= last_index + (len - last_index) / 2u32 {
// Item is closer to the last visited child and follows it. // Item is closer to the last visited child and follows it.
self.last_visited.get().unwrap().root() self.last_visited.get().unwrap()
.inclusively_following_siblings() .inclusively_following_siblings()
.nth((index - last_index) as usize).unwrap() .nth((index - last_index) as usize).unwrap()
} else { } else {
@ -154,7 +154,7 @@ impl ChildrenList {
} }
} else if index >= last_index / 2u32 { } else if index >= last_index / 2u32 {
// Item is closer to the last visited child and precedes it. // Item is closer to the last visited child and precedes it.
self.last_visited.get().unwrap().root() self.last_visited.get().unwrap()
.inclusively_preceding_siblings() .inclusively_preceding_siblings()
.nth((last_index - index) as usize).unwrap() .nth((last_index - index) as usize).unwrap()
} else { } else {
@ -165,7 +165,7 @@ impl ChildrenList {
.nth(index as usize) .nth(index as usize)
.unwrap() .unwrap()
}; };
self.last_visited.set(Some(JS::from_rooted(&last_visited))); self.last_visited.set(Some(last_visited.r()));
self.last_index.set(index); self.last_index.set(index);
Some(last_visited) Some(last_visited)
} }
@ -178,7 +178,7 @@ impl ChildrenList {
} }
let index = list.last_index.get(); let index = list.last_index.get();
if index < len { if index < len {
list.last_visited.set(Some(JS::from_ref(added[index as usize]))); list.last_visited.set(Some(added[index as usize]));
} else if index / 2u32 >= len { } else if index / 2u32 >= len {
// If last index is twice as large as the number of added nodes, // If last index is twice as large as the number of added nodes,
// updating only it means that less nodes will be traversed if // updating only it means that less nodes will be traversed if
@ -187,7 +187,7 @@ impl ChildrenList {
} else { } else {
// If last index is not twice as large but still larger, // If last index is not twice as large but still larger,
// it's better to update it to the number of added nodes. // it's better to update it to the number of added nodes.
list.last_visited.set(Some(JS::from_ref(next))); list.last_visited.set(Some(next));
list.last_index.set(len); list.last_index.set(len);
} }
} }
@ -198,7 +198,7 @@ impl ChildrenList {
added: &[&Node], added: &[&Node],
next: Option<&Node>) { next: Option<&Node>) {
let index = list.last_index.get(); let index = list.last_index.get();
if removed == &*list.last_visited.get().unwrap().root() { if removed == &*list.last_visited.get().unwrap() {
let visited = match (prev, added, next) { let visited = match (prev, added, next) {
(None, _, None) => { (None, _, None) => {
// Such cases where parent had only one child should // Such cases where parent had only one child should
@ -213,7 +213,7 @@ impl ChildrenList {
prev prev
}, },
}; };
list.last_visited.set(Some(JS::from_ref(visited))); list.last_visited.set(Some(visited));
} else if added.len() != 1 { } else if added.len() != 1 {
// The replaced child isn't the last visited one, and there are // The replaced child isn't the last visited one, and there are
// 0 or more than 1 nodes to replace it. Special care must be // 0 or more than 1 nodes to replace it. Special care must be
@ -250,13 +250,13 @@ impl ChildrenList {
self.last_visited.set(None); self.last_visited.set(None);
self.last_index.set(0u32); self.last_index.set(0u32);
} else if index < len as u32 { } else if index < len as u32 {
self.last_visited.set(Some(JS::from_ref(added[index as usize]))); self.last_visited.set(Some(added[index as usize]));
} else { } else {
// Setting last visited to parent's last child serves no purpose, // Setting last visited to parent's last child serves no purpose,
// so the middle is arbitrarily chosen here in case the caller // so the middle is arbitrarily chosen here in case the caller
// wants random access. // wants random access.
let middle = len / 2; let middle = len / 2;
self.last_visited.set(Some(JS::from_ref(added[middle]))); self.last_visited.set(Some(added[middle]));
self.last_index.set(middle as u32); self.last_index.set(middle as u32);
} }
}, },
@ -264,8 +264,7 @@ impl ChildrenList {
} }
fn reset(&self) { fn reset(&self) {
self.last_visited.set( self.last_visited.set(self.node.root().GetFirstChild().as_ref().map(Root::r));
self.node.root().GetFirstChild().map(|node| JS::from_rooted(&node)));
self.last_index.set(0u32); self.last_index.set(0u32);
} }
} }

2
components/script/dom/uievent.rs
View file

@ -92,7 +92,7 @@ impl UIEventMethods for UIEvent {
} }
event.InitEvent(type_, can_bubble, cancelable); event.InitEvent(type_, can_bubble, cancelable);
self.view.set(view.map(JS::from_ref)); self.view.set(view);
self.detail.set(detail); self.detail.set(detail);
} }
} }

2
components/script/dom/webglprogram.rs
View file

@ -86,7 +86,7 @@ impl WebGLProgram {
return Err(WebGLError::InvalidOperation); return Err(WebGLError::InvalidOperation);
} }
shader_slot.set(Some(JS::from_ref(shader))); shader_slot.set(Some(shader));
self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::AttachShader(self.id, shader.id()))).unwrap(); self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::AttachShader(self.id, shader.id()))).unwrap();

2
components/script/dom/webglrenderingcontext.rs
View file

@ -360,7 +360,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
if let Some(texture) = texture { if let Some(texture) = texture {
match texture.bind(target) { match texture.bind(target) {
Ok(_) => slot.set(Some(JS::from_ref(texture))), Ok(_) => slot.set(Some(texture)),
Err(err) => return self.webgl_error(err), Err(err) => return self.webgl_error(err),
} }
} else { } else {