servo: Merge #17980 - Introduce a new flag and use it to be more permissive...

servo: Merge #17980 - Introduce a new flag and use it to be more permissive about cousin sharing (from bholley:fix_initial_sharing); r=emilio

https://bugzilla.mozilla.org/show_bug.cgi?id=1387116

Source-Repo: https://github.com/servo/servo
Source-Revision: b701d726f2840d61573d213f2e2008335dc9831e

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 4f81898b45e85740b61ae598a7f855708d8c0f2b

servo/components/style/data.rs

@@ -22,8 +22,17 @@ bitflags! {
     flags RestyleFlags: u8 {
         /// Whether the styles changed for this restyle.
         const WAS_RESTYLED = 1 << 0,
+        /// Whether the last traversal of this element did not do
+        /// any style computation. This is not true during the initial
+        /// styling pass, nor is it true when we restyle (in which case
+        /// WAS_RESTYLED is set).
+        ///
+        /// This bit always corresponds to the last time the element was
+        /// traversed, so each traversal simply updates it with the appropriate
+        /// value.
+        const TRAVERSED_WITHOUT_STYLING = 1 << 1,
         /// Whether we reframed/reconstructed any ancestor or self.
-        const ANCESTOR_WAS_RECONSTRUCTED = 1 << 1,
+        const ANCESTOR_WAS_RECONSTRUCTED = 1 << 2,
     }
 }
 
@@ -98,14 +107,25 @@ impl RestyleData {
     /// to do a post-traversal.
     pub fn set_restyled(&mut self) {
         self.flags.insert(WAS_RESTYLED);
+        self.flags.remove(TRAVERSED_WITHOUT_STYLING);
     }
 
-    /// Mark this element as restyled, which is useful to know whether we need
-    /// to do a post-traversal.
+    /// Returns true if this element was restyled.
     pub fn is_restyle(&self) -> bool {
         self.flags.contains(WAS_RESTYLED)
     }
 
+    /// Mark that we traversed this element without computing any style for it.
+    pub fn set_traversed_without_styling(&mut self) {
+        self.flags.insert(TRAVERSED_WITHOUT_STYLING);
+    }
+
+    /// Returns whether the element was traversed without computing any style for
+    /// it.
+    pub fn traversed_without_styling(&self) -> bool {
+        self.flags.contains(TRAVERSED_WITHOUT_STYLING)
+    }
+
     /// Returns whether this element has been part of a restyle.
     pub fn contains_restyle_data(&self) -> bool {
         self.is_restyle() || !self.hint.is_empty() || !self.damage.is_empty()

servo/components/style/sharing/checks.rs

@@ -13,18 +13,11 @@ use dom::TElement;
 use servo_arc::Arc;
 use sharing::{StyleSharingCandidate, StyleSharingTarget};
 
-/// Whether, given two elements, they have pointer-equal computed values.
+/// Whether styles may be shared across the children of the given parent elements.
+/// This is used to share style across cousins.
 ///
 /// Both elements need to be styled already.
-///
-/// This is used to know whether we can share style across cousins (if the two
-/// parents have the same style).
-///
-/// We can only prove that they have the same class list, state, etc if they've
-/// been restyled at the same time, otherwise the invalidation pass could make
-/// them keep the same computed values even though now we wouldn't be able to
-/// prove we match the same selectors.
-pub fn same_computed_values<E>(first: Option<E>, second: Option<E>) -> bool
+pub fn can_share_style_across_parents<E>(first: Option<E>, second: Option<E>) -> bool
     where E: TElement,
 {
     let (first, second) = match (first, second) {
@@ -37,13 +30,18 @@ pub fn same_computed_values<E>(first: Option<E>, second: Option<E>) -> bool
     let first_data = first.borrow_data().unwrap();
     let second_data = second.borrow_data().unwrap();
 
-    // FIXME(emilio): This check effectively disables cousin style sharing
-    // on the initial style.
-    //
-    // This is pretty bad, se the discussion in bug 1381821 for mor details.
+    // If a parent element was already styled and we traversed past it without
+    // restyling it, that may be because our clever invalidation logic was able
+    // to prove that the styles of that element would remain unchanged despite
+    // changes to the id or class attributes. However, style sharing relies in
+    // the strong guarantee that all the classes and ids up the respective parent
+    // chains are identical. As such, if we skipped styling for one (or both) of
+    // the parents on this traversal, we can't share styles across cousins.
     //
-    // Bug 1387116 tracks fixing this, and various solutions are listed there.
-    if !first_data.restyle.is_restyle() || !second_data.restyle.is_restyle() {
+    // This is a somewhat conservative check. We could tighten it by having the
+    // invalidation logic explicitly flag elements for which it ellided styling.
+    if first_data.restyle.traversed_without_styling() ||
+       second_data.restyle.traversed_without_styling() {
         return false;
     }
 

servo/components/style/sharing/mod.rs

@@ -610,13 +610,14 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
             }
         }
 
-        // Check that we have the same parent, or at least the same pointer
-        // identity for parent computed style. The latter check allows us to
-        // share style between cousins if the parents shared style.
+        // Check that we have the same parent, or at least that the parents
+        // share styles and permit sharing across their children. The latter
+        // check allows us to share style between cousins if the parents
+        // shared style.
         let parent = target.traversal_parent();
         let candidate_parent = candidate.element.traversal_parent();
         if parent != candidate_parent &&
-           !checks::same_computed_values(parent, candidate_parent) {
+           !checks::can_share_style_across_parents(parent, candidate_parent) {
             miss!(Parent)
         }
 

servo/components/style/traversal.rs

@@ -528,6 +528,9 @@ where
         // evaluate the worklet code. In the case that the size hasn't changed,
         // this will result in increased concurrency between script and layout.
         notify_paint_worklet(context, data);
+    } else {
+        debug_assert!(data.has_styles());
+        data.restyle.set_traversed_without_styling();
     }
 
     // Now that matching and cascading is done, clear the bits corresponding to