servo: Merge #8029 - Some cleanup in layout (from Ms2ger:cleanup-layout); r=pcwalton

Source-Repo: https://github.com/servo/servo
Source-Revision: e0c8a88410277843714a20d5fced73a392fad861

servo/components/layout/css/matching.rs

@@ -363,14 +363,30 @@ impl StyleSharingCandidateCache {
 
 /// The results of attempting to share a style.
 pub enum StyleSharingResult {
-    /// We didn't find anybody to share the style with. The boolean indicates whether the style
+    /// We didn't find anybody to share the style with.
-    /// is shareable at all.
+    CannotShare,
-    CannotShare(bool),
     /// The node's style can be shared. The integer specifies the index in the LRU cache that was
     /// hit and the damage that was done.
     StyleWasShared(usize, RestyleDamage),
 }
 
+pub trait ElementMatchMethods {
+    fn match_element(&self,
+                     stylist: &Stylist,
+                     parent_bf: Option<&BloomFilter>,
+                     applicable_declarations: &mut ApplicableDeclarations)
+                     -> bool;
+
+    /// Attempts to share a style with another node. This method is unsafe because it depends on
+    /// the `style_sharing_candidate_cache` having only live nodes in it, and we have no way to
+    /// guarantee that at the type system level yet.
+    unsafe fn share_style_if_possible(&self,
+                                      style_sharing_candidate_cache:
+                                        &mut StyleSharingCandidateCache,
+                                      parent: Option<LayoutNode>)
+                                      -> StyleSharingResult;
+}
+
 pub trait MatchMethods {
     /// Inserts and removes the matching `Descendant` selectors from a bloom
     /// filter. This is used to speed up CSS selector matching to remove
@@ -385,21 +401,6 @@ pub trait MatchMethods {
     /// called to reset the bloom filter after an `insert`.
     fn remove_from_bloom_filter(&self, bf: &mut BloomFilter);
 
-    fn match_node(&self,
-                  stylist: &Stylist,
-                  parent_bf: Option<&BloomFilter>,
-                  applicable_declarations: &mut ApplicableDeclarations,
-                  shareable: &mut bool);
-
-    /// Attempts to share a style with another node. This method is unsafe because it depends on
-    /// the `style_sharing_candidate_cache` having only live nodes in it, and we have no way to
-    /// guarantee that at the type system level yet.
-    unsafe fn share_style_if_possible(&self,
-                                      style_sharing_candidate_cache:
-                                        &mut StyleSharingCandidateCache,
-                                      parent: Option<LayoutNode>)
-                                      -> StyleSharingResult;
-
     unsafe fn cascade_node(&self,
                            layout_context: &SharedLayoutContext,
                            parent: Option<LayoutNode>,
@@ -420,7 +421,9 @@ trait PrivateMatchMethods {
                                    shareable: bool,
                                    animate_properties: bool)
                                    -> RestyleDamage;
+}
 
+trait PrivateElementMatchMethods {
     fn share_style_with_candidate_if_possible(&self,
                                               parent_node: Option<LayoutNode>,
                                               candidate: &StyleSharingCandidate)
@@ -504,13 +507,13 @@ impl<'ln> PrivateMatchMethods for LayoutNode<'ln> {
         *style = Some(this_style);
         damage
     }
+}
 
+impl<'ln> PrivateElementMatchMethods for LayoutElement<'ln> {
     fn share_style_with_candidate_if_possible(&self,
                                               parent_node: Option<LayoutNode>,
                                               candidate: &StyleSharingCandidate)
                                               -> Option<Arc<ComputedValues>> {
-        let element = self.as_element().unwrap();
-
         let parent_node = match parent_node {
             Some(ref parent_node) if parent_node.as_element().is_some() => parent_node,
             Some(_) | None => return None,
@@ -528,7 +531,7 @@ impl<'ln> PrivateMatchMethods for LayoutNode<'ln> {
                 }
 
                 // Check tag names, classes, etc.
-                if !candidate.can_share_style_with(&element) {
+                if !candidate.can_share_style_with(self) {
                     return None
                 }
 
@@ -541,35 +544,34 @@ impl<'ln> PrivateMatchMethods for LayoutNode<'ln> {
     }
 }
 
-impl<'ln> MatchMethods for LayoutNode<'ln> {
+impl<'ln> ElementMatchMethods for LayoutElement<'ln> {
-    fn match_node(&self,
+    fn match_element(&self,
-                  stylist: &Stylist,
+                     stylist: &Stylist,
-                  parent_bf: Option<&BloomFilter>,
+                     parent_bf: Option<&BloomFilter>,
-                  applicable_declarations: &mut ApplicableDeclarations,
+                     applicable_declarations: &mut ApplicableDeclarations)
-                  shareable: &mut bool) {
+                     -> bool {
-        let element = self.as_element().unwrap();
+        let style_attribute = self.style_attribute().as_ref();
-        let style_attribute = element.style_attribute().as_ref();
 
         applicable_declarations.normal_shareable =
-            stylist.push_applicable_declarations(&element,
+            stylist.push_applicable_declarations(self,
                                                  parent_bf,
                                                  style_attribute,
                                                  None,
                                                  &mut applicable_declarations.normal);
-        stylist.push_applicable_declarations(&element,
+        stylist.push_applicable_declarations(self,
                                              parent_bf,
                                              None,
                                              Some(PseudoElement::Before),
                                              &mut applicable_declarations.before);
-        stylist.push_applicable_declarations(&element,
+        stylist.push_applicable_declarations(self,
                                              parent_bf,
                                              None,
                                              Some(PseudoElement::After),
                                              &mut applicable_declarations.after);
 
-        *shareable = applicable_declarations.normal_shareable &&
+        applicable_declarations.normal_shareable &&
-            applicable_declarations.before.is_empty() &&
+        applicable_declarations.before.is_empty() &&
-            applicable_declarations.after.is_empty()
+        applicable_declarations.after.is_empty()
     }
 
     unsafe fn share_style_if_possible(&self,
@@ -578,25 +580,22 @@ impl<'ln> MatchMethods for LayoutNode<'ln> {
                                       parent: Option<LayoutNode>)
                                       -> StyleSharingResult {
         if opts::get().disable_share_style_cache {
-            return StyleSharingResult::CannotShare(false)
+            return StyleSharingResult::CannotShare
         }
-        let ok = {
+
-            if let Some(element) = self.as_element() {
+        if self.style_attribute().is_some() {
-                element.style_attribute().is_none() &&
+            return StyleSharingResult::CannotShare
-                    element.get_attr(&ns!(""), &atom!("id")).is_none()
+        }
-            } else {
+        if self.get_attr(&ns!(""), &atom!("id")).is_some() {
-                false
+            return StyleSharingResult::CannotShare
-            }
-        };
-        if !ok {
-            return StyleSharingResult::CannotShare(false)
         }
 
         for (i, &(ref candidate, ())) in style_sharing_candidate_cache.iter().enumerate() {
             match self.share_style_with_candidate_if_possible(parent.clone(), candidate) {
                 Some(shared_style) => {
                     // Yay, cache hit. Share the style.
-                    let mut layout_data_ref = self.mutate_layout_data();
+                    let node = self.as_node();
+                    let mut layout_data_ref = node.mutate_layout_data();
                     let shared_data = &mut layout_data_ref.as_mut().unwrap().shared_data;
                     let style = &mut shared_data.style;
                     let damage = incremental::compute_damage(style, &*shared_style);
@@ -607,9 +606,11 @@ impl<'ln> MatchMethods for LayoutNode<'ln> {
             }
         }
 
-        StyleSharingResult::CannotShare(true)
+        StyleSharingResult::CannotShare
     }
+}
 
+impl<'ln> MatchMethods for LayoutNode<'ln> {
     // The below two functions are copy+paste because I can't figure out how to
     // write a function which takes a generic function. I don't think it can
     // be done.

servo/components/layout/parallel.rs

@@ -165,21 +165,18 @@ trait ParallelPostorderDomTraversal : PostorderDomTraversal {
     /// The only communication between siblings is that they both
     /// fetch-and-subtract the parent's children count.
     fn run_parallel(&self,
-                    mut unsafe_node: UnsafeLayoutNode,
+                    unsafe_node: UnsafeLayoutNode,
                     proxy: &mut WorkerProxy<SharedLayoutContext, UnsafeLayoutNodeList>) {
+        // Get a real layout node.
+        let mut node: LayoutNode = unsafe {
+            layout_node_from_unsafe_layout_node(&unsafe_node)
+        };
         loop {
-            // Get a real layout node.
-            let node: LayoutNode = unsafe {
-                layout_node_from_unsafe_layout_node(&unsafe_node)
-            };
-
             // Perform the appropriate operation.
             self.process(node);
 
             let shared_layout_context = proxy.user_data();
-            let layout_context = LayoutContext::new(shared_layout_context);
+            let parent = match node.layout_parent_node(shared_layout_context) {
-
-            let parent = match node.layout_parent_node(layout_context.shared) {
                 None => break,
                 Some(parent) => parent,
             };
@@ -188,18 +185,18 @@ trait ParallelPostorderDomTraversal : PostorderDomTraversal {
                 &*parent.borrow_layout_data_unchecked()
             };
             let parent_layout_data = parent_layout_data.as_ref().expect("no layout data");
-            unsafe_node = layout_node_to_unsafe_layout_node(&parent);
 
             if parent_layout_data
                 .data
                 .parallel
                 .children_count
-                .fetch_sub(1, Ordering::Relaxed) == 1 {
+                .fetch_sub(1, Ordering::Relaxed) != 1 {
-                // We were the last child of our parent. Construct flows for our parent.
-            } else {
                 // Get out of here and find another node to work on.
                 break
             }
+
+            // We were the last child of our parent. Construct flows for our parent.
+            node = parent;
         }
     }
 }

servo/components/layout/traversal.rs

@@ -6,7 +6,7 @@
 
 use construct::FlowConstructor;
 use context::LayoutContext;
-use css::matching::{ApplicableDeclarations, MatchMethods, StyleSharingResult};
+use css::matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult};
 use flow::{MutableFlowUtils, PostorderFlowTraversal, PreorderFlowTraversal};
 use flow::{self, Flow};
 use incremental::{self, BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, RestyleDamage};
@@ -165,26 +165,42 @@ impl<'a> PreorderDomTraversal for RecalcStyleForNode<'a> {
             // Check to see whether we can share a style with someone.
             let style_sharing_candidate_cache =
                 &mut self.layout_context.style_sharing_candidate_cache();
-            let sharing_result = unsafe {
+
-                node.share_style_if_possible(style_sharing_candidate_cache,
+            let sharing_result = match node.as_element() {
-                                             parent_opt.clone())
+                Some(element) => {
+                    unsafe {
+                        element.share_style_if_possible(style_sharing_candidate_cache,
+                                                        parent_opt.clone())
+                    }
+                },
+                None => StyleSharingResult::CannotShare,
             };
+
             // Otherwise, match and cascade selectors.
             match sharing_result {
-                StyleSharingResult::CannotShare(mut shareable) => {
+                StyleSharingResult::CannotShare => {
                     let mut applicable_declarations = ApplicableDeclarations::new();
 
-                    if node.as_element().is_some() {
+                    let shareable_element = match node.as_element() {
-                        // Perform the CSS selector matching.
+                        Some(element) => {
-                        let stylist = unsafe { &*self.layout_context.shared.stylist };
+                            // Perform the CSS selector matching.
-                        node.match_node(stylist,
+                            let stylist = unsafe { &*self.layout_context.shared.stylist };
-                                        Some(&*bf),
+                            if element.match_element(stylist,
-                                        &mut applicable_declarations,
+                                                     Some(&*bf),
-                                        &mut shareable);
+                                                     &mut applicable_declarations) {
-                    } else if node.has_changed() {
+                                Some(element)
-                        ThreadSafeLayoutNode::new(&node).set_restyle_damage(
+                            } else {
-                            incremental::rebuild_and_reflow())
+                                None
-                    }
+                            }
+                        },
+                        None => {
+                            if node.has_changed() {
+                                ThreadSafeLayoutNode::new(&node).set_restyle_damage(
+                                    incremental::rebuild_and_reflow())
+                            }
+                            None
+                        },
+                    };
 
                     // Perform the CSS cascade.
                     unsafe {
@@ -196,10 +212,8 @@ impl<'a> PreorderDomTraversal for RecalcStyleForNode<'a> {
                     }
 
                     // Add ourselves to the LRU cache.
-                    if shareable {
+                    if let Some(element) = shareable_element {
-                        if let Some(element) = node.as_element() {
+                        style_sharing_candidate_cache.insert_if_possible(&element);
-                            style_sharing_candidate_cache.insert_if_possible(&element);
-                        }
                     }
                 }
                 StyleSharingResult::StyleWasShared(index, damage) => {