diff --git a/components/style/animation.rs b/components/style/animation.rs index d090ecb1ecb..2d421ce06a9 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -9,7 +9,7 @@ use bezier::Bezier; use context::SharedStyleContext; use dom::{OpaqueNode, TElement}; use font_metrics::FontMetricsProvider; -use properties::{self, CascadeFlags, ComputedValues, LonghandId}; +use properties::{self, CascadeMode, ComputedValues, LonghandId}; use properties::animated_properties::AnimatedProperty; use properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection; use properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState; @@ -504,9 +504,8 @@ where Some(previous_style), Some(previous_style), Some(previous_style), - /* visited_style = */ None, font_metrics_provider, - CascadeFlags::empty(), + CascadeMode::Unvisited { visited_rules: None }, context.quirks_mode(), /* rule_cache = */ None, &mut Default::default(), diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index 1e97b72ff6c..df1b873a1b7 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -10,7 +10,7 @@ use cssparser::ToCss; use gecko_bindings::structs::{self, CSSPseudoElementType}; -use properties::{CascadeFlags, ComputedValues, PropertyFlags}; +use properties::{ComputedValues, PropertyFlags}; use properties::longhands::display::computed_value::T as Display; use selector_parser::{NonTSPseudoClass, PseudoElementCascadeType, SelectorImpl}; use std::fmt; @@ -55,14 +55,12 @@ impl PseudoElement { PseudoElementCascadeType::Lazy } - /// The CascadeFlags needed to cascade this pseudo-element. + /// Whether cascading this pseudo-element makes it inherit all properties, + /// even reset ones. /// - /// This is only needed to support the broken INHERIT_ALL pseudo mode for - /// Servo. + /// This is used in Servo for anonymous boxes, though it's likely broken. #[inline] - pub fn cascade_flags(&self) -> CascadeFlags { - CascadeFlags::empty() - } + pub fn inherits_all(&self) -> bool { false } /// Whether the pseudo-element should inherit from the default computed /// values instead of from the parent element. diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index e884a03ae4e..66e256a242a 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -3225,10 +3225,9 @@ impl<'a> StyleBuilder<'a> { parent_style: Option<<&'a ComputedValues>, parent_style_ignoring_first_line: Option<<&'a ComputedValues>, pseudo: Option<<&'a PseudoElement>, - cascade_flags: CascadeFlags, + cascade_mode: CascadeMode, rules: Option, custom_properties: Option>, - visited_style: Option>, ) -> Self { debug_assert_eq!(parent_style.is_some(), parent_style_ignoring_first_line.is_some()); #[cfg(feature = "gecko")] @@ -3239,18 +3238,18 @@ impl<'a> StyleBuilder<'a> { let reset_style = device.default_computed_values(); let inherited_style = parent_style.unwrap_or(reset_style); let inherited_style_ignoring_first_line = parent_style_ignoring_first_line.unwrap_or(reset_style); - // FIXME(bz): INHERIT_ALL seems like a fundamentally broken idea. I'm + // FIXME(bz): inherits_all seems like a fundamentally broken idea. I'm // 99% sure it should give incorrect behavior for table anonymous box // backgrounds, for example. This code doesn't attempt to make it play // nice with inherited_style_ignoring_first_line. - let reset_style = if cascade_flags.contains(CascadeFlags::INHERIT_ALL) { + let reset_style = if pseudo.map_or(false, |p| p.inherits_all()) { inherited_style } else { reset_style }; let mut flags = inherited_style.flags.inherited(); - if cascade_flags.contains(CascadeFlags::VISITED_DEPENDENT_ONLY) { + if matches!(cascade_mode, CascadeMode::Visited { .. }) { flags.insert(ComputedValueFlags::IS_STYLE_IF_VISITED); } @@ -3265,7 +3264,7 @@ impl<'a> StyleBuilder<'a> { custom_properties, writing_mode: inherited_style.writing_mode, flags, - visited_style, + visited_style: None, % for style_struct in data.active_style_structs(): % if style_struct.inherited: ${style_struct.ident}: StyleStructRef::Borrowed(inherited_style.${style_struct.name_lower}_arc()), @@ -3421,6 +3420,11 @@ impl<'a> StyleBuilder<'a> { // produced by this builder. This assumes that the caller doesn't need // to adjust or process visited style, so we can just build visited // style here for simplicity. + // + // FIXME(emilio): This doesn't set the IS_STYLE_IF_VISITED flag + // correctly, though right now it doesn't matter. + // + // We can probably kill that flag now. let visited_style = parent.and_then(|parent| { parent.visited_style().map(|style| { Self::for_inheritance( @@ -3430,16 +3434,17 @@ impl<'a> StyleBuilder<'a> { ).build() }) }); - Self::new( + let mut ret = Self::new( device, parent, parent, pseudo, - CascadeFlags::empty(), + CascadeMode::Unvisited { visited_rules: None }, /* rules = */ None, parent.and_then(|p| p.custom_properties().cloned()), - visited_style, - ) + ); + ret.visited_style = visited_style; + ret } /// Returns whether we have a visited style. @@ -3650,18 +3655,6 @@ static CASCADE_PROPERTY: [CascadePropertyFn; ${len(data.longhands)}] = [ % endfor ]; -bitflags! { - /// A set of flags to tweak the behavior of the `cascade` function. - pub struct CascadeFlags: u8 { - /// Whether to inherit all styles from the parent. If this flag is not - /// present, non-inherited styles are reset to their initial values. - const INHERIT_ALL = 1; - - /// Whether to only cascade properties that are visited dependent. - const VISITED_DEPENDENT_ONLY = 1 << 1; - } -} - /// Performs the CSS cascade, computing new styles for an element from its parent style. /// /// The arguments are: @@ -3684,9 +3677,43 @@ pub fn cascade( parent_style: Option<<&ComputedValues>, parent_style_ignoring_first_line: Option<<&ComputedValues>, layout_parent_style: Option<<&ComputedValues>, - visited_style: Option>, + visited_rules: Option<<&StrongRuleNode>, font_metrics_provider: &FontMetricsProvider, - flags: CascadeFlags, + quirks_mode: QuirksMode, + rule_cache: Option<<&RuleCache>, + rule_cache_conditions: &mut RuleCacheConditions, + element: Option, +) -> Arc +where + E: TElement, +{ + cascade_rules( + device, + pseudo, + rule_node, + guards, + parent_style, + parent_style_ignoring_first_line, + layout_parent_style, + font_metrics_provider, + CascadeMode::Unvisited { visited_rules }, + quirks_mode, + rule_cache, + rule_cache_conditions, + element, + ) +} + +fn cascade_rules( + device: &Device, + pseudo: Option<<&PseudoElement>, + rule_node: &StrongRuleNode, + guards: &StylesheetGuards, + parent_style: Option<<&ComputedValues>, + parent_style_ignoring_first_line: Option<<&ComputedValues>, + layout_parent_style: Option<<&ComputedValues>, + font_metrics_provider: &FontMetricsProvider, + cascade_mode: CascadeMode, quirks_mode: QuirksMode, rule_cache: Option<<&RuleCache>, rule_cache_conditions: &mut RuleCacheConditions, @@ -3697,33 +3724,29 @@ where { debug_assert_eq!(parent_style.is_some(), parent_style_ignoring_first_line.is_some()); let empty = SmallBitVec::new(); - - let property_restriction = pseudo.and_then(|p| p.property_restriction()); - + let restriction = pseudo.and_then(|p| p.property_restriction()); let iter_declarations = || { rule_node.self_and_ancestors().flat_map(|node| { let cascade_level = node.cascade_level(); - let source = node.style_source(); - - let declarations = if source.is_some() { - source.as_ref().unwrap().read(cascade_level.guard(guards)).declaration_importance_iter() - } else { - // The root node has no style source. - DeclarationImportanceIterator::new(&[], &empty) - }; let node_importance = node.importance(); + let declarations = match node.style_source() { + Some(source) => { + source.read(cascade_level.guard(guards)).declaration_importance_iter() + } + None => DeclarationImportanceIterator::new(&[], &empty), + }; declarations // Yield declarations later in source order (with more precedence) first. .rev() .filter_map(move |(declaration, declaration_importance)| { - if let Some(property_restriction) = property_restriction { + if let Some(restriction) = restriction { // declaration.id() is either a longhand or a custom // property. Custom properties are always allowed, but // longhands are only allowed if they have our - // property_restriction flag set. + // restriction flag set. if let PropertyDeclarationId::Longhand(id) = declaration.id() { - if !id.flags().contains(property_restriction) { + if !id.flags().contains(restriction) { return None } } @@ -3747,9 +3770,8 @@ where parent_style, parent_style_ignoring_first_line, layout_parent_style, - visited_style, font_metrics_provider, - flags, + cascade_mode, quirks_mode, rule_cache, rule_cache_conditions, @@ -3757,6 +3779,22 @@ where ) } +/// Whether we're cascading for visited or unvisited styles. +#[derive(Clone, Copy)] +pub enum CascadeMode<'a> { + /// We're cascading for unvisited styles. + Unvisited { + /// The visited rules that should match the visited style. + visited_rules: Option<<&'a StrongRuleNode>, + }, + /// We're cascading for visited styles. + Visited { + /// The writing mode of our unvisited style, needed to correctly resolve + /// logical properties.. + writing_mode: WritingMode, + }, +} + /// NOTE: This function expects the declaration with more priority to appear /// first. pub fn apply_declarations<'a, E, F, I>( @@ -3768,9 +3806,8 @@ pub fn apply_declarations<'a, E, F, I>( parent_style: Option<<&ComputedValues>, parent_style_ignoring_first_line: Option<<&ComputedValues>, layout_parent_style: Option<<&ComputedValues>, - visited_style: Option>, font_metrics_provider: &FontMetricsProvider, - flags: CascadeFlags, + cascade_mode: CascadeMode, quirks_mode: QuirksMode, rule_cache: Option<<&RuleCache>, rule_cache_conditions: &mut RuleCacheConditions, @@ -3788,16 +3825,8 @@ where ::std::ptr::eq(parent_style.unwrap(), parent_style_ignoring_first_line.unwrap()) || parent_style.unwrap().pseudo() == Some(PseudoElement::FirstLine)); - let (inherited_style, layout_parent_style) = match parent_style { - Some(parent_style) => { - (parent_style, - layout_parent_style.unwrap_or(parent_style)) - }, - None => { - (device.default_computed_values(), - device.default_computed_values()) - } - }; + let inherited_style = + parent_style.unwrap_or(device.default_computed_values()); let custom_properties = { let mut builder = @@ -3822,10 +3851,9 @@ where parent_style, parent_style_ignoring_first_line, pseudo, - flags, + cascade_mode, Some(rules.clone()), custom_properties, - visited_style, ), cached_system_font: None, in_media_query: false, @@ -3887,7 +3915,7 @@ where // Only a few properties are allowed to depend on the visited state // of links. When cascading visited styles, we can save time by // only processing these properties. - if flags.contains(CascadeFlags::VISITED_DEPENDENT_ONLY) && + if matches!(cascade_mode, CascadeMode::Visited { .. }) && !physical_longhand_id.is_visited_dependent() { continue } @@ -3952,9 +3980,43 @@ where (CASCADE_PROPERTY[discriminant])(&*declaration, &mut context); } % if category_to_cascade_now == "early": - let writing_mode = - WritingMode::new(context.builder.get_inherited_box()); + let writing_mode = match cascade_mode { + CascadeMode::Unvisited { .. } => { + WritingMode::new(context.builder.get_inherited_box()) + } + CascadeMode::Visited { writing_mode } => writing_mode, + }; + context.builder.writing_mode = writing_mode; + if let CascadeMode::Unvisited { visited_rules: Some(visited_rules) } = cascade_mode { + let is_link = pseudo.is_none() && element.unwrap().is_link(); + macro_rules! visited_parent { + ($parent:expr) => { + if is_link { + $parent + } else { + $parent.map(|p| p.visited_style().unwrap_or(p)) + } + } + } + // We could call apply_declarations directly, but that'd cause + // another instantiation of this function which is not great. + context.builder.visited_style = Some(cascade_rules( + device, + pseudo, + visited_rules, + guards, + visited_parent!(parent_style), + visited_parent!(parent_style_ignoring_first_line), + visited_parent!(layout_parent_style), + font_metrics_provider, + CascadeMode::Visited { writing_mode }, + quirks_mode, + rule_cache, + &mut *context.rule_cache_conditions.borrow_mut(), + element, + )); + } let mut _skip_font_family = false; @@ -4097,11 +4159,12 @@ where builder.clear_modified_reset(); - StyleAdjuster::new(&mut builder).adjust( - layout_parent_style, - element, - flags, - ); + if matches!(cascade_mode, CascadeMode::Unvisited { .. }) { + StyleAdjuster::new(&mut builder).adjust( + layout_parent_style.unwrap_or(inherited_style), + element, + ); + } if builder.modified_reset() || !apply_reset { // If we adjusted any reset structs, we can't cache this ComputedValues. @@ -4110,8 +4173,7 @@ where // back again. (Aside from being wasted effort, it will be wrong, since // context.rule_cache_conditions won't be set appropriately if we // didn't compute those reset properties.) - context.rule_cache_conditions.borrow_mut() - .set_uncacheable(); + context.rule_cache_conditions.borrow_mut().set_uncacheable(); } builder.build() diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index 9ce31f3cc89..f1ba0698e6c 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -223,19 +223,19 @@ impl PseudoElement { /// properties... Also, I guess it just could do all: inherit on the /// stylesheet, though chances are that'd be kinda slow if we don't cache /// them... - pub fn cascade_flags(&self) -> CascadeFlags { + pub fn inherits_all(&self) -> bool { match *self { PseudoElement::After | PseudoElement::Before | PseudoElement::Selection | PseudoElement::DetailsContent | - PseudoElement::DetailsSummary => CascadeFlags::empty(), + PseudoElement::DetailsSummary | // Anonymous table flows shouldn't inherit their parents properties in order // to avoid doubling up styles such as transformations. PseudoElement::ServoAnonymousTableCell | PseudoElement::ServoAnonymousTableRow | PseudoElement::ServoText | - PseudoElement::ServoInputText => CascadeFlags::empty(), + PseudoElement::ServoInputText => false, // For tables, we do want style to inherit, because TableWrapper is // responsible for handling clipping and scrolling, while Table is @@ -248,7 +248,7 @@ impl PseudoElement { PseudoElement::ServoTableWrapper | PseudoElement::ServoAnonymousBlock | PseudoElement::ServoInlineBlockWrapper | - PseudoElement::ServoInlineAbsolute => CascadeFlags::INHERIT_ALL, + PseudoElement::ServoInlineAbsolute => true, } } diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 754abab0993..5a65429a550 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -7,7 +7,7 @@ use app_units::Au; use dom::TElement; -use properties::{self, CascadeFlags, ComputedValues, StyleBuilder}; +use properties::{self, ComputedValues, StyleBuilder}; use properties::computed_value_flags::ComputedValueFlags; use properties::longhands::display::computed_value::T as Display; use properties::longhands::float::computed_value::T as Float; @@ -681,10 +681,14 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { &mut self, layout_parent_style: &ComputedValues, element: Option, - flags: CascadeFlags, ) where E: TElement, { + debug_assert!( + !self.style.flags.contains(ComputedValueFlags::IS_STYLE_IF_VISITED), + "Adjusting visited styles is wasted work" + ); + if cfg!(debug_assertions) { if element .and_then(|e| e.implemented_pseudo_element()) @@ -705,15 +709,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { // "Should always have an element around for non-pseudo styles" // ); - // Don't adjust visited styles, visited-dependent properties aren't - // affected by these adjustments and it'd be just wasted work anyway. - // - // It also doesn't make much sense to adjust them, since we don't - // cascade most properties anyway, and they wouldn't be looked up. - if flags.contains(CascadeFlags::VISITED_DEPENDENT_ONLY) { - return; - } - self.adjust_for_visited(element); #[cfg(feature = "gecko")] { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 5acdcf33bb0..820f4dbcda7 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -20,7 +20,7 @@ use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps}; #[cfg(feature = "gecko")] use malloc_size_of::MallocUnconditionalShallowSizeOf; use media_queries::Device; -use properties::{self, CascadeFlags, ComputedValues}; +use properties::{self, CascadeMode, ComputedValues}; use properties::{AnimationRules, PropertyDeclarationBlock}; use rule_cache::{RuleCache, RuleCacheConditions}; use rule_tree::{CascadeLevel, RuleTree, ShadowCascadeOrder, StrongRuleNode, StyleSource}; @@ -845,55 +845,18 @@ impl Stylist { { debug_assert!(pseudo.is_some() || element.is_some(), "Huh?"); - let cascade_flags = pseudo.map_or(CascadeFlags::empty(), |p| p.cascade_flags()); - // We need to compute visited values if we have visited rules or if our // parent has visited values. - let mut visited_values = None; - if inputs.visited_rules.is_some() || parent_style.and_then(|s| s.visited_style()).is_some() - { - // At this point inputs may have visited rules, or rules. - let rule_node = match inputs.visited_rules.as_ref() { - Some(rules) => rules, - None => inputs.rules.as_ref().unwrap_or(self.rule_tree.root()), - }; - - let inherited_style; - let inherited_style_ignoring_first_line; - let layout_parent_style_for_visited; - if pseudo.is_none() && element.unwrap().is_link() { - // We just want to use our parent style as our parent. - inherited_style = parent_style; - inherited_style_ignoring_first_line = parent_style_ignoring_first_line; - layout_parent_style_for_visited = layout_parent_style; - } else { - // We want to use the visited bits (if any) from our parent - // style as our parent. - inherited_style = parent_style - .map(|parent_style| parent_style.visited_style().unwrap_or(parent_style)); - inherited_style_ignoring_first_line = parent_style_ignoring_first_line - .map(|parent_style| parent_style.visited_style().unwrap_or(parent_style)); - layout_parent_style_for_visited = layout_parent_style - .map(|parent_style| parent_style.visited_style().unwrap_or(parent_style)); + let visited_rules = match inputs.visited_rules.as_ref() { + Some(rules) => Some(rules), + None => { + if parent_style.and_then(|s| s.visited_style()).is_some() { + Some(inputs.rules.as_ref().unwrap_or(self.rule_tree.root())) + } else { + None + } } - - visited_values = Some(properties::cascade::( - &self.device, - pseudo, - rule_node, - guards, - inherited_style, - inherited_style_ignoring_first_line, - layout_parent_style_for_visited, - None, - font_metrics, - cascade_flags | CascadeFlags::VISITED_DEPENDENT_ONLY, - self.quirks_mode, - rule_cache, - rule_cache_conditions, - element, - )); - } + }; // Read the comment on `precomputed_values_for_pseudo` to see why it's // difficult to assert that display: contents nodes never arrive here @@ -909,9 +872,8 @@ impl Stylist { parent_style, parent_style_ignoring_first_line, layout_parent_style, - visited_values, + visited_rules, font_metrics, - cascade_flags, self.quirks_mode, rule_cache, rule_cache_conditions, @@ -1581,9 +1543,8 @@ impl Stylist { Some(parent_style), Some(parent_style), Some(parent_style), - None, &metrics, - CascadeFlags::empty(), + CascadeMode::Unvisited { visited_rules: None }, self.quirks_mode, /* rule_cache = */ None, &mut Default::default(),