style: Make Invalidation work in terms of a dependency, not a selector.

That way we can look at the parent dependency as described in the previous
patch. An alternative would be to add a:

    parent_dependency: Option<&'a Dependency>

on construction to `Invalidation`, but this way seems slightly better to avoid
growing the struct. It's not even one more indirection because the selector is
contained directly in the Dependency struct.

Differential Revision: https://phabricator.services.mozilla.com/D71422
This commit is contained in:
Emilio Cobos Álvarez 2020-04-23 19:20:10 +00:00
parent c1bc588c93
commit 4b5de772c6
5 changed files with 315 additions and 202 deletions

View file

@ -9,6 +9,7 @@ use crate::context::QuirksMode;
use crate::dom::{TDocument, TElement, TNode, TShadowRoot};
use crate::invalidation::element::invalidator::{DescendantInvalidationLists, Invalidation};
use crate::invalidation::element::invalidator::{InvalidationProcessor, InvalidationVector};
use crate::invalidation::element::invalidation_map::Dependency;
use crate::Atom;
use selectors::attr::CaseSensitivity;
use selectors::matching::{self, MatchingContext, MatchingMode};
@ -130,7 +131,7 @@ where
{
results: &'a mut Q::Output,
matching_context: MatchingContext<'a, E::Impl>,
selector_list: &'a SelectorList<E::Impl>,
dependencies: &'a [Dependency],
}
impl<'a, E, Q> InvalidationProcessor<'a, E> for QuerySelectorProcessor<'a, E, Q>
@ -143,6 +144,11 @@ where
true
}
fn check_outer_dependency(&mut self, _: &Dependency, _: E) -> bool {
debug_assert!(false, "How? We should only have parent-less dependencies here!");
true
}
fn collect_invalidations(
&mut self,
element: E,
@ -171,11 +177,10 @@ where
self_invalidations
};
for selector in self.selector_list.0.iter() {
for dependency in self.dependencies.iter() {
target_vector.push(Invalidation::new(
selector,
dependency,
self.matching_context.current_host.clone(),
0,
))
}
@ -642,10 +647,13 @@ pub fn query_selector<E, Q>(
if root_element.is_some() || !invalidation_may_be_useful {
query_selector_slow::<E, Q>(root, selector_list, results, &mut matching_context);
} else {
let dependencies = selector_list.0.iter().map(|selector| {
Dependency::for_full_selector_invalidation(selector.clone())
}).collect::<SmallVec<[_; 5]>>();
let mut processor = QuerySelectorProcessor::<E, Q> {
results,
matching_context,
selector_list,
dependencies: &dependencies,
};
for node in root.dom_children() {

View file

@ -6,6 +6,7 @@
use crate::dom::TElement;
use crate::element_state::DocumentState;
use crate::invalidation::element::invalidation_map::Dependency;
use crate::invalidation::element::invalidator::{DescendantInvalidationLists, InvalidationVector};
use crate::invalidation::element::invalidator::{Invalidation, InvalidationProcessor};
use crate::invalidation::element::state_and_attributes;
@ -65,6 +66,11 @@ where
E: TElement,
I: Iterator<Item = &'a CascadeData>,
{
fn check_outer_dependency(&mut self, _: &Dependency, _: E) -> bool {
debug_assert!(false, "how, we should only have parent-less dependencies here!");
true
}
fn collect_invalidations(
&mut self,
_element: E,
@ -81,10 +87,14 @@ where
// We pass `None` as a scope, as document state selectors aren't
// affected by the current scope.
//
// FIXME(emilio): We should really pass the relevant host for
// self.rules, so that we invalidate correctly if the selector
// happens to have something like :host(:-moz-window-inactive)
// for example.
self_invalidations.push(Invalidation::new(
&dependency.selector,
&dependency.dependency,
/* scope = */ None,
0,
));
}
}

View file

@ -57,7 +57,7 @@ pub struct Dependency {
///
/// We'd generate:
///
/// * One dependency for .quz (offset: 0, parent: None)
/// * One dependency for .qux (offset: 0, parent: None)
/// * One dependency for .baz pointing to B with parent being a
/// dependency pointing to C.
/// * One dependency from .bar pointing to C (parent: None)
@ -88,6 +88,22 @@ pub enum DependencyInvalidationKind {
}
impl Dependency {
/// Creates a dummy dependency to invalidate the whole selector.
///
/// This is necessary because document state invalidation wants to
/// invalidate all elements in the document.
///
/// The offset is such as that Invalidation::new(self) returns a zero
/// offset. That is, it points to a virtual "combinator" outside of the
/// selector, so calling combinator() on such a dependency will panic.
pub fn for_full_selector_invalidation(selector: Selector<SelectorImpl>) -> Self {
Self {
selector_offset: selector.len() + 1,
selector,
parent: None,
}
}
/// Returns the combinator to the right of the partial selector this
/// dependency represents.
///
@ -147,14 +163,14 @@ impl SelectorMapEntry for StateDependency {
/// The same, but for document state selectors.
#[derive(Clone, Debug, MallocSizeOf)]
pub struct DocumentStateDependency {
/// The selector that is affected. We don't need to track an offset, since
/// when it changes it changes for the whole document anyway.
/// We track `Dependency` even though we don't need to track an offset,
/// since when it changes it changes for the whole document anyway.
#[cfg_attr(
feature = "gecko",
ignore_malloc_size_of = "CssRules have primary refs, we measure there"
)]
#[cfg_attr(feature = "servo", ignore_malloc_size_of = "Arc")]
pub selector: Selector<SelectorImpl>,
pub dependency: Dependency,
/// The state this dependency is affected by.
pub state: DocumentState,
}
@ -269,7 +285,7 @@ impl InvalidationMap {
if !document_state.is_empty() {
let dep = DocumentStateDependency {
state: document_state,
selector: selector.clone(),
dependency: Dependency::for_full_selector_invalidation(selector.clone()),
};
self.document_state_selectors.try_push(dep)?;
}

View file

@ -7,10 +7,10 @@
use crate::context::StackLimitChecker;
use crate::dom::{TElement, TNode, TShadowRoot};
use crate::selector_parser::SelectorImpl;
use crate::invalidation::element::invalidation_map::{Dependency, DependencyInvalidationKind};
use selectors::matching::matches_compound_selector_from;
use selectors::matching::{CompoundSelectorMatchingResult, MatchingContext};
use selectors::parser::{Combinator, Component, Selector};
use selectors::parser::{Combinator, Component};
use selectors::OpaqueElement;
use smallvec::SmallVec;
use std::fmt;
@ -34,6 +34,27 @@ where
false
}
/// When a dependency from a :where or :is selector matches, it may still be
/// the case that we don't need to invalidate the full style. Consider the
/// case of:
///
/// div .foo:where(.bar *, .baz) .qux
///
/// We can get to the `*` part after a .bar class change, but you only need
/// to restyle the element if it also matches .foo.
///
/// Similarly, you only need to restyle .baz if the whole result of matching
/// the selector changes.
///
/// This function is called to check the result of matching the "outer"
/// dependency that we generate for the parent of the `:where` selector,
/// that is, in the case above it should match
/// `div .foo:where(.bar *, .baz)`.
///
/// Returning true unconditionally here is over-optimistic and may
/// over-invalidate.
fn check_outer_dependency(&mut self, dependency: &Dependency, element: E) -> bool;
/// The matching context that should be used to process invalidations.
fn matching_context(&mut self) -> &mut MatchingContext<'a, E::Impl>;
@ -127,7 +148,11 @@ enum InvalidationKind {
/// relative to a current element we are processing, must be restyled.
#[derive(Clone)]
pub struct Invalidation<'a> {
selector: &'a Selector<SelectorImpl>,
/// The dependency that generated this invalidation.
///
/// Note that the offset inside the dependency is not really useful after
/// construction.
dependency: &'a Dependency,
/// The right shadow host from where the rule came from, if any.
///
/// This is needed to ensure that we match the selector with the right
@ -138,6 +163,8 @@ pub struct Invalidation<'a> {
///
/// This order is a "parse order" offset, that is, zero is the leftmost part
/// of the selector written as parsed / serialized.
///
/// It is initialized from the offset from `dependency`.
offset: usize,
/// Whether the invalidation was already matched by any previous sibling or
/// ancestor.
@ -149,16 +176,21 @@ pub struct Invalidation<'a> {
}
impl<'a> Invalidation<'a> {
/// Create a new invalidation for a given selector and offset.
/// Create a new invalidation for matching a dependency.
pub fn new(
selector: &'a Selector<SelectorImpl>,
dependency: &'a Dependency,
scope: Option<OpaqueElement>,
offset: usize,
) -> Self {
debug_assert!(
dependency.selector_offset == dependency.selector.len() + 1 ||
dependency.invalidation_kind() != DependencyInvalidationKind::Element,
"No point to this, if the dependency matched the element we should just invalidate it"
);
Self {
selector,
dependency,
scope,
offset,
// + 1 to go past the combinator.
offset: dependency.selector.len() + 1 - dependency.selector_offset,
matched_by_any_previous: false,
}
}
@ -174,7 +206,7 @@ impl<'a> Invalidation<'a> {
// for the weird pseudos in <input type="number">.
//
// We should be able to do better here!
match self.selector.combinator_at_parse_order(self.offset - 1) {
match self.dependency.selector.combinator_at_parse_order(self.offset - 1) {
Combinator::Descendant | Combinator::LaterSibling | Combinator::PseudoElement => true,
Combinator::Part |
Combinator::SlotAssignment |
@ -188,7 +220,7 @@ impl<'a> Invalidation<'a> {
return InvalidationKind::Descendant(DescendantInvalidationKind::Dom);
}
match self.selector.combinator_at_parse_order(self.offset - 1) {
match self.dependency.selector.combinator_at_parse_order(self.offset - 1) {
Combinator::Child | Combinator::Descendant | Combinator::PseudoElement => {
InvalidationKind::Descendant(DescendantInvalidationKind::Dom)
},
@ -206,7 +238,7 @@ impl<'a> fmt::Debug for Invalidation<'a> {
use cssparser::ToCss;
f.write_str("Invalidation(")?;
for component in self.selector.iter_raw_parse_order_from(self.offset) {
for component in self.dependency.selector.iter_raw_parse_order_from(self.offset) {
if matches!(*component, Component::Combinator(..)) {
break;
}
@ -763,36 +795,86 @@ where
context.current_host = invalidation.scope;
matches_compound_selector_from(
&invalidation.selector,
&invalidation.dependency.selector,
invalidation.offset,
context,
&self.element,
)
};
let mut invalidated_self = false;
let mut matched = false;
match matching_result {
let next_invalidation = match matching_result {
CompoundSelectorMatchingResult::NotMatched => {
return SingleInvalidationResult {
invalidated_self: false,
matched: false,
}
},
CompoundSelectorMatchingResult::FullyMatched => {
debug!(" > Invalidation matched completely");
matched = true;
invalidated_self = true;
// We matched completely. If we're an inner selector now we need
// to go outside our selector and carry on invalidating.
let mut cur_dependency = invalidation.dependency;
loop {
cur_dependency = match cur_dependency.parent {
None => return SingleInvalidationResult {
invalidated_self: true,
matched: true,
},
Some(ref p) => &**p,
};
debug!(" > Checking outer dependency {:?}", cur_dependency);
// The inner selector changed, now check if the full
// previous part of the selector did, before keeping
// checking for descendants.
if !self.processor.check_outer_dependency(cur_dependency, self.element) {
return SingleInvalidationResult {
invalidated_self: false,
matched: false,
}
}
if cur_dependency.invalidation_kind() == DependencyInvalidationKind::Element {
continue;
}
debug!(" > Generating invalidation");
break Invalidation::new(cur_dependency, invalidation.scope)
}
},
CompoundSelectorMatchingResult::Matched {
next_combinator_offset,
} => {
let next_combinator = invalidation
Invalidation {
dependency: invalidation.dependency,
scope: invalidation.scope,
offset: next_combinator_offset + 1,
matched_by_any_previous: false,
}
},
};
debug_assert_ne!(
next_invalidation.offset,
0,
"Rightmost selectors shouldn't generate more invalidations",
);
let mut invalidated_self = false;
let next_combinator = next_invalidation
.dependency
.selector
.combinator_at_parse_order(next_combinator_offset);
matched = true;
.combinator_at_parse_order(next_invalidation.offset - 1);
if matches!(next_combinator, Combinator::PseudoElement) {
// This will usually be the very next component, except for
// the fact that we store compound selectors the other way
// around, so there could also be state pseudo-classes.
let pseudo_selector = invalidation
let pseudo_selector = next_invalidation
.dependency
.selector
.iter_raw_parse_order_from(next_combinator_offset + 1)
.iter_raw_parse_order_from(next_invalidation.offset)
.skip_while(|c| matches!(**c, Component::NonTSPseudoClass(..)))
.next()
.unwrap();
@ -802,7 +884,7 @@ where
_ => unreachable!(
"Someone seriously messed up selector parsing: \
{:?} at offset {:?}: {:?}",
invalidation.selector, next_combinator_offset, pseudo_selector,
next_invalidation.dependency, next_invalidation.offset, pseudo_selector,
),
};
@ -849,13 +931,6 @@ where
}
}
let next_invalidation = Invalidation {
selector: invalidation.selector,
scope: invalidation.scope,
offset: next_combinator_offset + 1,
matched_by_any_previous: false,
};
debug!(
" > Invalidation matched, next: {:?}, ({:?})",
next_invalidation, next_combinator
@ -951,13 +1026,10 @@ where
},
}
}
},
CompoundSelectorMatchingResult::NotMatched => {},
}
SingleInvalidationResult {
invalidated_self,
matched,
matched: true,
}
}
}

View file

@ -85,8 +85,8 @@ pub fn check_dependency<E, W>(
dependency: &Dependency,
element: &E,
wrapper: &W,
context: &mut MatchingContext<'_, SelectorImpl>,
)
mut context: &mut MatchingContext<'_, E::Impl>,
) -> bool
where
E: TElement,
W: selectors::Element<Impl = E::Impl>,
@ -166,6 +166,14 @@ where
true
}
fn check_outer_dependency(&mut self, dependency: &Dependency, element: E) -> bool {
// We cannot assert about `element` having a snapshot here (in fact it
// most likely won't), because it may be an arbitrary descendant or
// later-sibling of the element we started invalidating with.
let wrapper = ElementWrapper::new(element, &*self.shared_context.snapshot_map);
check_dependency(dependency, &element, &wrapper, &mut self.matching_context)
}
fn matching_context(&mut self) -> &mut MatchingContext<'a, E::Impl> {
&mut self.matching_context
}
@ -447,7 +455,7 @@ where
/// Check whether a dependency should be taken into account.
#[inline]
fn check_dependency(&mut self, dependency: &Dependency) -> bool {
check_dependency(&self.element, &self.wrapper, &mut self.matching_context)
check_dependency(dependency, &self.element, &self.wrapper, &mut self.matching_context)
}
fn scan_dependency(&mut self, dependency: &'selectors Dependency) {
@ -484,9 +492,8 @@ where
debug_assert_ne!(dependency.selector_offset, dependency.selector.len());
let invalidation = Invalidation::new(
&dependency.selector,
&dependency,
self.matching_context.current_host.clone(),
dependency.selector.len() - dependency.selector_offset + 1,
);
match invalidation_kind {