style: Isolate all the restyling related logic in the invalidator in an InvalidationProcessor trait.

Ditto, no change in behavior.

components/style/data.rs

@@ -244,7 +244,7 @@ impl ElementData {
             return InvalidationResult::empty();
         }
 
-        use invalidation::element::collector::StateAndAttrInvalidationCollector;
+        use invalidation::element::collector::StateAndAttrInvalidationProcessor;
         use invalidation::element::invalidator::TreeStyleInvalidator;
 
         debug!("invalidate_style_if_needed: {:?}, flags: {:?}, has_snapshot: {}, \
@@ -259,18 +259,21 @@ impl ElementData {
             return InvalidationResult::empty();
         }
 
+        let processor = StateAndAttrInvalidationProcessor;
         let invalidator = TreeStyleInvalidator::new(
             element,
             Some(self),
             shared_context,
             stack_limit_checker,
             nth_index_cache,
+            &processor,
         );
 
-        let result =
+        let result = invalidator.invalidate();
-            invalidator.invalidate::<StateAndAttrInvalidationCollector>();
+
         unsafe { element.set_handled_snapshot() }
         debug_assert!(element.handled_snapshot());
+
         result
     }
 

components/style/invalidation/element/collector.rs

@@ -2,7 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-//! A collector for invalidations due to state and attribute changes.
+//! An invalidation processor for style changes due to state and attribute
+//! changes.
 
 use Atom;
 use context::{QuirksMode, SharedStyleContext};
@@ -11,7 +12,7 @@ use dom::TElement;
 use element_state::{ElementState, IN_VISITED_OR_UNVISITED_STATE};
 use invalidation::element::element_wrapper::{ElementSnapshot, ElementWrapper};
 use invalidation::element::invalidation_map::*;
-use invalidation::element::invalidator::{InvalidationVector, Invalidation, InvalidationCollector};
+use invalidation::element::invalidator::{InvalidationVector, Invalidation, InvalidationProcessor};
 use invalidation::element::restyle_hints::*;
 use selector_map::SelectorMap;
 use selector_parser::Snapshot;
@@ -48,11 +49,13 @@ where
     invalidates_self: bool,
 }
 
-///  A collector for state and attribute invalidations.
+/// An invalidation processor for style changes due to state and attribute
-pub struct StateAndAttrInvalidationCollector;
+/// changes.
+pub struct StateAndAttrInvalidationProcessor;
 
-impl InvalidationCollector for StateAndAttrInvalidationCollector {
+impl InvalidationProcessor for StateAndAttrInvalidationProcessor {
     fn collect_invalidations<E>(
+        &self,
         element: E,
         mut data: Option<&mut ElementData>,
         nth_index_cache: Option<&mut NthIndexCache>,
@@ -172,6 +175,96 @@ impl InvalidationCollector for StateAndAttrInvalidationCollector {
 
         invalidated_self
     }
+
+    fn should_process_descendants<E>(
+        &self,
+        _element: E,
+        data: Option<&mut ElementData>,
+    ) -> bool
+    where
+        E: TElement,
+    {
+        let data = match data {
+            None => return false,
+            Some(ref data) => data,
+        };
+
+        // FIXME(emilio): should check only RESTYLE_DESCENDANTS.
+        //
+        // Also, could probably return false if data.styles.is_display_none()
+        // returns true.
+        !data.hint.contains_subtree()
+    }
+
+    fn recursion_limit_exceeded<E>(
+        &self,
+        _element: E,
+        data: Option<&mut ElementData>,
+    )
+    where
+        E: TElement,
+    {
+        if let Some(data) = data {
+            data.hint.insert(RESTYLE_DESCENDANTS);
+        }
+    }
+
+    fn invalidated_child<E>(
+        &self,
+        element: E,
+        _data: Option<&mut ElementData>,
+        child: E,
+    )
+    where
+        E: TElement,
+    {
+        if child.get_data().is_none() {
+            return;
+        }
+
+        // The child may not be a flattened tree child of the current element,
+        // but may be arbitrarily deep.
+        //
+        // Since we keep the traversal flags in terms of the flattened tree,
+        // we need to propagate it as appropriate.
+        let mut current = child.traversal_parent();
+        while let Some(parent) = current.take() {
+            if parent == element {
+                break;
+            }
+
+            unsafe { parent.set_dirty_descendants() };
+            current = parent.traversal_parent();
+        }
+    }
+
+    fn invalidated_descendants<E>(
+        &self,
+        element: E,
+        data: Option<&mut ElementData>,
+    )
+    where
+        E: TElement,
+    {
+        // FIXME(emilio): We probably want to walk the flattened tree here too,
+        // and remove invalidated_child instead, or something like that.
+        if data.as_ref().map_or(false, |d| !d.styles.is_display_none()) {
+            unsafe { element.set_dirty_descendants() };
+        }
+    }
+
+    fn invalidated_self<E>(
+        &self,
+        _element: E,
+        data: Option<&mut ElementData>,
+    )
+    where
+        E: TElement,
+    {
+        if let Some(data) = data {
+            data.hint.insert(RESTYLE_SELF);
+        }
+    }
 }
 
 impl<'a, 'b, E> Collector<'a, 'b, E>

components/style/invalidation/element/invalidator.rs

@@ -8,7 +8,6 @@
 use context::{SharedStyleContext, StackLimitChecker};
 use data::ElementData;
 use dom::{TElement, TNode};
-use invalidation::element::restyle_hints::*;
 use selector_parser::SelectorImpl;
 use selectors::NthIndexCache;
 use selectors::matching::{MatchingContext, MatchingMode, VisitedHandlingMode};
@@ -19,11 +18,15 @@ use smallvec::SmallVec;
 use std::fmt;
 
 /// A trait to abstract the collection of invalidations for a given pass.
-pub trait InvalidationCollector {
+///
+/// The `data` argument is a mutable reference to the element's style data, if
+/// any.
+pub trait InvalidationProcessor {
     /// Collect invalidations for a given element's descendants and siblings.
     ///
     /// Returns whether the element itself was invalidated.
     fn collect_invalidations<E>(
+        &self,
         element: E,
         data: Option<&mut ElementData>,
         nth_index_cache: Option<&mut NthIndexCache>,
@@ -33,12 +36,62 @@ pub trait InvalidationCollector {
     ) -> bool
     where
         E: TElement;
+
+    /// Returns whether the invalidation process should process the descendants
+    /// of the given element.
+    fn should_process_descendants<E>(
+        &self,
+        element: E,
+        data: Option<&mut ElementData>,
+    ) -> bool
+    where
+        E: TElement;
+
+    /// Executes an arbitrary action when the recursion limit is exceded (if
+    /// any).
+    fn recursion_limit_exceeded<E>(
+        &self,
+        element: E,
+        data: Option<&mut ElementData>,
+    )
+    where
+        E: TElement;
+
+    /// Executes an arbitrary action when a direct child is invalidated.
+    fn invalidated_child<E>(
+        &self,
+        element: E,
+        data: Option<&mut ElementData>,
+        child: E,
+    )
+    where
+        E: TElement;
+
+    /// Executes an action when `Self` is invalidated.
+    fn invalidated_self<E>(
+        &self,
+        element: E,
+        data: Option<&mut ElementData>,
+    )
+    where
+        E: TElement;
+
+    /// Executes an action when any descendant of `Self` is invalidated.
+    fn invalidated_descendants<E>(
+        &self,
+        element: E,
+        data: Option<&mut ElementData>,
+    )
+    where
+        E: TElement;
 }
 
 /// The struct that takes care of encapsulating all the logic on where and how
 /// element styles need to be invalidated.
-pub struct TreeStyleInvalidator<'a, 'b: 'a, E>
+pub struct TreeStyleInvalidator<'a, 'b: 'a, E, P: 'a>
-    where E: TElement,
+where
+    E: TElement,
+    P: InvalidationProcessor
 {
     element: E,
     // TODO(emilio): It's tempting enough to just avoid running invalidation for
@@ -54,6 +107,10 @@ pub struct TreeStyleInvalidator<'a, 'b: 'a, E>
     shared_context: &'a SharedStyleContext<'b>,
     stack_limit_checker: Option<&'a StackLimitChecker>,
     nth_index_cache: Option<&'a mut NthIndexCache>,
+
+    // TODO(emilio): Make a mutable reference, we're going to need that for
+    // QS/QSA.
+    processor: &'a P,
 }
 
 /// A vector of invalidations, optimized for small invalidation sets.
@@ -182,8 +239,10 @@ impl InvalidationResult {
     }
 }
 
-impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
+impl<'a, 'b: 'a, E, P: 'a> TreeStyleInvalidator<'a, 'b, E, P>
-    where E: TElement,
+where
+    E: TElement,
+    P: InvalidationProcessor,
 {
     /// Trivially constructs a new `TreeStyleInvalidator`.
     pub fn new(
@@ -192,6 +251,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
         shared_context: &'a SharedStyleContext<'b>,
         stack_limit_checker: Option<&'a StackLimitChecker>,
         nth_index_cache: Option<&'a mut NthIndexCache>,
+        processor: &'a P,
     ) -> Self {
         Self {
             element,
@@ -199,17 +259,18 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
             shared_context,
             stack_limit_checker,
             nth_index_cache,
+            processor,
         }
     }
 
     /// Perform the invalidation pass.
-    pub fn invalidate<C: InvalidationCollector>(mut self) -> InvalidationResult {
+    pub fn invalidate(mut self) -> InvalidationResult {
         debug!("StyleTreeInvalidator::invalidate({:?})", self.element);
 
         let mut descendant_invalidations = InvalidationVector::new();
         let mut sibling_invalidations = InvalidationVector::new();
 
-        let invalidated_self = C::collect_invalidations(
+        let invalidated_self = self.processor.collect_invalidations(
             self.element,
             self.data.as_mut().map(|d| &mut **d),
             self.nth_index_cache.as_mut().map(|c| &mut **c),
@@ -246,14 +307,14 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
 
         while let Some(sibling) = current {
             let mut sibling_data = sibling.mutate_data();
-            let sibling_data = sibling_data.as_mut().map(|d| &mut **d);
 
             let mut sibling_invalidator = TreeStyleInvalidator::new(
                 sibling,
-                sibling_data,
+                sibling_data.as_mut().map(|d| &mut **d),
                 self.shared_context,
                 self.stack_limit_checker,
                 self.nth_index_cache.as_mut().map(|c| &mut **c),
+                self.processor,
             );
 
             let mut invalidations_for_descendants = InvalidationVector::new();
@@ -310,14 +371,14 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
         sibling_invalidations: &mut InvalidationVector,
     ) -> bool {
         let mut child_data = child.mutate_data();
-        let child_data = child_data.as_mut().map(|d| &mut **d);
 
         let mut child_invalidator = TreeStyleInvalidator::new(
             child,
-            child_data,
+            child_data.as_mut().map(|d| &mut **d),
             self.shared_context,
             self.stack_limit_checker,
             self.nth_index_cache.as_mut().map(|c| &mut **c),
+            self.processor,
         );
 
         let mut invalidations_for_descendants = InvalidationVector::new();
@@ -341,16 +402,12 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
         //
         // Since we keep the traversal flags in terms of the flattened tree,
         // we need to propagate it as appropriate.
-        if invalidated_child && child.get_data().is_some() {
+        if invalidated_child {
-            let mut current = child.traversal_parent();
+            self.processor.invalidated_child(
-            while let Some(parent) = current.take() {
+                self.element,
-                if parent == self.element {
+                self.data.as_mut().map(|d| &mut **d),
-                    break;
+                child,
-                }
+            );
-
-                unsafe { parent.set_dirty_descendants() };
-                current = parent.traversal_parent();
-            }
         }
 
         let invalidated_descendants = child_invalidator.invalidate_descendants(
@@ -424,20 +481,22 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
                self.element);
         debug!(" > {:?}", invalidations);
 
-        match self.data {
+        let should_process =
-            None => return false,
+            self.processor.should_process_descendants(
-            Some(ref data) => {
+                self.element,
-                // FIXME(emilio): Only needs to check RESTYLE_DESCENDANTS,
+                self.data.as_mut().map(|d| &mut **d),
-                // really.
+            );
-                if data.hint.contains_subtree() {
+
+        if !should_process {
             return false;
         }
-            }
-        }
 
         if let Some(checker) = self.stack_limit_checker {
             if checker.limit_exceeded() {
-                self.data.as_mut().unwrap().hint.insert(RESTYLE_DESCENDANTS);
+                self.processor.recursion_limit_exceeded(
+                    self.element,
+                    self.data.as_mut().map(|d| &mut **d)
+                );
                 return true;
             }
         }
@@ -467,8 +526,11 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
 
         any_descendant |= self.invalidate_nac(invalidations);
 
-        if any_descendant && self.data.as_ref().map_or(false, |d| !d.styles.is_display_none()) {
+        if any_descendant {
-            unsafe { self.element.set_dirty_descendants() };
+            self.processor.invalidated_descendants(
+                self.element,
+                self.data.as_mut().map(|d| &mut **d)
+            );
         }
 
         any_descendant
@@ -730,9 +792,10 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
         }
 
         if invalidated_self {
-            if let Some(ref mut data) = self.data {
+            self.processor.invalidated_self(
-                data.hint.insert(RESTYLE_SELF);
+                self.element,
-            }
+                self.data.as_mut().map(|d| &mut **d),
+            );
         }
 
         SingleInvalidationResult { invalidated_self, matched, }