Skip to content

Commit

Permalink
Bug 1336646 - Use the bloom filter for manual style resolves and pass…
Browse files Browse the repository at this point in the history
… a mutable StyleContext into match_element. r=emilio

We need to do something here to avoid a double-borrow when passing a mutable
StyleContext to match_element. After some discussion on IRC, we decided that
building the bloom filter for this case is probably worthwhile.
  • Loading branch information
bholley committed Feb 9, 2017
1 parent 8aec1cc commit e7a8f5e
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 15 deletions.
14 changes: 11 additions & 3 deletions components/style/bloom.rs
Expand Up @@ -66,7 +66,7 @@ impl<E: TElement> StyleBloom<E> {

/// Push an element to the bloom filter, knowing that it's a child of the
/// last element parent.
fn push(&mut self, element: E) {
pub fn push(&mut self, element: E) {
if cfg!(debug_assertions) {
if self.elements.is_empty() {
assert!(element.parent_element().is_none());
Expand All @@ -86,12 +86,20 @@ impl<E: TElement> StyleBloom<E> {
popped
}

fn clear(&mut self) {
/// Returns true if the bloom filter is empty.
pub fn is_empty(&self) -> bool {
self.elements.is_empty()
}


/// Clears the bloom filter.
pub fn clear(&mut self) {
self.filter.clear();
self.elements.clear();
}

fn rebuild(&mut self, mut element: E) -> usize {
/// Rebuilds the bloom filter up to the parent of the given element.
pub fn rebuild(&mut self, mut element: E) -> usize {
self.clear();

while let Some(parent) = element.parent_element() {
Expand Down
10 changes: 5 additions & 5 deletions components/style/matching.rs
Expand Up @@ -577,8 +577,7 @@ impl<E: TElement> PrivateMatchMethods for E {}
pub trait MatchMethods : TElement {
/// Runs selector matching of this element, and returns the result.
fn match_element(&self,
context: &StyleContext<Self>,
parent_bf: Option<&BloomFilter>)
context: &mut StyleContext<Self>)
-> MatchResults
{
let mut applicable_declarations =
Expand All @@ -591,7 +590,7 @@ pub trait MatchMethods : TElement {
// Compute the primary rule node.
let mut primary_relations =
stylist.push_applicable_declarations(self,
parent_bf,
Some(context.thread_local.bloom_filter.filter()),
style_attribute,
animation_rules,
None,
Expand All @@ -604,8 +603,9 @@ pub trait MatchMethods : TElement {
SelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| {
debug_assert!(applicable_declarations.is_empty());
let pseudo_animation_rules = self.get_animation_rules(Some(&pseudo));
stylist.push_applicable_declarations(self, parent_bf, None,
pseudo_animation_rules,
stylist.push_applicable_declarations(self,
Some(context.thread_local.bloom_filter.filter()),
None, pseudo_animation_rules,
Some(&pseudo),
&mut applicable_declarations,
MatchingReason::ForStyling);
Expand Down
26 changes: 19 additions & 7 deletions components/style/traversal.rs
Expand Up @@ -312,7 +312,7 @@ pub trait DomTraversal<E: TElement> : Sync {
}

/// Helper for the function below.
fn resolve_style_internal<E, F>(context: &StyleContext<E>, element: E, ensure_data: &F)
fn resolve_style_internal<E, F>(context: &mut StyleContext<E>, element: E, ensure_data: &F)
-> Option<E>
where E: TElement,
F: Fn(E),
Expand All @@ -324,12 +324,22 @@ fn resolve_style_internal<E, F>(context: &StyleContext<E>, element: E, ensure_da
// If the Element isn't styled, we need to compute its style.
if data.get_styles().is_none() {
// Compute the parent style if necessary.
if let Some(parent) = element.parent_element() {
display_none_root = resolve_style_internal(context, parent, ensure_data);
let parent = element.parent_element();
if let Some(p) = parent {
display_none_root = resolve_style_internal(context, p, ensure_data);
}

// Maintain the bloom filter. If it doesn't exist, we need to build it
// from scratch. Otherwise we just need to push the parent.
if context.thread_local.bloom_filter.is_empty() {
context.thread_local.bloom_filter.rebuild(element);
} else {
context.thread_local.bloom_filter.push(parent.unwrap());
context.thread_local.bloom_filter.assert_complete(element);
}

// Compute our style.
let match_results = element.match_element(context, None);
let match_results = element.match_element(context);
let shareable = match_results.primary_is_shareable();
element.cascade_node(context, &mut data, element.parent_element(),
match_results.primary,
Expand All @@ -355,13 +365,16 @@ fn resolve_style_internal<E, F>(context: &StyleContext<E>, element: E, ensure_da
/// first styled Element, ignoring pending restyles. The resolved style is
/// made available via a callback, and can be dropped by the time this function
/// returns in the display:none subtree case.
pub fn resolve_style<E, F, G, H>(context: &StyleContext<E>, element: E,
pub fn resolve_style<E, F, G, H>(context: &mut StyleContext<E>, element: E,
ensure_data: &F, clear_data: &G, callback: H)
where E: TElement,
F: Fn(E),
G: Fn(E),
H: FnOnce(&ElementStyles)
{
// Clear the bloom filter, just in case the caller is reusing TLS.
context.thread_local.bloom_filter.clear();

// Resolve styles up the tree.
let display_none_root = resolve_style_internal(context, element, ensure_data);

Expand Down Expand Up @@ -499,8 +512,7 @@ fn compute_style<E, D>(_traversal: &D,
// Perform the CSS selector matching.
context.thread_local.statistics.elements_matched += 1;

let filter = context.thread_local.bloom_filter.filter();
Some(element.match_element(context, Some(filter)))
Some(element.match_element(context))
}
}
}
Expand Down

0 comments on commit e7a8f5e

Please sign in to comment.