diff --git a/servo/components/style/data.rs b/servo/components/style/data.rs index 8e63fb525013e6d9be67ec1c345b8335ce4037c3..07870fad7a6957a146e52fd0733e42d9cd09f608 100644 --- a/servo/components/style/data.rs +++ b/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() diff --git a/servo/components/style/sharing/checks.rs b/servo/components/style/sharing/checks.rs index 259cadc958ba76f0299f9a4c1ae4161a97041e04..31296aed44677d4e85dcf92e6c770f109706f758 100644 --- a/servo/components/style/sharing/checks.rs +++ b/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; } diff --git a/servo/components/style/sharing/mod.rs b/servo/components/style/sharing/mod.rs index caa6f4afc869c33db63ac8bcaaa09221664fdeb8..27d911362f724e03baf1eca0dfd245050e825346 100644 --- a/servo/components/style/sharing/mod.rs +++ b/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) } diff --git a/servo/components/style/traversal.rs b/servo/components/style/traversal.rs index 39206303ae3d039029c0cd24b8d5eedfe3da5b7e..cd90f5687532a8bfdd5b03aac92ef3fc6fa480d8 100644 --- a/servo/components/style/traversal.rs +++ b/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