Implement attribute restyle hints.

Fixes #6942.

components/layout/layout_task.rs

@@ -1145,10 +1145,8 @@ impl LayoutTask {
 
         let modified_elements = document.drain_modified_elements();
         if !needs_dirtying {
-            for &(el, old_state) in modified_elements.iter() {
-                let hint = rw_data.stylist.restyle_hint_for_state_change(&el,
-                                                                         el.get_state(),
-                                                                         old_state);
+            for (el, snapshot) in modified_elements {
+                let hint = rw_data.stylist.compute_restyle_hint(&el, &snapshot, el.get_state());
                 el.note_restyle_hint(hint);
             }
         }

components/layout/wrapper.rs

@@ -58,7 +58,6 @@ use selectors::states::*;
 use smallvec::VecLike;
 use std::borrow::ToOwned;
 use std::cell::{Ref, RefMut};
-use std::iter::FromIterator;
 use std::marker::PhantomData;
 use std::mem;
 use std::sync::Arc;
@@ -69,7 +68,7 @@ use style::legacy::UnsignedIntegerAttribute;
 use style::node::TElementAttributes;
 use style::properties::ComputedValues;
 use style::properties::{PropertyDeclaration, PropertyDeclarationBlock};
-use style::restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint};
+use style::restyle_hints::{ElementSnapshot, RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint};
 use url::Url;
 use util::str::{is_whitespace, search_index};
 
@@ -375,15 +374,13 @@ impl<'le> LayoutDocument<'le> {
         self.as_node().children().find(LayoutNode::is_element)
     }
 
-    pub fn drain_modified_elements(&self) -> Vec<(LayoutElement, ElementState)> {
-        unsafe {
-            let elements = self.document.drain_modified_elements();
-            Vec::from_iter(elements.iter().map(|&(el, state)|
-                (LayoutElement {
-                    element: el,
-                    chain: PhantomData,
-                }, state)))
-        }
+    pub fn drain_modified_elements(&self) -> Vec<(LayoutElement, ElementSnapshot)> {
+        let elements =  unsafe { self.document.drain_modified_elements() };
+        elements.into_iter().map(|(el, snapshot)|
+            (LayoutElement {
+                element: el,
+                chain: PhantomData,
+            }, snapshot)).collect()
     }
 }
 

components/script/dom/attr.rs

@@ -148,13 +148,18 @@ impl AttrMethods for Attr {
 impl Attr {
     pub fn set_value(&self, mut value: AttrValue, owner: &Element) {
         assert!(Some(owner) == self.owner().r());
+        owner.will_mutate_attr();
         mem::swap(&mut *self.value.borrow_mut(), &mut value);
         if self.identifier.namespace == ns!("") {
             vtable_for(owner.upcast()).attribute_mutated(
                 self, AttributeMutation::Set(Some(&value)));
         }
     }
 
+    pub fn identifier(&self) -> &AttrIdentifier {
+        &self.identifier
+    }
+
     pub fn value(&self) -> Ref<AttrValue> {
         self.value.borrow()
     }

components/script/dom/bindings/trace.rs

@@ -84,6 +84,7 @@ use std::sync::mpsc::{Receiver, Sender};
 use string_cache::{Atom, Namespace, QualName};
 use style::attr::{AttrIdentifier, AttrValue};
 use style::properties::PropertyDeclarationBlock;
+use style::restyle_hints::ElementSnapshot;
 use style::values::specified::Length;
 use url::Url;
 use util::str::{DOMString, LengthOrPercentageOrAuto};
@@ -292,6 +293,7 @@ no_jsmanaged_fields!(DOMString);
 no_jsmanaged_fields!(Mime);
 no_jsmanaged_fields!(AttrIdentifier);
 no_jsmanaged_fields!(AttrValue);
+no_jsmanaged_fields!(ElementSnapshot);
 
 impl JSTraceable for Box<ScriptChan + Send> {
     #[inline]

components/script/dom/document.rs

@@ -88,7 +88,6 @@ use net_traits::{AsyncResponseTarget, PendingAsyncLoad};
 use num::ToPrimitive;
 use script_task::{MainThreadScriptMsg, Runnable};
 use script_traits::{MouseButton, TouchEventType, TouchId, UntrustedNodeAddress};
-use selectors::states::*;
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
 use std::boxed::FnBox;
@@ -102,6 +101,7 @@ use std::rc::Rc;
 use std::sync::Arc;
 use std::sync::mpsc::channel;
 use string_cache::{Atom, QualName};
+use style::restyle_hints::ElementSnapshot;
 use style::stylesheets::Stylesheet;
 use time;
 use url::Url;
@@ -188,8 +188,9 @@ pub struct Document {
     /// This field is set to the document itself for inert documents.
     /// https://html.spec.whatwg.org/multipage/#appropriate-template-contents-owner-document
     appropriate_template_contents_owner_document: MutNullableHeap<JS<Document>>,
-    /// For each element that has had a state change since the last restyle, track the original state.
-    modified_elements: DOMRefCell<HashMap<JS<Element>, ElementState>>,
+    /// For each element that has had a state or attribute change since the last restyle,
+    /// track the original condition of the element.
+    modified_elements: DOMRefCell<HashMap<JS<Element>, ElementSnapshot>>,
     /// http://w3c.github.io/touch-events/#dfn-active-touch-point
     active_touch_points: DOMRefCell<Vec<JS<Touch>>>,
 }
@@ -1275,7 +1276,7 @@ pub enum DocumentSource {
 #[allow(unsafe_code)]
 pub trait LayoutDocumentHelpers {
     unsafe fn is_html_document_for_layout(&self) -> bool;
-    unsafe fn drain_modified_elements(&self) -> Vec<(LayoutJS<Element>, ElementState)>;
+    unsafe fn drain_modified_elements(&self) -> Vec<(LayoutJS<Element>, ElementSnapshot)>;
 }
 
 #[allow(unsafe_code)]
@@ -1287,7 +1288,7 @@ impl LayoutDocumentHelpers for LayoutJS<Document> {
 
     #[inline]
     #[allow(unrooted_must_root)]
-    unsafe fn drain_modified_elements(&self) -> Vec<(LayoutJS<Element>, ElementState)> {
+    unsafe fn drain_modified_elements(&self) -> Vec<(LayoutJS<Element>, ElementSnapshot)> {
         let mut elements = (*self.unsafe_get()).modified_elements.borrow_mut_for_layout();
         let drain = elements.drain();
         let layout_drain = drain.map(|(k, v)| (k.to_layout(), v));
@@ -1457,7 +1458,21 @@ impl Document {
 
     pub fn element_state_will_change(&self, el: &Element) {
         let mut map = self.modified_elements.borrow_mut();
-        map.entry(JS::from_ref(el)).or_insert(el.get_state());
+        let snapshot = map.entry(JS::from_ref(el)).or_insert(ElementSnapshot::new());
+        if snapshot.state.is_none() {
+            snapshot.state = Some(el.get_state());
+        }
+    }
+
+    pub fn element_attr_will_change(&self, el: &Element) {
+        let mut map = self.modified_elements.borrow_mut();
+        let mut snapshot = map.entry(JS::from_ref(el)).or_insert(ElementSnapshot::new());
+        if snapshot.attrs.is_none() {
+            let attrs = el.attrs().iter()
+                          .map(|attr| (attr.identifier().clone(), attr.value().clone()))
+                          .collect();
+            snapshot.attrs = Some(attrs);
+        }
     }
 }
 

components/script/dom/element.rs

@@ -65,6 +65,7 @@ use html5ever::serialize::TraversalScope;
 use html5ever::serialize::TraversalScope::{ChildrenOnly, IncludeNode};
 use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks};
 use selectors::matching::{DeclarationBlock, matches};
+use selectors::matching::{common_style_affecting_attributes, rare_style_affecting_attributes};
 use selectors::parser::{AttrSelector, NamespaceConstraint, parse_author_origin_selector_list_from_str};
 use selectors::states::*;
 use smallvec::VecLike;
@@ -851,6 +852,7 @@ impl Element {
                               name: Atom,
                               namespace: Namespace,
                               prefix: Option<Atom>) {
+        self.will_mutate_attr();
         let window = window_from_node(self);
         let in_empty_ns = namespace == ns!("");
         let attr = Attr::new(&window, local_name, value, name, namespace, prefix, Some(self));
@@ -963,6 +965,7 @@ impl Element {
         let idx = self.attrs.borrow().iter().position(|attr| find(&attr));
 
         idx.map(|idx| {
+            self.will_mutate_attr();
             let attr = Root::from_ref(&*(*self.attrs.borrow())[idx]);
             self.attrs.borrow_mut().remove(idx);
             attr.set_owner(None);
@@ -1075,6 +1078,11 @@ impl Element {
         assert!(&**local_name == local_name.to_ascii_lowercase());
         self.set_attribute(local_name, AttrValue::UInt(DOMString(value.to_string()), value));
     }
+
+    pub fn will_mutate_attr(&self) {
+        let node = self.upcast::<Node>();
+        node.owner_doc().element_attr_will_change(self);
+    }
 }
 
 impl ElementMethods for Element {
@@ -1459,6 +1467,10 @@ impl ElementMethods for Element {
     }
 }
 
+pub fn fragment_affecting_attributes() -> [Atom; 3] {
+    [atom!("width"), atom!("height"), atom!("src")]
+}
+
 impl VirtualMethods for Element {
     fn super_type(&self) -> Option<&VirtualMethods> {
         Some(self.upcast::<Node>() as &VirtualMethods)
@@ -1468,18 +1480,16 @@ impl VirtualMethods for Element {
         self.super_type().unwrap().attribute_mutated(attr, mutation);
         let node = self.upcast::<Node>();
         let doc = node.owner_doc();
-        let damage = match attr.local_name() {
+        match attr.local_name() {
             &atom!(style) => {
                 // Modifying the `style` attribute might change style.
                 *self.style_attribute.borrow_mut() =
                     mutation.new_value(attr).map(|value| {
                         parse_style_attribute(&value, &doc.base_url())
                     });
-                NodeDamage::NodeStyleDamaged
-            },
-            &atom!(class) => {
-                // Modifying a class can change style.
-                NodeDamage::NodeStyleDamaged
+                if node.is_in_doc() {
+                    doc.content_changed(node, NodeDamage::NodeStyleDamaged);
+                }
             },
             &atom!(id) => {
                 *self.id_attribute.borrow_mut() =
@@ -1510,16 +1520,22 @@ impl VirtualMethods for Element {
                         }
                     }
                 }
-                NodeDamage::NodeStyleDamaged
             },
-            _ => {
-                // Modifying any other attribute might change arbitrary things.
-                NodeDamage::OtherNodeDamage
+            _ if attr.namespace() == &ns!("") => {
+                if fragment_affecting_attributes().iter().any(|a| a == attr.local_name()) ||
+                   common_style_affecting_attributes().iter().any(|a| &a.atom == attr.local_name()) ||
+                   rare_style_affecting_attributes().iter().any(|a| a == attr.local_name())
+                {
+                    doc.content_changed(node, NodeDamage::OtherNodeDamage);
+                }
             },
+            _ => {},
         };
-        if node.is_in_doc() {
-            doc.content_changed(node, damage);
-        }
+
+        // Make sure we rev the version even if we didn't dirty the node. If we
+        // don't do this, various attribute-dependent htmlcollections (like those
+        // generated by getElementsByClassName) might become stale.
+        node.rev_version();
     }
 
     fn parse_plain_attribute(&self, name: &Atom, value: DOMString) -> AttrValue {

components/script/dom/node.rs

@@ -488,13 +488,7 @@ impl Node {
         self.dirty_impl(damage, true)
     }
 
-    pub fn dirty(&self, damage: NodeDamage) {
-        self.dirty_impl(damage, false)
-    }
-
-    pub fn dirty_impl(&self, damage: NodeDamage, force_ancestors: bool) {
-
-        // 0. Set version counter
+    pub fn rev_version(&self) {
         // The new version counter is 1 plus the max of the node's current version counter,
         // its descendants version, and the document's version. Normally, this will just be
         // the document's version, but we do have to deal with the case where the node has moved
@@ -505,6 +499,15 @@ impl Node {
             ancestor.inclusive_descendants_version.set(version);
         }
         doc.inclusive_descendants_version.set(version);
+    }
+
+    pub fn dirty(&self, damage: NodeDamage) {
+        self.dirty_impl(damage, false)
+    }
+
+    pub fn dirty_impl(&self, damage: NodeDamage, force_ancestors: bool) {
+        // 0. Set version counter
+        self.rev_version();
 
         // 1. Dirty self.
         match damage {