Hoist RestyleDamage onto TElement.

Instead of maintaining a dummy restyle damage field for text nodes, we can just
perform the necessary parent inheritance and is_changed adjustments on the fly
in ThreadSafeLayoutNode, simplifying the requirements on the style system.

MozReview-Commit-ID: DCqiCRLsLUF

components/layout/traversal.rs

@@ -16,7 +16,7 @@ use std::mem;
 use style::atomic_refcell::AtomicRefCell;
 use style::context::{LocalStyleContext, SharedStyleContext, StyleContext};
 use style::data::NodeData;
-use style::dom::{TNode, TRestyleDamage};
+use style::dom::TNode;
 use style::selector_impl::ServoSelectorImpl;
 use style::traversal::{DomTraversalContext, recalc_style_at, remove_from_bloom_filter};
 use style::traversal::RestyleResult;
@@ -111,7 +111,7 @@ fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: O
         // Always reconstruct if incremental layout is turned off.
         let nonincremental_layout = opts::get().nonincremental_layout;
         if nonincremental_layout || node.has_dirty_descendants() ||
-           node.restyle_damage() != N::ConcreteRestyleDamage::empty() {
+           tnode.restyle_damage() != RestyleDamage::empty() {
             let mut flow_constructor = FlowConstructor::new(context);
             if nonincremental_layout || !flow_constructor.repair_if_possible(&tnode) {
                 flow_constructor.process(&tnode);
@@ -121,9 +121,7 @@ fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: O
             }
         }
 
-        // Reset the layout damage in this node. It's been propagated to the
+        tnode.clear_restyle_damage();
-        // flow by the flow constructor.
-        tnode.set_restyle_damage(RestyleDamage::empty());
     }
 
     unsafe { node.clear_dirty_bits(); }

components/script/layout_wrapper.rs

@@ -236,41 +236,12 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
         debug_assert!(self.is_text_node());
         let mut data = self.get_partial_layout_data().unwrap().borrow_mut();
         data.style_data.style_text_node(style);
-        if self.has_changed() {
-            data.restyle_damage = RestyleDamage::rebuild_and_reflow();
-        } else {
-            // FIXME(bholley): This is necessary to make it correct to use restyle
-            // damage in construct_flows_at to determine whether to reconstruct
-            // text nodes. Without it, we fail cascade-import-dynamic-002.htm.
-            //
-            // Long-term, We should teach layout how to correctly propagate
-            // style changes from elements to child text nodes so that we don't
-            // need to do this explicitly here. This will likely all be rolled
-            // into a patch where we stop styling text nodes from the style
-            // system and instead generate the styles on the fly during frame
-            // construction / repair.
-            let parent = self.parent_node().unwrap();
-            let parent_data = parent.get_partial_layout_data().unwrap().borrow();
-            data.restyle_damage = parent_data.restyle_damage;
-        }
     }
 
     fn borrow_data(&self) -> Option<AtomicRef<NodeData>> {
         self.get_style_data().map(|d| d.borrow())
     }
 
-    fn restyle_damage(self) -> RestyleDamage {
-        self.get_partial_layout_data().unwrap().borrow().restyle_damage
-    }
-
-    fn set_restyle_damage(self, damage: RestyleDamage) {
-        let mut damage = damage;
-        if self.has_changed() {
-            damage = RestyleDamage::rebuild_and_reflow();
-        }
-        self.get_partial_layout_data().unwrap().borrow_mut().restyle_damage = damage;
-    }
-
     fn parent_node(&self) -> Option<ServoLayoutNode<'ln>> {
         unsafe {
             self.node.parent_node_ref().map(|node| self.new_with_this_lifetime(&node))
@@ -300,14 +271,6 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
             self.node.next_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
         }
     }
-
-    #[inline]
-    fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             current_cv: Option<&'a Arc<ComputedValues>>,
-                                             _pseudo_element: Option<&PseudoElement>)
-                                             -> Option<&'a Arc<ComputedValues>> {
-        current_cv
-    }
 }
 
 pub struct ServoChildrenIterator<'a> {
@@ -531,6 +494,19 @@ impl<'le> TElement for ServoLayoutElement<'le> {
     fn attr_equals(&self, namespace: &Namespace, attr: &Atom, val: &Atom) -> bool {
         self.get_attr(namespace, attr).map_or(false, |x| x == val)
     }
+
+    fn set_restyle_damage(self, damage: RestyleDamage) {
+        let node = self.as_node();
+        node.get_partial_layout_data().unwrap().borrow_mut().restyle_damage = damage;
+    }
+
+    #[inline]
+    fn existing_style_for_restyle_damage<'a>(&'a self,
+                                             current_cv: Option<&'a Arc<ComputedValues>>,
+                                             _pseudo_element: Option<&PseudoElement>)
+                                             -> Option<&'a Arc<ComputedValues>> {
+        current_cv
+    }
 }
 
 impl<'le> PartialEq for ServoLayoutElement<'le> {
@@ -885,11 +861,22 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
     }
 
     fn restyle_damage(self) -> RestyleDamage {
-        self.node.restyle_damage()
+        if self.node.has_changed() {
+            RestyleDamage::rebuild_and_reflow()
+        } else if self.is_text_node() {
+            let parent = self.node.parent_node().unwrap();
+            let parent_data = parent.get_partial_layout_data().unwrap().borrow();
+            parent_data.restyle_damage
+        } else {
+            self.node.get_partial_layout_data().unwrap().borrow().restyle_damage
+        }
     }
 
-    fn set_restyle_damage(self, damage: RestyleDamage) {
+    fn clear_restyle_damage(self) {
-        self.node.set_restyle_damage(damage)
+        if self.is_element() {
+            let mut data = self.node.get_partial_layout_data().unwrap().borrow_mut();
+            data.restyle_damage = RestyleDamage::empty();
+        }
     }
 
     fn can_be_fragmented(&self) -> bool {

components/script_layout_interface/wrapper_traits.rs

@@ -353,7 +353,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + NodeInfo + PartialEq + Sized {
 
     fn restyle_damage(self) -> RestyleDamage;
 
-    fn set_restyle_damage(self, damage: RestyleDamage);
+    fn clear_restyle_damage(self);
 
     /// Returns true if this node contributes content. This is used in the implementation of
     /// `empty_cells` per CSS 2.1 § 17.6.1.1.

components/style/dom.rs

@@ -215,12 +215,6 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo {
     /// Immutable borrows the NodeData.
     fn borrow_data(&self) -> Option<AtomicRef<NodeData>>;
 
-    /// Get the description of how to account for recent style changes.
-    fn restyle_damage(self) -> Self::ConcreteRestyleDamage;
-
-    /// Set the restyle damage field.
-    fn set_restyle_damage(self, damage: Self::ConcreteRestyleDamage);
-
     fn parent_node(&self) -> Option<Self>;
 
     fn first_child(&self) -> Option<Self>;
@@ -230,14 +224,6 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo {
     fn prev_sibling(&self) -> Option<Self>;
 
     fn next_sibling(&self) -> Option<Self>;
-
-    /// XXX: It's a bit unfortunate we need to pass the current computed values
-    /// as an argument here, but otherwise Servo would crash due to double
-    /// borrows to return it.
-    fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             current_computed_values: Option<&'a Arc<ComputedValues>>,
-                                             pseudo: Option<&PseudoElement>)
-        -> Option<&'a <Self::ConcreteRestyleDamage as TRestyleDamage>::PreExistingComputedValues>;
 }
 
 pub trait TDocument : Sized + Copy + Clone {
@@ -273,6 +259,17 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre
     fn has_attr(&self, namespace: &Namespace, attr: &Atom) -> bool;
     fn attr_equals(&self, namespace: &Namespace, attr: &Atom, value: &Atom) -> bool;
 
+    /// Set the restyle damage field.
+    fn set_restyle_damage(self, damage: <Self::ConcreteNode as TNode>::ConcreteRestyleDamage);
+
+    /// XXX: It's a bit unfortunate we need to pass the current computed values
+    /// as an argument here, but otherwise Servo would crash due to double
+    /// borrows to return it.
+    fn existing_style_for_restyle_damage<'a>(&'a self,
+                                             current_computed_values: Option<&'a Arc<ComputedValues>>,
+                                             pseudo: Option<&PseudoElement>)
+        -> Option<&'a <<Self::ConcreteNode as TNode>::ConcreteRestyleDamage as TRestyleDamage>::PreExistingComputedValues>;
+
     /// Properly marks nodes as dirty in response to restyle hints.
     fn note_restyle_hint<C: DomTraversalContext<Self::ConcreteNode>>(&self, hint: RestyleHint) {
         // Bail early if there's no restyling to do.

components/style/gecko/wrapper.rs

@@ -273,19 +273,6 @@ impl<'ln> TNode for GeckoNode<'ln> {
         self.get_node_data().map(|x| x.borrow())
     }
 
-    fn restyle_damage(self) -> Self::ConcreteRestyleDamage {
-        // Not called from style, only for layout.
-        unimplemented!();
-    }
-
-    fn set_restyle_damage(self, damage: Self::ConcreteRestyleDamage) {
-        // FIXME(bholley): Gecko currently relies on the dirty bit being set to
-        // drive the post-traversal. This will go away soon.
-        unsafe { self.set_flags(NODE_IS_DIRTY_FOR_SERVO as u32) }
-
-        unsafe { Gecko_StoreStyleDifference(self.0, damage.0) }
-    }
-
     fn parent_node(&self) -> Option<GeckoNode<'ln>> {
         unsafe { self.0.mParent.as_ref().map(GeckoNode) }
     }
@@ -306,23 +293,6 @@ impl<'ln> TNode for GeckoNode<'ln> {
         unsafe { self.0.mNextSibling.as_ref().map(GeckoNode::from_content) }
     }
 
-    fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             current_cv: Option<&'a Arc<ComputedValues>>,
-                                             pseudo: Option<&PseudoElement>)
-                                             -> Option<&'a nsStyleContext> {
-        if current_cv.is_none() {
-            // Don't bother in doing an ffi call to get null back.
-            return None;
-        }
-
-        unsafe {
-            let atom_ptr = pseudo.map(|p| p.as_atom().as_ptr())
-                                 .unwrap_or(ptr::null_mut());
-            let context_ptr = Gecko_GetStyleContext(self.0, atom_ptr);
-            context_ptr.as_ref()
-        }
-    }
-
     fn needs_dirty_on_viewport_size_changed(&self) -> bool {
         // Gecko's node doesn't have the DIRTY_ON_VIEWPORT_SIZE_CHANGE flag,
         // so we force them to be dirtied on viewport size change, regardless if
@@ -464,6 +434,31 @@ impl<'le> TElement for GeckoElement<'le> {
                                        /* ignoreCase = */ false)
         }
     }
+
+    fn set_restyle_damage(self, damage: GeckoRestyleDamage) {
+        // FIXME(bholley): Gecko currently relies on the dirty bit being set to
+        // drive the post-traversal. This will go away soon.
+        unsafe { self.as_node().set_flags(NODE_IS_DIRTY_FOR_SERVO as u32) }
+
+        unsafe { Gecko_StoreStyleDifference(self.as_node().0, damage.0) }
+    }
+
+    fn existing_style_for_restyle_damage<'a>(&'a self,
+                                             current_cv: Option<&'a Arc<ComputedValues>>,
+                                             pseudo: Option<&PseudoElement>)
+                                             -> Option<&'a nsStyleContext> {
+        if current_cv.is_none() {
+            // Don't bother in doing an ffi call to get null back.
+            return None;
+        }
+
+        unsafe {
+            let atom_ptr = pseudo.map(|p| p.as_atom().as_ptr())
+                                 .unwrap_or(ptr::null_mut());
+            let context_ptr = Gecko_GetStyleContext(self.as_node().0, atom_ptr);
+            context_ptr.as_ref()
+        }
+    }
 }
 
 impl<'le> PartialEq for GeckoElement<'le> {

components/style/matching.rs

@@ -739,7 +739,7 @@ pub trait ElementMatchMethods : TElement {
                     // can decide more easily if it knows that it's a child of
                     // replaced content, or similar stuff!
                     let damage =
-                        match node.existing_style_for_restyle_damage(data.previous_styles().map(|x| &x.primary), None) {
+                        match self.existing_style_for_restyle_damage(data.previous_styles().map(|x| &x.primary), None) {
                             Some(ref source) => {
                                 <<Self as TElement>::ConcreteNode as TNode>
                                 ::ConcreteRestyleDamage::compute(source, &shared_style)
@@ -847,7 +847,7 @@ pub trait MatchMethods : TNode {
                               pseudo: Option<&PseudoElement>)
                               -> Self::ConcreteRestyleDamage
     {
-        match self.existing_style_for_restyle_damage(old_style, pseudo) {
+        match self.as_element().unwrap().existing_style_for_restyle_damage(old_style, pseudo) {
             Some(ref source) => {
                 Self::ConcreteRestyleDamage::compute(source,
                                                      new_style)
@@ -955,7 +955,7 @@ pub trait MatchMethods : TNode {
         data.finish_styling(new_styles);
         // Drop the mutable borrow early, since Servo's set_restyle_damage also borrows.
         mem::drop(data);
-        self.set_restyle_damage(damage);
+        self.as_element().unwrap().set_restyle_damage(damage);
 
         restyle_result
     }

components/style/traversal.rs

@@ -7,7 +7,7 @@
 use atomic_refcell::AtomicRefCell;
 use context::{LocalStyleContext, SharedStyleContext, StyleContext};
 use data::NodeData;
-use dom::{OpaqueNode, StylingMode, TNode, UnsafeNode};
+use dom::{OpaqueNode, StylingMode, TElement, TNode, UnsafeNode};
 use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult};
 use selectors::bloom::BloomFilter;
 use selectors::matching::StyleRelations;
@@ -370,7 +370,7 @@ pub fn recalc_style_at<'a, N, C, D>(context: &'a C,
                     STYLE_SHARING_CACHE_HITS.fetch_add(1, Ordering::Relaxed);
                 }
                 style_sharing_candidate_cache.touch(index);
-                node.set_restyle_damage(damage);
+                node.as_element().unwrap().set_restyle_damage(damage);
             }
         }
     }