Change get_attr() to get_attr_val_for_layout().

The old code was used by both layout and script, but was erroneously
borrowing for the layout case (which causes parallelism
problems). script now uses only `value_ref()` or `get_attribute()`, and
layout now has its own unsafe version that dances around the borrows of
`@mut Attr`.
This commit is contained in:
Jack Moffitt 2014-01-22 17:02:21 -07:00
parent 539cf58f73
commit c443bcbfff
9 changed files with 50 additions and 34 deletions

View file

@ -385,9 +385,9 @@ impl<'le> TElement for LayoutElement<'le> {
self.element.namespace.to_str().unwrap_or("") self.element.namespace.to_str().unwrap_or("")
} }
fn get_attr(&self, ns_url: Option<~str>, name: &str) -> Option<~str> { fn get_attr(&self, ns_url: Option<~str>, name: &str) -> Option<&'static str> {
let namespace = Namespace::from_str(ns_url); let namespace = Namespace::from_str(ns_url);
self.element.get_attr(namespace, name) unsafe { self.element.get_attr_val_for_layout(namespace, name) }
} }
fn get_link(&self) -> Option<~str> { fn get_link(&self) -> Option<~str> {
@ -397,7 +397,9 @@ impl<'le> TElement for LayoutElement<'le> {
// selector-link // selector-link
ElementNodeTypeId(HTMLAnchorElementTypeId) | ElementNodeTypeId(HTMLAnchorElementTypeId) |
ElementNodeTypeId(HTMLAreaElementTypeId) | ElementNodeTypeId(HTMLAreaElementTypeId) |
ElementNodeTypeId(HTMLLinkElementTypeId) => self.get_attr(None, "href"), ElementNodeTypeId(HTMLLinkElementTypeId) => {
self.get_attr(None, "href").map(|val| val.to_owned())
}
_ => None, _ => None,
} }
} }

View file

@ -65,6 +65,10 @@ impl Attr {
util::swap(&mut self.value, &mut value); util::swap(&mut self.value, &mut value);
value value
} }
pub fn value_ref<'a>(&'a self) -> &'a str {
self.value.as_slice()
}
} }
impl Attr { impl Attr {

View file

@ -366,7 +366,7 @@ impl Document {
pub fn GetElementsByName(&self, name: DOMString) -> @mut HTMLCollection { pub fn GetElementsByName(&self, name: DOMString) -> @mut HTMLCollection {
self.createHTMLCollection(|elem| self.createHTMLCollection(|elem|
elem.get_attr(Null, "name").is_some() && eq_slice(elem.get_attr(Null, "name").unwrap(), name)) elem.get_attribute(Null, "name").is_some() && eq_slice(elem.get_attribute(Null, "name").unwrap().value_ref(), name))
} }
pub fn createHTMLCollection(&self, callback: |elem: &Element| -> bool) -> @mut HTMLCollection { pub fn createHTMLCollection(&self, callback: |elem: &Element| -> bool) -> @mut HTMLCollection {
@ -456,9 +456,9 @@ fn foreach_ided_elements(root: &AbstractNode, callback: |&DOMString, &AbstractNo
} }
node.with_imm_element(|element| { node.with_imm_element(|element| {
match element.get_attr(Null, "id") { match element.get_attribute(Null, "id") {
Some(id) => { Some(id) => {
callback(&id.to_str(), &node); callback(&id.Value(), &node);
} }
None => () None => ()
} }

View file

@ -23,8 +23,10 @@ use layout_interface::{ContentBoxesResponse, ContentChangedDocumentDamage};
use layout_interface::{MatchSelectorsDocumentDamage}; use layout_interface::{MatchSelectorsDocumentDamage};
use style; use style;
use std::str::eq;
use std::ascii::StrAsciiExt; use std::ascii::StrAsciiExt;
use std::cast;
use std::str::eq;
use std::unstable::raw::Box;
pub struct Element { pub struct Element {
node: Node, node: Node,
@ -155,9 +157,19 @@ impl Element {
}).map(|&x| x) }).map(|&x| x)
} }
// FIXME(pcwalton): This is kind of confusingly named relative to the above... pub unsafe fn get_attr_val_for_layout(&self, namespace: Namespace, name: &str)
pub fn get_attr(&self, namespace: Namespace, name: &str) -> Option<~str> { -> Option<&'static str> {
self.get_attribute(namespace, name).map(|attr| attr.value.clone()) // FIXME: only case-insensitive in the HTML namespace (as opposed to SVG, etc.)
let name = name.to_ascii_lower();
self.attrs.iter().find(|attr: & &@mut Attr| {
// unsafely avoid a borrow because this is accessed by many tasks
// during parallel layout
let attr: ***Box<Attr> = cast::transmute(attr);
name == (***attr).data.local_name && (***attr).data.namespace == namespace
}).map(|attr| {
let attr: **Box<Attr> = cast::transmute(attr);
cast::transmute((**attr).data.value.as_slice())
})
} }
pub fn set_attr(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString) pub fn set_attr(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString)
@ -352,8 +364,8 @@ impl Element {
} }
pub fn get_string_attribute(&self, name: &str) -> DOMString { pub fn get_string_attribute(&self, name: &str) -> DOMString {
match self.get_attr(Null, name) { match self.get_attribute(Null, name) {
Some(x) => x, Some(x) => x.Value(),
None => ~"" None => ~""
} }
} }
@ -390,7 +402,7 @@ impl Element {
} }
pub fn GetAttribute(&self, name: DOMString) -> Option<DOMString> { pub fn GetAttribute(&self, name: DOMString) -> Option<DOMString> {
self.get_attr(Null, name).map(|s| s.to_owned()) self.get_attribute(Null, name).map(|s| s.Value())
} }
pub fn GetAttributeNS(&self, namespace: Option<DOMString>, local_name: DOMString) -> Option<DOMString> { pub fn GetAttributeNS(&self, namespace: Option<DOMString>, local_name: DOMString) -> Option<DOMString> {

View file

@ -11,7 +11,6 @@ use dom::window::Window;
use js::jsapi::JSTracer; use js::jsapi::JSTracer;
use std::str::eq_slice; use std::str::eq_slice;
use style::TElement;
pub struct HTMLDocument { pub struct HTMLDocument {
parent: Document parent: Document
@ -46,7 +45,7 @@ impl HTMLDocument {
pub fn Links(&self) -> @mut HTMLCollection { pub fn Links(&self) -> @mut HTMLCollection {
self.parent.createHTMLCollection(|elem| self.parent.createHTMLCollection(|elem|
(eq_slice(elem.tag_name, "a") || eq_slice(elem.tag_name, "area")) (eq_slice(elem.tag_name, "a") || eq_slice(elem.tag_name, "area"))
&& elem.get_attr(Null, "href").is_some()) && elem.get_attribute(Null, "href").is_some())
} }
pub fn Forms(&self) -> @mut HTMLCollection { pub fn Forms(&self) -> @mut HTMLCollection {
@ -59,7 +58,7 @@ impl HTMLDocument {
pub fn Anchors(&self) -> @mut HTMLCollection { pub fn Anchors(&self) -> @mut HTMLCollection {
self.parent.createHTMLCollection(|elem| self.parent.createHTMLCollection(|elem|
eq_slice(elem.tag_name, "a") && elem.get_attr(Null, "name").is_some()) eq_slice(elem.tag_name, "a") && elem.get_attribute(Null, "name").is_some())
} }
pub fn Applets(&self) -> @mut HTMLCollection { pub fn Applets(&self) -> @mut HTMLCollection {

View file

@ -15,7 +15,6 @@ use layout_interface::{ContentBoxQuery, ContentBoxResponse};
use servo_net::image_cache_task; use servo_net::image_cache_task;
use servo_net::image_cache_task::ImageCacheTask; use servo_net::image_cache_task::ImageCacheTask;
use servo_util::url::make_url; use servo_util::url::make_url;
use style::TElement;
pub struct HTMLImageElement { pub struct HTMLImageElement {
htmlelement: HTMLElement, htmlelement: HTMLElement,
@ -41,7 +40,7 @@ impl HTMLImageElement {
/// prefetching the image. This method must be called after `src` is changed. /// prefetching the image. This method must be called after `src` is changed.
pub fn update_image(&mut self, image_cache: ImageCacheTask, url: Option<Url>) { pub fn update_image(&mut self, image_cache: ImageCacheTask, url: Option<Url>) {
let elem = &mut self.htmlelement.element; let elem = &mut self.htmlelement.element;
let src_opt = elem.get_attr(Null, "src").map(|x| x.to_str()); let src_opt = elem.get_attribute(Null, "src").map(|x| x.Value());
match src_opt { match src_opt {
None => {} None => {}
Some(src) => { Some(src) => {

View file

@ -28,7 +28,7 @@ use std::comm::{Port, SharedChan};
use std::from_str::FromStr; use std::from_str::FromStr;
use std::str::eq_slice; use std::str::eq_slice;
use std::str; use std::str;
use style::{Stylesheet, TElement}; use style::Stylesheet;
macro_rules! handle_element( macro_rules! handle_element(
($document: expr, ($document: expr,
@ -339,11 +339,11 @@ pub fn parse_html(cx: *JSContext,
// Handle CSS style sheets from <link> elements // Handle CSS style sheets from <link> elements
ElementNodeTypeId(HTMLLinkElementTypeId) => { ElementNodeTypeId(HTMLLinkElementTypeId) => {
node.with_imm_element(|element| { node.with_imm_element(|element| {
match (element.get_attr(Null, "rel"), element.get_attr(Null, "href")) { match (element.get_attribute(Null, "rel"), element.get_attribute(Null, "href")) {
(Some(rel), Some(href)) => { (Some(rel), Some(href)) => {
if "stylesheet" == rel { if "stylesheet" == rel.value_ref() {
debug!("found CSS stylesheet: {:s}", href); debug!("found CSS stylesheet: {:s}", href.value_ref());
let url = make_url(href.to_str(), Some(url2.clone())); let url = make_url(href.Value(), Some(url2.clone()));
css_chan2.send(CSSTaskNewFile(UrlProvenance(url))); css_chan2.send(CSSTaskNewFile(UrlProvenance(url)));
} }
} }
@ -357,7 +357,7 @@ pub fn parse_html(cx: *JSContext,
node.with_mut_iframe_element(|iframe_element| { node.with_mut_iframe_element(|iframe_element| {
let sandboxed = iframe_element.is_sandboxed(); let sandboxed = iframe_element.is_sandboxed();
let elem = &mut iframe_element.htmlelement.element; let elem = &mut iframe_element.htmlelement.element;
let src_opt = elem.get_attr(Null, "src").map(|x| x.to_str()); let src_opt = elem.get_attribute(Null, "src").map(|x| x.Value());
for src in src_opt.iter() { for src in src_opt.iter() {
let iframe_url = make_url(src.clone(), Some(url2.clone())); let iframe_url = make_url(src.clone(), Some(url2.clone()));
iframe_element.frame = Some(iframe_url.clone()); iframe_element.frame = Some(iframe_url.clone());
@ -453,10 +453,10 @@ pub fn parse_html(cx: *JSContext,
unsafe { unsafe {
let scriptnode: AbstractNode = NodeWrapping::from_hubbub_node(script); let scriptnode: AbstractNode = NodeWrapping::from_hubbub_node(script);
scriptnode.with_imm_element(|script| { scriptnode.with_imm_element(|script| {
match script.get_attr(Null, "src") { match script.get_attribute(Null, "src") {
Some(src) => { Some(src) => {
debug!("found script: {:s}", src); debug!("found script: {:s}", src.Value());
let new_url = make_url(src.to_str(), Some(url3.clone())); let new_url = make_url(src.Value(), Some(url3.clone()));
js_chan2.send(JSTaskNewFile(new_url)); js_chan2.send(JSTaskNewFile(new_url));
} }
None => { None => {

View file

@ -786,8 +786,8 @@ impl ScriptTask {
let mut anchors = doc_node.traverse_preorder().filter(|node| node.is_anchor_element()); let mut anchors = doc_node.traverse_preorder().filter(|node| node.is_anchor_element());
anchors.find(|node| { anchors.find(|node| {
node.with_imm_element(|elem| { node.with_imm_element(|elem| {
match elem.get_attr(Null, "name") { match elem.get_attribute(Null, "name") {
Some(name) => eq_slice(name, fragid), Some(name) => eq_slice(name.value_ref(), fragid),
None => false None => false
} }
}) })
@ -885,15 +885,15 @@ impl ScriptTask {
fn load_url_from_element(&self, page: @mut Page, element: &Element) { fn load_url_from_element(&self, page: @mut Page, element: &Element) {
// if the node's element is "a," load url from href attr // if the node's element is "a," load url from href attr
let attr = element.get_attr(Null, "href"); let attr = element.get_attribute(Null, "href");
for href in attr.iter() { for href in attr.iter() {
debug!("ScriptTask: clicked on link to {:s}", *href); debug!("ScriptTask: clicked on link to {:s}", href.Value());
let click_frag = href.starts_with("#"); let click_frag = href.value_ref().starts_with("#");
let current_url = page.url.as_ref().map(|&(ref url, _)| { let current_url = page.url.as_ref().map(|&(ref url, _)| {
url.clone() url.clone()
}); });
debug!("ScriptTask: current url is {:?}", current_url); debug!("ScriptTask: current url is {:?}", current_url);
let url = make_url(href.to_owned(), current_url); let url = make_url(href.Value(), current_url);
if click_frag { if click_frag {
match self.find_fragment_node(page, url.fragment.unwrap()) { match self.find_fragment_node(page, url.fragment.unwrap()) {

View file

@ -18,7 +18,7 @@ pub trait TNode<E:TElement> : Clone {
} }
pub trait TElement { pub trait TElement {
fn get_attr(&self, namespace: Option<~str>, attr: &str) -> Option<~str>; fn get_attr(&self, namespace: Option<~str>, attr: &str) -> Option<&'static str>;
fn get_link(&self) -> Option<~str>; fn get_link(&self) -> Option<~str>;
fn get_local_name<'a>(&'a self) -> &'a str; fn get_local_name<'a>(&'a self) -> &'a str;
fn get_namespace_url<'a>(&'a self) -> &'a str; fn get_namespace_url<'a>(&'a self) -> &'a str;