Auto merge of #17245 - bzbarsky:share-pseudo-styles, r=emilio

Share styles for elements with eager pseudo-elements attached to them

<!-- 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
- [X] These changes fix https://bugzilla.mozilla.org/show_bug.cgi?id=1329361

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

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

<!-- 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/17245)
<!-- Reviewable:end -->

components/style/data.rs

@@ -294,6 +294,26 @@ impl EagerPseudoStyles {
             },
         }
     }
+
+    /// Returns whether this EagerPseudoStyles has the same set of
+    /// pseudos as the given one.
+    pub fn has_same_pseudos_as(&self, other: &EagerPseudoStyles) -> bool {
+        // We could probably just compare self.keys() to other.keys(), but that
+        // seems like it'll involve a bunch more moving stuff around and
+        // whatnot.
+        match (&self.0, &other.0) {
+            (&Some(ref our_arr), &Some(ref other_arr)) => {
+                for i in 0..EAGER_PSEUDO_COUNT {
+                    if our_arr[i].is_some() != other_arr[i].is_some() {
+                        return false
+                    }
+                }
+                true
+            },
+            (&None, &None) => true,
+            _ => false,
+        }
+    }
 }
 
 /// The styles associated with a node, including the styles for any

components/style/sharing/checks.rs

@@ -11,21 +11,9 @@ use bloom::StyleBloom;
 use context::{SelectorFlagsMap, SharedStyleContext};
 use dom::TElement;
 use element_state::*;
-use selectors::matching::StyleRelations;
 use sharing::{StyleSharingCandidate, StyleSharingTarget};
 use stylearc::Arc;
 
-/// Determines, based on the results of selector matching, whether it's worth to
-/// try to share style with this element, that is, to try to insert the element
-/// in the chache.
-#[inline]
-pub fn relations_are_shareable(relations: &StyleRelations) -> bool {
-    use selectors::matching::*;
-    // If we start sharing things that are AFFECTED_BY_PSEUDO_ELEMENTS, we need
-    // to track revalidation selectors on a per-pseudo-element basis.
-    !relations.intersects(AFFECTED_BY_PSEUDO_ELEMENTS)
-}
-
 /// Whether, given two elements, they have pointer-equal computed values.
 ///
 /// Both elements need to be styled already.

components/style/sharing/mod.rs

@@ -32,14 +32,12 @@
 //! * We check that the target and candidate have the same inline style and
 //!   presentation hint declarations.  This addresses constraint 3.
 //!
-//! * We ensure that elements that have pseudo-element styles are not inserted
-//!   into the cache.  This partially addresses constraint 4.
-//!
 //! * We ensure that a target matches a candidate only if they have the same
 //!   matching result for all selectors that target either elements or the
-//!   originating elements of pseudo-elements.  This addresses the second half
+//!   originating elements of pseudo-elements.  This addresses constraint 4
-//!   of constraint 4 (because it prevents a target that has pseudo-element
+//!   (because it prevents a target that has pseudo-element styles from matching
-//!   styles from matching any candidate) as well as constraint 2.
+//!   a candidate that has different pseudo-element styles) as well as
+//!   constraint 2.
 //!
 //! The actual checks that ensure that elements match the same rules are
 //! conceptually split up into two pieces.  First, we do various checks on
@@ -71,10 +69,11 @@ use bit_vec::BitVec;
 use bloom::StyleBloom;
 use cache::{LRUCache, LRUCacheMutIterator};
 use context::{SelectorFlagsMap, SharedStyleContext, StyleContext};
-use data::{ComputedStyle, ElementData, ElementStyles};
+use data::{ElementData, ElementStyles};
 use dom::{TElement, SendElement};
 use matching::{ChildCascadeRequirement, MatchMethods};
 use properties::ComputedValues;
+use selector_parser::RestyleDamage;
 use selectors::matching::{ElementSelectorFlags, VisitedHandlingMode, StyleRelations};
 use smallvec::SmallVec;
 use std::mem;
@@ -359,6 +358,61 @@ impl<E: TElement> StyleSharingTarget<E> {
             self.validation_data.take();
         result
     }
+
+    fn accumulate_damage_when_sharing(&self,
+                                      shared_context: &SharedStyleContext,
+                                      shared_style: &ElementStyles,
+                                      data: &mut ElementData) -> ChildCascadeRequirement {
+        // Accumulate restyle damage for the case when our sharing
+        // target managed to share style.  This can come from several
+        // sources:
+        //
+        // 1) We matched a different set of eager pseudos (which
+        //    should cause a reconstruct).
+        // 2) We have restyle damage from the eager pseudo computed
+        //    styles.
+        // 3) We have restyle damage from our own computed styles.
+        if data.has_styles() {
+            // We used to have pseudos (because we had styles).
+            // Check for damage from the set of pseudos changing or
+            // pseudos being restyled.
+            let (styles, restyle_data) = data.styles_and_restyle_mut();
+            if let Some(restyle_data) = restyle_data {
+                let old_pseudos = &styles.pseudos;
+                let new_pseudos = &shared_style.pseudos;
+                if !old_pseudos.has_same_pseudos_as(new_pseudos) {
+                    restyle_data.damage |= RestyleDamage::reconstruct();
+                } else {
+                    // It's a bit unfortunate that we have to keep
+                    // mapping PseudoElements back to indices
+                    // here....
+                    for pseudo in old_pseudos.keys() {
+                        let old_values =
+                            old_pseudos.get(&pseudo).unwrap().values.as_ref().map(|v| &**v);
+                        let new_values =
+                            new_pseudos.get(&pseudo).unwrap().values();
+                        self.element.accumulate_damage(
+                            &shared_context,
+                            Some(restyle_data),
+                            old_values,
+                            new_values,
+                            Some(&pseudo)
+                        );
+                    }
+                }
+            }
+        }
+
+        let old_values = data.get_styles_mut()
+                             .and_then(|s| s.primary.values.take());
+        self.element.accumulate_damage(
+            &shared_context,
+            data.get_restyle_mut(),
+            old_values.as_ref().map(|v| &**v),
+            shared_style.primary.values(),
+            None
+        )
+    }
 }
 
 /// A cache miss result.
@@ -461,11 +515,6 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
 
         // These are things we don't check in the candidate match because they
         // are either uncommon or expensive.
-        if !checks::relations_are_shareable(&relations) {
-            debug!("Failing to insert to the cache: {:?}", relations);
-            return;
-        }
-
         let box_style = style.get_box();
         if box_style.specifies_transitions() {
             debug!("Failing to insert to the cache: transitions");
@@ -548,26 +597,13 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
                 Ok(shared_style) => {
                     // Yay, cache hit. Share the style.
 
-                    // Accumulate restyle damage.
                     debug_assert_eq!(data.has_styles(), data.has_restyle());
-                    let old_values = data.get_styles_mut()
-                                         .and_then(|s| s.primary.values.take());
-                    let child_cascade_requirement =
-                        target.accumulate_damage(
-                            &shared_context,
-                            data.get_restyle_mut(),
-                            old_values.as_ref().map(|v| &**v),
-                            shared_style.values(),
-                            None
-                        );
 
-                    // We never put elements with pseudo style into the style
+                    let child_cascade_requirement =
-                    // sharing cache, so we can just mint an ElementStyles
+                        target.accumulate_damage_when_sharing(shared_context,
-                    // directly here.
+                                                              &shared_style,
-                    //
+                                                              data);
-                    // See https://bugzilla.mozilla.org/show_bug.cgi?id=1329361
+                    data.set_styles(shared_style);
-                    let styles = ElementStyles::new(shared_style);
-                    data.set_styles(styles);
 
                     return StyleSharingResult::StyleWasShared(i, child_cascade_requirement)
                 }
@@ -598,7 +634,7 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
                       shared: &SharedStyleContext,
                       bloom: &StyleBloom<E>,
                       selector_flags_map: &mut SelectorFlagsMap<E>)
-                      -> Result<ComputedStyle, CacheMiss> {
+                      -> Result<ElementStyles, CacheMiss> {
         macro_rules! miss {
             ($miss: ident) => {
                 return Err(CacheMiss::$miss);
@@ -674,6 +710,6 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
 
         debug!("Sharing style between {:?} and {:?}",
                target.element, candidate.element);
-        Ok(data.styles().primary.clone())
+        Ok(data.styles().clone())
     }
 }