Skip to content

Commit

Permalink
Don't reconstruct the layout object when going from no pseudo to pseu…
Browse files Browse the repository at this point in the history
…do with no content for ::before and ::after.

Fixes Gecko bug 1376073.  r=emilio
  • Loading branch information
bzbarsky committed Jul 28, 2017
1 parent 12a49dc commit 91d4956
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 12 deletions.
2 changes: 1 addition & 1 deletion components/style/context.rs
Expand Up @@ -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<CascadeInputs>; 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));
Expand Down
15 changes: 14 additions & 1 deletion components/style/data.rs
Expand Up @@ -162,20 +162,33 @@ 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 {
self.0.is_none()
}

/// 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<ComputedValues>> {
debug_assert!(pseudo.is_eager());
Expand Down
17 changes: 17 additions & 0 deletions components/style/gecko/pseudo_element.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
}
31 changes: 21 additions & 10 deletions components/style/matching.rs
Expand Up @@ -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) {
Expand All @@ -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;
}
}
}
}
Expand Down Expand Up @@ -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();
Expand Down
17 changes: 17 additions & 0 deletions components/style/servo/selector_parser.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -181,6 +183,21 @@ impl PseudoElement {
pub fn property_restriction(&self) -> Option<PropertyFlags> {
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.
Expand Down

0 comments on commit 91d4956

Please sign in to comment.