Bug 1381821: Prevent cousin sharing if we haven't restyled the parents. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 03 Aug 2017 16:39:33 +0200
changeset 620537 3b8c5089d459a3147ece636993fbc147d1176b57
parent 620532 82e09163b1b6be886a0cae447d7fa1eea9b7a604
child 620538 1b9002c0b983562bcd808d875411f0f223b3c837
push id72072
push userbmo:emilio+bugs@crisal.io
push dateThu, 03 Aug 2017 14:57:50 +0000
reviewersbz
bugs1381821
milestone57.0a1
Bug 1381821: Prevent cousin sharing if we haven't restyled the parents. r?bz MozReview-Commit-ID: COKdmHHokGx
servo/components/style/sharing/checks.rs
--- a/servo/components/style/sharing/checks.rs
+++ b/servo/components/style/sharing/checks.rs
@@ -14,27 +14,50 @@ use servo_arc::Arc;
 use sharing::{StyleSharingCandidate, StyleSharingTarget};
 
 /// Whether, given two elements, they have pointer-equal computed values.
 ///
 /// 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
     where E: TElement,
 {
-    let (a, b) = match (first, second) {
+    let (first, second) = match (first, second) {
         (Some(f), Some(s)) => (f, s),
         _ => return false,
     };
 
-    let eq = Arc::ptr_eq(a.borrow_data().unwrap().styles.primary(),
-                         b.borrow_data().unwrap().styles.primary());
-    eq
+    debug_assert_ne!(first, second);
+
+    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.
+    //
+    // It's kind of unfortunate, but probably not too bad. An option to avoid
+    // this is making a bunch more of revalidation selectors. Other one is
+    // adding a per-traversal flag of whether we're initially styling and make
+    // it go away properly... See the discussion in bug 1381821 for more
+    // details.
+    if !first_data.restyle.is_restyle() || !second_data.restyle.is_restyle() {
+        return false;
+    }
+
+    let same_computed_values =
+        Arc::ptr_eq(first_data.styles.primary(), second_data.styles.primary());
+
+    same_computed_values
 }
 
 /// Whether two elements have the same same style attribute (by pointer identity).
 pub fn have_same_style_attribute<E>(
     target: &mut StyleSharingTarget<E>,
     candidate: &mut StyleSharingCandidate<E>
 ) -> bool
     where E: TElement,