Auto merge of #17888 - emilio:o-to-the-n, r=heycam

style: Avoid exponential blowup when processing invalidations of the same kind.

Bug: 1383981
Reviewed-by: heycam
MozReview-Commit-ID: HZ97UwvblkQ

components/style/invalidation/element/invalidator.rs

@@ -35,6 +35,16 @@ pub struct TreeStyleInvalidator<'a, 'b: 'a, E>
 
 type InvalidationVector = SmallVec<[Invalidation; 10]>;
 
+/// The kind of invalidation we're processing.
+///
+/// We can use this to avoid pushing invalidations of the same kind to our
+/// descendants or siblings.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+enum InvalidationKind {
+    Descendant,
+    Sibling,
+}
+
 /// An `Invalidation` is a complex selector that describes which elements,
 /// relative to a current element we are processing, must be restyled.
 ///
@@ -46,6 +56,37 @@ type InvalidationVector = SmallVec<[Invalidation; 10]>;
 struct Invalidation {
     selector: Selector<SelectorImpl>,
     offset: usize,
+    /// Whether the invalidation was already matched by any previous sibling or
+    /// ancestor.
+    ///
+    /// If this is the case, we can avoid pushing invalidations generated by
+    /// this one if the generated invalidation is effective for all the siblings
+    /// or descendants after us.
+    matched_by_any_previous: bool,
+}
+
+impl Invalidation {
+    /// Whether this invalidation is effective for the next sibling or
+    /// descendant after us.
+    fn effective_for_next(&self) -> bool {
+        // TODO(emilio): For pseudo-elements this should be mostly false, except
+        // for the weird pseudos in <input type="number">.
+        //
+        // We should be able to do better here!
+        match self.selector.combinator_at(self.offset) {
+            Combinator::NextSibling |
+            Combinator::Child => false,
+            _ => true,
+        }
+    }
+
+    fn kind(&self) -> InvalidationKind {
+        if self.selector.combinator_at(self.offset).is_ancestor() {
+            InvalidationKind::Descendant
+        } else {
+            InvalidationKind::Sibling
+        }
+    }
 }
 
 impl fmt::Debug for Invalidation {
@@ -67,9 +108,9 @@ impl fmt::Debug for Invalidation {
 struct InvalidationResult {
     /// Whether the element itself was invalidated.
     invalidated_self: bool,
-    /// Whether the invalidation we've processed is effective for the next
+    /// Whether the invalidation matched, either invalidating the element or
-    /// sibling or descendant after us.
+    /// generating another invalidation.
-    effective_for_next: bool,
+    matched: bool,
 }
 
 impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
@@ -460,14 +501,16 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
             let result = self.process_invalidation(
                 &sibling_invalidations[i],
                 descendant_invalidations,
-                &mut new_sibling_invalidations
+                &mut new_sibling_invalidations,
+                InvalidationKind::Sibling,
             );
 
             invalidated_self |= result.invalidated_self;
-            if !result.effective_for_next {
+            sibling_invalidations[i].matched_by_any_previous |= result.matched;
-                sibling_invalidations.remove(i);
+            if sibling_invalidations[i].effective_for_next() {
-            } else {
                 i += 1;
+            } else {
+                sibling_invalidations.remove(i);
             }
         }
 
@@ -493,10 +536,13 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
                 invalidation,
                 descendant_invalidations,
                 sibling_invalidations,
+                InvalidationKind::Descendant,
             );
 
             invalidated |= result.invalidated_self;
-            if result.effective_for_next {
+            if invalidation.effective_for_next() {
+                let mut invalidation = invalidation.clone();
+                invalidation.matched_by_any_previous |= result.matched;
                 descendant_invalidations.push(invalidation.clone());
             }
         }
@@ -514,10 +560,11 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
         &mut self,
         invalidation: &Invalidation,
         descendant_invalidations: &mut InvalidationVector,
-        sibling_invalidations: &mut InvalidationVector
+        sibling_invalidations: &mut InvalidationVector,
+        invalidation_kind: InvalidationKind,
     ) -> InvalidationResult {
-        debug!("TreeStyleInvalidator::process_invalidation({:?}, {:?})",
+        debug!("TreeStyleInvalidator::process_invalidation({:?}, {:?}, {:?})",
-               self.element, invalidation);
+               self.element, invalidation, invalidation_kind);
 
         let mut context =
             MatchingContext::new_for_visited(
@@ -535,14 +582,17 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
         );
 
         let mut invalidated_self = false;
+        let mut matched = false;
         match matching_result {
             CompoundSelectorMatchingResult::Matched { next_combinator_offset: 0 } => {
                 debug!(" > Invalidation matched completely");
+                matched = true;
                 invalidated_self = true;
             }
             CompoundSelectorMatchingResult::Matched { next_combinator_offset } => {
                 let next_combinator =
                     invalidation.selector.combinator_at(next_combinator_offset);
+                matched = true;
 
                 if matches!(next_combinator, Combinator::PseudoElement) {
                     let pseudo_selector =
@@ -578,14 +628,90 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
                 let next_invalidation = Invalidation {
                     selector: invalidation.selector.clone(),
                     offset: next_combinator_offset,
+                    matched_by_any_previous: false,
                 };
 
                 debug!(" > Invalidation matched, next: {:?}, ({:?})",
                         next_invalidation, next_combinator);
-                if next_combinator.is_ancestor() {
+
-                    descendant_invalidations.push(next_invalidation);
+                let next_invalidation_kind = next_invalidation.kind();
+
+                // We can skip pushing under some circumstances, and we should
+                // because otherwise the invalidation list could grow
+                // exponentially.
+                //
+                //  * First of all, both invalidations need to be of the same
+                //    kind. This is because of how we propagate them going to
+                //    the right of the tree for sibling invalidations and going
+                //    down the tree for children invalidations. A sibling
+                //    invalidation that ends up generating a children
+                //    invalidation ends up (correctly) in five different lists,
+                //    not in the same list five different times.
+                //
+                //  * Then, the invalidation needs to be matched by a previous
+                //    ancestor/sibling, in order to know that this invalidation
+                //    has been generated already.
+                //
+                //  * Finally, the new invalidation needs to be
+                //    `effective_for_next()`, in order for us to know that it is
+                //    still in the list, since we remove the dependencies that
+                //    aren't from the lists for our children / siblings.
+                //
+                // To go through an example, let's imagine we are processing a
+                // dom subtree like:
+                //
+                //   <div><address><div><div/></div></address></div>
+                //
+                // And an invalidation list with a single invalidation like:
+                //
+                //   [div div div]
+                //
+                // When we process the invalidation list for the outer div, we
+                // match it, and generate a `div div` invalidation, so for the
+                // <address> child we have:
+                //
+                //   [div div div, div div]
+                //
+                // With the first of them marked as `matched`.
+                //
+                // When we process the <address> child, we don't match any of
+                // them, so both invalidations go untouched to our children.
+                //
+                // When we process the second <div>, we match _both_
+                // invalidations.
+                //
+                // However, when matching the first, we can tell it's been
+                // matched, and not push the corresponding `div div`
+                // invalidation, since we know it's necessarily already on the
+                // list.
+                //
+                // Thus, without skipping the push, we'll arrive to the
+                // innermost <div> with:
+                //
+                //   [div div div, div div, div div, div]
+                //
+                // While skipping it, we won't arrive here with duplicating
+                // dependencies:
+                //
+                //   [div div div, div div, div]
+                //
+                let can_skip_pushing =
+                    next_invalidation_kind == invalidation_kind &&
+                    invalidation.matched_by_any_previous &&
+                    next_invalidation.effective_for_next();
+
+                if can_skip_pushing {
+                    debug!(" > Can avoid push, since the invalidation had \
+                           already been matched before");
                 } else {
-                    sibling_invalidations.push(next_invalidation);
+                    match next_invalidation_kind {
+                        InvalidationKind::Descendant => {
+                            descendant_invalidations.push(next_invalidation);
+                        }
+                        InvalidationKind::Sibling => {
+                            sibling_invalidations.push(next_invalidation);
+                        }
+                    }
                 }
             }
             CompoundSelectorMatchingResult::NotMatched => {}
@@ -597,21 +723,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
             }
         }
 
-        // TODO(emilio): For pseudo-elements this should be mostly false, except
+        InvalidationResult { invalidated_self, matched, }
-        // for the weird pseudos in <input type="number">.
-        //
-        // We should be able to do better here!
-        let effective_for_next =
-            match invalidation.selector.combinator_at(invalidation.offset) {
-                Combinator::NextSibling |
-                Combinator::Child => false,
-                _ => true,
-            };
-
-        InvalidationResult {
-            invalidated_self: invalidated_self,
-            effective_for_next: effective_for_next,
-        }
     }
 }
 
@@ -830,12 +942,14 @@ impl<'a, 'b: 'a, E> InvalidationCollector<'a, 'b, E>
             self.descendant_invalidations.push(Invalidation {
                 selector: dependency.selector.clone(),
                 offset: dependency.selector_offset,
+                matched_by_any_previous: false,
             });
         } else if dependency.affects_later_siblings() {
             debug_assert_ne!(dependency.selector_offset, 0);
             self.sibling_invalidations.push(Invalidation {
                 selector: dependency.selector.clone(),
                 offset: dependency.selector_offset,
+                matched_by_any_previous: false,
             });
         }
     }