Serialize style attributes on demand…

… rather than every time that CSSOM `Element.style` is modified.

components/atoms/static_atoms.txt

@@ -70,3 +70,8 @@ gattserverdisconnected
 onchange
 
 reftest-wait
+
+icon
+apple-touch-icon
+stylesheet
+alternate

components/layout/construct.rs

@@ -1689,8 +1689,8 @@ impl<N> ObjectElement for N  where N: ThreadSafeLayoutNode {
     fn has_object_data(&self) -> bool {
         let elem = self.as_element().unwrap();
         let type_and_data = (
-            elem.get_attr(&ns!(), &local_name!("type")),
-            elem.get_attr(&ns!(), &local_name!("data")),
+            elem.get_attr_enum(&ns!(), &local_name!("type")),
+            elem.get_attr_enum(&ns!(), &local_name!("data")).map(|a| a.as_string()),
         );
         match type_and_data {
             (None, Some(uri)) => is_image_data(uri),
@@ -1700,9 +1700,10 @@ impl<N> ObjectElement for N  where N: ThreadSafeLayoutNode {
 
     fn object_data(&self) -> Option<ServoUrl> {
         let elem = self.as_element().unwrap();
+
         let type_and_data = (
-            elem.get_attr(&ns!(), &local_name!("type")),
-            elem.get_attr(&ns!(), &local_name!("data")),
+            elem.get_attr_enum(&ns!(), &local_name!("type")),
+            elem.get_attr_enum(&ns!(), &local_name!("data")).map(|a| a.as_string()),
         );
         match type_and_data {
             (None, Some(uri)) if is_image_data(uri) => ServoUrl::parse(uri).ok(),

components/layout/fragment.rs

@@ -628,8 +628,9 @@ impl TableColumnFragmentInfo {
     /// Create the information specific to an table column fragment.
     pub fn new<N: ThreadSafeLayoutNode>(node: &N) -> TableColumnFragmentInfo {
         let element = node.as_element().unwrap();
-        let span = element.get_attr(&ns!(), &local_name!("span"))
-                          .and_then(|string| string.parse().ok())
+        // FIXME(emilio): Parse this as AttrValue::UInt beforehand.
+        let span = element.get_attr_enum(&ns!(), &local_name!("span"))
+                          .and_then(|a| a.as_string().parse().ok())
                           .unwrap_or(0);
         TableColumnFragmentInfo {
             span: span,

components/script/dom/attr.rs

@@ -11,7 +11,7 @@ use dom::bindings::reflector::{Reflector, reflect_dom_object};
 use dom::bindings::str::DOMString;
 use dom::element::{AttributeMutation, Element};
 use dom::mutationobserver::{Mutation, MutationObserver};
-use dom::node::Node;
+use dom::node::{Node, document_from_node};
 use dom::virtualmethods::vtable_for;
 use dom::window::Window;
 use dom_struct::dom_struct;
@@ -21,6 +21,7 @@ use std::borrow::ToOwned;
 use std::cell::Ref;
 use std::mem;
 use style::attr::{AttrIdentifier, AttrValue};
+use style_traits::ToCss;
 
 // https://dom.spec.whatwg.org/#interface-attr
 #[dom_struct]
@@ -29,7 +30,7 @@ pub struct Attr {
     identifier: AttrIdentifier,
     value: DOMRefCell<AttrValue>,
 
-    /// the element that owns this attribute.
+    /// The element that owns this attribute.
     owner: MutNullableJS<Element>,
 }
 
@@ -41,6 +42,9 @@ impl Attr {
                      prefix: Option<Prefix>,
                      owner: Option<&Element>)
                      -> Attr {
+        if let (&AttrValue::Declaration(..), None) = (&value, owner) {
+            panic!("AttrValue::Declaration without an owner element is not allowed.")
+        }
         Attr {
             reflector_: Reflector::new(),
             identifier: AttrIdentifier {
@@ -98,7 +102,12 @@ impl AttrMethods for Attr {
     // https://dom.spec.whatwg.org/#dom-attr-value
     fn Value(&self) -> DOMString {
         // FIXME(ajeffrey): convert directly from AttrValue to DOMString
-        DOMString::from(&**self.value())
+        DOMString::from(self.value().serialize(&mut |block| {
+            let owner = self.owner.get().expect("get AttrValue::Declaration without an owner element");
+            let doc = document_from_node(&*owner);
+            let guard = doc.style_shared_lock().read();
+            block.read_with(&guard).to_css_string()
+        }))
     }
 
     // https://dom.spec.whatwg.org/#dom-attr-value
@@ -174,11 +183,11 @@ impl Attr {
     pub fn set_value(&self, mut value: AttrValue, owner: &Element) {
         let name = self.local_name().clone();
         let namespace = self.namespace().clone();
-        let old_value = DOMString::from(&**self.value());
+        let old_value = self.Value();
         let mutation = Mutation::Attribute { name, namespace, old_value };
         MutationObserver::queue_a_mutation_record(owner.upcast::<Node>(), mutation);
 
-        assert!(Some(owner) == self.owner().r());
+        assert_eq!(Some(owner), self.owner().r());
         owner.will_mutate_attr(self);
         self.swap_value(&mut value);
         if self.identifier.namespace == ns!() {
@@ -189,6 +198,9 @@ impl Attr {
 
     /// Used to swap the attribute's value without triggering mutation events
     pub fn swap_value(&self, value: &mut AttrValue) {
+        if let (&AttrValue::Declaration(..), None) = (&*value, self.owner.get()) {
+            panic!("AttrValue::Declaration without an owner element is not allowed.")
+        }
         mem::swap(&mut *self.value.borrow_mut(), value);
     }
 
@@ -235,7 +247,6 @@ impl Attr {
 #[allow(unsafe_code)]
 pub trait AttrHelpersForLayout {
     unsafe fn value_forever(&self) -> &'static AttrValue;
-    unsafe fn value_ref_forever(&self) -> &'static str;
     unsafe fn value_atom_forever(&self) -> Option<Atom>;
     unsafe fn value_tokens_forever(&self) -> Option<&'static [Atom]>;
     unsafe fn local_name_atom_forever(&self) -> LocalName;
@@ -250,11 +261,6 @@ impl AttrHelpersForLayout for LayoutJS<Attr> {
         mem::transmute::<&AttrValue, &AttrValue>((*self.unsafe_get()).value.borrow_for_layout())
     }
 
-    #[inline]
-    unsafe fn value_ref_forever(&self) -> &'static str {
-        &**self.value_forever()
-    }
-
     #[inline]
     unsafe fn value_atom_forever(&self) -> Option<Atom> {
         let value = (*self.unsafe_get()).value.borrow_for_layout();

components/script/dom/cssstyledeclaration.rs

@@ -57,12 +57,10 @@ impl CSSStyleOwner {
                 let document = document_from_node(&**el);
                 let shared_lock = document.style_shared_lock();
                 let mut attr = el.style_attribute().borrow_mut().take();
-                let result = if attr.is_some() {
-                    let lock = attr.as_ref().unwrap();
+                let result = if let Some(ref lock) = attr {
                     let mut guard = shared_lock.write();
                     let mut pdb = lock.write_with(&mut guard);
-                    let result = f(&mut pdb, &mut changed);
-                    result
+                    f(&mut pdb, &mut changed)
                 } else {
                     let mut pdb = PropertyDeclarationBlock::new();
                     let result = f(&mut pdb, &mut changed);
@@ -85,11 +83,7 @@ impl CSSStyleOwner {
                     //
                     // [1]: https://github.com/whatwg/html/issues/2306
                     if let Some(pdb) = attr {
-                        let guard = shared_lock.read();
-                        let serialization = pdb.read_with(&guard).to_css_string();
-                        el.set_attribute(&local_name!("style"),
-                                         AttrValue::Declaration(serialization,
-                                                                pdb));
+                        el.set_attribute(&local_name!("style"), AttrValue::Declaration(pdb));
                     }
                 } else {
                     // Remember to put it back.

components/script/dom/document.rs

@@ -715,7 +715,7 @@ impl Document {
         let check_anchor = |node: &HTMLAnchorElement| {
             let elem = node.upcast::<Element>();
             elem.get_attribute(&ns!(), &local_name!("name"))
-                .map_or(false, |attr| &**attr.value() == name)
+                .map_or(false, |attr| &**attr.value().as_atom() == name)
         };
         let doc_node = self.upcast::<Node>();
         doc_node.traverse_preorder()
@@ -3204,7 +3204,7 @@ impl DocumentMethods for Document {
                 return false;
             }
             element.get_attribute(&ns!(), &local_name!("name"))
-                   .map_or(false, |attr| &**attr.value() == &*name)
+                   .map_or(false, |attr| &*attr.value().as_atom() == &*name)
         })
     }
 

components/script/dom/element.rs

@@ -114,6 +114,7 @@ use style::stylearc::Arc;
 use style::thread_state;
 use style::values::{CSSFloat, Either};
 use style::values::specified;
+use style_traits::ToCss;
 use stylesheet_loader::StylesheetOwner;
 
 // TODO: Update focus state when the top-level browsing context gains or loses system focus,
@@ -294,8 +295,6 @@ impl Element {
 pub trait RawLayoutElementHelpers {
     unsafe fn get_attr_for_layout<'a>(&'a self, namespace: &Namespace, name: &LocalName)
                                       -> Option<&'a AttrValue>;
-    unsafe fn get_attr_val_for_layout<'a>(&'a self, namespace: &Namespace, name: &LocalName)
-                                      -> Option<&'a str>;
     unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue>;
 }
 
@@ -322,14 +321,6 @@ impl RawLayoutElementHelpers for Element {
         })
     }
 
-    #[inline]
-    unsafe fn get_attr_val_for_layout<'a>(&'a self, namespace: &Namespace, name: &LocalName)
-                                          -> Option<&'a str> {
-        get_attr_for_layout(self, namespace, name).map(|attr| {
-            attr.value_ref_forever()
-        })
-    }
-
     #[inline]
     unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue> {
         let attrs = self.attrs.borrow_for_layout();
@@ -363,7 +354,7 @@ pub trait LayoutElementHelpers {
     fn style_attribute(&self) -> *const Option<Arc<Locked<PropertyDeclarationBlock>>>;
     fn local_name(&self) -> &LocalName;
     fn namespace(&self) -> &Namespace;
-    fn get_lang_for_layout(&self) -> String;
+    fn get_lang_for_layout(&self) -> Atom;
     fn get_checked_state_for_layout(&self) -> bool;
     fn get_indeterminate_state_for_layout(&self) -> bool;
     fn get_state_for_layout(&self) -> ElementState;
@@ -507,15 +498,31 @@ impl LayoutElementHelpers for LayoutJS<Element> {
 
 
         let size = if let Some(this) = self.downcast::<HTMLInputElement>() {
-            // FIXME(pcwalton): More use of atoms, please!
-            match (*self.unsafe_get()).get_attr_val_for_layout(&ns!(), &local_name!("type")) {
+            match (*self.unsafe_get()).get_attr_for_layout(&ns!(), &local_name!("type")).map(|a| a.as_atom()) {
                 // Not text entry widget
-                Some("hidden") | Some("date") | Some("month") | Some("week") |
-                Some("time") | Some("datetime-local") | Some("number") | Some("range") |
-                Some("color") | Some("checkbox") | Some("radio") | Some("file") |
-                Some("submit") | Some("image") | Some("reset") | Some("button") => {
+                Some(&atom!("hidden")) |
+                Some(&atom!("date")) |
+                Some(&atom!("month")) |
+                Some(&atom!("week")) |
+                Some(&atom!("time")) |
+                Some(&atom!("datetime-local")) |
+                Some(&atom!("number")) |
+                Some(&atom!("checkbox")) |
+                Some(&atom!("radio")) |
+                Some(&atom!("file")) |
+                Some(&atom!("submit")) |
+                Some(&atom!("image")) |
+                Some(&atom!("reset")) |
+                Some(&atom!("button")) => {
                     None
                 },
+                // FIXME(emilio): make "color" "range" a static atom.
+                Some(other) if *other == Atom::from("color") => {
+                    None
+                }
+                Some(other) if *other == Atom::from("range") => {
+                    None
+                }
                 // Others
                 _ => {
                     match this.size_for_layout() {
@@ -726,18 +733,18 @@ impl LayoutElementHelpers for LayoutJS<Element> {
     }
 
     #[allow(unsafe_code)]
-    fn get_lang_for_layout(&self) -> String {
+    fn get_lang_for_layout(&self) -> Atom {
         unsafe {
             let mut current_node = Some(self.upcast::<Node>());
             while let Some(node) = current_node {
                 current_node = node.parent_node_ref();
                 match node.downcast::<Element>().map(|el| el.unsafe_get()) {
                     Some(elem) => {
-                        if let Some(attr) = (*elem).get_attr_val_for_layout(&ns!(xml), &local_name!("lang")) {
-                            return attr.to_owned();
+                        if let Some(attr) = (*elem).get_attr_for_layout(&ns!(xml), &local_name!("lang")) {
+                            return attr.as_atom().clone();
                         }
-                        if let Some(attr) = (*elem).get_attr_val_for_layout(&ns!(), &local_name!("lang")) {
-                            return attr.to_owned();
+                        if let Some(attr) = (*elem).get_attr_for_layout(&ns!(), &local_name!("lang")) {
+                            return attr.as_atom().clone();
                         }
                     }
                     None => continue
@@ -745,7 +752,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
             }
             // TODO: Check meta tags for a pragma-set default language
             // TODO: Check HTTP Content-Language header
-            String::new()
+            atom!("")
         }
     }
 
@@ -861,13 +868,21 @@ impl Element {
             });
 
             if let Some(attr) = attr {
-                return (**attr.value()).into();
+                return self.serialize_attr(&attr).into();
             }
         }
 
         ns!()
     }
 
+    pub fn serialize_attr(&self, attr: &Attr) -> String {
+        attr.value().serialize(&mut |block| {
+            let doc = document_from_node(self);
+            let guard = doc.style_shared_lock().read();
+            block.read_with(&guard).to_css_string()
+        })
+    }
+
     pub fn style_attribute(&self) -> &DOMRefCell<Option<Arc<Locked<PropertyDeclarationBlock>>>> {
         &self.style_attribute
     }
@@ -942,7 +957,7 @@ impl Element {
                     // Step 2.
                     for attr in element.attrs.borrow().iter() {
                         if attr.prefix() == Some(&namespace_prefix!("xmlns")) &&
-                           **attr.value() == *namespace {
+                            self.serialize_attr(attr) == *namespace {
                             return Some(attr.LocalName());
                         }
                     }
@@ -1012,7 +1027,7 @@ impl Element {
     pub fn push_attribute(&self, attr: &Attr) {
         let name = attr.local_name().clone();
         let namespace = attr.namespace().clone();
-        let old_value = DOMString::from(&**attr.value());
+        let old_value = DOMString::from(self.serialize_attr(attr));
         let mutation = Mutation::Attribute { name, namespace, old_value };
         MutationObserver::queue_a_mutation_record(&self.node, mutation);
 
@@ -1146,7 +1161,7 @@ impl Element {
 
             let name = attr.local_name().clone();
             let namespace = attr.namespace().clone();
-            let old_value = DOMString::from(&**attr.value());
+            let old_value = DOMString::from(self.serialize_attr(&attr));
             let mutation = Mutation::Attribute { name, namespace, old_value, };
             MutationObserver::queue_a_mutation_record(&self.node, mutation);
 
@@ -2190,36 +2205,16 @@ impl VirtualMethods for Element {
                 // Modifying the `style` attribute might change style.
                 *self.style_attribute.borrow_mut() = match mutation {
                     AttributeMutation::Set(..) => {
-                        // This is the fast path we use from
-                        // CSSStyleDeclaration.
-                        //
-                        // Juggle a bit to keep the borrow checker happy
-                        // while avoiding the extra clone.
-                        let is_declaration = match *attr.value() {
-                            AttrValue::Declaration(..) => true,
-                            _ => false,
-                        };
-
-                        let block = if is_declaration {
-                            let mut value = AttrValue::String(String::new());
-                            attr.swap_value(&mut value);
-                            let (serialization, block) = match value {
-                                AttrValue::Declaration(s, b) => (s, b),
-                                _ => unreachable!(),
-                            };
-                            let mut value = AttrValue::String(serialization);
-                            attr.swap_value(&mut value);
-                            block
+                        if let AttrValue::Declaration(ref block) = *attr.value() {
+                            Some(block.clone())
                         } else {
                             let win = window_from_node(self);
-                            Arc::new(doc.style_shared_lock().wrap(parse_style_attribute(
-                                &attr.value(),
+                            Some(Arc::new(doc.style_shared_lock().wrap(parse_style_attribute(
+                                attr.value().as_string(),
                                 &doc.base_url(),
                                 win.css_error_reporter(),
-                                doc.quirks_mode())))
-                        };
-
-                        Some(block)
+                                doc.quirks_mode()))))
+                        }
                     }
                     AttributeMutation::Removed => {
                         None
@@ -2397,15 +2392,21 @@ impl<'a> ::selectors::Element for Root<Element> {
                     local_name: &LocalName,
                     operation: &AttrSelectorOperation<&String>)
                     -> bool {
+        let mut serialize = |block: &Locked<PropertyDeclarationBlock>| {
+            let doc = document_from_node(&**self);
+            // This is porbably slow, but it only happens for silly selectors like [style*=color]
+            let guard = doc.style_shared_lock().read();
+            block.read_with(&guard).to_css_string()
+        };
         match *ns {
             NamespaceConstraint::Specific(ref ns) => {
                 self.get_attribute(ns, local_name)
-                    .map_or(false, |attr| attr.value().eval_selector(operation))
+                    .map_or(false, |attr| attr.value().eval_selector(operation, &mut serialize))
             }
             NamespaceConstraint::Any => {
                 self.attrs.borrow().iter().any(|attr| {
                     attr.local_name() == local_name &&
-                    attr.value().eval_selector(operation)
+                    attr.value().eval_selector(operation, &mut serialize)
                 })
             }
         }
@@ -2461,7 +2462,12 @@ impl<'a> ::selectors::Element for Root<Element> {
 
             NonTSPseudoClass::ServoCaseSensitiveTypeAttr(ref expected_value) => {
                 self.get_attribute(&ns!(), &local_name!("type"))
-                    .map_or(false, |attr| attr.value().eq(expected_value))
+                    .map_or(false, |attr| {
+                        match *attr.value() {
+                            AttrValue::Atom(ref atom) => atom == expected_value,
+                            _ => false,
+                        }
+                    })
             }
 
             // FIXME(heycam): This is wrong, since extended_filtering accepts

components/script/dom/htmlanchorelement.rs

@@ -67,7 +67,7 @@ impl HTMLAnchorElement {
         let attribute = self.upcast::<Element>().get_attribute(&ns!(), &local_name!("href"));
         *self.url.borrow_mut() = attribute.and_then(|attribute| {
             let document = document_from_node(self);
-            document.base_url().join(&attribute.value()).ok()
+            document.base_url().join(attribute.value().as_string()).ok()
         });
     }
 
@@ -276,7 +276,7 @@ impl HTMLAnchorElementMethods for HTMLAnchorElement {
                     // Step 3.
                     None => String::new(),
                     // Step 4.
-                    Some(attribute) => (**attribute.value()).to_owned(),
+                    Some(attribute) => attribute.value().as_string().to_owned(),
                 }
             },
             // Step 5.

components/script/dom/htmlbaseelement.rs

@@ -46,7 +46,7 @@ impl HTMLBaseElement {
                      that have a base url.");
         let document = document_from_node(self);
         let base = document.fallback_base_url();
-        let parsed = base.join(&href.value());
+        let parsed = base.join(href.value().as_string());
         parsed.unwrap_or(base)
     }
 

components/script/dom/htmlbodyelement.rs

@@ -167,15 +167,16 @@ impl VirtualMethods for HTMLBodyElement {
                     &local_name!("onoffline") | &local_name!("ononline") |
                     &local_name!("onpagehide") | &local_name!("onpageshow") |
                     &local_name!("onpopstate") | &local_name!("onstorage") |
-                    &local_name!("onresize") | &local_name!("onunload") | &local_name!("onerror")
-                      => {
-                          let evtarget = window.upcast::<EventTarget>(); // forwarded event
-                          let source_line = 1; //TODO(#9604) obtain current JS execution line
-                          evtarget.set_event_handler_uncompiled(window.get_url(),
-                                                                source_line,
-                                                                &name[2..],
-                                                                DOMString::from((**attr.value()).to_owned()));
-                          false
+                    &local_name!("onresize") | &local_name!("onunload") | &local_name!("onerror") => {
+                        let evtarget = window.upcast::<EventTarget>(); // forwarded event
+                        let source_line = 1; //TODO(#9604) obtain current JS execution line
+                        evtarget.set_event_handler_uncompiled(
+                            window.get_url(),
+                            source_line,
+                            &name[2..],
+                            DOMString::from(attr.value().as_string())
+                        );
+                        false
                     }
                     _ => true, // HTMLElement::attribute_mutated will take care of this.
                 }

components/script/dom/htmlbuttonelement.rs

@@ -202,7 +202,8 @@ impl VirtualMethods for HTMLButtonElement {
             &local_name!("type") => {
                 match mutation {
                     AttributeMutation::Set(_) => {
-                        let value = match &**attr.value() {
+                        // FIXME(emilio): Match on Atoms instead.
+                        let value = match &**attr.value().as_atom() {
                             "reset" => ButtonType::Reset,
                             "button" => ButtonType::Button,
                             "menu" => ButtonType::Menu,

components/script/dom/htmlelement.rs

@@ -439,8 +439,10 @@ impl HTMLElement {
     pub fn get_custom_attr(&self, local_name: DOMString) -> Option<DOMString> {
         // FIXME(ajeffrey): Convert directly from DOMString to LocalName
         let local_name = LocalName::from(to_snake_case(local_name));
-        self.upcast::<Element>().get_attribute(&ns!(), &local_name).map(|attr| {
-            DOMString::from(&**attr.value()) // FIXME(ajeffrey): Convert directly from AttrValue to DOMString
+        let element = self.upcast::<Element>();
+        element.get_attribute(&ns!(), &local_name).map(|attr| {
+            // FIXME(ajeffrey): Convert directly from AttrValue to DOMString
+            DOMString::from(element.serialize_attr(&attr))
         })
     }
 
@@ -557,7 +559,7 @@ impl VirtualMethods for HTMLElement {
                                                       source_line,
                                                       &name[2..],
                                                       // FIXME(ajeffrey): Convert directly from AttrValue to DOMString
-                                                      DOMString::from(&**attr.value()));
+                                                      DOMString::from(attr.value().as_string()));
             },
             _ => {}
         }

components/script/dom/htmlheadelement.rs

@@ -57,7 +57,7 @@ impl HTMLHeadElement {
         for meta in candidates {
             if let Some(content) = meta.get_attribute(&ns!(), &local_name!("content")).r() {
                 let content = content.value();
-                let content_val = content.trim();
+                let content_val = content.as_string().trim();
                 if !content_val.is_empty() {
                     doc.set_referrer_policy(determine_policy_for_token(content_val));
                     return;

components/script/dom/htmliframeelement.rs

@@ -105,6 +105,7 @@ impl HTMLIFrameElement {
         let element = self.upcast::<Element>();
         element.get_attribute(&ns!(), &local_name!("src")).and_then(|src| {
             let url = src.value();
+            let url = url.as_string();
             if url.is_empty() {
                 None
             } else {

components/script/dom/htmlimageelement.rs

@@ -687,6 +687,7 @@ impl HTMLImageElement {
         };
 
         let value = usemap_attr.value();
+        let value = value.as_string();
 
         if value.len() == 0 || !value.is_char_boundary(1) {
             return None

components/script/dom/htmlinputelement.rs

@@ -217,7 +217,8 @@ impl LayoutHTMLInputElementHelpers for LayoutJS<HTMLInputElement> {
         unsafe fn get_raw_attr_value(input: LayoutJS<HTMLInputElement>, default: &str) -> String {
             let elem = input.upcast::<Element>();
             let value = (*elem.unsafe_get())
-                .get_attr_val_for_layout(&ns!(), &local_name!("value"))
+                .get_attr_for_layout(&ns!(), &local_name!("value"))
+                .map(|v| v.as_string())
                 .unwrap_or(default);
             String::from(value)
         }
@@ -992,7 +993,7 @@ impl VirtualMethods for HTMLInputElement {
                 self.update_placeholder_shown_state();
             },
             &local_name!("value") if !self.value_changed.get() => {
-                let value = mutation.new_value(attr).map(|value| (**value).to_owned());
+                let value = mutation.new_value(attr).map(|value| value.as_string().to_owned());
                 self.textinput.borrow_mut().set_content(
                     value.map_or(DOMString::new(), DOMString::from));
                 self.update_placeholder_shown_state();
@@ -1031,7 +1032,7 @@ impl VirtualMethods for HTMLInputElement {
                     placeholder.clear();
                     if let AttributeMutation::Set(_) = mutation {
                         placeholder.extend(
-                            attr.value().chars().filter(|&c| c != '\n' && c != '\r'));
+                            attr.value().as_string().chars().filter(|&c| c != '\n' && c != '\r'));
                     }
                 }
                 self.update_placeholder_shown_state();

components/script/dom/htmllinkelement.rs

@@ -122,30 +122,28 @@ impl HTMLLinkElement {
     }
 
     pub fn is_alternate(&self) -> bool {
-        let rel = get_attr(self.upcast(), &local_name!("rel"));
+        let as_element: &Element = self.upcast();
+        let rel = as_element.get_attribute(&ns!(), &local_name!("rel"));
         match rel {
-            Some(ref value) => {
-                value.split(HTML_SPACE_CHARACTERS)
-                    .any(|s| s.eq_ignore_ascii_case("alternate"))
+            Some(value) => {
+                value
+                    .value()
+                    .as_tokens()
+                    .iter()
+                    .any(|s| s.eq_ignore_ascii_case(&atom!("alternate")))
             },
             None => false,
         }
     }
 }
 
-fn get_attr(element: &Element, local_name: &LocalName) -> Option<String> {
-    let elem = element.get_attribute(&ns!(), local_name);
-    elem.map(|e| {
-        let value = e.value();
-        (**value).to_owned()
-    })
-}
-
-fn string_is_stylesheet(value: &Option<String>) -> bool {
-    match *value {
-        Some(ref value) => {
-            value.split(HTML_SPACE_CHARACTERS)
-                .any(|s| s.eq_ignore_ascii_case("stylesheet"))
+fn is_stylesheet(attr: &Option<Root<Attr>>) -> bool {
+    match *attr {
+        Some(ref attr) => {
+            attr.value()
+                .as_tokens()
+                .iter()
+                .any(|s| s.eq_ignore_ascii_case(&atom!("stylesheet")))
         },
         None => false,
     }
@@ -154,11 +152,14 @@ fn string_is_stylesheet(value: &Option<String>) -> bool {
 /// Favicon spec usage in accordance with CEF implementation:
 /// only url of icon is required/used
 /// https://html.spec.whatwg.org/multipage/#rel-icon
-fn is_favicon(value: &Option<String>) -> bool {
-    match *value {
-        Some(ref value) => {
-            value.split(HTML_SPACE_CHARACTERS)
-                .any(|s| s.eq_ignore_ascii_case("icon") || s.eq_ignore_ascii_case("apple-touch-icon"))
+fn is_favicon(attr: &Option<Root<Attr>>) -> bool {
+    match *attr {
+        Some(ref attr) => {
+            attr.value()
+                .as_tokens()
+                .iter()
+                .any(|s| s.eq_ignore_ascii_case(&atom!("icon")) ||
+                        s.eq_ignore_ascii_case(&atom!("apple-touch-icon")))
         },
         None => false,
     }
@@ -174,28 +175,35 @@ impl VirtualMethods for HTMLLinkElement {
         if !self.upcast::<Node>().is_in_doc() || mutation.is_removal() {
             return;
         }
+        let as_element: &Element = self.upcast();
 
-        let rel = get_attr(self.upcast(), &local_name!("rel"));
+        let rel = as_element.get_attribute(&ns!(), &local_name!("rel"));
         match attr.local_name() {
             &local_name!("href") => {
-                if string_is_stylesheet(&rel) {
-                    self.handle_stylesheet_url(&attr.value());
+                if is_stylesheet(&rel) {
+                    self.handle_stylesheet_url(attr.value().as_string());
                 } else if is_favicon(&rel) {
-                    let sizes = get_attr(self.upcast(), &local_name!("sizes"));
-                    self.handle_favicon_url(rel.as_ref().unwrap(), &attr.value(), &sizes);
+                    let sizes = as_element.get_attribute(&ns!(), &local_name!("sizes"));
+                    self.handle_favicon_url(
+                        rel.as_ref().unwrap().value().as_string(),
+                        attr.value().as_string(),
+                        sizes.as_ref().map(|s| &**s));
                 }
             },
             &local_name!("sizes") => {
                 if is_favicon(&rel) {
-                    if let Some(ref href) = get_attr(self.upcast(), &local_name!("href")) {
-                        self.handle_favicon_url(rel.as_ref().unwrap(), href, &Some(attr.value().to_string()));
+                    if let Some(ref href) = as_element.get_attribute(&ns!(), &local_name!("href")) {
+                        self.handle_favicon_url(
+                            rel.as_ref().unwrap().value().as_string(),
+                            href.value().as_string(),
+                            Some(attr));
                     }
                 }
             },
             &local_name!("media") => {
-                if string_is_stylesheet(&rel) {
+                if is_stylesheet(&rel) {
                     if let Some(href) = self.upcast::<Element>().get_attribute(&ns!(), &local_name!("href")) {
-                        self.handle_stylesheet_url(&href.value());
+                        self.handle_stylesheet_url(href.value().as_string());
                     }
                 }
             },
@@ -216,18 +224,21 @@ impl VirtualMethods for HTMLLinkElement {
         }
 
         if tree_in_doc {
-            let element = self.upcast();
+            let as_element: &Element = self.upcast();
 
-            let rel = get_attr(element, &local_name!("rel"));
-            let href = get_attr(element, &local_name!("href"));
-            let sizes = get_attr(self.upcast(), &local_name!("sizes"));
+            let rel = as_element.get_attribute(&ns!(), &local_name!("rel"));
+            let href = as_element.get_attribute(&ns!(), &local_name!("href"));
+            let sizes = as_element.get_attribute(&ns!(), &local_name!("sizes"));
 
             match href {
-                Some(ref href) if string_is_stylesheet(&rel) => {
-                    self.handle_stylesheet_url(href);
+                Some(ref href) if is_stylesheet(&rel) => {
+                    self.handle_stylesheet_url(href.value().as_string());
                 }
                 Some(ref href) if is_favicon(&rel) => {
-                    self.handle_favicon_url(rel.as_ref().unwrap(), href, &sizes);
+                    self.handle_favicon_url(
+                        rel.as_ref().unwrap().value().as_string(),
+                        href.value().as_string(),
+                        sizes.as_ref().map(|s| &**s));
                 }
                 _ => {}
             }
@@ -275,7 +286,7 @@ impl HTMLLinkElement {
         let mq_attribute = element.get_attribute(&ns!(), &local_name!("media"));
         let value = mq_attribute.r().map(|a| a.value());
         let mq_str = match value {
-            Some(ref value) => &***value,
+            Some(ref value) => value.as_string(),
             None => "",
         };
 
@@ -291,7 +302,7 @@ impl HTMLLinkElement {
         let im_attribute = element.get_attribute(&ns!(), &local_name!("integrity"));
         let integrity_val = im_attribute.r().map(|a| a.value());
         let integrity_metadata = match integrity_val {
-            Some(ref value) => &***value,
+            Some(ref value) => value.as_string(),
             None => "",
         };
 
@@ -305,15 +316,17 @@ impl HTMLLinkElement {
         }, link_url, cors_setting, integrity_metadata.to_owned());
     }
 
-    fn handle_favicon_url(&self, rel: &str, href: &str, sizes: &Option<String>) {
+    fn handle_favicon_url(&self, rel: &str, href: &str, sizes: Option<&Attr>) {
         let document = document_from_node(self);
         match document.base_url().join(href) {
             Ok(url) => {
                 let event = ConstellationMsg::NewFavicon(url.clone());
                 document.window().upcast::<GlobalScope>().constellation_chan().send(event).unwrap();
 
-                let mozbrowser_event = match *sizes {
-                    Some(ref sizes) => MozBrowserEvent::IconChange(rel.to_owned(), url.to_string(), sizes.to_owned()),
+                let mozbrowser_event = match sizes {
+                    Some(sizes) => MozBrowserEvent::IconChange(
+                        rel.to_owned(), url.to_string(), sizes.value().as_string().to_owned()
+                    ),
                     None => MozBrowserEvent::IconChange(rel.to_owned(), url.to_string(), "".to_owned())
                 };
                 document.trigger_mozbrowser_event(mozbrowser_event);

components/script/dom/htmlmetaelement.rs

@@ -21,7 +21,6 @@ use dom_struct::dom_struct;
 use html5ever::{LocalName, Prefix};
 use parking_lot::RwLock;
 use servo_config::prefs::PREFS;
-use std::ascii::AsciiExt;
 use std::sync::atomic::AtomicBool;
 use style::attr::AttrValue;
 use style::media_queries::MediaList;
@@ -77,7 +76,7 @@ impl HTMLMetaElement {
     fn process_attributes(&self) {
         let element = self.upcast::<Element>();
         if let Some(name) = element.get_attribute(&ns!(), &local_name!("name")).r() {
-            let name = name.value().to_ascii_lowercase();
+            let name = name.value().as_atom().to_ascii_lowercase();
             let name = name.trim_matches(HTML_SPACE_CHARACTERS);
 
             if name == "viewport" {
@@ -97,8 +96,9 @@ impl HTMLMetaElement {
         let element = self.upcast::<Element>();
         if let Some(content) = element.get_attribute(&ns!(), &local_name!("content")).r() {
             let content = content.value();
+            let content = content.as_string();
             if !content.is_empty() {
-                if let Some(translated_rule) = ViewportRule::from_meta(&**content) {
+                if let Some(translated_rule) = ViewportRule::from_meta(content) {
                     let document = self.upcast::<Node>().owner_doc();
                     let shared_lock = document.style_shared_lock();
                     let rule = CssRule::Viewport(Arc::new(shared_lock.wrap(translated_rule)));
@@ -125,7 +125,7 @@ impl HTMLMetaElement {
     fn process_referrer_attribute(&self) {
         let element = self.upcast::<Element>();
         if let Some(name) = element.get_attribute(&ns!(), &local_name!("name")).r() {
-            let name = name.value().to_ascii_lowercase();
+            let name = name.value().as_atom().to_ascii_lowercase();
             let name = name.trim_matches(HTML_SPACE_CHARACTERS);
 
             if name == "referrer" {

components/script/dom/htmlscriptelement.rs

@@ -349,13 +349,13 @@ impl HTMLScriptElement {
         let event_attribute = element.get_attribute(&ns!(), &local_name!("event"));
         match (for_attribute.r(), event_attribute.r()) {
             (Some(for_attribute), Some(event_attribute)) => {
-                let for_value = for_attribute.value().to_ascii_lowercase();
+                let for_value = for_attribute.value().as_string().to_ascii_lowercase();
                 let for_value = for_value.trim_matches(HTML_SPACE_CHARACTERS);
                 if for_value != "window" {
                     return;
                 }
 
-                let event_value = event_attribute.value().to_ascii_lowercase();
+                let event_value = event_attribute.value().as_string().to_ascii_lowercase();
                 let event_value = event_value.trim_matches(HTML_SPACE_CHARACTERS);
                 if event_value != "onload" && event_value != "onload()" {
                     return;
@@ -366,7 +366,7 @@ impl HTMLScriptElement {
 
         // Step 13.
         let encoding = element.get_attribute(&ns!(), &local_name!("charset"))
-                              .and_then(|charset| encoding_from_whatwg_label(&charset.value()))
+                              .and_then(|charset| encoding_from_whatwg_label(charset.value().as_string()))
                               .unwrap_or_else(|| doc.encoding());
 
         // Step 14.
@@ -380,7 +380,7 @@ impl HTMLScriptElement {
         let im_attribute = element.get_attribute(&ns!(), &local_name!("integrity"));
         let integrity_val = im_attribute.r().map(|a| a.value());
         let integrity_metadata = match integrity_val {
-            Some(ref value) => &***value,
+            Some(ref value) => value.as_string(),
             None => "",
         };
 
@@ -394,6 +394,7 @@ impl HTMLScriptElement {
 
             // Step 20.1.
             let src = src.value();
+            let src = src.as_string();
 
             // Step 20.2.
             if src.is_empty() {
@@ -407,7 +408,7 @@ impl HTMLScriptElement {
             let url = match base_url.join(&src) {
                 Ok(url) => url,
                 Err(_) => {
-                    warn!("error parsing URL for script {}", &**src);
+                    warn!("error parsing URL for script {}", src);
                     self.queue_error_event();
                     return;
                 },
@@ -593,26 +594,28 @@ impl HTMLScriptElement {
         let element = self.upcast::<Element>();
         let type_attr = element.get_attribute(&ns!(), &local_name!("type"));
         let is_js = match type_attr.as_ref().map(|s| s.value()) {
-            Some(ref s) if s.is_empty() => {
+            Some(ref s) if s.as_string().is_empty() => {
                 // type attr exists, but empty means js
                 debug!("script type empty, inferring js");
                 true
             },
             Some(s) => {
-                debug!("script type={}", &**s);
+                let s = s.as_string();
+                debug!("script type={}", s);
                 SCRIPT_JS_MIMES.contains(&s.to_ascii_lowercase().trim_matches(HTML_SPACE_CHARACTERS))
             },
             None => {
                 debug!("no script type");
                 let language_attr = element.get_attribute(&ns!(), &local_name!("language"));
                 let is_js = match language_attr.as_ref().map(|s| s.value()) {
-                    Some(ref s) if s.is_empty() => {
+                    Some(ref s) if s.as_string().is_empty() => {
                         debug!("script language empty, inferring js");
                         true
                     },
                     Some(s) => {
-                        debug!("script language={}", &**s);
-                        let mut language = format!("text/{}", &**s);
+                        let s = s.as_string();
+                        debug!("script language={}", s);
+                        let mut language = format!("text/{}", s);
                         language.make_ascii_lowercase();
                         SCRIPT_JS_MIMES.contains(&&*language)
                     },

components/script/dom/htmlstyleelement.rs

@@ -79,9 +79,13 @@ impl HTMLStyleElement {
         let doc = document_from_node(self);
 
         let mq_attribute = element.get_attribute(&ns!(), &local_name!("media"));
+        let attr_value;
         let mq_str = match mq_attribute {
-            Some(a) => String::from(&**a.value()),
-            None => String::new(),
+            Some(ref a) => {
+                attr_value = a.value();
+                attr_value.as_string()
+            }
+            None => "",
         };
 
         let data = node.GetTextContent().expect("Element.textContent must be a string");
@@ -92,7 +96,7 @@ impl HTMLStyleElement {
                                                       PARSING_MODE_DEFAULT,
                                                       doc.quirks_mode());
         let shared_lock = node.owner_doc().style_shared_lock().clone();
-        let mut input = ParserInput::new(&mq_str);
+        let mut input = ParserInput::new(mq_str);
         let mq = Arc::new(shared_lock.wrap(
                     parse_media_query_list(&context, &mut CssParser::new(&mut input))));
         let loader = StylesheetLoader::for_element(self.upcast());

components/script/dom/htmltableelement.rs

@@ -427,12 +427,15 @@ impl VirtualMethods for HTMLTableElement {
             local_name!("border") => {
                 // According to HTML5 § 14.3.9, invalid values map to 1px.
                 self.border.set(mutation.new_value(attr).map(|value| {
-                    parse_unsigned_integer(value.chars()).unwrap_or(1)
+                    value.as_uint()
                 }));
             }
             local_name!("cellspacing") => {
                 self.cellspacing.set(mutation.new_value(attr).and_then(|value| {
-                    parse_unsigned_integer(value.chars()).ok()
+                    match *value {
+                        AttrValue::UInt(_, v) => Some(v),
+                        _ => None,
+                    }
                 }));
             },
             _ => {},
@@ -444,6 +447,13 @@ impl VirtualMethods for HTMLTableElement {
             local_name!("border") => AttrValue::from_u32(value.into(), 1),
             local_name!("width") => AttrValue::from_nonzero_dimension(value.into()),
             local_name!("bgcolor") => AttrValue::from_legacy_color(value.into()),
+            local_name!("cellspacing") => {
+                // FIXME(emilio): make AttrValue::from_u32 fallible instead.
+                match parse_unsigned_integer(value.chars()) {
+                    Ok(v) => AttrValue::UInt(value.into(), v),
+                    Err(..) => AttrValue::String(value.into()),
+                }
+            }
             _ => self.super_type().unwrap().parse_plain_attribute(local_name, value),
         }
     }

components/script/dom/htmltextareaelement.rs

@@ -330,7 +330,7 @@ impl VirtualMethods for HTMLTextAreaElement {
                     let mut placeholder = self.placeholder.borrow_mut();
                     placeholder.clear();
                     if let AttributeMutation::Set(_) = mutation {
-                        placeholder.push_str(&attr.value());
+                        placeholder.push_str(attr.value().as_string());
                     }
                 }
                 self.update_placeholder_shown_state();

components/script/dom/node.rs

@@ -2302,12 +2302,12 @@ impl NodeMethods for Node {
         fn is_equal_element_attrs(node: &Node, other: &Node) -> bool {
             let element = node.downcast::<Element>().unwrap();
             let other_element = other.downcast::<Element>().unwrap();
-            assert!(element.attrs().len() == other_element.attrs().len());
+            assert_eq!(element.attrs().len(), other_element.attrs().len());
             element.attrs().iter().all(|attr| {
                 other_element.attrs().iter().any(|other_attr| {
-                    (*attr.namespace() == *other_attr.namespace()) &&
-                    (attr.local_name() == other_attr.local_name()) &&
-                    (**attr.value() == **other_attr.value())
+                    *attr.namespace() == *other_attr.namespace() &&
+                    attr.local_name() == other_attr.local_name() &&
+                    element.serialize_attr(attr) == other_element.serialize_attr(other_attr)
                 })
             })
         }

components/script/dom/servoparser/html.rs

@@ -128,11 +128,11 @@ impl<'a> Serialize for &'a Node {
                     let attrs = elem.attrs().iter().map(|attr| {
                         let qname = QualName::new(None, attr.namespace().clone(),
                                                   attr.local_name().clone());
-                        let value = attr.value().clone();
+                        let value = elem.serialize_attr(attr);
                         (qname, value)
                     }).collect::<Vec<_>>();
                     let attr_refs = attrs.iter().map(|&(ref qname, ref value)| {
-                        let ar: AttrRef = (&qname, &**value);
+                        let ar: AttrRef = (&qname, &*value);
                         ar
                     });
                     serializer.start_elem(name.clone(), attr_refs)?;

components/script/dom/servoparser/mod.rs

@@ -899,8 +899,8 @@ impl TreeSink for Sink {
     fn is_mathml_annotation_xml_integration_point(&self, handle: &JS<Node>) -> bool {
         let elem = handle.downcast::<Element>().unwrap();
         elem.get_attribute(&ns!(), &local_name!("encoding")).map_or(false, |attr| {
-            attr.value().eq_ignore_ascii_case("text/html")
-                || attr.value().eq_ignore_ascii_case("application/xhtml+xml")
+            attr.value().as_string().eq_ignore_ascii_case("text/html")
+                || attr.value().as_string().eq_ignore_ascii_case("application/xhtml+xml")
         })
     }
 

components/script/layout_wrapper.rs

@@ -78,6 +78,7 @@ use style::selector_parser::{PseudoElement, SelectorImpl, extended_filtering};
 use style::shared_lock::{SharedRwLock as StyleSharedRwLock, Locked as StyleLocked};
 use style::str::is_whitespace;
 use style::stylearc::Arc;
+use style_traits::ToCss;
 
 #[derive(Copy, Clone)]
 pub struct ServoLayoutNode<'a> {
@@ -412,7 +413,7 @@ impl<'le> TElement for ServoLayoutElement<'le> {
 
     #[inline]
     fn has_attr(&self, namespace: &Namespace, attr: &LocalName) -> bool {
-        self.get_attr(namespace, attr).is_some()
+        self.get_attr_enum(namespace, attr).is_some()
     }
 
     #[inline]
@@ -520,9 +521,9 @@ impl<'le> TElement for ServoLayoutElement<'le> {
 
     #[inline]
     fn lang_attr(&self) -> Option<SelectorAttrValue> {
-        self.get_attr(&ns!(xml), &local_name!("lang"))
-            .or_else(|| self.get_attr(&ns!(), &local_name!("lang")))
-            .map(|v| String::from(v as &str))
+        self.get_attr_enum(&ns!(xml), &local_name!("lang"))
+            .or_else(|| self.get_attr_enum(&ns!(), &local_name!("lang")))
+            .map(|v| v.as_atom().to_string())
     }
 
     fn match_element_lang(&self,
@@ -544,8 +545,8 @@ impl<'le> TElement for ServoLayoutElement<'le> {
         // do this, we should make `get_lang_for_layout` return an Option,
         // so we can decide when to fall back to the Content-Language check.
         let element_lang = match override_lang {
-            Some(Some(lang)) => lang,
-            Some(None) => String::new(),
+            Some(Some(lang)) => Atom::from(lang),
+            Some(None) => atom!(""),
             None => self.element.get_lang_for_layout(),
         };
         extended_filtering(&element_lang, &*value)
@@ -582,12 +583,6 @@ impl<'le> ServoLayoutElement<'le> {
     }
 
     #[inline]
-    fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str> {
-        unsafe {
-            (*self.element.unsafe_get()).get_attr_val_for_layout(namespace, name)
-        }
-    }
-
     fn get_style_data(&self) -> Option<&StyleData> {
         unsafe {
             self.get_style_and_layout_data().map(|d| &*d.ptr.get())
@@ -674,16 +669,24 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
                     local_name: &LocalName,
                     operation: &AttrSelectorOperation<&String>)
                     -> bool {
+        let mut serialize = |block: &StyleLocked<PropertyDeclarationBlock>| {
+            let node = self.element.upcast::<Node>();
+            let doc = unsafe { node.owner_doc_for_layout() };
+            let lock = unsafe { doc.style_shared_lock() };
+            // This is porbably slow, but it only happens for silly selectors like [style*=color]
+            let guard = lock.read();
+            block.read_with(&guard).to_css_string()
+        };
         match *ns {
             NamespaceConstraint::Specific(ref ns) => {
                 self.get_attr_enum(ns, local_name)
-                    .map_or(false, |value| value.eval_selector(operation))
+                    .map_or(false, |value| value.eval_selector(operation, &mut serialize))
             }
             NamespaceConstraint::Any => {
                 let values = unsafe {
                     (*self.element.unsafe_get()).get_attr_vals_for_layout(local_name)
                 };
-                values.iter().any(|value| value.eval_selector(operation))
+                values.iter().any(|value| value.eval_selector(operation, &mut serialize))
             }
         }
     }
@@ -752,7 +755,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
             },
             NonTSPseudoClass::ServoCaseSensitiveTypeAttr(ref expected_value) => {
                 self.get_attr_enum(&ns!(), &local_name!("type"))
-                    .map_or(false, |attr| attr == expected_value)
+                    .map_or(false, |attr| &*attr.as_atom() == expected_value)
             }
             NonTSPseudoClass::ReadOnly =>
                 !self.element.get_state_for_layout().contains(pseudo_class.state_flag()),
@@ -774,15 +777,13 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
 
     #[inline]
     fn is_link(&self) -> bool {
-        unsafe {
-            match self.as_node().script_type_id() {
-                // https://html.spec.whatwg.org/multipage/#selector-link
-                NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLAnchorElement)) |
-                NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLAreaElement)) |
-                NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLLinkElement)) =>
-                    (*self.element.unsafe_get()).get_attr_val_for_layout(&ns!(), &local_name!("href")).is_some(),
-                _ => false,
-            }
+        match self.as_node().script_type_id() {
+            // https://html.spec.whatwg.org/multipage/#selector-link
+            NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLAnchorElement)) |
+            NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLAreaElement)) |
+            NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLLinkElement)) =>
+                self.get_attr_enum(&ns!(), &local_name!("href")).is_some(),
+            _ => false,
         }
     }
 
@@ -1145,10 +1146,6 @@ impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> {
         self.element.get_attr_enum(namespace, name)
     }
 
-    fn get_attr<'a>(&'a self, namespace: &Namespace, name: &LocalName) -> Option<&'a str> {
-        self.element.get_attr(namespace, name)
-    }
-
     fn style_data(&self) -> AtomicRef<ElementData> {
         self.element.get_data()
             .expect("Unstyled layout node?")
@@ -1227,18 +1224,7 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> {
                     local_name: &LocalName,
                     operation: &AttrSelectorOperation<&String>)
                     -> bool {
-        match *ns {
-            NamespaceConstraint::Specific(ref ns) => {
-                self.get_attr_enum(ns, local_name)
-                    .map_or(false, |value| value.eval_selector(operation))
-            }
-            NamespaceConstraint::Any => {
-                let values = unsafe {
-                    (*self.element.element.unsafe_get()).get_attr_vals_for_layout(local_name)
-                };
-                values.iter().any(|v| v.eval_selector(operation))
-            }
-        }
+        self.element.attr_matches(ns, local_name, operation)
     }
 
     fn match_non_ts_pseudo_class<F>(&self,

components/script/script_thread.rs

@@ -2101,13 +2101,15 @@ impl ScriptThread {
                                                 .inclusive_ancestors()
                                                 .filter_map(Root::downcast::<HTMLAnchorElement>)
                                                 .next() {
-                        let status = anchor.upcast::<Element>()
-                                           .get_attribute(&ns!(), &local_name!("href"))
-                                           .and_then(|href| {
-                                               let value = href.value();
-                                               let url = document.url();
-                                               url.join(&value).map(|url| url.to_string()).ok()
-                                           });
+                        let status =
+                            anchor.upcast::<Element>()
+                                .get_attribute(&ns!(), &local_name!("href"))
+                                .and_then(|href| {
+                                    let value = href.value();
+                                    let url = document.url();
+                                    url.join(value.as_string())
+                                        .map(|url| url.to_string()).ok()
+                                });
 
                         let event = ConstellationMsg::NodeStatus(status);
                         self.constellation_chan.send(event).unwrap();

components/script_layout_interface/wrapper_traits.rs

@@ -334,9 +334,6 @@ pub trait ThreadSafeLayoutElement: Clone + Copy + Sized + Debug +
     unsafe fn unsafe_get(self) ->
         <<Self::ConcreteThreadSafeLayoutNode as ThreadSafeLayoutNode>::ConcreteNode as TNode>::ConcreteElement;
 
-    #[inline]
-    fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str>;
-
     fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue>;
 
     fn style_data(&self) -> AtomicRef<ElementData>;
@@ -376,7 +373,7 @@ pub trait ThreadSafeLayoutElement: Clone + Copy + Sized + Debug +
     fn get_details_content_pseudo(&self) -> Option<Self> {
         if self.get_local_name() == &local_name!("details") &&
            self.get_namespace() == &ns!(html) {
-            let display = if self.get_attr(&ns!(), &local_name!("open")).is_some() {
+            let display = if self.get_attr_enum(&ns!(), &local_name!("open")).is_some() {
                 None // Specified by the stylesheet
             } else {
                 Some(display::T::none)

components/style/attr.rs

@@ -48,20 +48,7 @@ pub enum AttrValue {
     Dimension(String, LengthOrPercentageOrAuto),
     Url(String, Option<ServoUrl>),
 
-    /// Note that this variant is only used transitively as a fast path to set
-    /// the property declaration block relevant to the style of an element when
-    /// set from the inline declaration of that element (that is,
-    /// `element.style`).
-    ///
-    /// This can, as of this writing, only correspond to the value of the
-    /// `style` element, and is set from its relevant CSSInlineStyleDeclaration,
-    /// and then converted to a string in Element::attribute_mutated.
-    ///
-    /// Note that we don't necessarily need to do that (we could just clone the
-    /// declaration block), but that avoids keeping a refcounted
-    /// declarationblock for longer than needed.
-    Declaration(String,
-                #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
+    Declaration(#[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
                 Arc<Locked<PropertyDeclarationBlock>>)
 }
 
@@ -283,6 +270,18 @@ impl AttrValue {
         }
     }
 
+    /// Assumes the `AttrValue` is a `String` and returns its value
+    ///
+    /// ## Panics
+    ///
+    /// Panics if the `AttrValue` is not a `String`
+    pub fn as_string(&self) -> &str {
+        match *self {
+            AttrValue::String(ref s) => &s,
+            _ => panic!("String not found"),
+        }
+    }
+
     /// Assumes the `AttrValue` is a `Length` and returns its value
     ///
     /// ## Panics
@@ -351,18 +350,20 @@ impl AttrValue {
         }
     }
 
-    pub fn eval_selector(&self, selector: &AttrSelectorOperation<&String>) -> bool {
+    pub fn eval_selector<F>(&self, selector: &AttrSelectorOperation<&String>,
+                            serialize_declaration_block: &mut F)
+                            -> bool
+                            where F: FnMut(&Locked<PropertyDeclarationBlock>) -> String {
         // FIXME(SimonSapin) this can be more efficient by matching on `(self, selector)` variants
         // and doing Atom comparisons instead of string comparisons where possible,
         // with SelectorImpl::AttrValue changed to Atom.
-        selector.eval_str(self)
+        let serialization = self.serialize(serialize_declaration_block);
+        selector.eval_str(&serialization)
     }
-}
 
-impl ::std::ops::Deref for AttrValue {
-    type Target = str;
-
-    fn deref(&self) -> &str {
+    /// Serializes this attribute value into its string form.
+    pub fn serialize<F>(&self, serialize_declaration_block: &mut F) -> String
+    where F: FnMut(&Locked<PropertyDeclarationBlock>) -> String {
         match *self {
             AttrValue::String(ref value) |
                 AttrValue::TokenList(ref value, _) |
@@ -372,18 +373,9 @@ impl ::std::ops::Deref for AttrValue {
                 AttrValue::Color(ref value, _) |
                 AttrValue::Int(ref value, _) |
                 AttrValue::Url(ref value, _) |
-                AttrValue::Declaration(ref value, _) |
-                AttrValue::Dimension(ref value, _) => &value,
-            AttrValue::Atom(ref value) => &value,
-        }
-    }
-}
-
-impl PartialEq<Atom> for AttrValue {
-    fn eq(&self, other: &Atom) -> bool {
-        match *self {
-            AttrValue::Atom(ref value) => value == other,
-            _ => other == &**self,
+                AttrValue::Dimension(ref value, _) => value.clone(),
+            AttrValue::Atom(ref value) => String::from(&**value),
+            AttrValue::Declaration(ref block) => serialize_declaration_block(block),
         }
     }
 }

components/style/gecko/snapshot.rs

@@ -15,6 +15,7 @@ use gecko_bindings::structs::ServoElementSnapshotFlags as Flags;
 use gecko_bindings::structs::ServoElementSnapshotTable;
 use invalidation::element::element_wrapper::ElementSnapshot;
 use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity, NamespaceConstraint};
+use shared_lock::SharedRwLockReadGuard;
 use string_cache::{Atom, Namespace};
 
 /// A snapshot of a Gecko element.
@@ -85,7 +86,8 @@ impl GeckoElementSnapshot {
     pub fn attr_matches(&self,
                         ns: &NamespaceConstraint<&Namespace>,
                         local_name: &Atom,
-                        operation: &AttrSelectorOperation<&Atom>)
+                        operation: &AttrSelectorOperation<&Atom>,
+                        _: &SharedRwLockReadGuard)
                         -> bool {
         unsafe {
             match *operation {

components/style/invalidation/element/element_wrapper.rs

@@ -6,9 +6,10 @@
 //! against a past state of the element.
 
 use {Atom, CaseSensitivityExt, LocalName, Namespace};
+use context::SharedStyleContext;
 use dom::TElement;
 use element_state::ElementState;
-use selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl, Snapshot, SnapshotMap, AttrValue};
+use selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl, Snapshot, AttrValue};
 use selectors::Element;
 use selectors::attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint};
 use selectors::matching::{ElementSelectorFlags, LocalMatchingContext, MatchingContext};
@@ -68,18 +69,18 @@ pub struct ElementWrapper<'a, E>
 {
     element: E,
     cached_snapshot: Cell<Option<&'a Snapshot>>,
-    snapshot_map: &'a SnapshotMap,
+    shared_context: &'a SharedStyleContext<'a>,
 }
 
 impl<'a, E> ElementWrapper<'a, E>
     where E: TElement,
 {
     /// Trivially constructs an `ElementWrapper`.
-    pub fn new(el: E, snapshot_map: &'a SnapshotMap) -> Self {
+    pub fn new(el: E, shared_context: &'a SharedStyleContext) -> Self {
         ElementWrapper {
             element: el,
             cached_snapshot: Cell::new(None),
-            snapshot_map: snapshot_map,
+            shared_context: shared_context,
         }
     }
 
@@ -93,7 +94,7 @@ impl<'a, E> ElementWrapper<'a, E>
             return Some(s);
         }
 
-        let snapshot = self.snapshot_map.get(&self.element);
+        let snapshot = self.shared_context.snapshot_map.get(&self.element);
         debug_assert!(snapshot.is_some(), "has_snapshot lied!");
 
         self.cached_snapshot.set(snapshot);
@@ -260,27 +261,27 @@ impl<'a, E> Element for ElementWrapper<'a, E>
 
     fn parent_element(&self) -> Option<Self> {
         self.element.parent_element()
-            .map(|e| ElementWrapper::new(e, self.snapshot_map))
+            .map(|e| ElementWrapper::new(e, self.shared_context))
     }
 
     fn first_child_element(&self) -> Option<Self> {
         self.element.first_child_element()
-            .map(|e| ElementWrapper::new(e, self.snapshot_map))
+            .map(|e| ElementWrapper::new(e, self.shared_context))
     }
 
     fn last_child_element(&self) -> Option<Self> {
         self.element.last_child_element()
-            .map(|e| ElementWrapper::new(e, self.snapshot_map))
+            .map(|e| ElementWrapper::new(e, self.shared_context))
     }
 
     fn prev_sibling_element(&self) -> Option<Self> {
         self.element.prev_sibling_element()
-            .map(|e| ElementWrapper::new(e, self.snapshot_map))
+            .map(|e| ElementWrapper::new(e, self.shared_context))
     }
 
     fn next_sibling_element(&self) -> Option<Self> {
         self.element.next_sibling_element()
-            .map(|e| ElementWrapper::new(e, self.snapshot_map))
+            .map(|e| ElementWrapper::new(e, self.shared_context))
     }
 
     fn is_html_element_in_html_document(&self) -> bool {
@@ -302,7 +303,7 @@ impl<'a, E> Element for ElementWrapper<'a, E>
                     -> bool {
         match self.snapshot() {
             Some(snapshot) if snapshot.has_attrs() => {
-                snapshot.attr_matches(ns, local_name, operation)
+                snapshot.attr_matches(ns, local_name, operation, self.shared_context.guards.author)
             }
             _ => self.element.attr_matches(ns, local_name, operation)
         }
@@ -336,6 +337,6 @@ impl<'a, E> Element for ElementWrapper<'a, E>
 
     fn pseudo_element_originating_element(&self) -> Option<Self> {
         self.element.closest_non_native_anonymous_ancestor()
-            .map(|e| ElementWrapper::new(e, self.snapshot_map))
+            .map(|e| ElementWrapper::new(e, self.shared_context))
     }
 }

components/style/invalidation/element/invalidator.rs

@@ -97,7 +97,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
         let shared_context = self.shared_context;
 
         let wrapper =
-            ElementWrapper::new(self.element, shared_context.snapshot_map);
+            ElementWrapper::new(self.element, shared_context);
         let state_changes = wrapper.state_changes();
         let snapshot = wrapper.snapshot().expect("has_snapshot lied");
 

components/style/servo/selector_parser.rs

@@ -13,18 +13,20 @@ use dom::{OpaqueNode, TElement, TNode};
 use element_state::ElementState;
 use fnv::FnvHashMap;
 use invalidation::element::element_wrapper::ElementSnapshot;
+use properties::PropertyDeclarationBlock;
 use selector_parser::{AttrValue as SelectorAttrValue, ElementExt, PseudoElementCascadeType, SelectorParser};
 use selectors::Element;
 use selectors::attr::{AttrSelectorOperation, NamespaceConstraint, CaseSensitivity};
 use selectors::parser::{SelectorMethods, SelectorParseError};
 use selectors::visitor::SelectorVisitor;
+use shared_lock::{Locked, SharedRwLockReadGuard};
 use std::ascii::AsciiExt;
 use std::borrow::Cow;
 use std::fmt;
 use std::fmt::Debug;
 use std::mem;
 use std::ops::{Deref, DerefMut};
-use style_traits::{ParseError, StyleParseError};
+use style_traits::{ParseError, StyleParseError, ToCss as ServoToCss};
 
 /// A pseudo-element, both public and private.
 ///
@@ -626,7 +628,7 @@ impl ElementSnapshot for ServoElementSnapshot {
     fn lang_attr(&self) -> Option<SelectorAttrValue> {
         self.get_attr(&ns!(xml), &local_name!("lang"))
             .or_else(|| self.get_attr(&ns!(), &local_name!("lang")))
-            .map(|v| String::from(v as &str))
+            .map(|v| v.as_string().to_owned())
     }
 }
 
@@ -635,15 +637,19 @@ impl ServoElementSnapshot {
     pub fn attr_matches(&self,
                         ns: &NamespaceConstraint<&Namespace>,
                         local_name: &LocalName,
-                        operation: &AttrSelectorOperation<&String>)
+                        operation: &AttrSelectorOperation<&String>,
+                        guard: &SharedRwLockReadGuard)
                         -> bool {
+        let mut serialize = |block: &Locked<PropertyDeclarationBlock>| {
+            block.read_with(guard).to_css_string()
+        };
         match *ns {
             NamespaceConstraint::Specific(ref ns) => {
                 self.get_attr(ns, local_name)
-                    .map_or(false, |value| value.eval_selector(operation))
+                    .map_or(false, |value| value.eval_selector(operation, &mut serialize))
             }
             NamespaceConstraint::Any => {
-                self.any_attr_ignore_ns(local_name, |value| value.eval_selector(operation))
+                self.any_attr_ignore_ns(local_name, |value| value.eval_selector(operation, &mut serialize))
             }
         }
     }