From 91d4956da58282f966a79ab685d288fdb908f45f Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 28 Jul 2017 17:41:53 -0400 Subject: [PATCH] Don't reconstruct the layout object when going from no pseudo to pseudo with no content for ::before and ::after. Fixes Gecko bug 1376073. r=emilio --- components/style/context.rs | 2 +- components/style/data.rs | 15 ++++++++++- components/style/gecko/pseudo_element.rs | 17 +++++++++++++ components/style/matching.rs | 31 +++++++++++++++-------- components/style/servo/selector_parser.rs | 17 +++++++++++++ 5 files changed, 70 insertions(+), 12 deletions(-) diff --git a/components/style/context.rs b/components/style/context.rs index 9393c3522d87..997a3f375740 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -232,7 +232,7 @@ impl Clone for EagerPseudoCascadeInputs { impl EagerPseudoCascadeInputs { /// Construct inputs from previous cascade results, if any. fn new_from_style(styles: &EagerPseudoStyles) -> Self { - EagerPseudoCascadeInputs(styles.as_array().map(|styles| { + EagerPseudoCascadeInputs(styles.as_optional_array().map(|styles| { let mut inputs: [Option; EAGER_PSEUDO_COUNT] = Default::default(); for i in 0..EAGER_PSEUDO_COUNT { inputs[i] = styles[i].as_ref().map(|s| CascadeInputs::new_from_style(s)); diff --git a/components/style/data.rs b/components/style/data.rs index c835593fed2c..bce4d4502f5c 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -162,6 +162,13 @@ impl fmt::Debug for EagerPseudoArray { } } +// Can't use [None; EAGER_PSEUDO_COUNT] here because it complains +// about Copy not being implemented for our Arc type. +#[cfg(feature = "gecko")] +const EMPTY_PSEUDO_ARRAY: &'static EagerPseudoArrayInner = &[None, None, None, None]; +#[cfg(feature = "servo")] +const EMPTY_PSEUDO_ARRAY: &'static EagerPseudoArrayInner = &[None, None, None]; + impl EagerPseudoStyles { /// Returns whether there are any pseudo styles. pub fn is_empty(&self) -> bool { @@ -169,13 +176,19 @@ impl EagerPseudoStyles { } /// Grabs a reference to the list of styles, if they exist. - pub fn as_array(&self) -> Option<&EagerPseudoArrayInner> { + pub fn as_optional_array(&self) -> Option<&EagerPseudoArrayInner> { match self.0 { None => None, Some(ref x) => Some(&x.0), } } + /// Grabs a reference to the list of styles or a list of None if + /// there are no styles to be had. + pub fn as_array(&self) -> &EagerPseudoArrayInner { + self.as_optional_array().unwrap_or(EMPTY_PSEUDO_ARRAY) + } + /// Returns a reference to the style for a given eager pseudo, if it exists. pub fn get(&self, pseudo: &PseudoElement) -> Option<&Arc> { debug_assert!(pseudo.is_eager()); diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index 7765122bd61a..cf37567252c0 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -12,6 +12,8 @@ use cssparser::{ToCss, serialize_identifier}; use gecko_bindings::structs::{self, CSSPseudoElementType}; use properties::{PropertyFlags, APPLIES_TO_FIRST_LETTER, APPLIES_TO_FIRST_LINE}; use properties::APPLIES_TO_PLACEHOLDER; +use properties::ComputedValues; +use properties::longhands::display::computed_value as display; use selector_parser::{NonTSPseudoClass, PseudoElementCascadeType, SelectorImpl}; use std::fmt; use string_cache::Atom; @@ -132,4 +134,19 @@ impl PseudoElement { _ => None, } } + + /// Whether this pseudo-element should actually exist if it has + /// the given styles. + pub fn should_exist(&self, style: &ComputedValues) -> bool + { + let display = style.get_box().clone_display(); + if display == display::T::none { + return false; + } + if self.is_before_or_after() && style.ineffective_content_property() { + return false; + } + + true + } } diff --git a/components/style/matching.rs b/components/style/matching.rs index 4396b5167171..eeb06259dd49 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -522,18 +522,13 @@ pub trait MatchMethods : TElement { ); if data.styles.pseudos.is_empty() && old_styles.pseudos.is_empty() { - return cascade_requirement; - } - - // If it matched a different number of pseudos, reconstruct. - if data.styles.pseudos.is_empty() != old_styles.pseudos.is_empty() { - data.restyle.damage |= RestyleDamage::reconstruct(); + // This is the common case; no need to examine pseudos here. return cascade_requirement; } let pseudo_styles = - old_styles.pseudos.as_array().unwrap().iter().zip( - data.styles.pseudos.as_array().unwrap().iter()); + old_styles.pseudos.as_array().iter().zip( + data.styles.pseudos.as_array().iter()); for (i, (old, new)) in pseudo_styles.enumerate() { match (old, new) { @@ -548,8 +543,21 @@ pub trait MatchMethods : TElement { } (&None, &None) => {}, _ => { - data.restyle.damage |= RestyleDamage::reconstruct(); - return cascade_requirement; + // It's possible that we're switching from not having + // ::before/::after at all to having styles for them but not + // actually having a useful pseudo-element. Check for that + // case. + let pseudo = PseudoElement::from_eager_index(i); + let new_pseudo_should_exist = + new.as_ref().map_or(false, + |s| pseudo.should_exist(s)); + let old_pseudo_should_exist = + old.as_ref().map_or(false, + |s| pseudo.should_exist(s)); + if new_pseudo_should_exist != old_pseudo_should_exist { + data.restyle.damage |= RestyleDamage::reconstruct(); + return cascade_requirement; + } } } } @@ -790,6 +798,9 @@ pub trait MatchMethods : TElement { } if pseudo.map_or(false, |p| p.is_before_or_after()) { + // FIXME(bz) This duplicates some of the logic in + // PseudoElement::should_exist, but it's not clear how best to share + // that logic without redoing the "get the display" work. let old_style_generates_no_pseudo = old_style_is_display_none || old_values.ineffective_content_property(); diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index ae47b993ebcc..c88b4faa3ad9 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -13,7 +13,9 @@ use dom::{OpaqueNode, TElement, TNode}; use element_state::ElementState; use fnv::FnvHashMap; use invalidation::element::element_wrapper::ElementSnapshot; +use properties::ComputedValues; use properties::PropertyFlags; +use properties::longhands::display::computed_value as display; use selector_parser::{AttrValue as SelectorAttrValue, ElementExt, PseudoElementCascadeType, SelectorParser}; use selectors::Element; use selectors::attr::{AttrSelectorOperation, NamespaceConstraint, CaseSensitivity}; @@ -181,6 +183,21 @@ impl PseudoElement { pub fn property_restriction(&self) -> Option { None } + + /// Whether this pseudo-element should actually exist if it has + /// the given styles. + pub fn should_exist(&self, style: &ComputedValues) -> bool + { + let display = style.get_box().clone_display(); + if display == display::T::none { + return false; + } + if self.is_before_or_after() && style.ineffective_content_property() { + return false; + } + + true + } } /// The type used for storing pseudo-class string arguments.