From 8705e3f39f3f6e374f222f5388c1febf02399fd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 27 May 2023 06:02:50 +0200 Subject: [PATCH] style: Compute layer order during CascadeData rebuild For that, deal with children in add_rule recursively, to keep the current layer name up-to-date in block layer rules. This is not the final version of this code, as right now something like this: @layer A { ... } @layer B { ... } @layer A.A { ... } Would give A.A more priority over B, which is not correct. There are tests for this incoming in wpt sync and such, but that can be tweaked later. Differential Revision: https://phabricator.services.mozilla.com/D124335 --- components/style/stylesheets/layer_rule.rs | 51 +++++- components/style/stylesheets/mod.rs | 2 +- components/style/stylesheets/rule_parser.rs | 7 +- .../style/stylesheets/rules_iterator.rs | 120 +++++++------- components/style/stylist.rs | 146 +++++++++++++++--- 5 files changed, 236 insertions(+), 90 deletions(-) diff --git a/components/style/stylesheets/layer_rule.rs b/components/style/stylesheets/layer_rule.rs index c8a04ca6563..8d51558eccd 100644 --- a/components/style/stylesheets/layer_rule.rs +++ b/components/style/stylesheets/layer_rule.rs @@ -20,8 +20,37 @@ use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, ToCss}; /// A ``: https://drafts.csswg.org/css-cascade-5/#typedef-layer-name -#[derive(Clone, Debug, ToShmem)] -pub struct LayerName(SmallVec<[AtomIdent; 1]>); +#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToShmem)] +pub struct LayerName(pub SmallVec<[AtomIdent; 1]>); + +impl LayerName { + /// Returns an empty layer name (which isn't a valid final state, so caller + /// is responsible to fill up the name before use). + pub fn new_empty() -> Self { + Self(Default::default()) + } + + /// Returns a synthesized name for an anonymous layer. + pub fn new_anonymous() -> Self { + use std::sync::atomic::{AtomicUsize, Ordering}; + static NEXT_ANONYMOUS_LAYER_NAME: AtomicUsize = AtomicUsize::new(0); + + let mut name = SmallVec::new(); + let next_id = NEXT_ANONYMOUS_LAYER_NAME.fetch_add(1, Ordering::Relaxed); + // The parens don't _technically_ prevent conflicts with authors, as + // authors could write escaped parens as part of the identifier, I + // think, but highly reduces the possibility. + name.push(AtomIdent::from(&*format!("-moz-anon-layer({})", next_id))); + + LayerName(name) + } + + /// Returns the names of the layers. That is, for a layer like `foo.bar`, + /// it'd return [foo, bar]. + pub fn layer_names(&self) -> &[AtomIdent] { + &self.0 + } +} impl Parse for LayerName { fn parse<'i, 't>( @@ -82,10 +111,13 @@ impl ToCss for LayerName { pub enum LayerRuleKind { /// A block `@layer ? { ... }` Block { - /// The layer name, or `None` if anonymous. - name: Option, + /// The layer name. + name: LayerName, /// The nested rules. rules: Arc>, + /// Whether the layer name is synthesized (and thus shouldn't be + /// serialized). + is_anonymous: bool, }, /// A statement `@layer , , ;` Statement { @@ -116,8 +148,9 @@ impl ToCssWithGuard for LayerRule { LayerRuleKind::Block { ref name, ref rules, + ref is_anonymous, } => { - if let Some(ref name) = *name { + if !*is_anonymous { name.to_css(&mut CssWriter::new(dest))?; dest.write_char(' ')?; } @@ -151,8 +184,13 @@ impl DeepCloneWithLock for LayerRule { LayerRuleKind::Block { ref name, ref rules, + ref is_anonymous, } => LayerRuleKind::Block { - name: name.clone(), + name: if *is_anonymous { + LayerName::new_anonymous() + } else { + name.clone() + }, rules: Arc::new( lock.wrap( rules @@ -160,6 +198,7 @@ impl DeepCloneWithLock for LayerRule { .deep_clone_with_lock(lock, guard, params), ), ), + is_anonymous: *is_anonymous, }, LayerRuleKind::Statement { ref names } => LayerRuleKind::Statement { names: names.clone(), diff --git a/components/style/stylesheets/mod.rs b/components/style/stylesheets/mod.rs index b4e10ee368e..360a17b2f9a 100644 --- a/components/style/stylesheets/mod.rs +++ b/components/style/stylesheets/mod.rs @@ -11,7 +11,7 @@ mod font_face_rule; pub mod font_feature_values_rule; pub mod import_rule; pub mod keyframes_rule; -mod layer_rule; +pub mod layer_rule; mod loader; mod media_rule; mod namespace_rule; diff --git a/components/style/stylesheets/rule_parser.rs b/components/style/stylesheets/rule_parser.rs index 2998aba8fad..7fb76a8d7a9 100644 --- a/components/style/stylesheets/rule_parser.rs +++ b/components/style/stylesheets/rule_parser.rs @@ -577,9 +577,9 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { )))) }, AtRulePrelude::Layer(names) => { - let name = match names.len() { - 0 => None, - 1 => Some(names.into_iter().next().unwrap()), + let (name, is_anonymous) = match names.len() { + 0 => (LayerName::new_anonymous(), true), + 1 => (names.into_iter().next().unwrap(), false), _ => return Err(input.new_error(BasicParseErrorKind::AtRuleBodyInvalid)), }; Ok(CssRule::Layer(Arc::new(self.shared_lock.wrap( @@ -587,6 +587,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { kind: LayerRuleKind::Block { name, rules: self.parse_nested_rules(input, CssRuleType::Layer), + is_anonymous, }, source_location: start.source_location(), }, diff --git a/components/style/stylesheets/rules_iterator.rs b/components/style/stylesheets/rules_iterator.rs index b1921e63e07..d98d70981df 100644 --- a/components/style/stylesheets/rules_iterator.rs +++ b/components/style/stylesheets/rules_iterator.rs @@ -51,67 +51,65 @@ where pub fn skip_children(&mut self) { self.stack.pop(); } -} -fn children_of_rule<'a, C>( - rule: &'a CssRule, - device: &'a Device, - quirks_mode: QuirksMode, - guard: &'a SharedRwLockReadGuard<'_>, - effective: &mut bool, -) -> Option> -where - C: NestedRuleIterationCondition + 'static, -{ - *effective = true; - match *rule { - CssRule::Namespace(_) | - CssRule::Style(_) | - CssRule::FontFace(_) | - CssRule::CounterStyle(_) | - CssRule::Viewport(_) | - CssRule::Keyframes(_) | - CssRule::Page(_) | - CssRule::FontFeatureValues(_) => None, - CssRule::Import(ref import_rule) => { - let import_rule = import_rule.read_with(guard); - if !C::process_import(guard, device, quirks_mode, import_rule) { - *effective = false; - return None; - } - Some(import_rule.stylesheet.rules(guard).iter()) - }, - CssRule::Document(ref doc_rule) => { - let doc_rule = doc_rule.read_with(guard); - if !C::process_document(guard, device, quirks_mode, doc_rule) { - *effective = false; - return None; - } - Some(doc_rule.rules.read_with(guard).0.iter()) - }, - CssRule::Media(ref lock) => { - let media_rule = lock.read_with(guard); - if !C::process_media(guard, device, quirks_mode, media_rule) { - *effective = false; - return None; - } - Some(media_rule.rules.read_with(guard).0.iter()) - }, - CssRule::Supports(ref lock) => { - let supports_rule = lock.read_with(guard); - if !C::process_supports(guard, device, quirks_mode, supports_rule) { - *effective = false; - return None; - } - Some(supports_rule.rules.read_with(guard).0.iter()) - }, - CssRule::Layer(ref lock) => { - use crate::stylesheets::layer_rule::LayerRuleKind; + /// Returns the children of `rule`, and whether `rule` is effective. + pub fn children( + rule: &'a CssRule, + device: &'a Device, + quirks_mode: QuirksMode, + guard: &'a SharedRwLockReadGuard<'_>, + effective: &mut bool, + ) -> Option> { + *effective = true; + match *rule { + CssRule::Namespace(_) | + CssRule::Style(_) | + CssRule::FontFace(_) | + CssRule::CounterStyle(_) | + CssRule::Viewport(_) | + CssRule::Keyframes(_) | + CssRule::Page(_) | + CssRule::FontFeatureValues(_) => None, + CssRule::Import(ref import_rule) => { + let import_rule = import_rule.read_with(guard); + if !C::process_import(guard, device, quirks_mode, import_rule) { + *effective = false; + return None; + } + Some(import_rule.stylesheet.rules(guard).iter()) + }, + CssRule::Document(ref doc_rule) => { + let doc_rule = doc_rule.read_with(guard); + if !C::process_document(guard, device, quirks_mode, doc_rule) { + *effective = false; + return None; + } + Some(doc_rule.rules.read_with(guard).0.iter()) + }, + CssRule::Media(ref lock) => { + let media_rule = lock.read_with(guard); + if !C::process_media(guard, device, quirks_mode, media_rule) { + *effective = false; + return None; + } + Some(media_rule.rules.read_with(guard).0.iter()) + }, + CssRule::Supports(ref lock) => { + let supports_rule = lock.read_with(guard); + if !C::process_supports(guard, device, quirks_mode, supports_rule) { + *effective = false; + return None; + } + Some(supports_rule.rules.read_with(guard).0.iter()) + }, + CssRule::Layer(ref lock) => { + use crate::stylesheets::layer_rule::LayerRuleKind; - let layer_rule = lock.read_with(guard); - match layer_rule.kind { - LayerRuleKind::Block { ref rules, .. } => Some(rules.read_with(guard).0.iter()), - LayerRuleKind::Statement { .. } => None, + let layer_rule = lock.read_with(guard); + match layer_rule.kind { + LayerRuleKind::Block { ref rules, .. } => Some(rules.read_with(guard).0.iter()), + LayerRuleKind::Statement { .. } => None, + } } } } @@ -138,7 +136,7 @@ where }; let mut effective = true; - let children = children_of_rule::( + let children = Self::children( rule, self.device, self.quirks_mode, @@ -324,7 +322,7 @@ impl<'a, 'b> EffectiveRulesIterator<'a, 'b> { guard: &'a SharedRwLockReadGuard<'b>, rule: &'a CssRule, ) -> Self { - let children = children_of_rule::(rule, device, quirks_mode, guard, &mut false); + let children = RulesIterator::::children(rule, device, quirks_mode, guard, &mut false); EffectiveRulesIterator::new(device, quirks_mode, guard, children.unwrap_or([].iter())) } } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 677ba49729f..71a230ab1d6 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -25,11 +25,12 @@ use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; use crate::stylesheet_set::{DataValidity, DocumentStylesheetSet, SheetRebuildKind}; use crate::stylesheet_set::{DocumentStylesheetFlusher, SheetCollectionFlusher}; use crate::stylesheets::keyframes_rule::KeyframesAnimation; +use crate::stylesheets::layer_rule::LayerName; use crate::stylesheets::viewport_rule::{self, MaybeNew, ViewportRule}; use crate::stylesheets::{StyleRule, StylesheetInDocument, StylesheetContents}; #[cfg(feature = "gecko")] use crate::stylesheets::{CounterStyleRule, FontFaceRule, FontFeatureValuesRule, PageRule}; -use crate::stylesheets::{CssRule, Origin, OriginSet, PerOrigin, PerOriginIter}; +use crate::stylesheets::{CssRule, Origin, OriginSet, PerOrigin, PerOriginIter, EffectiveRulesIterator}; use crate::thread_state::{self, ThreadState}; use crate::{Atom, LocalName, Namespace, WeakAtom}; use fallible::FallibleVec; @@ -1931,6 +1932,12 @@ pub struct CascadeData { /// by name. animations: PrecomputedHashMap, + /// A map from cascade layer name to layer order. + layer_order: FxHashMap, + + /// The next layer order for this cascade data. + next_layer_order: u32, + /// Effective media query results cached from the last rebuild. effective_media_query_results: EffectiveMediaQueryResults, @@ -1971,6 +1978,8 @@ impl CascadeData { // somewhat gnarly. selectors_for_cache_revalidation: SelectorMap::new_without_attribute_bucketing(), animations: Default::default(), + layer_order: Default::default(), + next_layer_order: 0, extra_data: ExtraStyleData::default(), effective_media_query_results: EffectiveMediaQueryResults::new(), rules_source_order: 0, @@ -2129,16 +2138,20 @@ impl CascadeData { fn add_rule( &mut self, rule: &CssRule, - _device: &Device, + device: &Device, quirks_mode: QuirksMode, stylesheet: &S, guard: &SharedRwLockReadGuard, rebuild_kind: SheetRebuildKind, + current_layer: &mut LayerName, mut precomputed_pseudo_element_decls: Option<&mut PrecomputedPseudoElementDeclarations>, ) -> Result<(), FailedAllocationError> where S: StylesheetInDocument + 'static, { + // Handle leaf rules first, as those are by far the most common ones, + // and are always effective, so we can skip some checks. + let mut handled = true; match *rule { CssRule::Style(ref locked) => { let style_rule = locked.read_with(&guard); @@ -2240,22 +2253,6 @@ impl CascadeData { } self.rules_source_order += 1; }, - CssRule::Import(ref lock) => { - if rebuild_kind.should_rebuild_invalidation() { - let import_rule = lock.read_with(guard); - self.effective_media_query_results - .saw_effective(import_rule); - } - - // NOTE: effective_rules visits the inner stylesheet if - // appropriate. - }, - CssRule::Media(ref lock) => { - if rebuild_kind.should_rebuild_invalidation() { - let media_rule = lock.read_with(guard); - self.effective_media_query_results.saw_effective(media_rule); - } - }, CssRule::Keyframes(ref keyframes_rule) => { let keyframes_rule = keyframes_rule.read_with(guard); debug!("Found valid keyframes rule: {:?}", *keyframes_rule); @@ -2292,9 +2289,116 @@ impl CascadeData { CssRule::Page(ref rule) => { self.extra_data.add_page(rule); }, + CssRule::Viewport(..) => {}, + _ => { + handled = false; + }, + } + + if handled { + // Assert that there are no children, and that the rule is + // effective. + if cfg!(debug_assertions) { + let mut effective = false; + let children = EffectiveRulesIterator::children( + rule, + device, + quirks_mode, + guard, + &mut effective, + ); + debug_assert!(children.is_none()); + debug_assert!(effective); + } + return Ok(()); + } + + let mut effective = false; + let children = EffectiveRulesIterator::children( + rule, + device, + quirks_mode, + guard, + &mut effective, + ); + + if !effective { + return Ok(()); + } + + let mut layer_names_to_pop = 0; + match *rule { + CssRule::Import(ref lock) => { + if rebuild_kind.should_rebuild_invalidation() { + let import_rule = lock.read_with(guard); + self.effective_media_query_results + .saw_effective(import_rule); + } + + }, + CssRule::Media(ref lock) => { + if rebuild_kind.should_rebuild_invalidation() { + let media_rule = lock.read_with(guard); + self.effective_media_query_results.saw_effective(media_rule); + } + }, + CssRule::Layer(ref lock) => { + use crate::stylesheets::layer_rule::LayerRuleKind; + + fn maybe_register_layer(data: &mut CascadeData, layer: &LayerName) { + // TODO: Measure what's more common / expensive, if + // layer.clone() or the double hash lookup in the insert + // case. + if data.layer_order.get(layer).is_some() { + return; + } + data.layer_order.insert(layer.clone(), data.next_layer_order); + data.next_layer_order += 1; + } + + let layer_rule = lock.read_with(guard); + match layer_rule.kind { + LayerRuleKind::Block { ref name, .. } => { + for name in name.layer_names() { + current_layer.0.push(name.clone()); + maybe_register_layer(self, ¤t_layer); + layer_names_to_pop += 1; + } + } + LayerRuleKind::Statement { ref names } => { + for name in &**names { + for name in name.layer_names() { + current_layer.0.push(name.clone()); + maybe_register_layer(self, ¤t_layer); + current_layer.0.pop(); + } + } + } + } + }, // We don't care about any other rule. _ => {}, } + + if let Some(children) = children { + for child in children { + self.add_rule( + child, + device, + quirks_mode, + stylesheet, + guard, + rebuild_kind, + current_layer, + precomputed_pseudo_element_decls.as_deref_mut(), + )?; + } + } + + for _ in 0..layer_names_to_pop { + current_layer.0.pop(); + } + Ok(()) } @@ -2321,8 +2425,9 @@ impl CascadeData { self.effective_media_query_results.saw_effective(contents); } + let mut current_layer = LayerName::new_empty(); - for rule in stylesheet.effective_rules(device, guard) { + for rule in contents.rules(guard).iter() { self.add_rule( rule, device, @@ -2330,6 +2435,7 @@ impl CascadeData { stylesheet, guard, rebuild_kind, + &mut current_layer, precomputed_pseudo_element_decls.as_deref_mut(), )?; } @@ -2448,6 +2554,8 @@ impl CascadeData { host_rules.clear(); } self.animations.clear(); + self.layer_order.clear(); + self.next_layer_order = 0; self.extra_data.clear(); self.rules_source_order = 0; self.num_selectors = 0;