Eliminate RestyleResult.

The new incremental restyle algorithm will make traversal decisions in a more
centralized way.

MozReview-Commit-ID: GH6gFt4VUJO

components/layout/traversal.rs

@@ -19,7 +19,6 @@ use style::data::ElementData;
 use style::dom::{StylingMode, TElement, TNode};
 use style::traversal::{DomTraversalContext, put_thread_local_bloom_filter};
 use style::traversal::{recalc_style_at, remove_from_bloom_filter};
-use style::traversal::RestyleResult;
 use style::traversal::take_thread_local_bloom_filter;
 use util::opts;
 use wrapper::{GetRawData, LayoutNodeHelpers, LayoutNodeLayoutData};
@@ -73,7 +72,7 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
         }
     }
 
-    fn process_preorder(&self, node: N) -> RestyleResult {
+    fn process_preorder(&self, node: N) {
         // FIXME(pcwalton): Stop allocating here. Ideally this should just be
         // done by the HTML parser.
         node.initialize_data();
@@ -101,11 +100,9 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
             let parent = node.parent_node().unwrap().as_element();
             let bf = take_thread_local_bloom_filter(parent, self.root, self.context.shared_context());
             put_thread_local_bloom_filter(bf, &node.to_unsafe(), self.context.shared_context());
-
-            RestyleResult::Stop
         } else {
             let el = node.as_element().unwrap();
-            recalc_style_at::<_, _, Self>(&self.context, self.root, el)
+            recalc_style_at::<_, _, Self>(&self.context, self.root, el);
         }
     }
 
@@ -114,8 +111,13 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
     }
 
     fn should_traverse_child(parent: N::ConcreteElement, child: N) -> bool {
+        // If the parent is display:none, we don't need to do anything.
+        if parent.is_display_none() {
+            return false;
+        }
+
         // If this node has been marked as damaged in some way, we need to
-        // traverse it unconditionally for layout.
+        // traverse it for layout.
         if child.has_changed() {
             return true;
         }

components/style/dom.rs

@@ -11,6 +11,7 @@ use data::{ElementStyles, ElementData};
 use element_state::ElementState;
 use parking_lot::RwLock;
 use properties::{ComputedValues, PropertyDeclarationBlock};
+use properties::longhands::display::computed_value as display;
 use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint};
 use selector_impl::{ElementExt, PseudoElement};
 use selector_matching::ApplicableDeclarationBlock;
@@ -213,6 +214,13 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre
     /// traversal. Returns the number of children left to process.
     fn did_process_child(&self) -> isize;
 
+    /// Returns true if this element's current style is display:none. Only valid
+    /// to call after styling.
+    fn is_display_none(&self) -> bool {
+        self.borrow_data().unwrap()
+            .current_styles().primary
+            .get_box().clone_display() == display::T::none
+    }
 
     /// Returns true if this node has a styled layout frame that owns the style.
     fn frame_has_style(&self) -> bool { false }

components/style/gecko/traversal.rs

@@ -10,7 +10,6 @@ use gecko::context::StandaloneStyleContext;
 use gecko::wrapper::{GeckoElement, GeckoNode};
 use std::mem;
 use traversal::{DomTraversalContext, recalc_style_at};
-use traversal::RestyleResult;
 
 pub struct RecalcStyleOnly<'lc> {
     context: StandaloneStyleContext<'lc>,
@@ -30,14 +29,10 @@ impl<'lc, 'ln> DomTraversalContext<GeckoNode<'ln>> for RecalcStyleOnly<'lc> {
         }
     }
 
-    fn process_preorder(&self, node: GeckoNode<'ln>) -> RestyleResult {
+    fn process_preorder(&self, node: GeckoNode<'ln>) {
-        if node.is_text_node() {
+        if node.is_element() {
-            // Text nodes don't have children, so save the traversal algorithm
-            // the trouble of iterating the children.
-            RestyleResult::Stop
-        } else {
             let el = node.as_element().unwrap();
-            recalc_style_at::<_, _, Self>(&self.context, self.root, el)
+            recalc_style_at::<_, _, Self>(&self.context, self.root, el);
         }
     }
 
@@ -48,7 +43,11 @@ impl<'lc, 'ln> DomTraversalContext<GeckoNode<'ln>> for RecalcStyleOnly<'lc> {
     /// We don't use the post-order traversal for anything.
     fn needs_postorder_traversal(&self) -> bool { false }
 
-    fn should_traverse_child(_parent: GeckoElement<'ln>, child: GeckoNode<'ln>) -> bool {
+    fn should_traverse_child(parent: GeckoElement<'ln>, child: GeckoNode<'ln>) -> bool {
+        if parent.is_display_none() {
+            return false;
+        }
+
         match child.as_element() {
             Some(el) => el.styling_mode() != StylingMode::Stop,
             None => false, // Gecko restyle doesn't need to traverse text nodes.

components/style/matching.rs

@@ -30,7 +30,6 @@ use std::ops::Deref;
 use std::slice::IterMut;
 use std::sync::Arc;
 use string_cache::Atom;
-use traversal::RestyleResult;
 use util::opts;
 
 fn create_common_style_affecting_attributes_from_element<E: TElement>(element: &E)
@@ -482,7 +481,7 @@ pub enum StyleSharingResult<ConcreteRestyleDamage: TRestyleDamage> {
     /// LRU cache that was hit and the damage that was done, and the restyle
     /// result the original result of the candidate's styling, that is, whether
     /// it should stop the traversal or not.
-    StyleWasShared(usize, ConcreteRestyleDamage, RestyleResult),
+    StyleWasShared(usize, ConcreteRestyleDamage),
 }
 
 // Callers need to pass several boolean flags to cascade_node_pseudo_element.
@@ -730,15 +729,9 @@ pub trait MatchMethods : TElement {
                             }
                         };
 
-                    let restyle_result = if shared_style.get_box().clone_display() == display::T::none {
-                        RestyleResult::Stop
-                    } else {
-                        RestyleResult::Continue
-                    };
-
                     data.finish_styling(ElementStyles::new(shared_style));
 
-                    return StyleSharingResult::StyleWasShared(i, damage, restyle_result)
+                    return StyleSharingResult::StyleWasShared(i, damage)
                 }
                 Err(miss) => {
                     debug!("Cache miss: {:?}", miss);
@@ -857,7 +850,6 @@ pub trait MatchMethods : TElement {
                                     mut data: AtomicRefMut<ElementData>,
                                     parent: Option<Self>,
                                     applicable_declarations: &ApplicableDeclarations)
-                                    -> RestyleResult
         where Ctx: StyleContext<'a>
     {
         // Get our parent's style.
@@ -869,7 +861,7 @@ pub trait MatchMethods : TElement {
         let mut applicable_declarations_cache =
             context.local_context().applicable_declarations_cache.borrow_mut();
 
-        let (damage, restyle_result) = {
+        let damage = {
             // Update animations before the cascade. This may modify the value of the old primary
             // style.
             let cacheable = data.previous_styles_mut().map_or(true,
@@ -893,7 +885,7 @@ pub trait MatchMethods : TElement {
                                                      animate: true,
                                                  }));
 
-            let (damage, restyle_result) =
+            let damage =
                 self.compute_damage_and_cascade_pseudos(old_primary,
                                                         old_pseudos,
                                                         &new_styles.primary,
@@ -906,15 +898,13 @@ pub trait MatchMethods : TElement {
                 parent_style.unwrap().is_multicol()
             }));
 
-            (damage, restyle_result)
+            damage
         };
 
         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);
-
-        restyle_result
     }
 
     fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self,
@@ -925,7 +915,7 @@ pub trait MatchMethods : TElement {
                                                    context: &Ctx,
                                                    applicable_declarations: &ApplicableDeclarations,
                                                    mut applicable_declarations_cache: &mut ApplicableDeclarationsCache)
-                                                   -> (Self::ConcreteRestyleDamage, RestyleResult)
+                                                   -> Self::ConcreteRestyleDamage
         where Ctx: StyleContext<'a>
     {
         // Here we optimise the case of the style changing but both the
@@ -950,14 +940,18 @@ pub trait MatchMethods : TElement {
             debug!("Short-circuiting traversal: {:?} {:?} {:?}",
                    this_display, old_display, damage);
 
-            return (damage, RestyleResult::Stop);
+            return damage
         }
 
-        // Otherwise, we just compute the damage normally, and sum up the damage
+        // Compute the damage and sum up the damage related to pseudo-elements.
-        // related to pseudo-elements.
         let mut damage =
             self.compute_restyle_damage(old_primary, new_primary, None);
 
+        // If the new style is display:none, we don't need pseudo-elements styles.
+        if new_primary.get_box().clone_display() == display::T::none {
+            return damage;
+        }
+
         let rebuild_and_reflow =
             Self::ConcreteRestyleDamage::rebuild_and_reflow();
         let no_damage = Self::ConcreteRestyleDamage::empty();
@@ -1016,7 +1010,7 @@ pub trait MatchMethods : TElement {
             }
         });
 
-        (damage, RestyleResult::Continue)
+        damage
     }
 }
 

components/style/parallel.rs

@@ -11,8 +11,8 @@
 use dom::{OpaqueNode, StylingMode, TElement, TNode, UnsafeNode};
 use std::mem;
 use std::sync::atomic::Ordering;
-use traversal::{RestyleResult, DomTraversalContext};
 use traversal::{STYLE_SHARING_CACHE_HITS, STYLE_SHARING_CACHE_MISSES};
+use traversal::DomTraversalContext;
 use util::opts;
 use workqueue::{WorkQueue, WorkUnit, WorkerProxy};
 
@@ -83,8 +83,9 @@ fn top_down_dom<N, C>(unsafe_nodes: UnsafeNodeList,
 
         // Perform the appropriate traversal.
         let mut children_to_process = 0isize;
-        if let RestyleResult::Continue = context.process_preorder(node) {
+        context.process_preorder(node);
-            C::traverse_children(node.as_element().unwrap(), |kid| {
+        if let Some(el) = node.as_element() {
+            C::traverse_children(el, |kid| {
                 children_to_process += 1;
                 discovered_child_nodes.push(kid.to_unsafe())
             });

components/style/sequential.rs

@@ -5,7 +5,7 @@
 //! Implements sequential traversal over the DOM tree.
 
 use dom::{StylingMode, TElement, TNode};
-use traversal::{RestyleResult, DomTraversalContext};
+use traversal::DomTraversalContext;
 
 pub fn traverse_dom<N, C>(root: N,
                           shared: &C::SharedContext)
@@ -16,9 +16,9 @@ pub fn traverse_dom<N, C>(root: N,
         where N: TNode,
               C: DomTraversalContext<N>
     {
-        if let RestyleResult::Continue = context.process_preorder(node) {
+        context.process_preorder(node);
-            C::traverse_children(node.as_element().unwrap(),
+        if let Some(el) = node.as_element() {
-                                 |kid| doit::<N, C>(context, kid));
+            C::traverse_children(el, |kid| doit::<N, C>(context, kid));
         }
 
         if context.needs_postorder_traversal() {

components/style/traversal.rs

@@ -21,16 +21,6 @@ use util::opts;
 /// detected by ticking a generation number every layout.
 pub type Generation = u32;
 
-/// This enum tells us about whether we can stop restyling or not after styling
-/// an element.
-///
-/// So far this only happens where a display: none node is found.
-#[derive(Clone, Copy, PartialEq)]
-pub enum RestyleResult {
-    Continue,
-    Stop,
-}
-
 /// Style sharing candidate cache stats. These are only used when
 /// `-Z style-sharing-stats` is given.
 pub static STYLE_SHARING_CACHE_HITS: AtomicUsize = ATOMIC_USIZE_INIT;
@@ -175,7 +165,7 @@ pub trait DomTraversalContext<N: TNode> {
     fn new<'a>(&'a Self::SharedContext, OpaqueNode) -> Self;
 
     /// Process `node` on the way down, before its children have been processed.
-    fn process_preorder(&self, node: N) -> RestyleResult;
+    fn process_preorder(&self, node: N);
 
     /// Process `node` on the way up, after its children have been processed.
     ///
@@ -307,7 +297,7 @@ fn ensure_element_styled_internal<'a, E, C>(element: E,
 #[allow(unsafe_code)]
 pub fn recalc_style_at<'a, E, C, D>(context: &'a C,
                                     root: OpaqueNode,
-                                    element: E) -> RestyleResult
+                                    element: E)
     where E: TElement,
           C: StyleContext<'a>,
           D: DomTraversalContext<E::ConcreteNode>
@@ -315,7 +305,6 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C,
     // Get the style bloom filter.
     let mut bf = take_thread_local_bloom_filter(element.parent_element(), root, context.shared_context());
 
-    let mut restyle_result = RestyleResult::Continue;
     let mode = element.styling_mode();
     debug_assert!(mode != StylingMode::Stop, "Parent should not have enqueued us");
     if mode != StylingMode::Traverse {
@@ -361,8 +350,7 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C,
 
                 // Perform the CSS cascade.
                 unsafe {
-                    restyle_result = element.cascade_node(context, data,
+                    element.cascade_node(context, data, element.parent_element(),
-                                                          element.parent_element(),
                                          &applicable_declarations);
                 }
 
@@ -376,8 +364,7 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C,
                                                                      relations);
                 }
             }
-            StyleSharingResult::StyleWasShared(index, damage, cached_restyle_result) => {
+            StyleSharingResult::StyleWasShared(index, damage) => {
-                restyle_result = cached_restyle_result;
                 if opts::get().style_sharing_stats {
                     STYLE_SHARING_CACHE_HITS.fetch_add(1, Ordering::Relaxed);
                 }
@@ -394,7 +381,7 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C,
     // If we restyled this node, conservatively mark all our children as needing
     // processing. The eventual algorithm we're designing does this in a more granular
     // fashion.
-    if mode == StylingMode::Restyle && restyle_result == RestyleResult::Continue {
+    if mode == StylingMode::Restyle && !element.is_display_none() {
         for kid in element.as_node().children() {
             if let Some(kid) = kid.as_element() {
                 unsafe { let _ = D::prepare_for_styling(&kid); }
@@ -411,6 +398,4 @@ pub fn recalc_style_at<'a, E, C, D>(context: &'a C,
 
     // NB: flow construction updates the bloom filter on the way up.
     put_thread_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context());
-
-    restyle_result
 }