Auto merge of #12757 - emilio:stylo, r=bholley,pcwalton

stylo: Stop restyling display: none elements, remove the has_changed hack that made us use ReconstructFrame unconditionally.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

r? @bholley

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12757)
<!-- Reviewable:end -->

components/layout/animation.rs

@@ -135,7 +135,7 @@ pub fn recalc_style_for_animations(context: &SharedLayoutContext,
                 update_style_for_animation(&context.style_context,
                                            animation,
                                            &mut fragment.style);
-                damage |= RestyleDamage::compute(Some(&old_style), &fragment.style);
+                damage |= RestyleDamage::compute(&old_style, &fragment.style);
             }
         }
     });

components/layout/construct.rs

@@ -1374,12 +1374,20 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
         // We visit the kids first and reset their HAS_NEWLY_CONSTRUCTED_FLOW flags after checking
         // them.  NOTE: Make sure not to bail out early before resetting all the flags!
         let mut need_to_reconstruct = false;
+
+        // If the node has display: none, it's possible that we haven't even
+        // styled the children once, so we need to bailout early here.
+        if node.style(self.style_context()).get_box().clone_display() == display::T::none {
+            return false;
+        }
+
         for kid in node.children() {
             if kid.flags().contains(HAS_NEWLY_CONSTRUCTED_FLOW) {
                 kid.remove_flags(HAS_NEWLY_CONSTRUCTED_FLOW);
                 need_to_reconstruct = true
             }
         }
+
         if need_to_reconstruct {
             return false
         }

components/layout/query.rs

@@ -30,6 +30,7 @@ use std::ops::Deref;
 use std::sync::{Arc, Mutex};
 use string_cache::Atom;
 use style::computed_values;
+use style::context::StyleContext;
 use style::logical_geometry::{WritingMode, BlockFlowDirection, InlineBaseDirection};
 use style::properties::longhands::{display, position};
 use style::properties::style_structs;
@@ -37,7 +38,7 @@ use style::selector_impl::PseudoElement;
 use style::selector_matching::Stylist;
 use style::values::LocalToCss;
 use style_traits::cursor::Cursor;
-use wrapper::ThreadSafeLayoutNodeHelpers;
+use wrapper::{LayoutNodeLayoutData, ThreadSafeLayoutNodeHelpers};
 
 /// Mutable data belonging to the LayoutThread.
 ///
@@ -620,11 +621,39 @@ pub fn process_node_scroll_area_request< N: LayoutNode>(requested_node: N, layou
     }
 }
 
+/// Ensures that a node's data, and all its parents' is initialized. This is
+/// needed to resolve style lazily.
+fn ensure_node_data_initialized<N: LayoutNode>(node: &N) {
+    let mut cur = Some(node.clone());
+    while let Some(current) = cur {
+        if current.borrow_data().is_some() {
+            break;
+        }
+
+        current.initialize_data();
+        cur = current.parent_node();
+    }
+}
+
 /// Return the resolved value of property for a given (pseudo)element.
 /// https://drafts.csswg.org/cssom/#resolved-value
-pub fn process_resolved_style_request<N: LayoutNode>(
+pub fn process_resolved_style_request<'a, N, C>(requested_node: N,
-            requested_node: N, pseudo: &Option<PseudoElement>,
+                                                style_context: &'a C,
-            property: &Atom, layout_root: &mut FlowRef) -> Option<String> {
+                                                pseudo: &Option<PseudoElement>,
+                                                property: &Atom,
+                                                layout_root: &mut FlowRef) -> Option<String>
+    where N: LayoutNode,
+          C: StyleContext<'a>
+{
+    use style::traversal::ensure_node_styled;
+
+    // This node might have display: none, or it's style might be not up to
+    // date, so we might need to do style recalc.
+    //
+    // FIXME(emilio): Is a bit shame we have to do this instead of in style.
+    ensure_node_data_initialized(&requested_node);
+    ensure_node_styled(requested_node, style_context);
+
     let layout_node = requested_node.to_threadsafe();
     let layout_node = match *pseudo {
         Some(PseudoElement::Before) => layout_node.get_before_pseudo(),

components/layout/traversal.rs

@@ -16,6 +16,7 @@ use std::mem;
 use style::context::SharedStyleContext;
 use style::dom::TNode;
 use style::selector_impl::ServoSelectorImpl;
+use style::traversal::RestyleResult;
 use style::traversal::{DomTraversalContext, remove_from_bloom_filter, recalc_style_at};
 use util::opts;
 use wrapper::{LayoutNodeLayoutData, ThreadSafeLayoutNodeHelpers};
@@ -69,12 +70,12 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
         }
     }
 
-    fn process_preorder(&self, node: N) {
+    fn process_preorder(&self, node: N) -> RestyleResult {
-        // FIXME(pcwalton): Stop allocating here. Ideally this should just be done by the HTML
+        // FIXME(pcwalton): Stop allocating here. Ideally this should just be
-        // parser.
+        // done by the HTML parser.
         node.initialize_data();
 
-        recalc_style_at(&self.context, self.root, node);
+        recalc_style_at(&self.context, self.root, node)
     }
 
     fn process_postorder(&self, node: N) {

components/layout_thread/lib.rs

@@ -1237,8 +1237,13 @@ impl LayoutThread {
                 },
                 ReflowQueryType::ResolvedStyleQuery(node, ref pseudo, ref property) => {
                     let node = unsafe { ServoLayoutNode::new(&node) };
+                    let layout_context = LayoutContext::new(&shared_layout_context);
                     rw_data.resolved_style_response =
-                        process_resolved_style_request(node, pseudo, property, &mut root_flow);
+                        process_resolved_style_request(node,
+                                                       &layout_context,
+                                                       pseudo,
+                                                       property,
+                                                       &mut root_flow);
                 },
                 ReflowQueryType::OffsetParentQuery(node) => {
                     let node = unsafe { ServoLayoutNode::new(&node) };

components/script/layout_wrapper.rs

@@ -61,7 +61,7 @@ use style::dom::{PresentationalHintsSynthetizer, OpaqueNode, TDocument, TElement
 use style::element_state::*;
 use style::properties::{ComputedValues, PropertyDeclaration, PropertyDeclarationBlock};
 use style::refcell::{Ref, RefCell, RefMut};
-use style::selector_impl::{ElementSnapshot, NonTSPseudoClass, ServoSelectorImpl};
+use style::selector_impl::{ElementSnapshot, NonTSPseudoClass, PseudoElement, ServoSelectorImpl};
 use style::sink::Push;
 use style::str::is_whitespace;
 use url::Url;
@@ -266,7 +266,8 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
 
     #[inline]
     fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             current_cv: Option<&'a Arc<ComputedValues>>)
+                                             current_cv: Option<&'a Arc<ComputedValues>>,
+                                             _pseudo_element: Option<&PseudoElement>)
                                              -> Option<&'a Arc<ComputedValues>> {
         current_cv
     }

components/script_layout_interface/restyle_damage.rs

@@ -47,7 +47,11 @@ impl TRestyleDamage for RestyleDamage {
     /// For Servo the style source is always the computed values.
     type PreExistingComputedValues = Arc<ServoComputedValues>;
 
-    fn compute(old: Option<&Arc<ServoComputedValues>>,
+    fn empty() -> Self {
+        RestyleDamage::empty()
+    }
+
+    fn compute(old: &Arc<ServoComputedValues>,
                new: &Arc<ServoComputedValues>) -> RestyleDamage {
         compute_damage(old, new)
     }
@@ -150,13 +154,7 @@ macro_rules! add_if_not_equal(
     })
 );
 
-fn compute_damage(old: Option<&Arc<ServoComputedValues>>, new: &Arc<ServoComputedValues>) -> RestyleDamage {
+fn compute_damage(old: &ServoComputedValues, new: &ServoComputedValues) -> RestyleDamage {
-    let new = &**new;
-    let old: &ServoComputedValues = match old {
-        None => return RestyleDamage::rebuild_and_reflow(),
-        Some(cv) => &**cv,
-    };
-
     let mut damage = RestyleDamage::empty();
 
     // This should check every CSS property, as enumerated in the fields of

components/style/dom.rs

@@ -12,9 +12,10 @@ use element_state::ElementState;
 use properties::{ComputedValues, PropertyDeclaration, PropertyDeclarationBlock};
 use refcell::{Ref, RefMut};
 use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint};
-use selector_impl::ElementExt;
+use selector_impl::{ElementExt, PseudoElement};
 use selectors::matching::DeclarationBlock;
 use sink::Push;
+use std::fmt::Debug;
 use std::ops::BitOr;
 use std::sync::Arc;
 use string_cache::{Atom, Namespace};
@@ -44,7 +45,7 @@ impl OpaqueNode {
     }
 }
 
-pub trait TRestyleDamage : BitOr<Output=Self> + Copy {
+pub trait TRestyleDamage : Debug + PartialEq + BitOr<Output=Self> + Copy {
     /// The source for our current computed values in the cascade. This is a
     /// ComputedValues in Servo and a StyleContext in Gecko.
     ///
@@ -55,9 +56,11 @@ pub trait TRestyleDamage : BitOr<Output=Self> + Copy {
     /// This should be obtained via TNode::existing_style_for_restyle_damage
     type PreExistingComputedValues;
 
-    fn compute(old: Option<&Self::PreExistingComputedValues>,
+    fn compute(old: &Self::PreExistingComputedValues,
                new: &Arc<ComputedValues>) -> Self;
 
+    fn empty() -> Self;
+
     fn rebuild_and_reflow() -> Self;
 }
 
@@ -174,7 +177,8 @@ pub trait TNode : Sized + Copy + Clone {
     /// 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>>)
+                                             current_computed_values: Option<&'a Arc<ComputedValues>>,
+                                             pseudo: Option<&PseudoElement>)
         -> Option<&'a <Self::ConcreteRestyleDamage as TRestyleDamage>::PreExistingComputedValues>;
 }
 

components/style/matching.rs

@@ -12,6 +12,7 @@ use cache::{LRUCache, SimpleHashCache};
 use context::{StyleContext, SharedStyleContext};
 use data::PrivateStyleData;
 use dom::{TElement, TNode, TRestyleDamage};
+use properties::longhands::display::computed_value as display;
 use properties::{ComputedValues, PropertyDeclaration, cascade};
 use selector_impl::{ElementExt, TheSelectorImpl, PseudoElement};
 use selector_matching::{DeclarationBlock, Stylist};
@@ -25,6 +26,7 @@ use std::hash::{BuildHasherDefault, Hash, Hasher};
 use std::slice::Iter;
 use std::sync::Arc;
 use string_cache::{Atom, Namespace};
+use traversal::RestyleResult;
 use util::opts;
 
 fn create_common_style_affecting_attributes_from_element<E: TElement>(element: &E)
@@ -410,9 +412,11 @@ impl StyleSharingCandidateCache {
 pub enum StyleSharingResult<ConcreteRestyleDamage: TRestyleDamage> {
     /// We didn't find anybody to share the style with.
     CannotShare,
-    /// The node's style can be shared. The integer specifies the index in the LRU cache that was
+    /// The node's style can be shared. The integer specifies the index in the
-    /// hit and the damage that was done.
+    /// LRU cache that was hit and the damage that was done, and the restyle
-    StyleWasShared(usize, ConcreteRestyleDamage),
+    /// result the original result of the candidate's styling, that is, whether
+    /// it should stop the traversal or not.
+    StyleWasShared(usize, ConcreteRestyleDamage, RestyleResult),
 }
 
 trait PrivateMatchMethods: TNode {
@@ -424,22 +428,22 @@ trait PrivateMatchMethods: TNode {
                                             context: &Ctx,
                                             parent_style: Option<&Arc<ComputedValues>>,
                                             applicable_declarations: &[DeclarationBlock],
-                                            mut style: Option<&mut Arc<ComputedValues>>,
+                                            mut old_style: Option<&mut Arc<ComputedValues>>,
                                             applicable_declarations_cache:
                                              &mut ApplicableDeclarationsCache,
                                             shareable: bool,
                                             animate_properties: bool)
-                                            -> (Self::ConcreteRestyleDamage, Arc<ComputedValues>)
+                                            -> Arc<ComputedValues>
-    where Ctx: StyleContext<'a> {
+        where Ctx: StyleContext<'a>
+    {
         let mut cacheable = true;
         let shared_context = context.shared_context();
         if animate_properties {
             cacheable = !self.update_animations_for_cascade(shared_context,
-                                                            &mut style) && cacheable;
+                                                            &mut old_style) && cacheable;
         }
 
-        let this_style;
+        let (this_style, is_cacheable) = match parent_style {
-        match parent_style {
             Some(ref parent_style) => {
                 let cache_entry = applicable_declarations_cache.find(applicable_declarations);
                 let cached_computed_values = match cache_entry {
@@ -447,27 +451,25 @@ trait PrivateMatchMethods: TNode {
                     None => None,
                 };
 
-                let (the_style, is_cacheable) = cascade(shared_context.viewport_size,
+                cascade(shared_context.viewport_size,
                         applicable_declarations,
                         shareable,
                         Some(&***parent_style),
                         cached_computed_values,
-                                                        shared_context.error_reporter.clone());
+                        shared_context.error_reporter.clone())
-                cacheable = cacheable && is_cacheable;
-                this_style = the_style
             }
             None => {
-                let (the_style, is_cacheable) = cascade(shared_context.viewport_size,
+                cascade(shared_context.viewport_size,
                         applicable_declarations,
                         shareable,
                         None,
                         None,
-                                                        shared_context.error_reporter.clone());
+                        shared_context.error_reporter.clone())
-                cacheable = cacheable && is_cacheable;
-                this_style = the_style
             }
         };
 
+        cacheable = cacheable && is_cacheable;
+
         let mut this_style = Arc::new(this_style);
 
         if animate_properties {
@@ -482,7 +484,7 @@ trait PrivateMatchMethods: TNode {
 
             // Trigger transitions if necessary. This will reset `this_style` back
             // to its old value if it did trigger a transition.
-            if let Some(ref style) = style {
+            if let Some(ref style) = old_style {
                 animations_started |=
                     animation::start_transitions_if_applicable(
                         new_animations_sender,
@@ -496,21 +498,13 @@ trait PrivateMatchMethods: TNode {
         }
 
 
-        let existing_style =
-            self.existing_style_for_restyle_damage(style.map(|s| &*s));
-
-        // Calculate style difference.
-        let damage =
-            Self::ConcreteRestyleDamage::compute(existing_style, &this_style);
-
         // Cache the resolved style if it was cacheable.
         if cacheable {
             applicable_declarations_cache.insert(applicable_declarations.to_vec(),
                                                  this_style.clone());
         }
 
-        // Return the final style and the damage done to our caller.
+        this_style
-        (damage, this_style)
     }
 
     fn update_animations_for_cascade(&self,
@@ -646,16 +640,27 @@ pub trait ElementMatchMethods : TElement {
 
                 let style = &mut node.mutate_data().unwrap().style;
 
-                let damage = {
+                let damage =
-                    let source =
+                    match node.existing_style_for_restyle_damage((*style).as_ref(), None) {
-                        node.existing_style_for_restyle_damage((*style).as_ref());
+                        Some(ref source) => {
-                    let damage = <<Self as TElement>::ConcreteNode as TNode>
+                            <<Self as TElement>::ConcreteNode as TNode>
-                                     ::ConcreteRestyleDamage::compute(source, &shared_style);
+                            ::ConcreteRestyleDamage::compute(source, &shared_style)
-                    damage
+                        }
+                        None => {
+                            <<Self as TElement>::ConcreteNode as TNode>
+                            ::ConcreteRestyleDamage::rebuild_and_reflow()
+                        }
+                    };
+
+                let restyle_result = if shared_style.get_box().clone_display() == display::T::none {
+                    RestyleResult::Stop
+                } else {
+                    RestyleResult::Continue
                 };
 
                 *style = Some(shared_style);
-                return StyleSharingResult::StyleWasShared(i, damage)
+
+                return StyleSharingResult::StyleWasShared(i, damage, restyle_result)
             }
         }
 
@@ -714,11 +719,54 @@ pub trait MatchMethods : TNode {
         }
     }
 
+    fn compute_restyle_damage(&self,
+                              old_style: Option<&Arc<ComputedValues>>,
+                              new_style: &Arc<ComputedValues>,
+                              pseudo: Option<&PseudoElement>)
+                              -> Self::ConcreteRestyleDamage
+    {
+        match self.existing_style_for_restyle_damage(old_style, pseudo) {
+            Some(ref source) => {
+                Self::ConcreteRestyleDamage::compute(source,
+                                                     new_style)
+            }
+            None => {
+                // If there's no style source, two things can happen:
+                //
+                //  1. This is not an incremental restyle (old_style is none).
+                //     In this case we can't do too much than sending
+                //     rebuild_and_reflow.
+                //
+                //  2. This is an incremental restyle, but the old display value
+                //     is none, so there's no effective way for Gecko to get the
+                //     style source. In this case, we could return either
+                //     RestyleDamage::empty(), in the case both displays are
+                //     none, or rebuild_and_reflow, otherwise. The first case
+                //     should be already handled when calling this function, so
+                //     we can assert that the new display value is not none.
+                //
+                //     Also, this can be a text node (in which case we don't
+                //     care of watching the new display value).
+                //
+                // Unfortunately we can't strongly assert part of this, since
+                // we style some nodes that in Gecko never generate a frame,
+                // like children of replaced content. Arguably, we shouldn't be
+                // styling those here, but until we implement that we'll have to
+                // stick without the assertions.
+                debug_assert!(pseudo.is_none() ||
+                              new_style.get_box().clone_display() != display::T::none);
+                Self::ConcreteRestyleDamage::rebuild_and_reflow()
+            }
+        }
+    }
+
     unsafe fn cascade_node<'a, Ctx>(&self,
                                     context: &Ctx,
                                     parent: Option<Self>,
                                     applicable_declarations: &ApplicableDeclarations)
-    where Ctx: StyleContext<'a> {
+                                    -> RestyleResult
+        where Ctx: StyleContext<'a>
+    {
         // Get our parent's style. This must be unsafe so that we don't touch the parent's
         // borrow flags.
         //
@@ -735,72 +783,171 @@ pub trait MatchMethods : TNode {
         let mut applicable_declarations_cache =
             context.local_context().applicable_declarations_cache.borrow_mut();
 
-        let damage;
+        let (damage, restyle_result) = if self.is_text_node() {
-        if self.is_text_node() {
             let mut data_ref = self.mutate_data().unwrap();
             let mut data = &mut *data_ref;
             let cloned_parent_style = ComputedValues::style_for_child_text_node(parent_style.unwrap());
 
-            {
+            let damage =
-                let existing_style =
+                self.compute_restyle_damage(data.style.as_ref(), &cloned_parent_style, None);
-                    self.existing_style_for_restyle_damage(data.style.as_ref());
-                damage = Self::ConcreteRestyleDamage::compute(existing_style,
-                                                              &cloned_parent_style);
-            }
 
             data.style = Some(cloned_parent_style);
+
+            (damage, RestyleResult::Continue)
         } else {
-            damage = {
             let mut data_ref = self.mutate_data().unwrap();
             let mut data = &mut *data_ref;
-                let (mut damage, final_style) = self.cascade_node_pseudo_element(
+            let final_style =
-                    context,
+                self.cascade_node_pseudo_element(context, parent_style,
-                    parent_style,
                                                  &applicable_declarations.normal,
                                                  data.style.as_mut(),
                                                  &mut applicable_declarations_cache,
                                                  applicable_declarations.normal_shareable,
-                    true);
+                                                 /* should_animate = */ true);
 
-                data.style = Some(final_style);
+            let (damage, restyle_result) =
-
+                self.compute_damage_and_cascade_pseudos(final_style,
-                <Self::ConcreteElement as MatchAttr>::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| {
+                                                        data,
-                    let applicable_declarations_for_this_pseudo =
-                        applicable_declarations.per_pseudo.get(&pseudo).unwrap();
-
-                    if !applicable_declarations_for_this_pseudo.is_empty() {
-                        // NB: Transitions and animations should only work for
-                        // pseudo-elements ::before and ::after
-                        let should_animate_properties =
-                            <Self::ConcreteElement as MatchAttr>::Impl::pseudo_is_before_or_after(&pseudo);
-                        let (new_damage, style) = self.cascade_node_pseudo_element(
                                                         context,
-                            Some(data.style.as_ref().unwrap()),
+                                                        applicable_declarations,
-                            &*applicable_declarations_for_this_pseudo,
+                                                        &mut applicable_declarations_cache);
-                            data.per_pseudo.get_mut(&pseudo),
-                            &mut applicable_declarations_cache,
-                            false,
-                            should_animate_properties);
-                        data.per_pseudo.insert(pseudo, style);
 
-                        damage = damage | new_damage;
-                    }
-                });
-
-                damage
-            };
-
-            // This method needs to borrow the data as mutable, so make sure data_ref goes out of
-            // scope first.
             self.set_can_be_fragmented(parent.map_or(false, |p| {
                 p.can_be_fragmented() ||
                 parent_style.as_ref().unwrap().is_multicol()
             }));
+
+            (damage, restyle_result)
+        };
+
+
+        // This method needs to borrow the data as mutable, so make sure
+        // data_ref goes out of scope first.
+        self.set_restyle_damage(damage);
+
+        restyle_result
     }
 
-        // This method needs to borrow the data as mutable, so make sure data_ref goes out of
+    fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self,
-        // scope first.
+                                                   final_style: Arc<ComputedValues>,
-        self.set_restyle_damage(damage);
+                                                   data: &mut PrivateStyleData,
+                                                   context: &Ctx,
+                                                   applicable_declarations: &ApplicableDeclarations,
+                                                   mut applicable_declarations_cache: &mut ApplicableDeclarationsCache)
+                                                   -> (Self::ConcreteRestyleDamage, RestyleResult)
+        where Ctx: StyleContext<'a>
+    {
+        // Here we optimise the case of the style changing but both the
+        // previous and the new styles having display: none. In this
+        // case, we can always optimize the traversal, regardless of the
+        // restyle hint.
+        let this_display = final_style.get_box().clone_display();
+        if this_display == display::T::none {
+            let old_display = data.style.as_ref().map(|old_style| {
+                old_style.get_box().clone_display()
+            });
+
+            // If display passed from none to something, then we need to reflow,
+            // otherwise, we don't do anything.
+            let damage = match old_display {
+                Some(display) if display == this_display => {
+                    Self::ConcreteRestyleDamage::empty()
+                }
+                _ => Self::ConcreteRestyleDamage::rebuild_and_reflow()
+            };
+
+            debug!("Short-circuiting traversal: {:?} {:?} {:?}",
+                   this_display, old_display, damage);
+
+            data.style = Some(final_style);
+            return (damage, RestyleResult::Stop);
+        }
+
+        // Otherwise, we just compute the damage normally, and sum up the damage
+        // related to pseudo-elements.
+        let mut damage =
+            self.compute_restyle_damage(data.style.as_ref(), &final_style, None);
+
+        data.style = Some(final_style);
+
+        let data_per_pseudo = &mut data.per_pseudo;
+        let new_style = data.style.as_ref();
+
+        debug_assert!(new_style.is_some());
+
+        let rebuild_and_reflow =
+            Self::ConcreteRestyleDamage::rebuild_and_reflow();
+
+        <Self::ConcreteElement as MatchAttr>::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| {
+            use std::collections::hash_map::Entry;
+
+            let applicable_declarations_for_this_pseudo =
+                applicable_declarations.per_pseudo.get(&pseudo).unwrap();
+
+            let has_declarations =
+                !applicable_declarations_for_this_pseudo.is_empty();
+
+            // If there are declarations matching, we're going to need to
+            // recompute the style anyway, so do it now to simplify the logic
+            // below.
+            let pseudo_style_if_declarations = if has_declarations {
+                // NB: Transitions and animations should only work for
+                // pseudo-elements ::before and ::after
+                let should_animate_properties =
+                    <Self::ConcreteElement as MatchAttr>::Impl::pseudo_is_before_or_after(&pseudo);
+
+                Some(self.cascade_node_pseudo_element(context,
+                                                      new_style,
+                                                      &*applicable_declarations_for_this_pseudo,
+                                                      data_per_pseudo.get_mut(&pseudo),
+                                                      &mut applicable_declarations_cache,
+                                                      /* shareable = */ false,
+                                                      should_animate_properties))
+            } else {
+                None
+            };
+
+            // Let's see what we had before.
+            match data_per_pseudo.entry(pseudo.clone()) {
+                Entry::Vacant(vacant_entry) => {
+                    // If we had a vacant entry, and no rules that match, we're
+                    // fine so far.
+                    if !has_declarations {
+                        return;
+                    }
+
+                    // Otherwise, we need to insert the new computed styles, and
+                    // generate a rebuild_and_reflow damage.
+                    damage = damage | Self::ConcreteRestyleDamage::rebuild_and_reflow();
+                    vacant_entry.insert(pseudo_style_if_declarations.unwrap());
+                }
+                Entry::Occupied(mut occupied_entry) => {
+                    // If there was an existing style, and no declarations, we
+                    // need to remove us from the map, and ensure we're
+                    // reconstructing.
+                    if !has_declarations {
+                        damage = damage | Self::ConcreteRestyleDamage::rebuild_and_reflow();
+                        occupied_entry.remove();
+                        return;
+                    }
+
+                    // If there's a new style, we need to diff it and add the
+                    // damage, except if the damage was already
+                    // rebuild_and_reflow, in which case we can avoid it.
+                    if damage != rebuild_and_reflow {
+                        damage = damage |
+                                 self.compute_restyle_damage(Some(occupied_entry.get()),
+                                                             pseudo_style_if_declarations.as_ref().unwrap(),
+                                                             Some(&pseudo));
+                    }
+
+                    // And now, of course, use the new style.
+                    occupied_entry.insert(pseudo_style_if_declarations.unwrap());
+                }
+            }
+        });
+
+        (damage, RestyleResult::Continue)
     }
 }
 

components/style/parallel.rs

@@ -11,7 +11,7 @@
 use dom::{OpaqueNode, TNode, UnsafeNode};
 use std::mem;
 use std::sync::atomic::Ordering;
-use traversal::DomTraversalContext;
+use traversal::{RestyleResult, DomTraversalContext};
 use workqueue::{WorkQueue, WorkUnit, WorkerProxy};
 
 #[allow(dead_code)]
@@ -67,39 +67,40 @@ fn top_down_dom<N, C>(unsafe_nodes: UnsafeNodeList,
             continue;
         }
 
-        // Perform the appropriate traversal.
-        context.process_preorder(node);
-
         // Possibly enqueue the children.
         let mut children_to_process = 0isize;
+        // Perform the appropriate traversal.
+        if let RestyleResult::Continue = context.process_preorder(node) {
             for kid in node.children() {
-            // Trigger the hook pre-adding the kid to the list. This can (and in
+                // Trigger the hook pre-adding the kid to the list. This can
-            // fact uses to) change the result of the should_process operation.
+                // (and in fact uses to) change the result of the should_process
+                // operation.
                 //
-            // As of right now, this hook takes care of propagating the restyle
+                // As of right now, this hook takes care of propagating the
-            // flag down the tree. In the future, more accurate behavior is
+                // restyle flag down the tree. In the future, more accurate
-            // probably going to be needed.
+                // behavior is probably going to be needed.
                 context.pre_process_child_hook(node, kid);
                 if context.should_process(kid) {
                     children_to_process += 1;
                     discovered_child_nodes.push(kid.to_unsafe())
                 }
             }
-
-        // Reset the count of children.
-        {
-            let data = node.mutate_data().unwrap();
-            data.parallel.children_to_process
-                         .store(children_to_process,
-                                Ordering::Relaxed);
         }
 
+        // Reset the count of children if we need to do a bottom-up traversal
+        // after the top up.
+        if context.needs_postorder_traversal() {
+            node.mutate_data().unwrap()
+                .parallel.children_to_process
+                         .store(children_to_process,
+                                Ordering::Relaxed);
 
             // If there were no more children, start walking back up.
             if children_to_process == 0 {
                 bottom_up_dom::<N, C>(unsafe_nodes.1, unsafe_node, proxy)
             }
         }
+    }
 
     for chunk in discovered_child_nodes.chunks(CHUNK_SIZE) {
         proxy.push(WorkUnit {
@@ -123,7 +124,9 @@ fn top_down_dom<N, C>(unsafe_nodes: UnsafeNodeList,
 fn bottom_up_dom<N, C>(root: OpaqueNode,
                        unsafe_node: UnsafeNode,
                        proxy: &mut WorkerProxy<C::SharedContext, UnsafeNodeList>)
-                       where N: TNode, C: DomTraversalContext<N> {
+    where N: TNode,
+          C: DomTraversalContext<N>
+{
     let context = C::new(proxy.user_data(), root);
 
     // Get a real layout node.

components/style/selector_impl.rs

@@ -79,7 +79,8 @@ pub trait ElementExt: Element<Impl=TheSelectorImpl> {
 impl TheSelectorImpl {
     #[inline]
     pub fn each_eagerly_cascaded_pseudo_element<F>(mut fun: F)
-        where F: FnMut(<Self as SelectorImpl>::PseudoElement) {
+        where F: FnMut(PseudoElement)
+    {
         Self::each_pseudo_element(|pseudo| {
             if Self::pseudo_element_cascade_type(&pseudo).is_eager() {
                 fun(pseudo)
@@ -89,7 +90,8 @@ impl TheSelectorImpl {
 
     #[inline]
     pub fn each_precomputed_pseudo_element<F>(mut fun: F)
-        where F: FnMut(<Self as SelectorImpl>::PseudoElement) {
+        where F: FnMut(PseudoElement)
+    {
         Self::each_pseudo_element(|pseudo| {
             if Self::pseudo_element_cascade_type(&pseudo).is_precomputed() {
                 fun(pseudo)

components/style/sequential.rs

@@ -5,7 +5,7 @@
 //! Implements sequential traversal over the DOM tree.
 
 use dom::TNode;
-use traversal::DomTraversalContext;
+use traversal::{RestyleResult, DomTraversalContext};
 
 pub fn traverse_dom<N, C>(root: N,
                           shared: &C::SharedContext)
@@ -17,17 +17,19 @@ pub fn traverse_dom<N, C>(root: N,
               C: DomTraversalContext<N>
     {
         debug_assert!(context.should_process(node));
-        context.process_preorder(node);
+        if let RestyleResult::Continue = context.process_preorder(node) {
-
             for kid in node.children() {
                 context.pre_process_child_hook(node, kid);
                 if context.should_process(kid) {
                     doit::<N, C>(context, kid);
                 }
             }
+        }
 
+        if context.needs_postorder_traversal() {
             context.process_postorder(node);
         }
+    }
 
     let context = C::new(shared, root.opaque());
     if context.should_process(root) {

components/style/traversal.rs

@@ -18,6 +18,16 @@ use values::HasViewportPercentage;
 /// 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.
+pub enum RestyleResult {
+    Continue,
+    Stop,
+}
+
+
 /// A pair of the bloom filter used for css selector matching, and the node to
 /// which it applies. This is used to efficiently do `Descendant` selector
 /// matches. Thanks to the bloom filter, we can avoid walking up the tree
@@ -141,12 +151,23 @@ pub fn remove_from_bloom_filter<'a, N, C>(context: &C, root: OpaqueNode, node: N
 
 pub trait DomTraversalContext<N: TNode>  {
     type SharedContext: Sync + 'static;
+
     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);
+    fn process_preorder(&self, node: N) -> RestyleResult;
+
     /// Process `node` on the way up, after its children have been processed.
+    ///
+    /// This is only executed if `needs_postorder_traversal` returns true.
     fn process_postorder(&self, node: N);
 
+    /// Boolean that specifies whether a bottom up traversal should be
+    /// performed.
+    ///
+    /// If it's false, then process_postorder has no effect at all.
+    fn needs_postorder_traversal(&self) -> bool { true }
+
     /// Returns if the node should be processed by the preorder traversal (and
     /// then by the post-order one).
     ///
@@ -172,14 +193,88 @@ pub trait DomTraversalContext<N: TNode>  {
     }
 }
 
+pub fn ensure_node_styled<'a, N, C>(node: N,
+                                    context: &'a C)
+    where N: TNode,
+          C: StyleContext<'a>
+{
+    let mut display_none = false;
+    ensure_node_styled_internal(node, context, &mut display_none);
+}
+
+#[allow(unsafe_code)]
+fn ensure_node_styled_internal<'a, N, C>(node: N,
+                                         context: &'a C,
+                                         parents_had_display_none: &mut bool)
+    where N: TNode,
+          C: StyleContext<'a>
+{
+    use properties::longhands::display::computed_value as display;
+
+    // Ensure we have style data available. This must be done externally because
+    // there's no way to initialize the style data from the style system
+    // (because in Servo it's coupled with the layout data too).
+    //
+    // Ideally we'd have an initialize_data() or something similar but just for
+    // style data.
+    debug_assert!(node.borrow_data().is_some(),
+                  "Need to initialize the data before calling ensure_node_styled");
+
+    // We need to go to the root and ensure their style is up to date.
+    //
+    // This means potentially a bit of wasted work (usually not much). We could
+    // add a flag at the node at which point we stopped the traversal to know
+    // where should we stop, but let's not add that complication unless needed.
+    let parent = match node.parent_node() {
+        Some(parent) if parent.is_element() => Some(parent),
+        _ => None,
+    };
+
+    if let Some(parent) = parent {
+        ensure_node_styled_internal(parent, context, parents_had_display_none);
+    }
+
+    // Common case: our style is already resolved and none of our ancestors had
+    // display: none.
+    //
+    // We only need to mark whether we have display none, and forget about it,
+    // our style is up to date.
+    if let Some(ref style) = node.borrow_data().unwrap().style {
+        if !*parents_had_display_none {
+            *parents_had_display_none = style.get_box().clone_display() == display::T::none;
+            return;
+        }
+    }
+
+    // Otherwise, our style might be out of date. Time to do selector matching
+    // if appropriate and cascade the node.
+    //
+    // Note that we could add the bloom filter's complexity here, but that's
+    // probably not necessary since we're likely to be matching only a few
+    // nodes, at best.
+    let mut applicable_declarations = ApplicableDeclarations::new();
+    if let Some(element) = node.as_element() {
+        let stylist = &context.shared_context().stylist;
+
+        element.match_element(&**stylist,
+                              None,
+                              &mut applicable_declarations);
+    }
+
+    unsafe {
+        node.cascade_node(context, parent, &applicable_declarations);
+    }
+}
+
 /// Calculates the style for a single node.
 #[inline]
 #[allow(unsafe_code)]
 pub fn recalc_style_at<'a, N, C>(context: &'a C,
                                  root: OpaqueNode,
-                                 node: N)
+                                 node: N) -> RestyleResult
     where N: TNode,
-          C: StyleContext<'a> {
+          C: StyleContext<'a>
+{
     // Get the parent node.
     let parent_opt = match node.parent_node() {
         Some(parent) if parent.is_element() => Some(parent),
@@ -190,6 +285,7 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C,
     let mut bf = take_thread_local_bloom_filter(parent_opt, root, context.shared_context());
 
     let nonincremental_layout = opts::get().nonincremental_layout;
+    let mut restyle_result = RestyleResult::Continue;
     if nonincremental_layout || node.is_dirty() {
         // Remove existing CSS styles from nodes whose content has changed (e.g. text changed),
         // to force non-incremental reflow.
@@ -239,7 +335,7 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C,
 
                 // Perform the CSS cascade.
                 unsafe {
-                    node.cascade_node(context,
+                    restyle_result = node.cascade_node(context,
                                                     parent_opt,
                                                     &applicable_declarations);
                 }
@@ -249,7 +345,8 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C,
                     style_sharing_candidate_cache.insert_if_possible::<'ln, N>(&element);
                 }
             }
-            StyleSharingResult::StyleWasShared(index, damage) => {
+            StyleSharingResult::StyleWasShared(index, damage, restyle_result_cascade) => {
+                restyle_result = restyle_result_cascade;
                 style_sharing_candidate_cache.touch(index);
                 node.set_restyle_damage(damage);
             }
@@ -286,4 +383,10 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C,
             }
         }
     }
+
+    if nonincremental_layout {
+        RestyleResult::Continue
+    } else {
+        restyle_result
+    }
 }

ports/geckolib/traversal.rs

@@ -6,6 +6,7 @@ use context::StandaloneStyleContext;
 use std::mem;
 use style::context::SharedStyleContext;
 use style::dom::OpaqueNode;
+use style::traversal::RestyleResult;
 use style::traversal::{DomTraversalContext, recalc_style_at};
 use wrapper::GeckoNode;
 
@@ -27,13 +28,18 @@ impl<'lc, 'ln> DomTraversalContext<GeckoNode<'ln>> for RecalcStyleOnly<'lc> {
         }
     }
 
-    fn process_preorder(&self, node: GeckoNode<'ln>) {
+    fn process_preorder(&self, node: GeckoNode<'ln>) -> RestyleResult {
         // FIXME(pcwalton): Stop allocating here. Ideally this should just be done by the HTML
         // parser.
         node.initialize_data();
 
-        recalc_style_at(&self.context, self.root, node);
+        recalc_style_at(&self.context, self.root, node)
     }
 
-    fn process_postorder(&self, _: GeckoNode<'ln>) {}
+    fn process_postorder(&self, _: GeckoNode<'ln>) {
+        unreachable!();
+    }
+
+    /// We don't use the post-order traversal for anything.
+    fn needs_postorder_traversal(&self) -> bool { false }
 }

ports/geckolib/wrapper.rs

@@ -44,7 +44,7 @@ use style::dom::{TDocument, TElement, TNode, TRestyleDamage, UnsafeNode};
 use style::element_state::ElementState;
 use style::error_reporting::StdoutErrorReporter;
 use style::gecko_glue::ArcHelpers;
-use style::gecko_selector_impl::{GeckoSelectorImpl, NonTSPseudoClass};
+use style::gecko_selector_impl::{GeckoSelectorImpl, NonTSPseudoClass, PseudoElement};
 use style::parser::ParserContextExtraData;
 use style::properties::{ComputedValues, parse_style_attribute};
 use style::properties::{PropertyDeclaration, PropertyDeclarationBlock};
@@ -95,18 +95,21 @@ impl<'ln> GeckoNode<'ln> {
     }
 }
 
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, Debug, PartialEq)]
 pub struct GeckoRestyleDamage(nsChangeHint);
 
 impl TRestyleDamage for GeckoRestyleDamage {
     type PreExistingComputedValues = nsStyleContext;
-    fn compute(source: Option<&nsStyleContext>,
+
+    fn empty() -> Self {
+        use std::mem;
+        GeckoRestyleDamage(unsafe { mem::transmute(0u32) })
+    }
+
+    fn compute(source: &nsStyleContext,
                new_style: &Arc<ComputedValues>) -> Self {
         type Helpers = ArcHelpers<ServoComputedValues, ComputedValues>;
-        let context = match source {
+        let context = source as *const nsStyleContext as *mut nsStyleContext;
-            Some(ctx) => ctx as *const nsStyleContext as *mut nsStyleContext,
-            None => return Self::rebuild_and_reflow(),
-        };
 
         Helpers::borrow(new_style, |new_style| {
             let hint = unsafe { Gecko_CalcStyleDifference(context, new_style) };
@@ -194,10 +197,9 @@ impl<'ln> TNode for GeckoNode<'ln> {
         unimplemented!()
     }
 
-    fn has_changed(&self) -> bool {
+    // NOTE: This is not relevant for Gecko, since we get explicit restyle hints
-        // FIXME(bholley) - Implement this to allow incremental reflows!
+    // when a content has changed.
-        true
+    fn has_changed(&self) -> bool { false }
-    }
 
     unsafe fn set_changed(&self, _value: bool) {
         unimplemented!()
@@ -310,13 +312,21 @@ impl<'ln> TNode for GeckoNode<'ln> {
     }
 
     fn existing_style_for_restyle_damage<'a>(&'a self,
-                                             current_cv: Option<&'a Arc<ComputedValues>>)
+                                             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;
         }
 
+        if pseudo.is_some() {
+            // FIXME(emilio): This makes us reconstruct frame for pseudos every
+            // restyle, add a FFI call to get the style context associated with
+            // a PE.
+            return None;
+        }
+
         unsafe {
             let context_ptr = Gecko_GetStyleContext(self.node);
             context_ptr.as_ref()

tests/wpt/metadata-css/css-transitions-1_dev/html/hidden-container-001.htm.ini

@@ -1,5 +0,0 @@
-[hidden-container-001.htm]
-  type: testharness
-  [transition within display:none / values]
-    expected: FAIL
-

tests/wpt/mozilla/meta/MANIFEST.json

@@ -4420,6 +4420,18 @@
             "url": "/_mozilla/css/pseudo_element_a.html"
           }
         ],
+        "css/pseudo_element_restyle_no_rules.html": [
+          {
+            "path": "css/pseudo_element_restyle_no_rules.html",
+            "references": [
+              [
+                "/_mozilla/css/pseudo_element_restyle_no_rules_ref.html",
+                "=="
+              ]
+            ],
+            "url": "/_mozilla/css/pseudo_element_restyle_no_rules.html"
+          }
+        ],
         "css/pseudo_inherit.html": [
           {
             "path": "css/pseudo_inherit.html",
@@ -13586,6 +13598,18 @@
           "url": "/_mozilla/css/pseudo_element_a.html"
         }
       ],
+      "css/pseudo_element_restyle_no_rules.html": [
+        {
+          "path": "css/pseudo_element_restyle_no_rules.html",
+          "references": [
+            [
+              "/_mozilla/css/pseudo_element_restyle_no_rules_ref.html",
+              "=="
+            ]
+          ],
+          "url": "/_mozilla/css/pseudo_element_restyle_no_rules.html"
+        }
+      ],
       "css/pseudo_inherit.html": [
         {
           "path": "css/pseudo_inherit.html",

tests/wpt/mozilla/tests/css/pseudo_element_restyle_no_rules.html

@@ -0,0 +1,20 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Pseudo-Element is restyled correctly when matching rules become empty</title>
+<link rel="match" href="pseudo_element_restyle_no_rules_ref.html">
+<style>
+  p.whatever::before { content: " hey "; }
+</style>
+<p>
+  test.
+
+<script>
+var p = document.querySelector('p');
+var toggle = function() {
+  p.className = p.className == "whatever" ? "" : "whatever";
+};
+toggle();
+// Ensure a restyle happens.
+p.offsetTop;
+toggle()
+</script>

tests/wpt/mozilla/tests/css/pseudo_element_restyle_no_rules_ref.html

@@ -0,0 +1,5 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test reference.</title>
+<p>
+  test.