Skip to content

Commit

Permalink
Bug 1876962: Part 3 - Remove special case handling for :has in styl…
Browse files Browse the repository at this point in the history
…e sharing. r=firefox-style-system-reviewers,emilio

Was added in bug 1793012. No longer needed since `:has` is part of
reinvalidation selectors now. The new approach can be potentially-
pricey, but is a lot simpler.

Differential Revision: https://phabricator.services.mozilla.com/D200224
  • Loading branch information
dshin-moz committed Feb 12, 2024
1 parent f151ebf commit 7290a1e
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 61 deletions.
11 changes: 0 additions & 11 deletions servo/components/style/sharing/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,6 @@ where
return true;
}

// 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
Expand Down
30 changes: 0 additions & 30 deletions servo/components/style/sharing/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,13 +314,11 @@ 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> {
Expand Down Expand Up @@ -510,7 +508,6 @@ impl<E: TElement> SharingCache<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 {
Expand All @@ -519,7 +516,6 @@ impl<E: TElement> SharingCache<E> {
};
self.entries.insert(StyleSharingCandidate {
element,
considered_relative_selector,
validation_data,
});
}
Expand Down Expand Up @@ -659,27 +655,6 @@ 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 style
.style
.0
.flags
.intersects(ComputedValueFlags::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.
Expand Down Expand Up @@ -712,11 +687,6 @@ impl<E: TElement> StyleSharingCache<E> {
}
self.cache_mut().insert(
*element,
style
.style
.0
.flags
.intersects(ComputedValueFlags::CONSIDERED_RELATIVE_SELECTOR),
validation_data_holder,
);
}
Expand Down
21 changes: 1 addition & 20 deletions servo/components/style/style_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ use crate::selector_parser::{PseudoElement, SelectorImpl};
use crate::stylist::RuleInclusion;
use log::Level::Trace;
use selectors::matching::{
MatchingContext, MatchingForInvalidation, MatchingMode, NeedsSelectorFlags,
RelativeSelectorMatchingState, VisitedHandlingMode,
MatchingContext, MatchingForInvalidation, MatchingMode, NeedsSelectorFlags, VisitedHandlingMode,
};
use servo_arc::Arc;

Expand Down Expand Up @@ -509,24 +508,6 @@ where
}
}
}
// 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.
match matching_context.considered_relative_selector {
RelativeSelectorMatchingState::None => (),
RelativeSelectorMatchingState::Considered => {
matching_context
.extra_data
.cascade_input_flags
.insert(ComputedValueFlags::CONSIDERED_RELATIVE_SELECTOR);
},
RelativeSelectorMatchingState::ConsideredAnchor => {
matching_context.extra_data.cascade_input_flags.insert(
ComputedValueFlags::ANCHORS_RELATIVE_SELECTOR |
ComputedValueFlags::CONSIDERED_RELATIVE_SELECTOR,
);
},
};

MatchingResults {
rule_node,
Expand Down

0 comments on commit 7290a1e

Please sign in to comment.