From 349492b5e2754e2cff9d0a353cf0e34adb7d5772 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 14 Nov 2019 02:49:54 +0000 Subject: [PATCH] style: Fix cascade order of shadow parts. This moves the shadow cascade order into the cascade level, and refactors the code a bit for that. Differential Revision: https://phabricator.services.mozilla.com/D49988 --- components/style/applicable_declarations.rs | 83 +---- components/style/gecko/pseudo_element.rs | 6 + components/style/matching.rs | 4 +- components/style/properties/cascade.rs | 14 +- components/style/rule_collector.rs | 51 ++- components/style/rule_tree/mod.rs | 336 ++++++++++---------- components/style/selector_map.rs | 12 +- components/style/servo/selector_parser.rs | 6 + components/style/stylist.rs | 7 +- 9 files changed, 237 insertions(+), 282 deletions(-) diff --git a/components/style/applicable_declarations.rs b/components/style/applicable_declarations.rs index e4bf996a1d6..ab83e5737a9 100644 --- a/components/style/applicable_declarations.rs +++ b/components/style/applicable_declarations.rs @@ -5,11 +5,10 @@ //! Applicable declarations management. use crate::properties::PropertyDeclarationBlock; -use crate::rule_tree::{CascadeLevel, ShadowCascadeOrder, StyleSource}; +use crate::rule_tree::{CascadeLevel, StyleSource}; use crate::shared_lock::Locked; use servo_arc::Arc; use smallvec::SmallVec; -use std::fmt::{self, Debug}; /// List of applicable declarations. This is a transient structure that shuttles /// declarations between selector matching and inserting into the rule tree, and @@ -20,75 +19,27 @@ use std::fmt::{self, Debug}; /// However, it may depend a lot on workload, and stack space is cheap. pub type ApplicableDeclarationList = SmallVec<[ApplicableDeclarationBlock; 16]>; -/// Blink uses 18 bits to store source order, and does not check overflow [1]. -/// That's a limit that could be reached in realistic webpages, so we use -/// 24 bits and enforce defined behavior in the overflow case. -/// -/// [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/ -/// RuleSet.h?l=128&rcl=90140ab80b84d0f889abc253410f44ed54ae04f3 -const SOURCE_ORDER_SHIFT: usize = 0; -const SOURCE_ORDER_BITS: usize = 24; -const SOURCE_ORDER_MAX: u32 = (1 << SOURCE_ORDER_BITS) - 1; -const SOURCE_ORDER_MASK: u32 = SOURCE_ORDER_MAX << SOURCE_ORDER_SHIFT; - -/// We store up-to-15 shadow order levels. -/// -/// You'd need an element slotted across 16 components with ::slotted rules to -/// trigger this as of this writing, which looks... Unlikely. -const SHADOW_CASCADE_ORDER_SHIFT: usize = SOURCE_ORDER_BITS; -const SHADOW_CASCADE_ORDER_BITS: usize = 4; -const SHADOW_CASCADE_ORDER_MAX: u8 = (1 << SHADOW_CASCADE_ORDER_BITS) - 1; -const SHADOW_CASCADE_ORDER_MASK: u32 = - (SHADOW_CASCADE_ORDER_MAX as u32) << SHADOW_CASCADE_ORDER_SHIFT; - -const CASCADE_LEVEL_SHIFT: usize = SOURCE_ORDER_BITS + SHADOW_CASCADE_ORDER_BITS; -const CASCADE_LEVEL_BITS: usize = 4; -const CASCADE_LEVEL_MAX: u8 = (1 << CASCADE_LEVEL_BITS) - 1; -const CASCADE_LEVEL_MASK: u32 = (CASCADE_LEVEL_MAX as u32) << CASCADE_LEVEL_SHIFT; - /// Stores the source order of a block, the cascade level it belongs to, and the /// counter needed to handle Shadow DOM cascade order properly. -#[derive(Clone, Copy, Eq, MallocSizeOf, PartialEq)] -struct ApplicableDeclarationBits(u32); +/// +/// FIXME(emilio): Optimize storage. +#[derive(Clone, Copy, Eq, MallocSizeOf, PartialEq, Debug)] +struct ApplicableDeclarationBits { + source_order: u32, + cascade_level: CascadeLevel, +} impl ApplicableDeclarationBits { - fn new( - source_order: u32, - cascade_level: CascadeLevel, - shadow_cascade_order: ShadowCascadeOrder, - ) -> Self { - debug_assert!( - cascade_level as u8 <= CASCADE_LEVEL_MAX, - "Gotta find more bits!" - ); - let mut bits = ::std::cmp::min(source_order, SOURCE_ORDER_MAX); - bits |= ((shadow_cascade_order & SHADOW_CASCADE_ORDER_MAX) as u32) << - SHADOW_CASCADE_ORDER_SHIFT; - bits |= (cascade_level as u8 as u32) << CASCADE_LEVEL_SHIFT; - ApplicableDeclarationBits(bits) + fn new(source_order: u32, cascade_level: CascadeLevel) -> Self { + Self { source_order, cascade_level } } fn source_order(&self) -> u32 { - (self.0 & SOURCE_ORDER_MASK) >> SOURCE_ORDER_SHIFT - } - - fn shadow_cascade_order(&self) -> ShadowCascadeOrder { - ((self.0 & SHADOW_CASCADE_ORDER_MASK) >> SHADOW_CASCADE_ORDER_SHIFT) as ShadowCascadeOrder + self.source_order } fn level(&self) -> CascadeLevel { - let byte = ((self.0 & CASCADE_LEVEL_MASK) >> CASCADE_LEVEL_SHIFT) as u8; - unsafe { CascadeLevel::from_byte(byte) } - } -} - -impl Debug for ApplicableDeclarationBits { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("ApplicableDeclarationBits") - .field("source_order", &self.source_order()) - .field("shadow_cascade_order", &self.shadow_cascade_order()) - .field("level", &self.level()) - .finish() + self.cascade_level } } @@ -119,7 +70,7 @@ impl ApplicableDeclarationBlock { ) -> Self { ApplicableDeclarationBlock { source: StyleSource::from_declarations(declarations), - bits: ApplicableDeclarationBits::new(0, level, 0), + bits: ApplicableDeclarationBits::new(0, level), specificity: 0, } } @@ -131,11 +82,10 @@ impl ApplicableDeclarationBlock { order: u32, level: CascadeLevel, specificity: u32, - shadow_cascade_order: ShadowCascadeOrder, ) -> Self { ApplicableDeclarationBlock { source, - bits: ApplicableDeclarationBits::new(order, level, shadow_cascade_order), + bits: ApplicableDeclarationBits::new(order, level), specificity, } } @@ -155,9 +105,8 @@ impl ApplicableDeclarationBlock { /// Convenience method to consume self and return the right thing for the /// rule tree to iterate over. #[inline] - pub fn for_rule_tree(self) -> (StyleSource, CascadeLevel, ShadowCascadeOrder) { + pub fn for_rule_tree(self) -> (StyleSource, CascadeLevel) { let level = self.level(); - let cascade_order = self.bits.shadow_cascade_order(); - (self.source, level, cascade_order) + (self.source, level) } } diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index aa849ceee02..8d05dd9bbb1 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -140,6 +140,12 @@ impl PseudoElement { *self == PseudoElement::FieldsetContent } + /// Whether this pseudo-element is the ::-moz-color-swatch pseudo. + #[inline] + pub fn is_color_swatch(&self) -> bool { + *self == PseudoElement::MozColorSwatch + } + /// Whether this pseudo-element is lazily-cascaded. #[inline] pub fn is_lazy(&self) -> bool { diff --git a/components/style/matching.rs b/components/style/matching.rs index 958ec00f1d9..01246ed3878 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -137,12 +137,12 @@ trait PrivateMatchMethods: TElement { if replacements.contains(RestyleHint::RESTYLE_STYLE_ATTRIBUTE) { let style_attribute = self.style_attribute(); result |= replace_rule_node( - CascadeLevel::StyleAttributeNormal, + CascadeLevel::same_tree_author_normal(), style_attribute, primary_rules, ); result |= replace_rule_node( - CascadeLevel::StyleAttributeImportant, + CascadeLevel::same_tree_author_important(), style_attribute, primary_rules, ); diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 5266ed78356..5dc73b9abc1 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -346,16 +346,10 @@ fn should_ignore_declaration_when_ignoring_document_colors( return false; } - let is_style_attribute = matches!( - cascade_level, - CascadeLevel::StyleAttributeNormal | CascadeLevel::StyleAttributeImportant - ); - - // Don't override colors on pseudo-element's style attributes. The - // background-color on ::-moz-color-swatch is an example. Those are set - // as an author style (via the style attribute), but it's pretty - // important for it to show up for obvious reasons :) - if pseudo.is_some() && is_style_attribute { + // Don't override background-color on ::-moz-color-swatch. It is set as an + // author style (via the style attribute), but it's pretty important for it + // to show up for obvious reasons :) + if pseudo.map_or(false, |p| p.is_color_swatch()) && longhand_id == LonghandId::BackgroundColor { return false; } diff --git a/components/style/rule_collector.rs b/components/style/rule_collector.rs index dd50d846715..031a92ca728 100644 --- a/components/style/rule_collector.rs +++ b/components/style/rule_collector.rs @@ -79,7 +79,6 @@ where rules: &'a mut ApplicableDeclarationList, context: &'a mut MatchingContext<'b, E::Impl>, flags_setter: &'a mut F, - shadow_cascade_order: ShadowCascadeOrder, matches_user_and_author_rules: bool, matches_document_author_rules: bool, } @@ -132,7 +131,6 @@ where context, flags_setter, rules, - shadow_cascade_order: 0, matches_user_and_author_rules, matches_document_author_rules: matches_user_and_author_rules, } @@ -142,7 +140,7 @@ where let cascade_level = match origin { Origin::UserAgent => CascadeLevel::UANormal, Origin::User => CascadeLevel::UserNormal, - Origin::Author => CascadeLevel::SameTreeAuthorNormal, + Origin::Author => CascadeLevel::same_tree_author_normal(), }; let cascade_data = self.stylist.cascade_data().borrow_for_origin(origin); @@ -198,7 +196,6 @@ where ) { debug_assert!(shadow_host.shadow_root().is_some()); self.collect_rules_internal(Some(shadow_host), map, cascade_level); - self.shadow_cascade_order += 1; } #[inline] @@ -212,7 +209,6 @@ where let rule_hash_target = self.rule_hash_target; let rules = &mut self.rules; let flags_setter = &mut self.flags_setter; - let shadow_cascade_order = self.shadow_cascade_order; let start = rules.len(); self.context.with_shadow_host(shadow_host, |context| { map.get_all_matching_rules( @@ -222,16 +218,18 @@ where context, flags_setter, cascade_level, - shadow_cascade_order, ); }); sort_rules_from(rules, start); } - /// Collects the rules for the ::slotted pseudo-element. - fn collect_slotted_rules(&mut self) { + /// Collects the rules for the ::slotted pseudo-element and the :host + /// pseudo-class. + fn collect_host_and_slotted_rules(&mut self) { let mut slots = SmallVec::<[_; 3]>::new(); let mut current = self.rule_hash_target.assigned_slot(); + let mut shadow_cascade_order = ShadowCascadeOrder::for_outermost_shadow_tree(); + while let Some(slot) = current { debug_assert!( self.matches_user_and_author_rules, @@ -239,11 +237,16 @@ where ); slots.push(slot); current = slot.assigned_slot(); + shadow_cascade_order.dec(); } + self.collect_host_rules(shadow_cascade_order); + // Match slotted rules in reverse order, so that the outer slotted rules // come before the inner rules (and thus have less priority). for slot in slots.iter().rev() { + shadow_cascade_order.inc(); + let shadow = slot.containing_shadow().unwrap(); let data = match shadow.style_data() { Some(d) => d, @@ -253,10 +256,11 @@ where Some(r) => r, None => continue, }; + self.collect_rules_in_shadow_tree( shadow.host(), slotted_rules, - CascadeLevel::InnerShadowNormal, + CascadeLevel::AuthorNormal { shadow_cascade_order }, ); } } @@ -277,12 +281,12 @@ where let cascade_data = containing_shadow.style_data(); let host = containing_shadow.host(); if let Some(map) = cascade_data.and_then(|data| data.normal_rules(self.pseudo_element)) { - self.collect_rules_in_shadow_tree(host, map, CascadeLevel::SameTreeAuthorNormal); + self.collect_rules_in_shadow_tree(host, map, CascadeLevel::same_tree_author_normal()); } } /// Collects the rules for the :host pseudo-class. - fn collect_host_rules(&mut self) { + fn collect_host_rules(&mut self, shadow_cascade_order: ShadowCascadeOrder) { let shadow = match self.rule_hash_target.shadow_root() { Some(s) => s, None => return, @@ -307,7 +311,7 @@ where self.collect_rules_in_shadow_tree( rule_hash_target, host_rules, - CascadeLevel::InnerShadowNormal, + CascadeLevel::AuthorNormal { shadow_cascade_order }, ); } @@ -342,21 +346,18 @@ where .part_rules(self.pseudo_element), }; - // TODO(emilio): SameTreeAuthorNormal is a bit of a lie here, we may - // need an OuterTreeAuthorNormal cascade level or such, and change the - // cascade order, if we allow to forward parts to even outer trees. - // - // Though the current thing kinda works because we apply them after - // the outer tree, so as long as we don't allow forwarding we're - // good. + // TODO(emilio): Cascade order will need to increment for each tree when + // we implement forwarding. + let shadow_cascade_order = ShadowCascadeOrder::for_innermost_containing_tree(); if let Some(part_rules) = part_rules { let containing_host = containing_shadow.map(|s| s.host()); let element = self.element; let rule_hash_target = self.rule_hash_target; let rules = &mut self.rules; let flags_setter = &mut self.flags_setter; - let shadow_cascade_order = self.shadow_cascade_order; - let cascade_level = CascadeLevel::SameTreeAuthorNormal; + let cascade_level = CascadeLevel::AuthorNormal { + shadow_cascade_order, + }; let start = rules.len(); self.context.with_shadow_host(containing_host, |context| { rule_hash_target.each_part(|p| { @@ -368,7 +369,6 @@ where context, flags_setter, cascade_level, - shadow_cascade_order, ); } }); @@ -382,7 +382,7 @@ where self.rules .push(ApplicableDeclarationBlock::from_declarations( sa.clone_arc(), - CascadeLevel::StyleAttributeNormal, + CascadeLevel::same_tree_author_normal(), )); } } @@ -433,12 +433,11 @@ where if self.stylist.author_styles_enabled() == AuthorStylesEnabled::No { return; } - self.collect_host_rules(); - self.collect_slotted_rules(); + self.collect_host_and_slotted_rules(); self.collect_normal_rules_from_containing_shadow_tree(); self.collect_document_author_rules(); - self.collect_part_rules(); self.collect_style_attribute(); + self.collect_part_rules(); self.collect_animation_rules(); } } diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 02ebeed0452..304a539744f 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -169,15 +169,60 @@ const FREE_LIST_SENTINEL: *mut RuleNode = 0x01 as *mut RuleNode; /// another thread is currently adding an entry). We spin if we find this value. const FREE_LIST_LOCKED: *mut RuleNode = 0x02 as *mut RuleNode; -/// A counter to track how many inner shadow roots rules deep we are. -/// -/// This is used to handle: +/// A counter to track how many shadow root rules deep we are. This is used to +/// handle: /// /// https://drafts.csswg.org/css-scoping/#shadow-cascading /// -/// In particular, it'd be `0` for the innermost shadow host, `1` for the next, -/// and so on. -pub type ShadowCascadeOrder = u8; +/// See the static functions for the meaning of different values. +#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, Ord, PartialEq, PartialOrd)] +pub struct ShadowCascadeOrder(i8); + +impl ShadowCascadeOrder { + /// A level for the outermost shadow tree (the shadow tree we own, and the + /// ones from the slots we're slotted in). + #[inline] + pub fn for_outermost_shadow_tree() -> Self { + Self(-1) + } + + /// A level for the element's tree. + #[inline] + fn for_same_tree() -> Self { + Self(0) + } + + /// A level for the innermost containing tree (the one closest to the + /// element). + #[inline] + pub fn for_innermost_containing_tree() -> Self { + Self(1) + } + + /// Decrement the level, moving inwards. We should only move inwards if + /// we're traversing slots. + #[inline] + pub fn dec(&mut self) { + debug_assert!(self.0 < 0); + self.0 = self.0.saturating_sub(1); + } + + /// The level, moving inwards. We should only move inwards if we're + /// traversing slots. + #[inline] + pub fn inc(&mut self) { + debug_assert_ne!(self.0, -1); + self.0 = self.0.saturating_add(1); + } +} + +impl std::ops::Neg for ShadowCascadeOrder { + type Output = Self; + #[inline] + fn neg(self) -> Self { + Self(self.0.neg()) + } +} impl RuleTree { /// Construct a new rule tree. @@ -215,26 +260,20 @@ impl RuleTree { guards: &StylesheetGuards, ) -> StrongRuleNode where - I: Iterator, + I: Iterator, { use self::CascadeLevel::*; let mut current = self.root.clone(); - let mut last_level = current.get().level; let mut found_important = false; - let mut important_style_attr = None; - let mut important_same_tree = SmallVec::<[StyleSource; 4]>::new(); - let mut important_inner_shadow = SmallVec::<[SmallVec<[StyleSource; 4]>; 4]>::new(); - important_inner_shadow.push(SmallVec::new()); + let mut important_author = SmallVec::<[(StyleSource, ShadowCascadeOrder); 4]>::new(); let mut important_user = SmallVec::<[StyleSource; 4]>::new(); let mut important_ua = SmallVec::<[StyleSource; 4]>::new(); let mut transition = None; - let mut last_cascade_order = 0; - for (source, level, shadow_cascade_order) in iter { - debug_assert!(level >= last_level, "Not really ordered"); + for (source, level) in iter { debug_assert!(!level.is_important(), "Important levels handled internally"); let any_important = { let pdb = source.read(level.guard(guards)); @@ -244,29 +283,11 @@ impl RuleTree { if any_important { found_important = true; match level { - InnerShadowNormal => { - debug_assert!( - shadow_cascade_order >= last_cascade_order, - "Not really ordered" - ); - if shadow_cascade_order > last_cascade_order && - !important_inner_shadow.last().unwrap().is_empty() - { - last_cascade_order = shadow_cascade_order; - important_inner_shadow.push(SmallVec::new()); - } - important_inner_shadow - .last_mut() - .unwrap() - .push(source.clone()) + AuthorNormal { shadow_cascade_order } => { + important_author.push((source.clone(), shadow_cascade_order)); }, - SameTreeAuthorNormal => important_same_tree.push(source.clone()), UANormal => important_ua.push(source.clone()), UserNormal => important_user.push(source.clone()), - StyleAttributeNormal => { - debug_assert!(important_style_attr.is_none()); - important_style_attr = Some(source.clone()); - }, _ => {}, }; } @@ -290,7 +311,6 @@ impl RuleTree { } else { current = current.ensure_child(self.root.downgrade(), source, level); } - last_level = level; } // Early-return in the common case of no !important declarations. @@ -298,23 +318,35 @@ impl RuleTree { return current; } - // // Insert important declarations, in order of increasing importance, // followed by any transition rule. // - - for source in important_same_tree.drain() { - current = current.ensure_child(self.root.downgrade(), source, SameTreeAuthorImportant); + // Inner shadow wins over same-tree, which wins over outer-shadow. + // + // We negate the shadow cascade order to preserve the right PartialOrd + // behavior. + if !important_author.is_empty() && + important_author.first().unwrap().1 != important_author.last().unwrap().1 + { + // We only need to sort if the important rules come from + // different trees, but we need this sort to be stable. + // + // FIXME(emilio): This could maybe be smarter, probably by chunking + // the important rules while inserting, and iterating the outer + // chunks in reverse order. + // + // That is, if we have rules with levels like: -1 -1 -1 0 0 0 1 1 1, + // we're really only sorting the chunks, while keeping elements + // inside the same chunk already sorted. Seems like we could try to + // keep a SmallVec-of-SmallVecs with the chunks and just iterate the + // outer in reverse. + important_author.sort_by_key(|&(_, order)| -order); } - if let Some(source) = important_style_attr { - current = current.ensure_child(self.root.downgrade(), source, StyleAttributeImportant); - } - - for mut list in important_inner_shadow.drain().rev() { - for source in list.drain() { - current = current.ensure_child(self.root.downgrade(), source, InnerShadowImportant); - } + for (source, shadow_cascade_order) in important_author.drain() { + current = current.ensure_child(self.root.downgrade(), source, AuthorImportant { + shadow_cascade_order: -shadow_cascade_order, + }); } for source in important_user.drain() { @@ -359,11 +391,8 @@ impl RuleTree { I: Iterator, { let mut current = from; - let mut last_level = current.get().level; for (source, level) in iter { - debug_assert!(last_level <= level, "Not really ordered"); current = current.ensure_child(self.root.downgrade(), source, level); - last_level = level; } current } @@ -439,7 +468,6 @@ impl RuleTree { guards: &StylesheetGuards, important_rules_changed: &mut bool, ) -> Option { - debug_assert!(level.is_unique_per_element()); // TODO(emilio): Being smarter with lifetimes we could avoid a bit of // the refcount churn. let mut current = path.clone(); @@ -468,7 +496,16 @@ impl RuleTree { if current.get().level == level { *important_rules_changed |= level.is_important(); - if let Some(pdb) = pdb { + let current_decls = current + .get() + .source + .as_ref() + .unwrap() + .as_declarations(); + + // If the only rule at the level we're replacing is exactly the + // same as `pdb`, we're done, and `path` is still valid. + if let (Some(ref pdb), Some(ref current_decls)) = (pdb, current_decls) { // If the only rule at the level we're replacing is exactly the // same as `pdb`, we're done, and `path` is still valid. // @@ -478,25 +515,17 @@ impl RuleTree { // also equally valid. This is less likely, and would require an // in-place mutation of the source, which is, at best, fiddly, // so let's skip it for now. - let current_decls = current - .get() - .source - .as_ref() - .unwrap() - .as_declarations() - .expect("Replacing non-declarations style?"); - let is_here_already = ArcBorrow::ptr_eq(&pdb, ¤t_decls); + let is_here_already = ArcBorrow::ptr_eq(pdb, current_decls); if is_here_already { debug!("Picking the fast path in rule replacement"); return None; } } - current = current.parent().unwrap().clone(); + + if current_decls.is_some() { + current = current.parent().unwrap().clone(); + } } - debug_assert!( - current.get().level != level, - "Multiple rules should've been replaced?" - ); // Insert the rule if it's relevant at this level in the cascade. // @@ -611,46 +640,42 @@ const RULE_TREE_GC_INTERVAL: usize = 300; /// tree may affect an element connected to the document or an "outer" shadow /// tree. /// -/// We need to differentiate between rules from the same tree and "inner" shadow -/// trees in order to be able to find the right position for the style attribute -/// easily. Otherwise we wouldn't be able to avoid selector-matching when a -/// style attribute is added or removed. -/// /// [1]: https://drafts.csswg.org/css-cascade/#cascade-origin /// [2]: https://drafts.csswg.org/css-cascade/#preshint /// [3]: https://html.spec.whatwg.org/multipage/#presentational-hints /// [4]: https://drafts.csswg.org/css-scoping/#shadow-cascading #[repr(u8)] -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, PartialOrd)] -#[cfg_attr(feature = "servo", derive(MallocSizeOf))] +#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, PartialOrd)] pub enum CascadeLevel { /// Normal User-Agent rules. - UANormal = 0, + UANormal, /// User normal rules. UserNormal, /// Presentational hints. PresHints, - /// Shadow DOM styles from "inner" shadow trees. - /// - /// See above for why this is needed instead of merging InnerShadowNormal, - /// SameTreeAuthorNormal and StyleAttributeNormal inside something like - /// AuthorNormal. - InnerShadowNormal, - /// Author normal rules from the same tree the element is in. - SameTreeAuthorNormal, - /// Style attribute normal rules. - StyleAttributeNormal, + /// Shadow DOM styles from author styles. + AuthorNormal { + /// The order in the shadow tree hierarchy. This number is relative to + /// the tree of the element, and thus the only invariants that need to + /// be preserved is: + /// + /// * Zero is the same tree as the element that matched the rule. This + /// is important so that we can optimize style attribute insertions. + /// + /// * The levels are ordered in accordance with + /// https://drafts.csswg.org/css-scoping/#shadow-cascading + shadow_cascade_order: ShadowCascadeOrder, + }, /// SVG SMIL animations. SMILOverride, /// CSS animations and script-generated animations. Animations, - /// Author-supplied important rules from the same tree the element came - /// from. - SameTreeAuthorImportant, - /// Style attribute important rules. - StyleAttributeImportant, - /// Shadow DOM important rules. - InnerShadowImportant, + /// Author-supplied important rules. + AuthorImportant { + /// The order in the shadow tree hierarchy, inverted, so that PartialOrd + /// does the right thing. + shadow_cascade_order: ShadowCascadeOrder, + }, /// User important rules. UserImportant, /// User-agent important rules. @@ -662,12 +687,6 @@ pub enum CascadeLevel { } impl CascadeLevel { - /// Converts a raw byte to a CascadeLevel. - pub unsafe fn from_byte(byte: u8) -> Self { - debug_assert!(byte <= CascadeLevel::Transitions as u8); - mem::transmute(byte) - } - /// Select a lock guard for this level pub fn guard<'a>(&self, guards: &'a StylesheetGuards<'a>) -> &'a SharedRwLockReadGuard<'a> { match *self { @@ -679,16 +698,21 @@ impl CascadeLevel { } } - /// Returns whether this cascade level is unique per element, in which case - /// we can replace the path in the cascade without fear. - pub fn is_unique_per_element(&self) -> bool { - match *self { - CascadeLevel::Transitions | - CascadeLevel::Animations | - CascadeLevel::SMILOverride | - CascadeLevel::StyleAttributeNormal | - CascadeLevel::StyleAttributeImportant => true, - _ => false, + /// Returns the cascade level for author important declarations from the + /// same tree as the element. + #[inline] + pub fn same_tree_author_important() -> Self { + CascadeLevel::AuthorImportant { + shadow_cascade_order: ShadowCascadeOrder::for_same_tree(), + } + } + + /// Returns the cascade level for author normal declarations from the same + /// tree as the element. + #[inline] + pub fn same_tree_author_normal() -> Self { + CascadeLevel::AuthorNormal { + shadow_cascade_order: ShadowCascadeOrder::for_same_tree(), } } @@ -697,9 +721,7 @@ impl CascadeLevel { #[inline] pub fn is_important(&self) -> bool { match *self { - CascadeLevel::SameTreeAuthorImportant | - CascadeLevel::InnerShadowImportant | - CascadeLevel::StyleAttributeImportant | + CascadeLevel::AuthorImportant { .. } | CascadeLevel::UserImportant | CascadeLevel::UAImportant => true, _ => false, @@ -724,14 +746,10 @@ impl CascadeLevel { CascadeLevel::UAImportant | CascadeLevel::UANormal => Origin::UserAgent, CascadeLevel::UserImportant | CascadeLevel::UserNormal => Origin::User, CascadeLevel::PresHints | - CascadeLevel::InnerShadowNormal | - CascadeLevel::SameTreeAuthorNormal | - CascadeLevel::StyleAttributeNormal | + CascadeLevel::AuthorNormal { .. } | + CascadeLevel::AuthorImportant { .. } | CascadeLevel::SMILOverride | CascadeLevel::Animations | - CascadeLevel::SameTreeAuthorImportant | - CascadeLevel::StyleAttributeImportant | - CascadeLevel::InnerShadowImportant | CascadeLevel::Transitions => Origin::Author, } } @@ -1146,6 +1164,15 @@ impl StrongRuleNode { ) -> StrongRuleNode { use parking_lot::RwLockUpgradableReadGuard; + debug_assert!( + self.get().level <= level, + "Should be ordered (instead {:?} > {:?}), from {:?} and {:?}", + self.get().level, + level, + self.get().source, + source, + ); + let key = ChildKey(level, source.key()); let read_guard = self.get().children.upgradable_read(); @@ -1448,55 +1475,40 @@ impl StrongRuleNode { } }); - match node.cascade_level() { - // Non-author rules: - CascadeLevel::UANormal | - CascadeLevel::UAImportant | - CascadeLevel::UserNormal | - CascadeLevel::UserImportant => { - for (id, declaration) in longhands { - if properties.contains(id) { - // This property was set by a non-author rule. - // Stop looking for it in this element's rule - // nodes. - properties.remove(id); + let is_author = node.cascade_level().origin() == Origin::Author; + for (id, declaration) in longhands { + if !properties.contains(id) { + continue; + } - // However, if it is inherited, then it might be - // inherited from an author rule from an - // ancestor element's rule nodes. - if declaration.get_css_wide_keyword() == - Some(CSSWideKeyword::Inherit) - { - have_explicit_ua_inherit = true; - inherited_properties.insert(id); - } + if is_author { + if !author_colors_allowed { + // FIXME(emilio): this looks wrong, this should + // do: if color is not transparent, then return + // true, or something. + if let PropertyDeclaration::BackgroundColor(ref color) = + *declaration + { + return *color == Color::transparent(); } } - }, - // Author rules: - CascadeLevel::PresHints | - CascadeLevel::SameTreeAuthorNormal | - CascadeLevel::InnerShadowNormal | - CascadeLevel::StyleAttributeNormal | - CascadeLevel::SMILOverride | - CascadeLevel::Animations | - CascadeLevel::SameTreeAuthorImportant | - CascadeLevel::InnerShadowImportant | - CascadeLevel::StyleAttributeImportant | - CascadeLevel::Transitions => { - for (id, declaration) in longhands { - if properties.contains(id) { - if !author_colors_allowed { - if let PropertyDeclaration::BackgroundColor(ref color) = - *declaration - { - return *color == Color::transparent(); - } - } - return true; - } - } - }, + return true; + } + + // This property was set by a non-author rule. + // Stop looking for it in this element's rule + // nodes. + properties.remove(id); + + // However, if it is inherited, then it might be + // inherited from an author rule from an + // ancestor element's rule nodes. + if declaration.get_css_wide_keyword() == + Some(CSSWideKeyword::Inherit) + { + have_explicit_ua_inherit = true; + inherited_properties.insert(id); + } } } diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 52b939e0219..81144c2a4bf 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -10,7 +10,7 @@ use crate::context::QuirksMode; use crate::dom::TElement; use crate::hash::map as hash_map; use crate::hash::{HashMap, HashSet}; -use crate::rule_tree::{CascadeLevel, ShadowCascadeOrder}; +use crate::rule_tree::CascadeLevel; use crate::selector_parser::SelectorImpl; use crate::stylist::Rule; use crate::{Atom, LocalName, Namespace, WeakAtom}; @@ -171,7 +171,6 @@ impl SelectorMap { context: &mut MatchingContext, flags_setter: &mut F, cascade_level: CascadeLevel, - shadow_cascade_order: ShadowCascadeOrder, ) where E: TElement, F: FnMut(&E, ElementSelectorFlags), @@ -190,7 +189,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ); } @@ -203,7 +201,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ) } } @@ -217,7 +214,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ) } }); @@ -230,7 +226,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ) } @@ -242,7 +237,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ) } @@ -253,7 +247,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ); } @@ -265,7 +258,6 @@ impl SelectorMap { context: &mut MatchingContext, flags_setter: &mut F, cascade_level: CascadeLevel, - shadow_cascade_order: ShadowCascadeOrder, ) where E: TElement, F: FnMut(&E, ElementSelectorFlags), @@ -280,7 +272,7 @@ impl SelectorMap { flags_setter, ) { matching_rules.push( - rule.to_applicable_declaration_block(cascade_level, shadow_cascade_order), + rule.to_applicable_declaration_block(cascade_level), ); } } diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index 4fdd4d4ab37..72c47e65dda 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -173,6 +173,12 @@ impl PseudoElement { false } + /// Whether this pseudo-element is the ::-moz-color-swatch pseudo. + #[inline] + pub fn is_color_swatch(&self) -> bool { + false + } + /// Whether this pseudo-element is eagerly-cascaded. #[inline] pub fn is_eager(&self) -> bool { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index ab6fe309f3f..414210b7d20 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -18,7 +18,7 @@ use crate::properties::{self, CascadeMode, ComputedValues}; use crate::properties::{AnimationRules, PropertyDeclarationBlock}; use crate::rule_cache::{RuleCache, RuleCacheConditions}; use crate::rule_collector::{containing_shadow_ignoring_svg_use, RuleCollector}; -use crate::rule_tree::{CascadeLevel, RuleTree, ShadowCascadeOrder, StrongRuleNode, StyleSource}; +use crate::rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource}; use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, SelectorMap, SelectorMapEntry}; use crate::selector_parser::{PerPseudoElementMap, PseudoElement, SelectorImpl, SnapshotMap}; use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; @@ -1324,7 +1324,7 @@ impl Stylist { .declaration_importance_iter() .map(|(declaration, importance)| { debug_assert!(!importance.important()); - (declaration, CascadeLevel::StyleAttributeNormal) + (declaration, CascadeLevel::same_tree_author_normal()) }) }; @@ -1987,7 +1987,6 @@ impl CascadeData { self.rules_source_order, CascadeLevel::UANormal, selector.specificity(), - 0, )); continue; } @@ -2323,7 +2322,6 @@ impl Rule { pub fn to_applicable_declaration_block( &self, level: CascadeLevel, - shadow_cascade_order: ShadowCascadeOrder, ) -> ApplicableDeclarationBlock { let source = StyleSource::from_rule(self.style_rule.clone()); ApplicableDeclarationBlock::new( @@ -2331,7 +2329,6 @@ impl Rule { self.source_order, level, self.specificity(), - shadow_cascade_order, ) }