Bug 1367635 - Part 9: Don't use rule cache for property-restricted pseudo-elements. r?emilio
MozReview-Commit-ID: JebyVWzC7Q1
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -2583,17 +2583,17 @@ pub struct StyleBuilder<'a> {
/// The rule node representing the ordered list of rules matched for this
/// node.
pub rules: Option<StrongRuleNode>,
custom_properties: Option<Arc<::custom_properties::CustomPropertiesMap>>,
/// The pseudo-element this style will represent.
- pseudo: Option<<&'a PseudoElement>,
+ pub pseudo: Option<<&'a PseudoElement>,
/// Whether we have mutated any reset structs since the the last time
/// `clear_modified_reset` was called. This is used to tell whether the
/// `StyleAdjuster` did any work.
modified_reset: bool,
/// The writing mode flags.
///
--- a/servo/components/style/rule_cache.rs
+++ b/servo/components/style/rule_cache.rs
@@ -5,16 +5,17 @@
//! A cache from rule node to computed values, in order to cache reset
//! properties.
use values::computed::NonNegativeLength;
use fnv::FnvHashMap;
use logical_geometry::WritingMode;
use properties::{ComputedValues, StyleBuilder};
use rule_tree::StrongRuleNode;
+use selector_parser::PseudoElement;
use servo_arc::Arc;
use smallvec::SmallVec;
/// The conditions for caching and matching a style in the rule cache.
#[derive(Debug, Default, Clone)]
pub struct RuleCacheConditions {
uncacheable: bool,
font_size: Option<NonNegativeLength>,
@@ -88,16 +89,24 @@ impl RuleCache {
&self,
builder_with_early_props: &StyleBuilder,
) -> Option<&ComputedValues> {
if builder_with_early_props.is_style_if_visited() {
// FIXME(emilio): We can probably do better, does it matter much?
return None;
}
+ // A pseudo-element with property restrictions can result in different
+ // computed values if it's also used for a non-pseudo.
+ if builder_with_early_props.pseudo
+ .and_then(|p| p.property_restriction())
+ .is_some() {
+ return None;
+ }
+
let rules = match builder_with_early_props.rules {
Some(ref rules) => rules,
None => return None,
};
self.map.get(rules).and_then(|cached_values| {
for &(ref conditions, ref values) in cached_values.iter() {
if conditions.matches(builder_with_early_props) {
@@ -110,27 +119,34 @@ impl RuleCache {
}
/// Inserts a node into the rules cache if possible.
///
/// Returns whether the style was inserted into the cache.
pub fn insert_if_possible(
&mut self,
style: &Arc<ComputedValues>,
+ pseudo: Option<&PseudoElement>,
conditions: &RuleCacheConditions,
) -> bool {
if !conditions.cacheable() {
return false;
}
if style.is_style_if_visited() {
// FIXME(emilio): We can probably do better, does it matter much?
return false;
}
+ // A pseudo-element with property restrictions can result in different
+ // computed values if it's also used for a non-pseudo.
+ if pseudo.and_then(|p| p.property_restriction()).is_some() {
+ return false;
+ }
+
let rules = match style.rules {
Some(ref r) => r.clone(),
None => return false,
};
debug!("Inserting cached reset style with conditions {:?}", conditions);
self.map
.entry(rules)
--- a/servo/components/style/style_resolver.rs
+++ b/servo/components/style/style_resolver.rs
@@ -513,34 +513,36 @@ where
}
if self.element.is_native_anonymous() || pseudo.is_some() {
cascade_flags.insert(PROHIBIT_DISPLAY_CONTENTS);
} else if self.element.is_root() {
cascade_flags.insert(IS_ROOT_ELEMENT);
}
let implemented_pseudo = self.element.implemented_pseudo_element();
+ let pseudo = pseudo.or(implemented_pseudo.as_ref());
+
let mut conditions = Default::default();
let values =
cascade(
self.context.shared.stylist.device(),
- pseudo.or(implemented_pseudo.as_ref()),
+ pseudo,
rules.unwrap_or(self.context.shared.stylist.rule_tree().root()),
&self.context.shared.guards,
parent_style,
parent_style,
layout_parent_style,
style_if_visited,
&self.context.thread_local.font_metrics_provider,
cascade_flags,
self.context.shared.quirks_mode(),
Some(&self.context.thread_local.rule_cache),
&mut conditions,
);
self.context
.thread_local
.rule_cache
- .insert_if_possible(&values, &conditions);
+ .insert_if_possible(&values, pseudo, &conditions);
values
}
}