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.

Sign up for GitHub

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

Jump to bottom

Replaces rust-wapcaplet with a new interning module in Rust, and supports interning on CSS selector matching #1135

Closed
wants to merge 1 commit into from
Closed

Replaces rust-wapcaplet with a new interning module in Rust, and supports interning on CSS selector matching #1135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/components/main/layout/box.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use servo_net::local_image_cache::LocalImageCache;
use servo_util::range::*;
use servo_util::tree::{TreeNodeRef, ElementLike};
use extra::url::Url;
use servo_util::interning::intern_string;

/// Render boxes (`struct RenderBox`) are the leaves of the layout tree. They cannot position
/// themselves. In general, render boxes do not have a simple correspondence with CSS boxes as in
Expand Down Expand Up @@ -476,9 +477,9 @@ impl RenderBox {
pub fn image_width(&self, image_box: @mut ImageRenderBox) -> Au {
let attr_width: Option<int> = do self.with_base |base| {
do base.node.with_imm_element |elt| {
match elt.get_attr("width") {
match elt.get_attr(&intern_string("width")) {
Some(width) => {
FromStr::from_str(width)
FromStr::from_str(width.to_str_slice())
}
None => {
None
Expand All @@ -502,9 +503,9 @@ impl RenderBox {
pub fn image_height(&self, image_box: @mut ImageRenderBox) -> Au {
let attr_height: Option<int> = do self.with_base |base| {
do base.node.with_imm_element |elt| {
match elt.get_attr("height") {
match elt.get_attr(&intern_string("height")) {
Some(height) => {
FromStr::from_str(height)
FromStr::from_str(height.to_str_slice())
}
None => {
None
Expand Down
15 changes: 10 additions & 5 deletions src/components/script/dom/document.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ use std::hashmap::HashMap;

use std::cast;
use std::ptr;
use std::str::eq_slice;
use std::libc;
use std::ascii::StrAsciiExt;
use std::unstable::raw::Box;
use servo_util::interning::intern_string;

pub trait ReflectableDocument {
fn init_reflector(@mut self, cx: *JSContext);
Expand Down Expand Up @@ -210,7 +210,7 @@ impl Document {
}

pub fn GetElementsByTagName(&self, tag: &DOMString) -> @mut HTMLCollection {
self.createHTMLCollection(|elem| eq_slice(elem.tag_name, null_str_as_empty(tag)))
self.createHTMLCollection(|elem| elem.tag_name.eq(&intern_string(null_str_as_empty(tag))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this probably should not intern tag. elem.tag_name can be converted to string slice instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that createHTMLCollection loops over the entire tree, I think interning probably makes a lot of sense, but not every time the callback is called.

}

pub fn GetElementsByTagNameNS(&self, _ns: &DOMString, _tag: &DOMString) -> @mut HTMLCollection {
Expand Down Expand Up @@ -322,7 +322,7 @@ impl Document {
let new_title = @HTMLTitleElement {
htmlelement: HTMLElement::new(HTMLTitleElementTypeId, ~"title", abstract_self)
};
let new_title = unsafe {
let new_title = unsafe {
Node::as_abstract_node(self.get_cx(), new_title)
};
new_title.add_child(self.CreateTextNode(abstract_self, title));
Expand All @@ -338,8 +338,13 @@ impl Document {
}

pub fn GetElementsByName(&self, name: &DOMString) -> @mut HTMLCollection {
let name_interned = intern_string("name");
self.createHTMLCollection(|elem|
elem.get_attr("name").is_some() && eq_slice(elem.get_attr("name").unwrap(), null_str_as_empty(name)))
match elem.get_attr(&name_interned) {
None => false,
Some(ref name_attr) => (*name_attr).eq(&intern_string(null_str_as_empty(name))),
}
)
}

pub fn createHTMLCollection(&self, callback: &fn(elem: &Element) -> bool) -> @mut HTMLCollection {
Expand Down Expand Up @@ -395,7 +400,7 @@ fn foreach_ided_elements(root: &AbstractNode<ScriptView>,
}

do node.with_imm_element |element| {
match element.get_attr("id") {
match element.get_attr(&intern_string("id")) {
Some(id) => {
callback(&id.to_str(), &node);
}
Expand Down
74 changes: 49 additions & 25 deletions src/components/script/dom/element.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ use js::jsapi::{JSContext, JSObject};
use std::comm;
use std::hashmap::HashMap;
use std::ascii::StrAsciiExt;
use servo_util::interning::{intern_string, IntString};
use std::str::eq_slice;

pub struct Element {
node: Node<ScriptView>,
tag_name: ~str, // TODO: This should be an atom, not a ~str.
attrs: HashMap<~str, ~str>,
attrs_list: ~[~str], // store an order of attributes.
tag_name: IntString, // TODO: This should be an atom, not a ~str.
id: Option<IntString>,
classes: ~[IntString],
attrs: HashMap<IntString, IntString>,
attrs_list: ~[IntString], // store an order of attributes.
style_attribute: Option<style::PropertyDeclarationBlock>,
}

Expand Down Expand Up @@ -126,27 +130,38 @@ pub enum ElementTypeId {
//

impl ElementLike for Element {
fn get_local_name<'a>(&'a self) -> &'a str {
self.tag_name.as_slice()
fn get_local_name<'a>(&'a self) -> &'a IntString {
&self.tag_name
}

fn get_attr<'a>(&'a self, name: &str) -> Option<&'a str> {
fn get_attr<'a>(&'a self, name: &IntString) -> Option<&'a IntString> {
// FIXME: only case-insensitive in the HTML namespace (as opposed to SVG, etc.)
let name = name.to_ascii_lower();
let value: Option<&str> = self.attrs.find_equiv(&name).map(|value| {
let value: &str = *value;
let value: Option<&IntString> = self.attrs.find_equiv(name).map(|value| {
value
});

return value;
}

fn get_id<'a>(&'a self) -> Option<&'a IntString> {
match self.id {
None => None,
Some(ref id) => Some(id),
}
}

fn get_classes<'a>(&'a self) -> &'a [IntString] {
let c: &'a [IntString] = self.classes;
c
}
}

impl<'self> Element {
pub fn new(type_id: ElementTypeId, tag_name: ~str, document: AbstractDocument) -> Element {
Element {
node: Node::new(ElementNodeTypeId(type_id), document),
tag_name: tag_name,
tag_name: intern_string(tag_name),
id: None,
classes: ~[],
attrs: HashMap::new(),
attrs_list: ~[],
style_attribute: None,
Expand All @@ -157,24 +172,33 @@ impl<'self> Element {
abstract_self: AbstractNode<ScriptView>,
raw_name: &DOMString,
raw_value: &DOMString) {
let name = null_str_as_empty(raw_name).to_ascii_lower();
let value = null_str_as_empty(raw_value);
static WHITESPACE: &'static [char] = &'static [' ', '\t', '\n', '\r', '\x0C'];

let name = intern_string(null_str_as_empty(raw_name));
let value = intern_string(null_str_as_empty(raw_value));

// FIXME: reduce the time of `value.clone()`.
// FIXME: reduce the time of `value.clone()`
self.attrs.mangle(name.clone(), value.clone(),
|new_name: &~str, new_value: ~str| {
|new_name: &IntString, new_value: IntString| {
// register to the ordered list.
self.attrs_list.push(new_name.clone());
new_value
},
|_, old_value: &mut ~str, new_value: ~str| {
|_, old_value: &mut IntString, new_value: IntString| {
// update value.
*old_value = new_value;
});

if "style" == name {
self.style_attribute = Some(style::parse_style_attribute(
null_str_as_empty_ref(raw_value)));
if eq_slice(name.to_ascii_lower(), "id") {
self.id = Some(value);
} else if eq_slice(name.to_ascii_lower(), "class") {
for class in value.to_str_slice().split_iter(WHITESPACE) {
self.classes.push(intern_string(class));
}
}

if eq_slice("style", name.to_ascii_lower()) {
self.style_attribute = Some(style::parse_style_attribute(value.to_str_slice()));
}

// TODO: update owner document's id hashmap for `document.getElementById()`
Expand Down Expand Up @@ -205,7 +229,7 @@ impl<'self> Element {

impl Element {
pub fn TagName(&self) -> DOMString {
Some(self.tag_name.to_owned().to_ascii_upper())
Some(self.tag_name.to_str_slice().to_ascii_upper())
}

pub fn Id(&self) -> DOMString {
Expand All @@ -216,7 +240,7 @@ impl Element {
}

pub fn GetAttribute(&self, name: &DOMString) -> DOMString {
self.get_attr(null_str_as_empty_ref(name)).map(|s| s.to_owned())
self.get_attr(&intern_string(null_str_as_empty_ref(name))).map(|s| s.to_str())
}

pub fn GetAttributeNS(&self, _namespace: &DOMString, _localname: &DOMString) -> DOMString {
Expand Down Expand Up @@ -385,15 +409,15 @@ impl Element {
}

pub struct Attr {
name: ~str,
value: ~str,
name: IntString,
value: IntString,
}

impl Attr {
pub fn new(name: ~str, value: ~str) -> Attr {
Attr {
name: name,
value: value
name: intern_string(name),
value: intern_string(value)
}
}
}
17 changes: 9 additions & 8 deletions src/components/script/dom/htmldocument.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use servo_util::tree::{TreeNodeRef, ElementLike};

use std::ptr;
use std::str::eq_slice;
use servo_util::interning::intern_string;

pub struct HTMLDocument {
parent: Document
Expand Down Expand Up @@ -48,11 +49,11 @@ impl HTMLDocument {
}

pub fn Images(&self) -> @mut HTMLCollection {
self.parent.createHTMLCollection(|elem| eq_slice(elem.tag_name, "img"))
self.parent.createHTMLCollection(|elem| eq_slice(elem.tag_name.to_str_slice(), "img"))
}

pub fn Embeds(&self) -> @mut HTMLCollection {
self.parent.createHTMLCollection(|elem| eq_slice(elem.tag_name, "embed"))
self.parent.createHTMLCollection(|elem| eq_slice(elem.tag_name.to_str_slice(), "embed"))
}

pub fn Plugins(&self) -> @mut HTMLCollection {
Expand All @@ -61,26 +62,26 @@ impl HTMLDocument {

pub fn Links(&self) -> @mut HTMLCollection {
self.parent.createHTMLCollection(|elem|
(eq_slice(elem.tag_name, "a") || eq_slice(elem.tag_name, "area"))
&& elem.get_attr("href").is_some())
(eq_slice(elem.tag_name.to_str_slice(), "a") || eq_slice(elem.tag_name.to_str_slice(), "area"))
&& elem.get_attr(&intern_string("href")).is_some())
}

pub fn Forms(&self) -> @mut HTMLCollection {
self.parent.createHTMLCollection(|elem| eq_slice(elem.tag_name, "form"))
self.parent.createHTMLCollection(|elem| eq_slice(elem.tag_name.to_str_slice(), "form"))
}

pub fn Scripts(&self) -> @mut HTMLCollection {
self.parent.createHTMLCollection(|elem| eq_slice(elem.tag_name, "script"))
self.parent.createHTMLCollection(|elem| eq_slice(elem.tag_name.to_str_slice(), "script"))
}

pub fn Anchors(&self) -> @mut HTMLCollection {
self.parent.createHTMLCollection(|elem|
eq_slice(elem.tag_name, "a") && elem.get_attr("name").is_some())
eq_slice(elem.tag_name.to_str_slice(), "a") && elem.get_attr(&intern_string("name")).is_some())
}

pub fn Applets(&self) -> @mut HTMLCollection {
// FIXME: This should be return OBJECT elements containing applets.
self.parent.createHTMLCollection(|elem| eq_slice(elem.tag_name, "applet"))
self.parent.createHTMLCollection(|elem| eq_slice(elem.tag_name.to_str_slice(), "applet"))
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/components/script/dom/htmlimageelement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use servo_net::image_cache_task;
use servo_net::image_cache_task::ImageCacheTask;
use servo_util::url::make_url;
use servo_util::tree::ElementLike;
use servo_util::interning::intern_string;

pub struct HTMLImageElement {
htmlelement: HTMLElement,
Expand All @@ -23,7 +24,7 @@ impl HTMLImageElement {
/// prefetching the image. This method must be called after `src` is changed.
pub fn update_image(&mut self, image_cache: ImageCacheTask, url: Option<Url>) {
let elem = &mut self.htmlelement.element;
let src_opt = elem.get_attr("src").map(|x| x.to_str());
let src_opt = elem.get_attr(&intern_string("src")).map(|x| x.to_str());
match src_opt {
None => {}
Some(src) => {
Expand Down
3 changes: 2 additions & 1 deletion src/components/script/dom/htmlscriptelement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
use dom::bindings::utils::{DOMString, ErrorResult};
use dom::htmlelement::HTMLElement;
use servo_util::tree::ElementLike;
use servo_util::interning::intern_string;

pub struct HTMLScriptElement {
htmlelement: HTMLElement,
}

impl HTMLScriptElement {
pub fn Src(&self) -> DOMString {
self.htmlelement.element.get_attr("src").map(|s| s.to_str())
self.htmlelement.element.get_attr(&intern_string("src")).map(|s| s.to_str())
}

pub fn SetSrc(&mut self, _src: &DOMString) -> ErrorResult {
Expand Down
19 changes: 10 additions & 9 deletions src/components/script/html/hubbub_html_parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use servo_util::url::make_url;
use extra::url::Url;
use extra::future::Future;
use geom::size::Size2D;
use servo_util::interning::{intern_string};

macro_rules! handle_element(
($cx: expr,
Expand Down Expand Up @@ -110,7 +111,7 @@ type JSResult = ~[JSFile];

enum CSSMessage {
CSSTaskNewFile(StylesheetProvenance),
CSSTaskExit
CSSTaskExit
}

enum JSMessage {
Expand Down Expand Up @@ -365,7 +366,7 @@ pub fn parse_html(cx: *JSContext,

let (css_chan2, css_chan3, js_chan2) = (css_chan.clone(), css_chan.clone(), js_chan.clone());
let next_subpage_id = Cell::new(next_subpage_id);

parser.set_tree_handler(~hubbub::TreeHandler {
create_comment: |data: ~str| {
debug!("create comment");
Expand Down Expand Up @@ -405,10 +406,10 @@ pub fn parse_html(cx: *JSContext,
// Handle CSS style sheets from <link> elements
ElementNodeTypeId(HTMLLinkElementTypeId) => {
do node.with_imm_element |element| {
match (element.get_attr("rel"), element.get_attr("href")) {
match (element.get_attr(&intern_string("rel")), element.get_attr(&intern_string("href"))) {
(Some(rel), Some(href)) => {
if rel == "stylesheet" {
debug!("found CSS stylesheet: %s", href);
if eq_slice(rel.to_ascii_lower(), "stylesheet") {
debug!("found CSS stylesheet: %s", href.to_str_slice());
let url = make_url(href.to_str(), Some(url2.clone()));
css_chan2.send(CSSTaskNewFile(UrlProvenance(url)));
}
Expand All @@ -424,11 +425,11 @@ pub fn parse_html(cx: *JSContext,
let iframe_chan = iframe_chan.take();
let sandboxed = iframe_element.is_sandboxed();
let elem = &mut iframe_element.htmlelement.element;
let src_opt = elem.get_attr("src").map(|x| x.to_str());
let src_opt = elem.get_attr(&intern_string("src")).map(|x| x.to_str());
for src in src_opt.iter() {
let iframe_url = make_url(src.clone(), Some(url2.clone()));
iframe_element.frame = Some(iframe_url.clone());

// Size future
let (port, chan) = comm::oneshot();
let size_future = Future::from_port(port);
Expand Down Expand Up @@ -525,9 +526,9 @@ pub fn parse_html(cx: *JSContext,
unsafe {
let scriptnode: AbstractNode<ScriptView> = NodeWrapping::from_hubbub_node(script);
do scriptnode.with_imm_element |script| {
match script.get_attr("src") {
match script.get_attr(&intern_string("src")) {
Some(src) => {
debug!("found script: %s", src);
debug!("found script: %s", src.to_str_slice());
let new_url = make_url(src.to_str(), Some(url3.clone()));
js_chan2.send(JSTaskNewFile(new_url));
}
Expand Down