From 52d1b595151772c7d636152e2e3bf382632b3378 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 29 Jun 2017 16:22:13 -0700 Subject: [PATCH] Implement :visited handling for lazy pseudo-elements in stylo. Part 2 of the fix for Gecko bug 1364242: https://bugzilla.mozilla.org/show_bug.cgi?id=1364242 --- .../script_layout_interface/wrapper_traits.rs | 1 + components/style/context.rs | 7 +- components/style/matching.rs | 8 +- components/style/stylist.rs | 152 +++++++++++++----- ports/geckolib/glue.rs | 13 +- 5 files changed, 134 insertions(+), 47 deletions(-) diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index cac426156da..579ac767d13 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -426,6 +426,7 @@ pub trait ThreadSafeLayoutElement: Clone + Copy + Sized + Debug + &style_pseudo, RuleInclusion::All, data.styles.primary(), + /* is_probe = */ false, &ServoMetricsProvider) .unwrap() .clone() diff --git a/components/style/context.rs b/components/style/context.rs index 6a8f3535ce4..18822ca2930 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -184,7 +184,7 @@ impl Default for CascadeInputs { impl CascadeInputs { /// Construct inputs from previous cascade results, if any. - fn new_from_style(style: &Arc) -> Self { + pub fn new_from_style(style: &Arc) -> Self { CascadeInputs { rules: style.rules.clone(), visited_rules: style.get_visited_style().and_then(|v| v.rules.clone()), @@ -204,6 +204,11 @@ impl CascadeInputs { self.rules.as_mut() } + /// Gets a reference to the rule node, if any. + pub fn get_rules(&self) -> Option<&StrongRuleNode> { + self.rules.as_ref() + } + /// Gets a reference to the rule node. Panic if the element does not have /// rule node. pub fn rules(&self) -> &StrongRuleNode { diff --git a/components/style/matching.rs b/components/style/matching.rs index 167b18e7969..e20c4760abe 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -166,7 +166,7 @@ impl CascadeVisitedMode { /// Returns the computed values based on the cascade mode. In visited mode, /// visited values are only returned if they already exist. If they don't, /// we fallback to the regular, unvisited styles. - fn values<'a>(&self, values: &'a Arc) -> &'a Arc { + pub fn values<'a>(&self, values: &'a Arc) -> &'a Arc { if *self == CascadeVisitedMode::Visited && values.get_visited_style().is_some() { return values.visited_style(); } @@ -1211,8 +1211,10 @@ pub trait MatchMethods : TElement { // Compute rule nodes for eagerly-cascaded pseudo-elements. let mut matches_different_pseudos = false; SelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| { - // For pseudo-elements, we only try to match visited rules if there - // are also unvisited rules. (This matches Gecko's behavior.) + // For eager pseudo-elements, we only try to match visited rules if + // there are also unvisited rules. (This matches Gecko's behavior + // for probing pseudo-elements, and for eager pseudo-elements Gecko + // does not try to resolve style if the probe says there isn't any.) if visited_handling == VisitedHandlingMode::RelevantLinkVisited && !context.cascade_inputs().pseudos.has(&pseudo) { return diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 533ded9ec4e..da2bfd6a1e6 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -7,7 +7,7 @@ use {Atom, LocalName, Namespace}; use applicable_declarations::{ApplicableDeclarationBlock, ApplicableDeclarationList}; use bit_vec::BitVec; -use context::QuirksMode; +use context::{CascadeInputs, QuirksMode}; use dom::TElement; use element_state::ElementState; use error_reporting::create_error_reporter; @@ -16,6 +16,7 @@ use font_metrics::FontMetricsProvider; use gecko_bindings::structs::{nsIAtom, StyleRuleInclusion}; use invalidation::element::invalidation_map::InvalidationMap; use invalidation::media_queries::EffectiveMediaQueryResults; +use matching::CascadeVisitedMode; use media_queries::Device; use properties::{self, CascadeFlags, ComputedValues}; use properties::{AnimationRules, PropertyDeclarationBlock}; @@ -27,7 +28,7 @@ use selector_parser::{SelectorImpl, PseudoElement}; use selectors::attr::NamespaceConstraint; use selectors::bloom::BloomFilter; use selectors::matching::{ElementSelectorFlags, matches_selector, MatchingContext, MatchingMode}; -use selectors::matching::AFFECTED_BY_PRESENTATIONAL_HINTS; +use selectors::matching::{VisitedHandlingMode, AFFECTED_BY_PRESENTATIONAL_HINTS}; use selectors::parser::{AncestorHashes, Combinator, Component, Selector, SelectorAndHashes}; use selectors::parser::{SelectorIter, SelectorMethods}; use selectors::sink::Push; @@ -680,46 +681,89 @@ impl Stylist { pseudo: &PseudoElement, rule_inclusion: RuleInclusion, parent_style: &Arc, + is_probe: bool, font_metrics: &FontMetricsProvider) -> Option> where E: TElement, { - let rule_node = - self.lazy_pseudo_rules(guards, element, pseudo, rule_inclusion); - self.compute_pseudo_element_style_with_rulenode(rule_node.as_ref(), - guards, - parent_style, - font_metrics) + let cascade_inputs = + self.lazy_pseudo_rules(guards, element, pseudo, is_probe, rule_inclusion); + self.compute_pseudo_element_style_with_inputs(&cascade_inputs, + guards, + parent_style, + font_metrics) } - /// Computes a pseudo-element style lazily using the given rulenode. This - /// can be used for truly lazy pseudo-elements or to avoid redoing selector - /// matching for eager pseudo-elements when we need to recompute their style - /// with a new parent style. - pub fn compute_pseudo_element_style_with_rulenode(&self, - rule_node: Option<&StrongRuleNode>, - guards: &StylesheetGuards, - parent_style: &ComputedValues, - font_metrics: &FontMetricsProvider) - -> Option> + /// Computes a pseudo-element style lazily using the given CascadeInputs. + /// This can be used for truly lazy pseudo-elements or to avoid redoing + /// selector matching for eager pseudo-elements when we need to recompute + /// their style with a new parent style. + pub fn compute_pseudo_element_style_with_inputs(&self, + inputs: &CascadeInputs, + guards: &StylesheetGuards, + parent_style: &Arc, + font_metrics: &FontMetricsProvider) + -> Option> { - let rule_node = match rule_node { - Some(rule_node) => rule_node, - None => return None + // We may have only visited rules in cases when we are actually + // resolving, not probing, pseudo-element style. + if !inputs.has_rules() && !inputs.has_visited_rules() { + return None + } + + // We need to compute visited values if we have visited rules or if our + // parent has visited values. + let visited_values = if inputs.has_visited_rules() || parent_style.get_visited_style().is_some() { + // Slightly annoying: we know that inputs has either rules or + // visited rules, but we can't do inputs.rules() up front because + // maybe it just has visited rules, so can't unwrap_or. + let rule_node = match inputs.get_visited_rules() { + Some(rules) => rules, + None => inputs.rules() + }; + // We want to use the visited bits (if any) from our parent style as + // our parent. + let mode = CascadeVisitedMode::Visited; + let inherited_style = mode.values(parent_style); + let computed = + properties::cascade(&self.device, + rule_node, + guards, + Some(inherited_style), + Some(inherited_style), + None, + None, + &create_error_reporter(), + font_metrics, + CascadeFlags::empty(), + self.quirks_mode); + + Some(Arc::new(computed)) + } else { + None + }; + + // We may not have non-visited rules, if we only had visited ones. In + // that case we want to use the root rulenode for our non-visited rules. + let root; + let rules = if let Some(rules) = inputs.get_rules() { + rules + } else { + root = self.rule_tree.root(); + &root }; // Read the comment on `precomputed_values_for_pseudo` to see why it's // difficult to assert that display: contents nodes never arrive here // (tl;dr: It doesn't apply for replaced elements and such, but the // computed value is still "contents"). - // Bug 1364242: We need to add visited support for lazy pseudos let computed = properties::cascade(&self.device, - rule_node, + rules, guards, Some(parent_style), Some(parent_style), - None, + visited_values, None, &create_error_reporter(), font_metrics, @@ -729,7 +773,7 @@ impl Stylist { Some(Arc::new(computed)) } - /// Computes the rule node for a lazily-cascaded pseudo-element. + /// Computes the cascade inputs for a lazily-cascaded pseudo-element. /// /// See the documentation on lazy pseudo-elements in /// docs/components/style.md @@ -737,14 +781,15 @@ impl Stylist { guards: &StylesheetGuards, element: &E, pseudo: &PseudoElement, + is_probe: bool, rule_inclusion: RuleInclusion) - -> Option + -> CascadeInputs where E: TElement { let pseudo = pseudo.canonical(); debug_assert!(pseudo.is_lazy()); if self.pseudos_map.get(&pseudo).is_none() { - return None + return CascadeInputs::default() } // Apply the selector flags. We should be in sequential mode @@ -777,7 +822,7 @@ impl Stylist { } }; - // Bug 1364242: We need to add visited support for lazy pseudos + let mut inputs = CascadeInputs::default(); let mut declarations = ApplicableDeclarationList::new(); let mut matching_context = MatchingContext::new(MatchingMode::ForStatelessPseudoElement, @@ -792,19 +837,52 @@ impl Stylist { &mut declarations, &mut matching_context, &mut set_selector_flags); - if declarations.is_empty() { - return None - } - let rule_node = - self.rule_tree.insert_ordered_rules_with_important( + if !declarations.is_empty() { + let rule_node = self.rule_tree.insert_ordered_rules_with_important( declarations.into_iter().map(|a| a.order_and_level()), guards); - if rule_node == self.rule_tree.root() { - None - } else { - Some(rule_node) + if rule_node != self.rule_tree.root() { + inputs.set_rules(VisitedHandlingMode::AllLinksUnvisited, + rule_node); + } + }; + + if is_probe && !inputs.has_rules() { + // When probing, don't compute visited styles if we have no + // unvisited styles. + return inputs; } + + if matching_context.relevant_link_found { + let mut declarations = ApplicableDeclarationList::new(); + let mut matching_context = + MatchingContext::new_for_visited(MatchingMode::ForStatelessPseudoElement, + None, + VisitedHandlingMode::RelevantLinkVisited, + self.quirks_mode); + self.push_applicable_declarations(element, + Some(&pseudo), + None, + None, + AnimationRules(None, None), + rule_inclusion, + &mut declarations, + &mut matching_context, + &mut set_selector_flags); + if !declarations.is_empty() { + let rule_node = + self.rule_tree.insert_ordered_rules_with_important( + declarations.into_iter().map(|a| a.order_and_level()), + guards); + if rule_node != self.rule_tree.root() { + inputs.set_rules(VisitedHandlingMode::RelevantLinkVisited, + rule_node); + } + } + } + + inputs } /// Set a given device, which may change the styles that apply to the diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index d925584687f..faf69b35f8d 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -10,7 +10,7 @@ use selectors::Element; use std::env; use std::fmt::Write; use std::ptr; -use style::context::{QuirksMode, SharedStyleContext, StyleContext}; +use style::context::{CascadeInputs, QuirksMode, SharedStyleContext, StyleContext}; use style::context::ThreadLocalStyleContext; use style::data::{ElementData, ElementStyles, RestyleData}; use style::dom::{AnimationOnlyDirtyDescendants, DirtyDescendants}; @@ -1550,13 +1550,13 @@ fn get_pseudo_style( inherited_styles.unwrap_or(styles.primary()); let guards = StylesheetGuards::same(guard); let metrics = get_metrics_provider_for_product(); - let rule_node = match styles.pseudos.get(&pseudo) { - Some(styles) => styles.rules.as_ref(), - None => None, + let inputs = match styles.pseudos.get(&pseudo) { + Some(styles) => CascadeInputs::new_from_style(styles), + None => return None, }; doc_data.stylist - .compute_pseudo_element_style_with_rulenode( - rule_node, + .compute_pseudo_element_style_with_inputs( + &inputs, &guards, inherited_styles, &metrics) @@ -1588,6 +1588,7 @@ fn get_pseudo_style( &pseudo, rule_inclusion, base, + is_probe, &metrics) }, };