Track all node damage with PendingRestyles.

components/layout_thread/lib.rs

@@ -1084,11 +1084,8 @@ impl LayoutThread {
                         // NB: The dirty bit is propagated down the tree.
                         unsafe { node.set_dirty(); }
 
-                        let mut current = node.parent_node().and_then(|n| n.as_element());
+                        if let Some(p) = node.parent_node().and_then(|n| n.as_element()) {
-                        while let Some(el) = current {
+                            unsafe { p.note_dirty_descendant() };
-                            if el.has_dirty_descendants() { break; }
-                            unsafe { el.set_dirty_descendants(); }
-                            current = el.parent_element();
                         }
 
                         next = iter.next_skipping_children();
@@ -1120,8 +1117,16 @@ impl LayoutThread {
         let restyles = document.drain_pending_restyles();
         if !needs_dirtying {
             for (el, restyle) in restyles {
+                // Propagate the descendant bit up the ancestors. Do this before
+                // the restyle calculation so that we can also do it for new
+                // unstyled nodes, which the descendants bit helps us find.
+                if let Some(parent) = el.parent_element() {
+                    unsafe { parent.note_dirty_descendant() };
+                }
+
                 if el.get_data().is_none() {
-                    // If we haven't styled this node yet, we can ignore the restyle.
+                    // If we haven't styled this node yet, we don't need to track
+                    // a restyle.
                     continue;
                 }
 
@@ -1143,15 +1148,6 @@ impl LayoutThread {
                     let mut d = el.mutate_layout_data().unwrap();
                     d.base.restyle_damage |= restyle.damage;
                 }
-
-                // Propagate the descendant bit up the ancestors.
-                if !hint.is_empty() || !restyle.damage.is_empty() {
-                    let curr = el;
-                    while let Some(curr) = curr.parent_element() {
-                        if curr.has_dirty_descendants() { break }
-                        unsafe { curr.set_dirty_descendants(); }
-                    }
-                }
             }
         }
 

components/script/dom/document.rs

@@ -477,7 +477,7 @@ impl Document {
     }
 
     pub fn content_and_heritage_changed(&self, node: &Node, damage: NodeDamage) {
-        node.force_dirty_ancestors(damage);
+        node.dirty(damage);
     }
 
     /// Reflows and disarms the timer if the reflow timer has expired.
@@ -1990,12 +1990,12 @@ impl Document {
         self.id_map.borrow().get(&id).map(|ref elements| Root::from_ref(&*(*elements)[0]))
     }
 
-    fn ensure_pending_restyle(&self, el: &Element) -> RefMut<PendingRestyle> {
+    pub fn ensure_pending_restyle(&self, el: &Element) -> RefMut<PendingRestyle> {
         let map = self.pending_restyles.borrow_mut();
         RefMut::map(map, |m| m.entry(JS::from_ref(el)).or_insert_with(PendingRestyle::new))
     }
 
-    fn ensure_snapshot(&self, el: &Element) -> RefMut<Snapshot> {
+    pub fn ensure_snapshot(&self, el: &Element) -> RefMut<Snapshot> {
         let mut entry = self.ensure_pending_restyle(el);
         if entry.snapshot.is_none() {
             entry.snapshot = Some(Snapshot::new(el.html_element_in_html_document()));

components/script/dom/element.rs

@@ -84,13 +84,15 @@ use std::fmt;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicUsize, Ordering};
 use style::attr::{AttrValue, LengthOrPercentageOrAuto};
+use style::dom::TRestyleDamage;
 use style::element_state::*;
 use style::matching::{common_style_affecting_attributes, rare_style_affecting_attributes};
 use style::parser::ParserContextExtraData;
 use style::properties::{DeclaredValue, Importance};
 use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, parse_style_attribute};
 use style::properties::longhands::{background_image, border_spacing, font_family, font_size, overflow_x};
-use style::selector_impl::{NonTSPseudoClass, ServoSelectorImpl};
+use style::restyle_hints::RESTYLE_SELF;
+use style::selector_impl::{NonTSPseudoClass, RestyleDamage, ServoSelectorImpl};
 use style::selector_matching::ApplicableDeclarationBlock;
 use style::sink::Push;
 use style::values::CSSFloat;
@@ -201,6 +203,19 @@ impl Element {
             ElementBinding::Wrap)
     }
 
+    pub fn restyle(&self, damage: NodeDamage) {
+        let doc = self.node.owner_doc();
+        let mut restyle = doc.ensure_pending_restyle(self);
+
+        // FIXME(bholley): I think we should probably only do this for
+        // NodeStyleDamaged, but I'm preserving existing behavior.
+        restyle.hint |= RESTYLE_SELF;
+
+        if damage == NodeDamage::OtherNodeDamage {
+            restyle.damage = RestyleDamage::rebuild_and_reflow();
+        }
+    }
+
     // https://drafts.csswg.org/cssom-view/#css-layout-box
     // Elements that have a computed value of the display property
     // that is table-column or table-column-group

components/script/dom/node.rs

@@ -446,10 +446,6 @@ impl Node {
         self.set_flag(HAS_DIRTY_DESCENDANTS, state)
     }
 
-    pub fn force_dirty_ancestors(&self, damage: NodeDamage) {
-        self.dirty_impl(damage, true)
-    }
-
     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
@@ -464,30 +460,18 @@ impl Node {
     }
 
     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();
-
+        if !self.is_in_doc() {
-        // 1. Dirty self.
+            return;
-        match damage {
-            NodeDamage::NodeStyleDamaged => {}
-            NodeDamage::OtherNodeDamage => self.set_has_changed(true),
         }
 
-        if self.is_dirty() && !force_ancestors {
+        match self.type_id() {
-            return
+            NodeTypeId::CharacterData(CharacterDataTypeId::Text) =>
-        }
+                self.parent_node.get().unwrap().downcast::<Element>().unwrap().restyle(damage),
-
+            NodeTypeId::Element(_) =>
-        self.set_flag(IS_DIRTY, true);
+                self.downcast::<Element>().unwrap().restyle(damage),
-
+            _ => {},
-        // 4. Dirty ancestors.
+        };
-        for ancestor in self.ancestors() {
-            if !force_ancestors && ancestor.has_dirty_descendants() { break }
-            ancestor.set_has_dirty_descendants(true);
-        }
     }
 
     /// The maximum version number of this node's descendants, including itself

components/script/layout_wrapper.rs

@@ -529,6 +529,35 @@ impl<'le> ServoLayoutElement<'le> {
             self.get_style_and_layout_data().map(|d| &**d.ptr)
         }
     }
+
+    pub unsafe fn note_dirty_descendant(&self) {
+        use ::selectors::Element;
+
+        let mut current = Some(*self);
+        while let Some(el) = current {
+            // FIXME(bholley): Ideally we'd have the invariant that any element
+            // with has_dirty_descendants also has the bit set on all its ancestors.
+            // However, there are currently some corner-cases where we get that wrong.
+            // I have in-flight patches to fix all this stuff up, so we just always
+            // propagate this bit for now.
+            el.set_dirty_descendants();
+            current = el.parent_element();
+        }
+
+        debug_assert!(self.dirty_descendants_bit_is_propagated());
+    }
+
+    fn dirty_descendants_bit_is_propagated(&self) -> bool {
+        use ::selectors::Element;
+
+        let mut current = Some(*self);
+        while let Some(el) = current {
+            if !el.has_dirty_descendants() { return false; }
+            current = el.parent_element();
+        }
+
+        true
+    }
 }
 
 fn as_element<'le>(node: LayoutJS<Node>) -> Option<ServoLayoutElement<'le>> {