From e7a8f5ec30c68f9ff760186abc88daa9d5dc09fe Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Mon, 6 Feb 2017 12:57:25 -0800 Subject: [PATCH] Bug 1336646 - Use the bloom filter for manual style resolves and pass 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. --- components/style/bloom.rs | 14 +++++++++++--- components/style/matching.rs | 10 +++++----- components/style/traversal.rs | 26 +++++++++++++++++++------- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/components/style/bloom.rs b/components/style/bloom.rs index 72e7a173de8b..aba2d9fa5b82 100644 --- a/components/style/bloom.rs +++ b/components/style/bloom.rs @@ -66,7 +66,7 @@ impl StyleBloom { /// 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()); @@ -86,12 +86,20 @@ impl StyleBloom { 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() { diff --git a/components/style/matching.rs b/components/style/matching.rs index bacb0905389c..0a26255e82de 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -577,8 +577,7 @@ impl PrivateMatchMethods for E {} pub trait MatchMethods : TElement { /// Runs selector matching of this element, and returns the result. fn match_element(&self, - context: &StyleContext, - parent_bf: Option<&BloomFilter>) + context: &mut StyleContext) -> MatchResults { let mut applicable_declarations = @@ -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, @@ -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); diff --git a/components/style/traversal.rs b/components/style/traversal.rs index a438211495a6..f6e75a2efe8e 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -312,7 +312,7 @@ pub trait DomTraversal : Sync { } /// Helper for the function below. -fn resolve_style_internal(context: &StyleContext, element: E, ensure_data: &F) +fn resolve_style_internal(context: &mut StyleContext, element: E, ensure_data: &F) -> Option where E: TElement, F: Fn(E), @@ -324,12 +324,22 @@ fn resolve_style_internal(context: &StyleContext, 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, @@ -355,13 +365,16 @@ fn resolve_style_internal(context: &StyleContext, 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(context: &StyleContext, element: E, +pub fn resolve_style(context: &mut StyleContext, 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); @@ -499,8 +512,7 @@ fn compute_style(_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)) } } }