diff options
author | David Shin <dshin@mozilla.com> | 2023-05-25 14:35:18 +0000 |
---|---|---|
committer | Martin Robinson <mrobinson@igalia.com> | 2023-11-24 08:57:14 +0100 |
commit | ff8100d3963ac88fa0001bec40472134dbb4394b (patch) | |
tree | 7f6485012d046f07afd2cc7eda7f8b44b8a214c6 /components/style | |
parent | 5c0897c8eb6ef789dd14918b9b427935a244a847 (diff) | |
download | servo-ff8100d3963ac88fa0001bec40472134dbb4394b.tar.gz servo-ff8100d3963ac88fa0001bec40472134dbb4394b.zip |
style: Correct style sharing handling for any element that considered `:has()` in selector matching
For any element that anchors a `:has()` selector (i.e. Matches a selector that
contains a `:has()` on its rightmost side), we prevent style sharing altogether,
as evaluation of the `:has()` selector is required in the first place to
determine style sharing.
On the other hand, any element matching a rule containing `:has()` without
anchoring it can do style sharing for siblings, but not cousins.
Differential Revision: https://phabricator.services.mozilla.com/D176836
Diffstat (limited to 'components/style')
-rw-r--r-- | components/style/dom.rs | 4 | ||||
-rw-r--r-- | components/style/gecko/wrapper.rs | 8 | ||||
-rw-r--r-- | components/style/properties/computed_value_flags.rs | 5 | ||||
-rw-r--r-- | components/style/sharing/checks.rs | 9 | ||||
-rw-r--r-- | components/style/sharing/mod.rs | 37 | ||||
-rw-r--r-- | components/style/style_resolver.rs | 10 | ||||
-rw-r--r-- | components/style/traversal.rs | 8 |
7 files changed, 76 insertions, 5 deletions
diff --git a/components/style/dom.rs b/components/style/dom.rs index 68c8460bfd7..9f9a7af7b78 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -894,6 +894,10 @@ pub trait TElement: /// This will usually be the size of the content area of the primary box, /// but can be None if there is no box or if some axis lacks size containment. fn query_container_size(&self, display: &Display) -> euclid::default::Size2D<Option<app_units::Au>>; + + /// Returns true if this element anchors a relative selector, now or after + /// a DOM mutation. + fn anchors_relative_selector(&self) -> bool; } /// TNode and TElement aren't Send because we want to be careful and explicit diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index ac4f113c93e..eb3ab03c73c 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -895,6 +895,9 @@ fn selector_flags_to_node_flags(flags: ElementSelectorFlags) -> u32 { if flags.contains(ElementSelectorFlags::HAS_EMPTY_SELECTOR) { gecko_flags |= NODE_HAS_EMPTY_SELECTOR; } + if flags.contains(ElementSelectorFlags::ANCHORS_RELATIVE_SELECTOR) { + gecko_flags |= NODE_ANCHORS_RELATIVE_SELECTOR; + } gecko_flags } @@ -1729,6 +1732,11 @@ impl<'le> TElement for GeckoElement<'le> { } } } + + fn anchors_relative_selector(&self) -> bool { + use crate::gecko_bindings::structs::NODE_ANCHORS_RELATIVE_SELECTOR; + self.flags() & NODE_ANCHORS_RELATIVE_SELECTOR != 0 + } } impl<'le> PartialEq for GeckoElement<'le> { diff --git a/components/style/properties/computed_value_flags.rs b/components/style/properties/computed_value_flags.rs index 9b834efc222..d84311de084 100644 --- a/components/style/properties/computed_value_flags.rs +++ b/components/style/properties/computed_value_flags.rs @@ -118,6 +118,9 @@ bitflags! { /// A flag used to mark styles which have `container-type` of `size` or /// `inline-size`, or under one. const SELF_OR_ANCESTOR_HAS_SIZE_CONTAINER_TYPE = 1 << 23; + + /// Whether the style evaluated any relative selector. + const CONSIDERED_RELATIVE_SELECTOR = 1 << 24; } } @@ -150,7 +153,7 @@ impl ComputedValueFlags { /// Flags that are an input to the cascade. #[inline] fn cascade_input_flags() -> Self { - Self::USES_VIEWPORT_UNITS_ON_CONTAINER_QUERIES + Self::USES_VIEWPORT_UNITS_ON_CONTAINER_QUERIES | Self::CONSIDERED_RELATIVE_SELECTOR } /// Returns the flags that are always propagated to descendants. diff --git a/components/style/sharing/checks.rs b/components/style/sharing/checks.rs index 42c7110dce8..a691ac5c764 100644 --- a/components/style/sharing/checks.rs +++ b/components/style/sharing/checks.rs @@ -36,6 +36,15 @@ where // Cousins are a bit more complicated. // + // The fact that the candidate is here means that its element does not anchor + // the relative selector. However, it may have considered relative selector(s) + // to compute its style, i.e. there's a rule `<..> :has(<..>) <..> candidate`. + // In this case, evaluating style sharing requires evaluating the relative + // selector for the target anyway. + if candidate.considered_relative_selector { + return false; + } + // If a parent element was already styled and we traversed past it without // restyling it, that may be because our clever invalidation logic was able // to prove that the styles of that element would remain unchanged despite diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index dcc92de7f42..e376a6da412 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -67,6 +67,7 @@ use crate::applicable_declarations::ApplicableDeclarationBlock; use crate::bloom::StyleBloom; use crate::context::{SharedStyleContext, StyleContext}; +use crate::computed_value_flags::ComputedValueFlags; use crate::dom::{SendElement, TElement}; use crate::properties::ComputedValues; use crate::rule_tree::StrongRuleNode; @@ -275,11 +276,13 @@ pub struct StyleSharingCandidate<E: TElement> { /// The element. element: E, validation_data: ValidationData, + considered_relative_selector: bool, } struct FakeCandidate { _element: usize, _validation_data: ValidationData, + _considered_relative_selector: bool, } impl<E: TElement> Deref for StyleSharingCandidate<E> { @@ -466,13 +469,19 @@ impl<Candidate> SharingCacheBase<Candidate> { } impl<E: TElement> SharingCache<E> { - fn insert(&mut self, element: E, validation_data_holder: Option<&mut StyleSharingTarget<E>>) { + fn insert( + &mut self, + element: E, + considered_relative_selector: bool, + validation_data_holder: Option<&mut StyleSharingTarget<E>>, + ) { let validation_data = match validation_data_holder { Some(v) => v.take_validation_data(), None => ValidationData::default(), }; self.entries.insert(StyleSharingCandidate { element, + considered_relative_selector, validation_data, }); } @@ -612,6 +621,22 @@ impl<E: TElement> StyleSharingCache<E> { return; } + // If this element was considered for matching a relative selector, we can't + // share style, as that requires evaluating the relative selector in the + // first place. A couple notes: + // - This means that a document that contains a standalone `:has()` + // rule would basically turn style sharing off. + // - Since the flag is set on the element, we may be overly pessimistic: + // For example, given `<div class="foo"><div class="bar"></div></div>`, + // if we run into a `.foo:has(.bar) .baz` selector, we refuse any selector + // matching `.foo`, even if `:has()` may not even be used. Ideally we'd + // have something like `RelativeSelectorConsidered::RightMost`, but the + // element flag is required for invalidation, and this reduces more tracking. + if element.anchors_relative_selector() { + debug!("Failing to insert to the cache: may anchor relative selector"); + return; + } + // In addition to the above running animations check, we also need to // check CSS animation and transition styles since it's possible that // we are about to create CSS animations/transitions. @@ -642,7 +667,15 @@ impl<E: TElement> StyleSharingCache<E> { self.clear(); self.dom_depth = dom_depth; } - self.cache_mut().insert(*element, validation_data_holder); + self.cache_mut().insert( + *element, + style + .style + .0 + .flags + .intersects(ComputedValueFlags::CONSIDERED_RELATIVE_SELECTOR), + validation_data_holder, + ); } /// Clear the style sharing candidate cache. diff --git a/components/style/style_resolver.rs b/components/style/style_resolver.rs index ee69329e1a9..bd120159a0d 100644 --- a/components/style/style_resolver.rs +++ b/components/style/style_resolver.rs @@ -499,6 +499,16 @@ where } } + if matching_context.considered_relative_selector { + // This is a bit awkward - ideally, the flag is set directly where `considered_relative_selector` + // is; however, in that context, the implementation detail of `extra_data` is not visible, so + // it's done here. A trait for manipulating the flags is an option, but not worth it for a single flag. + matching_context + .extra_data + .cascade_input_flags + .insert(ComputedValueFlags::CONSIDERED_RELATIVE_SELECTOR); + } + MatchingResults { rule_node, flags: matching_context.extra_data.cascade_input_flags, diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 5102a5d1751..19329d9f517 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -222,7 +222,10 @@ pub trait DomTraversal<E: TElement>: Sync { if traversal_flags.for_animation_only() { return data.map_or(false, |d| d.has_styles()) && (el.has_animation_only_dirty_descendants() || - data.as_ref().unwrap().hint.has_animation_hint_or_recascade()); + data.as_ref() + .unwrap() + .hint + .has_animation_hint_or_recascade()); } // Non-incremental layout visits every node. @@ -420,7 +423,8 @@ pub fn recalc_style_at<E, D, F>( // Compute style for this element if necessary. if let Some(restyle_kind) = restyle_kind { - child_restyle_requirement = compute_style(traversal_data, context, element, data, restyle_kind); + child_restyle_requirement = + compute_style(traversal_data, context, element, data, restyle_kind); if !element.matches_user_and_content_rules() { // We must always cascade native anonymous subtrees, since they |