From dff8f78c4201f7ba799e00ae6cd179658e6b626b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 6 Jun 2023 17:30:03 +0200 Subject: [PATCH] style: Update CSSOM for layer rules to the spec Pretty mechanical. Tests are in https://wpt.live/css/css-cascade/layer-rules-cssom.html which (with a fix for @import tests which I'll submit separately) we pass. Sync for that is bug 1743936. Differential Revision: https://phabricator.services.mozilla.com/D133387 --- components/style/gecko/arc_types.rs | 18 ++- components/style/gecko/wrapper.rs | 26 +++- components/style/invalidation/stylesheets.rs | 6 +- components/style/stylesheets/layer_rule.rs | 133 +++++++++--------- components/style/stylesheets/mod.rs | 31 ++-- components/style/stylesheets/rule_parser.rs | 20 +-- .../style/stylesheets/rules_iterator.rs | 10 +- components/style/stylesheets/stylesheet.rs | 3 +- components/style/stylist.rs | 49 +++---- 9 files changed, 155 insertions(+), 141 deletions(-) diff --git a/components/style/gecko/arc_types.rs b/components/style/gecko/arc_types.rs index 05fcdb5b12d..4fb08c3a309 100644 --- a/components/style/gecko/arc_types.rs +++ b/components/style/gecko/arc_types.rs @@ -12,9 +12,10 @@ use crate::gecko::url::CssUrlData; use crate::gecko_bindings::structs::{ RawServoAnimationValue, RawServoCounterStyleRule, RawServoCssUrlData, RawServoDeclarationBlock, RawServoFontFaceRule, RawServoFontFeatureValuesRule, RawServoImportRule, RawServoKeyframe, - RawServoKeyframesRule, RawServoLayerRule, RawServoMediaList, RawServoMediaRule, - RawServoMozDocumentRule, RawServoNamespaceRule, RawServoPageRule, RawServoScrollTimelineRule, - RawServoStyleRule, RawServoStyleSheetContents, RawServoSupportsRule, ServoCssRules, + RawServoKeyframesRule, RawServoLayerBlockRule, RawServoLayerStatementRule, RawServoMediaList, + RawServoMediaRule, RawServoMozDocumentRule, RawServoNamespaceRule, RawServoPageRule, + RawServoScrollTimelineRule, RawServoStyleRule, RawServoStyleSheetContents, + RawServoSupportsRule, ServoCssRules, }; use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasFFI, Strong}; use crate::media_queries::MediaList; @@ -24,8 +25,8 @@ use crate::shared_lock::Locked; use crate::stylesheets::keyframes_rule::Keyframe; use crate::stylesheets::{ CounterStyleRule, CssRules, DocumentRule, FontFaceRule, FontFeatureValuesRule, ImportRule, - KeyframesRule, LayerRule, MediaRule, NamespaceRule, PageRule, ScrollTimelineRule, StyleRule, - StylesheetContents, SupportsRule, + KeyframesRule, LayerBlockRule, LayerStatementRule, MediaRule, NamespaceRule, PageRule, + ScrollTimelineRule, StyleRule, StylesheetContents, SupportsRule, }; use servo_arc::{Arc, ArcBorrow}; use std::{mem, ptr}; @@ -73,8 +74,11 @@ impl_arc_ffi!(Locked => RawServoKeyframe impl_arc_ffi!(Locked => RawServoKeyframesRule [Servo_KeyframesRule_AddRef, Servo_KeyframesRule_Release]); -impl_arc_ffi!(Locked => RawServoLayerRule - [Servo_LayerRule_AddRef, Servo_LayerRule_Release]); +impl_arc_ffi!(Locked => RawServoLayerBlockRule + [Servo_LayerBlockRule_AddRef, Servo_LayerBlockRule_Release]); + +impl_arc_ffi!(Locked => RawServoLayerStatementRule + [Servo_LayerStatementRule_AddRef, Servo_LayerStatementRule_Release]); impl_arc_ffi!(Locked => RawServoMediaList [Servo_MediaList_AddRef, Servo_MediaList_Release]); diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 2418591963b..b289fefb4bd 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1578,8 +1578,8 @@ impl<'le> TElement for GeckoElement<'le> { use crate::properties::longhands::_x_text_zoom::SpecifiedValue as SpecifiedZoom; use crate::properties::longhands::color::SpecifiedValue as SpecifiedColor; use crate::properties::longhands::text_align::SpecifiedValue as SpecifiedTextAlign; - use crate::values::specified::color::Color; use crate::stylesheets::layer_rule::LayerOrder; + use crate::values::specified::color::Color; lazy_static! { static ref TH_RULE: ApplicableDeclarationBlock = { let global_style_data = &*GLOBAL_STYLE_DATA; @@ -1588,7 +1588,11 @@ impl<'le> TElement for GeckoElement<'le> { Importance::Normal, ); let arc = Arc::new_leaked(global_style_data.shared_lock.wrap(pdb)); - ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints, LayerOrder::root()) + ApplicableDeclarationBlock::from_declarations( + arc, + ServoCascadeLevel::PresHints, + LayerOrder::root(), + ) }; static ref TABLE_COLOR_RULE: ApplicableDeclarationBlock = { let global_style_data = &*GLOBAL_STYLE_DATA; @@ -1597,7 +1601,11 @@ impl<'le> TElement for GeckoElement<'le> { Importance::Normal, ); let arc = Arc::new_leaked(global_style_data.shared_lock.wrap(pdb)); - ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints, LayerOrder::root()) + ApplicableDeclarationBlock::from_declarations( + arc, + ServoCascadeLevel::PresHints, + LayerOrder::root(), + ) }; static ref MATHML_LANG_RULE: ApplicableDeclarationBlock = { let global_style_data = &*GLOBAL_STYLE_DATA; @@ -1606,7 +1614,11 @@ impl<'le> TElement for GeckoElement<'le> { Importance::Normal, ); let arc = Arc::new_leaked(global_style_data.shared_lock.wrap(pdb)); - ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints, LayerOrder::root()) + ApplicableDeclarationBlock::from_declarations( + arc, + ServoCascadeLevel::PresHints, + LayerOrder::root(), + ) }; static ref SVG_TEXT_DISABLE_ZOOM_RULE: ApplicableDeclarationBlock = { let global_style_data = &*GLOBAL_STYLE_DATA; @@ -1615,7 +1627,11 @@ impl<'le> TElement for GeckoElement<'le> { Importance::Normal, ); let arc = Arc::new_leaked(global_style_data.shared_lock.wrap(pdb)); - ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints, LayerOrder::root()) + ApplicableDeclarationBlock::from_declarations( + arc, + ServoCascadeLevel::PresHints, + LayerOrder::root(), + ) }; }; diff --git a/components/style/invalidation/stylesheets.rs b/components/style/invalidation/stylesheets.rs index 1d1111170ed..3a9017582b2 100644 --- a/components/style/invalidation/stylesheets.rs +++ b/components/style/invalidation/stylesheets.rs @@ -541,6 +541,7 @@ impl StylesheetInvalidationSet { Page(..) | Viewport(..) | FontFeatureValues(..) | + LayerStatement(..) | FontFace(..) | Keyframes(..) | ScrollTimeline(..) | @@ -556,7 +557,7 @@ impl StylesheetInvalidationSet { self.collect_invalidations_for_rule(rule, guard, device, quirks_mode) }, - Document(..) | Import(..) | Media(..) | Supports(..) | Layer(..) => { + Document(..) | Import(..) | Media(..) | Supports(..) | LayerBlock(..) => { if !is_generic_change && !EffectiveRules::is_effective(guard, device, quirks_mode, rule) { @@ -597,7 +598,8 @@ impl StylesheetInvalidationSet { } } }, - Document(..) | Namespace(..) | Import(..) | Media(..) | Supports(..) | Layer(..) => { + Document(..) | Namespace(..) | Import(..) | Media(..) | Supports(..) | + LayerStatement(..) | LayerBlock(..) => { // Do nothing, relevant nested rules are visited as part of the // iteration. }, diff --git a/components/style/stylesheets/layer_rule.rs b/components/style/stylesheets/layer_rule.rs index f77a1e7c50e..3607e9f2ebf 100644 --- a/components/style/stylesheets/layer_rule.rs +++ b/components/style/stylesheets/layer_rule.rs @@ -159,71 +159,34 @@ impl ToCss for LayerName { } } -/// The kind of layer rule this is. #[derive(Debug, ToShmem)] -pub enum LayerRuleKind { - /// A block `@layer ? { ... }` - Block { - /// The layer name, or `None` if anonymous. - name: Option, - /// The nested rules. - rules: Arc>, - }, - /// A statement `@layer , , ;` - Statement { - /// The list of layers to sort. - names: Vec, - }, -} - -/// A [`@layer`][layer] rule. -/// -/// [layer]: https://drafts.csswg.org/css-cascade-5/#layering -#[derive(Debug, ToShmem)] -pub struct LayerRule { - /// The kind of layer rule we are. - pub kind: LayerRuleKind, - /// The source position where this media rule was found. +/// A block `@layer ? { ... }` +/// https://drafts.csswg.org/css-cascade-5/#layer-block +pub struct LayerBlockRule { + /// The layer name, or `None` if anonymous. + pub name: Option, + /// The nested rules. + pub rules: Arc>, + /// The source position where this rule was found. pub source_location: SourceLocation, } -impl ToCssWithGuard for LayerRule { +impl ToCssWithGuard for LayerBlockRule { fn to_css( &self, guard: &SharedRwLockReadGuard, dest: &mut crate::str::CssStringWriter, ) -> fmt::Result { dest.write_str("@layer")?; - match self.kind { - LayerRuleKind::Block { - ref name, - ref rules, - } => { - if let Some(ref name) = *name { - dest.write_char(' ')?; - name.to_css(&mut CssWriter::new(dest))?; - } - rules.read_with(guard).to_css_block(guard, dest) - }, - LayerRuleKind::Statement { ref names } => { - let mut writer = CssWriter::new(dest); - let mut first = true; - for name in &**names { - if first { - writer.write_char(' ')?; - } else { - writer.write_str(", ")?; - } - first = false; - name.to_css(&mut writer)?; - } - dest.write_char(';') - }, + if let Some(ref name) = self.name { + dest.write_char(' ')?; + name.to_css(&mut CssWriter::new(dest))?; } + self.rules.read_with(guard).to_css_block(guard, dest) } } -impl DeepCloneWithLock for LayerRule { +impl DeepCloneWithLock for LayerBlockRule { fn deep_clone_with_lock( &self, lock: &SharedRwLock, @@ -231,25 +194,57 @@ impl DeepCloneWithLock for LayerRule { params: &DeepCloneParams, ) -> Self { Self { - kind: match self.kind { - LayerRuleKind::Block { - ref name, - ref rules, - } => LayerRuleKind::Block { - name: name.clone(), - rules: Arc::new( - lock.wrap( - rules - .read_with(guard) - .deep_clone_with_lock(lock, guard, params), - ), - ), - }, - LayerRuleKind::Statement { ref names } => LayerRuleKind::Statement { - names: names.clone(), - }, - }, + name: self.name.clone(), + rules: Arc::new( + lock.wrap( + self.rules + .read_with(guard) + .deep_clone_with_lock(lock, guard, params), + ), + ), source_location: self.source_location.clone(), } } } + +/// A statement `@layer , , ;` +/// +/// https://drafts.csswg.org/css-cascade-5/#layer-empty +#[derive(Clone, Debug, ToShmem)] +pub struct LayerStatementRule { + /// The list of layers to sort. + pub names: Vec, + /// The source position where this rule was found. + pub source_location: SourceLocation, +} + +impl ToCssWithGuard for LayerStatementRule { + fn to_css( + &self, + _: &SharedRwLockReadGuard, + dest: &mut crate::str::CssStringWriter, + ) -> fmt::Result { + let mut writer = CssWriter::new(dest); + writer.write_str("@layer ")?; + let mut first = true; + for name in &*self.names { + if !first { + writer.write_str(", ")?; + } + first = false; + name.to_css(&mut writer)?; + } + writer.write_char(';') + } +} + +impl DeepCloneWithLock for LayerStatementRule { + fn deep_clone_with_lock( + &self, + _: &SharedRwLock, + _: &SharedRwLockReadGuard, + _: &DeepCloneParams, + ) -> Self { + self.clone() + } +} diff --git a/components/style/stylesheets/mod.rs b/components/style/stylesheets/mod.rs index f7098e0c151..f6348c6190d 100644 --- a/components/style/stylesheets/mod.rs +++ b/components/style/stylesheets/mod.rs @@ -51,7 +51,7 @@ pub use self::font_face_rule::FontFaceRule; pub use self::font_feature_values_rule::FontFeatureValuesRule; pub use self::import_rule::ImportRule; pub use self::keyframes_rule::KeyframesRule; -pub use self::layer_rule::LayerRule; +pub use self::layer_rule::{LayerBlockRule, LayerStatementRule}; pub use self::loader::StylesheetLoader; pub use self::media_rule::MediaRule; pub use self::namespace_rule::NamespaceRule; @@ -261,7 +261,8 @@ pub enum CssRule { Supports(Arc>), Page(Arc>), Document(Arc>), - Layer(Arc>), + LayerBlock(Arc>), + LayerStatement(Arc>), ScrollTimeline(Arc>), } @@ -304,9 +305,8 @@ impl CssRule { lock.unconditional_shallow_size_of(ops) + lock.read_with(guard).size_of(guard, ops) }, - // TODO(emilio): Add memory reporting for @layer rules. - CssRule::Layer(_) => 0, - CssRule::ScrollTimeline(_) => 0, + // TODO(emilio): Add memory reporting for these rules. + CssRule::LayerBlock(_) | CssRule::LayerStatement(_) | CssRule::ScrollTimeline(_) => 0, } } } @@ -341,8 +341,9 @@ pub enum CssRuleType { Viewport = 15, // After viewport, all rules should return 0 from the API, but we still need // a constant somewhere. - Layer = 16, - ScrollTimeline = 17, + LayerBlock = 16, + LayerStatement = 17, + ScrollTimeline = 18, } #[allow(missing_docs)] @@ -369,7 +370,8 @@ impl CssRule { CssRule::Supports(_) => CssRuleType::Supports, CssRule::Page(_) => CssRuleType::Page, CssRule::Document(_) => CssRuleType::Document, - CssRule::Layer(_) => CssRuleType::Layer, + CssRule::LayerBlock(_) => CssRuleType::LayerBlock, + CssRule::LayerStatement(_) => CssRuleType::LayerStatement, CssRule::ScrollTimeline(_) => CssRuleType::ScrollTimeline, } } @@ -504,9 +506,15 @@ impl DeepCloneWithLock for CssRule { lock.wrap(rule.deep_clone_with_lock(lock, guard, params)), )) }, - CssRule::Layer(ref arc) => { + CssRule::LayerStatement(ref arc) => { let rule = arc.read_with(guard); - CssRule::Layer(Arc::new( + CssRule::LayerStatement(Arc::new( + lock.wrap(rule.deep_clone_with_lock(lock, guard, params)), + )) + }, + CssRule::LayerBlock(ref arc) => { + let rule = arc.read_with(guard); + CssRule::LayerBlock(Arc::new( lock.wrap(rule.deep_clone_with_lock(lock, guard, params)), )) }, @@ -534,7 +542,8 @@ impl ToCssWithGuard for CssRule { CssRule::Supports(ref lock) => lock.read_with(guard).to_css(guard, dest), CssRule::Page(ref lock) => lock.read_with(guard).to_css(guard, dest), CssRule::Document(ref lock) => lock.read_with(guard).to_css(guard, dest), - CssRule::Layer(ref lock) => lock.read_with(guard).to_css(guard, dest), + CssRule::LayerBlock(ref lock) => lock.read_with(guard).to_css(guard, dest), + CssRule::LayerStatement(ref lock) => lock.read_with(guard).to_css(guard, dest), CssRule::ScrollTimeline(ref lock) => lock.read_with(guard).to_css(guard, dest), } } diff --git a/components/style/stylesheets/rule_parser.rs b/components/style/stylesheets/rule_parser.rs index 12ec7226bfc..3c0d8a5c231 100644 --- a/components/style/stylesheets/rule_parser.rs +++ b/components/style/stylesheets/rule_parser.rs @@ -17,14 +17,14 @@ use crate::stylesheets::document_rule::DocumentCondition; use crate::stylesheets::font_feature_values_rule::parse_family_name_list; use crate::stylesheets::import_rule::ImportLayer; use crate::stylesheets::keyframes_rule::parse_keyframe_list; -use crate::stylesheets::layer_rule::{LayerName, LayerRuleKind}; +use crate::stylesheets::layer_rule::{LayerBlockRule, LayerName, LayerStatementRule}; use crate::stylesheets::scroll_timeline_rule::ScrollTimelineDescriptors; use crate::stylesheets::stylesheet::Namespaces; use crate::stylesheets::supports_rule::SupportsCondition; use crate::stylesheets::{ viewport_rule, AllowImportRules, CorsMode, CssRule, CssRuleType, CssRules, DocumentRule, - FontFeatureValuesRule, KeyframesRule, LayerRule, MediaRule, NamespaceRule, PageRule, - RulesMutateError, ScrollTimelineRule, StyleRule, StylesheetLoader, SupportsRule, ViewportRule, + FontFeatureValuesRule, KeyframesRule, MediaRule, NamespaceRule, PageRule, RulesMutateError, + ScrollTimelineRule, StyleRule, StylesheetLoader, SupportsRule, ViewportRule, }; use crate::values::computed::font::FamilyName; use crate::values::{CssUrl, CustomIdent, KeyframesName, TimelineName}; @@ -613,13 +613,13 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { 0 | 1 => names.into_iter().next(), _ => return Err(input.new_error(BasicParseErrorKind::AtRuleBodyInvalid)), }; - Ok(CssRule::Layer(Arc::new(self.shared_lock.wrap(LayerRule { - kind: LayerRuleKind::Block { + Ok(CssRule::LayerBlock(Arc::new(self.shared_lock.wrap( + LayerBlockRule { name, - rules: self.parse_nested_rules(input, CssRuleType::Layer), + rules: self.parse_nested_rules(input, CssRuleType::LayerBlock), + source_location: start.source_location(), }, - source_location: start.source_location(), - })))) + )))) }, AtRulePrelude::Import(..) | AtRulePrelude::Namespace(..) => { // These rules don't have blocks. @@ -654,8 +654,8 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> { if names.is_empty() { return Err(()); } - CssRule::Layer(Arc::new(self.shared_lock.wrap(LayerRule { - kind: LayerRuleKind::Statement { names }, + CssRule::LayerStatement(Arc::new(self.shared_lock.wrap(LayerStatementRule { + names, source_location: start.source_location(), }))) }, diff --git a/components/style/stylesheets/rules_iterator.rs b/components/style/stylesheets/rules_iterator.rs index 0cbc7327441..417185953a0 100644 --- a/components/style/stylesheets/rules_iterator.rs +++ b/components/style/stylesheets/rules_iterator.rs @@ -70,6 +70,7 @@ where CssRule::Keyframes(_) | CssRule::ScrollTimeline(_) | CssRule::Page(_) | + CssRule::LayerStatement(_) | CssRule::FontFeatureValues(_) => None, CssRule::Import(ref import_rule) => { let import_rule = import_rule.read_with(guard); @@ -103,14 +104,9 @@ where } Some(supports_rule.rules.read_with(guard).0.iter()) }, - CssRule::Layer(ref lock) => { - use crate::stylesheets::layer_rule::LayerRuleKind; - + CssRule::LayerBlock(ref lock) => { 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, - } + Some(layer_rule.rules.read_with(guard).0.iter()) }, } } diff --git a/components/style/stylesheets/stylesheet.rs b/components/style/stylesheets/stylesheet.rs index e63d8187365..ab7df841d7f 100644 --- a/components/style/stylesheets/stylesheet.rs +++ b/components/style/stylesheets/stylesheet.rs @@ -363,7 +363,8 @@ impl SanitizationKind { CssRule::Import(..) | // TODO(emilio): Perhaps Layer should not be always sanitized? But // we sanitize @media and co, so this seems safer for now. - CssRule::Layer(..) => false, + CssRule::LayerStatement(..) | + CssRule::LayerBlock(..) => false, CssRule::FontFace(..) | CssRule::Namespace(..) | CssRule::Style(..) => true, diff --git a/components/style/stylist.rs b/components/style/stylist.rs index c0cc2f238bb..a99de9d2a87 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -2516,35 +2516,25 @@ impl CascadeData { self.effective_media_query_results.saw_effective(media_rule); } }, - CssRule::Layer(ref lock) => { - use crate::stylesheets::layer_rule::LayerRuleKind; - + CssRule::LayerBlock(ref lock) => { let layer_rule = lock.read_with(guard); - match layer_rule.kind { - LayerRuleKind::Block { ref name, .. } => { - children_layer_id = maybe_register_layers( - self, - name.as_ref(), - &mut current_layer, - &mut layer_names_to_pop, - ); - }, - LayerRuleKind::Statement { ref names } => { - for name in &**names { - let mut pushed = 0; - // There are no children, so we can ignore the - // return value. - maybe_register_layers( - self, - Some(name), - &mut current_layer, - &mut pushed, - ); - for _ in 0..pushed { - current_layer.0.pop(); - } - } - }, + children_layer_id = maybe_register_layers( + self, + layer_rule.name.as_ref(), + &mut current_layer, + &mut layer_names_to_pop, + ); + }, + CssRule::LayerStatement(ref lock) => { + let layer_rule = lock.read_with(guard); + for name in &*layer_rule.names { + let mut pushed = 0; + // There are no children, so we can ignore the + // return value. + maybe_register_layers(self, Some(name), &mut current_layer, &mut pushed); + for _ in 0..pushed { + current_layer.0.pop(); + } } }, // We don't care about any other rule. @@ -2660,7 +2650,8 @@ impl CascadeData { CssRule::Page(..) | CssRule::Viewport(..) | CssRule::Document(..) | - CssRule::Layer(..) | + CssRule::LayerBlock(..) | + CssRule::LayerStatement(..) | CssRule::FontFeatureValues(..) => { // Not affected by device changes. continue;