Skip to content

Commit

Permalink
Fix revalidation selectors when pseudo-elements are involved.
Browse files Browse the repository at this point in the history
  • Loading branch information
bzbarsky committed Jun 8, 2017
1 parent 24e944a commit 537cf52
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 9 deletions.
7 changes: 6 additions & 1 deletion components/selectors/parser.rs
Expand Up @@ -479,6 +479,9 @@ impl<'a, Impl: 'a + SelectorImpl> AncestorIter<'a, Impl> {
fn skip_until_ancestor(&mut self) {
loop {
while self.0.next().is_some() {}
// If this is ever changed to stop at the "pseudo-element"
// combinator, we will need to fix the way we compute hashes for
// revalidation selectors.
if self.0.next_sequence().map_or(true, |x| matches!(x, Combinator::Child | Combinator::Descendant)) {
break;
}
Expand Down Expand Up @@ -515,7 +518,9 @@ pub enum Combinator {
/// A dummy combinator we use to the left of pseudo-elements.
///
/// It serializes as the empty string, and acts effectively as a child
/// combinator.
/// combinator in most cases. If we ever actually start using a child
/// combinator for this, we will need to fix up the way hashes are computed
/// for revalidation selectors.
PseudoElement,
}

Expand Down
8 changes: 4 additions & 4 deletions components/style/sharing/mod.rs
Expand Up @@ -61,10 +61,10 @@
//! the up-front checks but would have different matching results for the
//! selector in question. In this case, "descendants" includes pseudo-elements,
//! so there is a single selector map of revalidation selectors that includes
//! both selectors targeting element and selectors targeting pseudo-elements.
//! This relies on matching an element against a pseudo-element-targeting
//! selector being a sensible operation that will effectively check whether that
//! element is a matching originating element for the selector.
//! both selectors targeting elements and selectors targeting pseudo-element
//! originating elements. We ensure that the pseudo-element parts of all these
//! selectors are effectively stripped off, so that matching them all against
//! elements makes sense.

use Atom;
use bit_vec::BitVec;
Expand Down
54 changes: 50 additions & 4 deletions components/style/stylist.rs
Expand Up @@ -160,7 +160,7 @@ pub struct Stylist {
/// on state that is not otherwise visible to the cache, like attributes or
/// tree-structural state like child index and pseudos).
#[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
selectors_for_cache_revalidation: SelectorMap<SelectorAndHashes<SelectorImpl>>,
selectors_for_cache_revalidation: SelectorMap<RevalidationSelectorAndHashes>,

/// The total number of selectors.
num_selectors: usize,
Expand Down Expand Up @@ -474,7 +474,8 @@ impl Stylist {

self.dependencies.note_selector(selector_and_hashes);
if needs_revalidation(&selector_and_hashes.selector) {
self.selectors_for_cache_revalidation.insert(selector_and_hashes.clone());
self.selectors_for_cache_revalidation.insert(
RevalidationSelectorAndHashes::new(&selector_and_hashes));
}
selector_and_hashes.selector.visit(&mut AttributeAndStateDependencyVisitor {
attribute_dependencies: &mut self.attribute_dependencies,
Expand Down Expand Up @@ -1134,7 +1135,7 @@ impl Stylist {
let mut results = BitVec::new();
self.selectors_for_cache_revalidation.lookup(*element, &mut |selector_and_hashes| {
results.push(matches_selector(&selector_and_hashes.selector,
0,
selector_and_hashes.selector_offset,
&selector_and_hashes.hashes,
element,
&mut matching_context,
Expand Down Expand Up @@ -1288,7 +1289,52 @@ impl<'a> SelectorVisitor for MappedIdVisitor<'a> {
}
}

/// Visitor determine whether a selector requires cache revalidation.
/// SelectorMapEntry implementation for use in our revalidation selector map.
#[derive(Clone, Debug)]
struct RevalidationSelectorAndHashes {
selector: Selector<SelectorImpl>,
selector_offset: usize,
hashes: AncestorHashes,
}

impl RevalidationSelectorAndHashes {
fn new(selector_and_hashes: &SelectorAndHashes<SelectorImpl>) -> Self {
// We basically want to check whether the first combinator is a
// pseudo-element combinator. If it is, we want to use the offset one
// past it. Otherwise, our offset is 0.
let mut index = 0;
let mut iter = selector_and_hashes.selector.iter();

// First skip over the first ComplexSelector. We can't check what sort
// of combinator we have until we do that.
for _ in &mut iter {
index += 1; // Simple selector
}

let offset = match iter.next_sequence() {
Some(Combinator::PseudoElement) => index + 1, // +1 for the combinator
_ => 0
};

RevalidationSelectorAndHashes {
selector: selector_and_hashes.selector.clone(),
selector_offset: offset,
hashes: selector_and_hashes.hashes.clone(),
}
}
}

impl SelectorMapEntry for RevalidationSelectorAndHashes {
fn selector(&self) -> SelectorIter<SelectorImpl> {
self.selector.iter_from(self.selector_offset)
}

fn hashes(&self) -> &AncestorHashes {
&self.hashes
}
}

/// Visitor to determine whether a selector requires cache revalidation.
///
/// Note that we just check simple selectors and eagerly return when the first
/// need for revalidation is found, so we don't need to store state on the
Expand Down

0 comments on commit 537cf52

Please sign in to comment.