author | Sebastian Hengst <archaeopteryx@coole-files.de> |
Sat, 28 Oct 2017 10:07:46 +0200 | |
changeset 388845 | 39400e032624c7cd7b3c63c62957c5c32aa8b008 |
parent 388844 | 6087df884cb256fce27d97574eedb12e91db73eb |
child 388866 | 8fe5c82c3dcd998eaa5f9b038d083abc764931af |
push id | 54277 |
push user | archaeopteryx@coole-files.de |
push date | Sat, 28 Oct 2017 08:08:04 +0000 |
treeherder | autoland@39400e032624 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | backout |
bugs | 272646 |
milestone | 58.0a1 |
backs out | fc813bf68348121aeac663dd48a23221c4116dbb |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/servo/components/style/dom.rs +++ b/servo/components/style/dom.rs @@ -383,25 +383,16 @@ pub trait TElement while let Some(parent) = curr.traversal_parent() { depth += 1; curr = parent; } depth } - /// The style scope of this element is a node that represents which rules - /// apply to the element. - /// - /// In Servo, where we don't know about Shadow DOM or XBL, the style scope - /// is always the document. - fn style_scope(&self) -> Self::ConcreteNode { - self.as_node().owner_doc().as_node() - } - /// Get this node's parent element from the perspective of a restyle /// traversal. fn traversal_parent(&self) -> Option<Self> { self.as_node().traversal_parent() } /// Get this node's children from the perspective of a restyle traversal. fn traversal_children(&self) -> LayoutIterator<Self::TraversalChildrenIterator>; @@ -742,26 +733,24 @@ pub trait TElement /// Returns the anonymous content for the current element's XBL binding, /// given if any. /// /// This is used in Gecko for XBL and shadow DOM. fn xbl_binding_anonymous_content(&self) -> Option<Self::ConcreteNode> { None } - /// Return the element which we can use to look up rules in the selector - /// maps. - /// - /// This is always the element itself, except in the case where we are an - /// element-backed pseudo-element, in which case we return the originating - /// element. + /// Returns the rule hash target given an element. fn rule_hash_target(&self) -> Self { let is_implemented_pseudo = self.implemented_pseudo_element().is_some(); + // NB: This causes use to rule has pseudo selectors based on the + // properties of the originating element (which is fine, given the + // find_first_from_right usage). if is_implemented_pseudo { self.closest_non_native_anonymous_ancestor().unwrap() } else { *self } } /// Implements Gecko's `nsBindingManager::WalkRules`.
--- a/servo/components/style/gecko/wrapper.rs +++ b/servo/components/style/gecko/wrapper.rs @@ -514,23 +514,18 @@ impl<'le> GeckoElement<'le> { fn get_extended_slots( &self, ) -> Option<&structs::FragmentOrElement_nsExtendedDOMSlots> { self.get_dom_slots() .and_then(|s| unsafe { s.mExtendedSlots.mPtr.as_ref() }) } #[inline] - fn may_be_in_binding_manager(&self) -> bool { - self.flags() & (structs::NODE_MAY_BE_IN_BINDING_MNGR as u32) != 0 - } - - #[inline] fn get_xbl_binding(&self) -> Option<GeckoXBLBinding<'le>> { - if !self.may_be_in_binding_manager() { + if self.flags() & (structs::NODE_MAY_BE_IN_BINDING_MNGR as u32) == 0 { return None; } unsafe { bindings::Gecko_GetXBLBinding(self.0).map(GeckoXBLBinding) } } #[inline] fn get_xbl_binding_with_content(&self) -> Option<GeckoXBLBinding<'le>> { @@ -553,21 +548,19 @@ impl<'le> GeckoElement<'le> { // where the binding parent is stored in a member variable // rather than in slots. So just get it through FFI for now. unsafe { bindings::Gecko_GetBindingParent(self.0).map(GeckoElement) } } else { let binding_parent = unsafe { self.get_non_xul_xbl_binding_parent_raw_content().as_ref() - }.map(GeckoNode::from_content).and_then(|n| n.as_element()); - - debug_assert!(binding_parent == unsafe { - bindings::Gecko_GetBindingParent(self.0).map(GeckoElement) - }); + }.map(GeckoNode::from_content) + .and_then(|n| n.as_element()); + debug_assert!(binding_parent == unsafe { bindings::Gecko_GetBindingParent(self.0).map(GeckoElement) }); binding_parent } } fn get_non_xul_xbl_binding_parent_raw_content(&self) -> *mut nsIContent { debug_assert!(!self.is_xul_element()); self.get_extended_slots() .map_or(ptr::null_mut(), |slots| slots.mBindingParent) @@ -927,38 +920,16 @@ impl<'le> TElement for GeckoElement<'le> fn before_pseudo_element(&self) -> Option<Self> { self.get_before_or_after_pseudo(/* is_before = */ true) } fn after_pseudo_element(&self) -> Option<Self> { self.get_before_or_after_pseudo(/* is_before = */ false) } - /// Ensure this accurately represents the rules that an element may ever - /// match, even in the native anonymous content case. - fn style_scope(&self) -> Self::ConcreteNode { - if self.implemented_pseudo_element().is_some() { - return self.closest_non_native_anonymous_ancestor().unwrap().style_scope(); - } - - if self.is_in_native_anonymous_subtree() { - return self.as_node().owner_doc().as_node(); - } - - if self.get_xbl_binding().is_some() { - return self.as_node(); - } - - if let Some(parent) = self.get_xbl_binding_parent() { - return parent.as_node(); - } - - self.as_node().owner_doc().as_node() - } - /// Execute `f` for each anonymous content child element (apart from /// ::before and ::after) whose originating element is `self`. fn each_anonymous_content_child<F>(&self, mut f: F) where F: FnMut(Self), { let array: *mut structs::nsTArray<*mut nsIContent> = unsafe { bindings::Gecko_GetAnonymousContentForElement(self.0) };
--- a/servo/components/style/sharing/checks.rs +++ b/servo/components/style/sharing/checks.rs @@ -1,16 +1,17 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ //! Different checks done during the style sharing process in order to determine //! quickly whether it's worth to share style, and whether two different //! elements can indeed share the same style. +use Atom; use bloom::StyleBloom; use context::{SelectorFlagsMap, SharedStyleContext}; use dom::TElement; use selectors::NthIndexCache; use sharing::{StyleSharingCandidate, StyleSharingTarget}; /// Determines whether a target and a candidate have compatible parents for sharing. pub fn parents_allow_sharing<E>( @@ -90,26 +91,24 @@ pub fn have_same_class<E>(target: &mut S /// Whether a given element and a candidate match the same set of "revalidation" /// selectors. /// /// Revalidation selectors are those that depend on the DOM structure, like /// :first-child, etc, or on attributes that we don't check off-hand (pretty /// much every attribute selector except `id` and `class`. #[inline] -pub fn revalidate<E>( - target: &mut StyleSharingTarget<E>, - candidate: &mut StyleSharingCandidate<E>, - shared_context: &SharedStyleContext, - bloom: &StyleBloom<E>, - nth_index_cache: &mut NthIndexCache, - selector_flags_map: &mut SelectorFlagsMap<E>, -) -> bool -where - E: TElement, +pub fn revalidate<E>(target: &mut StyleSharingTarget<E>, + candidate: &mut StyleSharingCandidate<E>, + shared_context: &SharedStyleContext, + bloom: &StyleBloom<E>, + nth_index_cache: &mut NthIndexCache, + selector_flags_map: &mut SelectorFlagsMap<E>) + -> bool + where E: TElement, { let stylist = &shared_context.stylist; let for_element = target.revalidation_match_results( stylist, bloom, nth_index_cache, selector_flags_map, @@ -126,39 +125,30 @@ where // comparing represent the same set of selectors. debug_assert_eq!(for_element.len(), for_candidate.len()); for_element == for_candidate } /// Checks whether we might have rules for either of the two ids. #[inline] -pub fn may_match_different_id_rules<E>( - shared_context: &SharedStyleContext, - element: E, - candidate: E, -) -> bool -where - E: TElement, +pub fn may_have_rules_for_ids(shared_context: &SharedStyleContext, + element_id: Option<&Atom>, + candidate_id: Option<&Atom>) -> bool { - let element_id = element.get_id(); - let candidate_id = candidate.get_id(); - - if element_id == candidate_id { - return false; - } - + // We shouldn't be called unless the ids are different. + debug_assert!(element_id.is_some() || candidate_id.is_some()); let stylist = &shared_context.stylist; let may_have_rules_for_element = match element_id { - Some(ref id) => stylist.may_have_rules_for_id(id, element), + Some(id) => stylist.may_have_rules_for_id(id), None => false }; if may_have_rules_for_element { return true; } match candidate_id { - Some(ref id) => stylist.may_have_rules_for_id(id, candidate), + Some(id) => stylist.may_have_rules_for_id(id), None => false } }
--- a/servo/components/style/sharing/mod.rs +++ b/servo/components/style/sharing/mod.rs @@ -201,21 +201,20 @@ impl ValidationData { #[inline] fn revalidation_match_results<E, F>( &mut self, element: E, stylist: &Stylist, bloom: &StyleBloom<E>, nth_index_cache: &mut NthIndexCache, bloom_known_valid: bool, - flags_setter: &mut F, + flags_setter: &mut F ) -> &SmallBitVec - where - E: TElement, - F: FnMut(&E, ElementSelectorFlags), + where E: TElement, + F: FnMut(&E, ElementSelectorFlags), { if self.revalidation_match_results.is_none() { // The bloom filter may already be set up for our element. // If it is, use it. If not, we must be in a candidate // (i.e. something in the cache), and the element is one // of our cousins, not a sibling. In that case, we'll // just do revalidation selector matching without a bloom // filter, to avoid thrashing the filter. @@ -226,22 +225,20 @@ impl ValidationData { } else { if bloom.current_parent() == element.traversal_parent() { Some(bloom.filter()) } else { None } }; self.revalidation_match_results = - Some(stylist.match_revalidation_selectors( - element, - bloom_to_use, - nth_index_cache, - flags_setter, - )); + Some(stylist.match_revalidation_selectors(&element, + bloom_to_use, + nth_index_cache, + flags_setter)); } self.revalidation_match_results.as_ref().unwrap() } } /// Information regarding a style sharing candidate, that is, an entry in the /// style sharing cache. @@ -659,25 +656,16 @@ impl<E: TElement> StyleSharingCache<E> { if target.is_native_anonymous() { debug_assert!(!candidate.element.is_native_anonymous(), "Why inserting NAC into the cache?"); trace!("Miss: Native Anonymous Content"); return None; } - // Note that in the XBL case, we should be able to assert that the - // scopes are different, since two elements with different XBL bindings - // need to necessarily have different style (and thus children of them - // would never pass the parent check). - if target.element.style_scope() != candidate.element.style_scope() { - trace!("Miss: Different style scopes"); - return None; - } - if *target.get_local_name() != *candidate.element.get_local_name() { trace!("Miss: Local Name"); return None; } if *target.get_namespace() != *candidate.element.get_namespace() { trace!("Miss: Namespace"); return None; @@ -697,27 +685,25 @@ impl<E: TElement> StyleSharingCache<E> { // We do not ignore visited state here, because Gecko // needs to store extra bits on visited style contexts, // so these contexts cannot be shared if target.element.get_state() != candidate.get_state() { trace!("Miss: User and Author State"); return None; } - // It's possible that there are no styles for either id. - let may_match_different_id_rules = - checks::may_match_different_id_rules( - shared, - target.element, - candidate.element, - ); - - if may_match_different_id_rules { - trace!("Miss: ID Attr"); - return None; + let element_id = target.element.get_id(); + let candidate_id = candidate.element.get_id(); + if element_id != candidate_id { + // It's possible that there are no styles for either id. + if checks::may_have_rules_for_ids(shared, element_id.as_ref(), + candidate_id.as_ref()) { + trace!("Miss: ID Attr"); + return None; + } } if !checks::have_same_style_attribute(target, candidate) { trace!("Miss: Style Attr"); return None; } if !checks::have_same_class(target, candidate) {
--- a/servo/components/style/stylist.rs +++ b/servo/components/style/stylist.rs @@ -1404,55 +1404,36 @@ impl Stylist { } else { debug!("skipping transition rules"); } } /// Given an id, returns whether there might be any rules for that id in any /// of our rule maps. #[inline] - pub fn may_have_rules_for_id<E>( - &self, - id: &Atom, - element: E, - ) -> bool - where - E: TElement, - { - let hash = id.get_hash(); - for (data, _) in self.cascade_data.iter_origins() { - if data.mapped_ids.might_contain_hash(hash) { - return true; - } - } - - let mut xbl_rules_may_contain = false; - - element.each_xbl_stylist(|stylist| { - xbl_rules_may_contain = xbl_rules_may_contain || - stylist.cascade_data.author.mapped_ids.might_contain_hash(hash) - }); - - xbl_rules_may_contain + pub fn may_have_rules_for_id(&self, id: &Atom) -> bool { + self.cascade_data + .iter_origins() + .any(|(d, _)| d.mapped_ids.might_contain_hash(id.get_hash())) } /// Returns the registered `@keyframes` animation for the specified name. #[inline] pub fn get_animation(&self, name: &Atom) -> Option<&KeyframesAnimation> { self.cascade_data .iter_origins() .filter_map(|(d, _)| d.animations.get(name)) .next() } /// Computes the match results of a given element against the set of /// revalidation selectors. pub fn match_revalidation_selectors<E, F>( &self, - element: E, + element: &E, bloom: Option<&BloomFilter>, nth_index_cache: &mut NthIndexCache, flags_setter: &mut F ) -> SmallBitVec where E: TElement, F: FnMut(&E, ElementSelectorFlags), { @@ -1468,50 +1449,32 @@ impl Stylist { // Note that, by the time we're revalidating, we're guaranteed that the // candidate and the entry have the same id, classes, and local name. // This means we're guaranteed to get the same rulehash buckets for all // the lookups, which means that the bitvecs are comparable. We verify // this in the caller by asserting that the bitvecs are same-length. let mut results = SmallBitVec::new(); for (data, _) in self.cascade_data.iter_origins() { data.selectors_for_cache_revalidation.lookup( - element, + *element, self.quirks_mode, &mut |selector_and_hashes| { results.push(matches_selector( &selector_and_hashes.selector, selector_and_hashes.selector_offset, Some(&selector_and_hashes.hashes), - &element, + element, &mut matching_context, flags_setter )); true } ); } - element.each_xbl_stylist(|stylist| { - stylist.cascade_data.author.selectors_for_cache_revalidation.lookup( - element, - stylist.quirks_mode, - &mut |selector_and_hashes| { - results.push(matches_selector( - &selector_and_hashes.selector, - selector_and_hashes.selector_offset, - Some(&selector_and_hashes.hashes), - &element, - &mut matching_context, - flags_setter - )); - true - } - ); - }); - results } /// Computes styles for a given declaration with parent_style. pub fn compute_for_declarations( &self, guards: &StylesheetGuards, parent_style: &ComputedValues,