From e47cb584480561df292f4d3f50b9433102c0db29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 24 Feb 2018 17:28:20 -0500 Subject: [PATCH] servo: Merge #20117 - style: Somewhat miscelaneous cleanup (from emilio:misc-cleanup); r=nox Source-Repo: https://github.com/servo/servo Source-Revision: bbfca28a4f3770896955375d01f1c489b4632fd3 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : cca02ea2c160395889cfd6eb60c62a065d490300 --- servo/components/layout/traversal.rs | 19 +- servo/components/layout_thread/dom_wrapper.rs | 30 +- servo/components/script/dom/element.rs | 8 +- .../script_layout_interface/wrapper_traits.rs | 8 +- servo/components/selectors/matching.rs | 10 +- servo/components/selectors/tree.rs | 33 +- servo/components/style/animation.rs | 8 +- servo/components/style/bloom.rs | 6 +- servo/components/style/context.rs | 44 +- servo/components/style/dom.rs | 51 +- servo/components/style/dom_apis.rs | 4 +- servo/components/style/gecko/snapshot.rs | 25 +- servo/components/style/gecko/wrapper.rs | 319 ++++++------- .../invalidation/element/element_wrapper.rs | 43 +- .../element/state_and_attributes.rs | 12 +- .../style/invalidation/stylesheets.rs | 4 +- servo/components/style/matching.rs | 445 ++++++++++-------- servo/components/style/parallel.rs | 29 +- servo/components/style/rule_tree/mod.rs | 18 +- servo/components/style/selector_map.rs | 18 +- .../components/style/servo/selector_parser.rs | 4 +- servo/components/style/sharing/checks.rs | 8 +- servo/components/style/sharing/mod.rs | 6 +- servo/components/style/style_resolver.rs | 4 +- servo/components/style/stylist.rs | 4 +- 25 files changed, 585 insertions(+), 575 deletions(-) diff --git a/servo/components/layout/traversal.rs b/servo/components/layout/traversal.rs index 78cb8d741f38..2d85ce23c9b9 100644 --- a/servo/components/layout/traversal.rs +++ b/servo/components/layout/traversal.rs @@ -47,14 +47,19 @@ impl<'a> RecalcStyleAndConstructFlows<'a> { #[allow(unsafe_code)] impl<'a, E> DomTraversal for RecalcStyleAndConstructFlows<'a> - where E: TElement, - E::ConcreteNode: LayoutNode, - E::FontMetricsProvider: Send, +where + E: TElement, + E::ConcreteNode: LayoutNode, + E::FontMetricsProvider: Send, { - fn process_preorder(&self, traversal_data: &PerLevelTraversalData, - context: &mut StyleContext, node: E::ConcreteNode, - note_child: F) - where F: FnMut(E::ConcreteNode) + fn process_preorder( + &self, + traversal_data: &PerLevelTraversalData, + context: &mut StyleContext, node: E::ConcreteNode, + note_child: F, + ) + where + F: FnMut(E::ConcreteNode) { // FIXME(pcwalton): Stop allocating here. Ideally this should just be // done by the HTML parser. diff --git a/servo/components/layout_thread/dom_wrapper.rs b/servo/components/layout_thread/dom_wrapper.rs index a909ce72e8f0..1eaf74e4e9d6 100644 --- a/servo/components/layout_thread/dom_wrapper.rs +++ b/servo/components/layout_thread/dom_wrapper.rs @@ -341,7 +341,7 @@ pub struct ServoLayoutElement<'le> { impl<'le> fmt::Debug for ServoLayoutElement<'le> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "<{}", self.element.local_name())?; - if let &Some(ref id) = unsafe { &*self.element.id_attribute() } { + if let Some(id) = self.id() { write!(f, " id={}", id)?; } write!(f, "> ({:#x})", self.as_node().opaque().0) @@ -372,7 +372,7 @@ impl<'le> TElement for ServoLayoutElement<'le> { } } - fn get_state(&self) -> ElementState { + fn state(&self) -> ElementState { self.element.get_state_for_layout() } @@ -382,9 +382,9 @@ impl<'le> TElement for ServoLayoutElement<'le> { } #[inline] - fn get_id(&self) -> Option { + fn id(&self) -> Option<&Atom> { unsafe { - (*self.element.id_attribute()).clone() + (*self.element.id_attribute()).as_ref() } } @@ -692,12 +692,12 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { } #[inline] - fn get_local_name(&self) -> &LocalName { + fn local_name(&self) -> &LocalName { self.element.local_name() } #[inline] - fn get_namespace(&self) -> &Namespace { + fn namespace(&self) -> &Namespace { self.element.namespace() } @@ -787,7 +787,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { fn is_html_slot_element(&self) -> bool { unsafe { self.element.is_html_element() && - self.get_local_name() == &local_name!("slot") + self.local_name() == &local_name!("slot") } } @@ -1025,8 +1025,8 @@ impl Iterator for ThreadSafeLayoutNodeChildrenIterator Iterator for ThreadSafeLayoutNodeChildrenIterator ::selectors::Element for ServoThreadSafeLayoutElement<'le> { } #[inline] - fn get_local_name(&self) -> &LocalName { - self.element.get_local_name() + fn local_name(&self) -> &LocalName { + self.element.local_name() } #[inline] - fn get_namespace(&self) -> &Namespace { - self.element.get_namespace() + fn namespace(&self) -> &Namespace { + self.element.namespace() } fn match_pseudo_element( diff --git a/servo/components/script/dom/element.rs b/servo/components/script/dom/element.rs index 9c88c1391fd7..c8407c3277fb 100644 --- a/servo/components/script/dom/element.rs +++ b/servo/components/script/dom/element.rs @@ -2628,12 +2628,12 @@ impl<'a> SelectorsElement for DomRoot { }) } - fn get_local_name(&self) -> &LocalName { - self.local_name() + fn local_name(&self) -> &LocalName { + Element::local_name(self) } - fn get_namespace(&self) -> &Namespace { - self.namespace() + fn namespace(&self) -> &Namespace { + Element::namespace(self) } fn match_non_ts_pseudo_class( diff --git a/servo/components/script_layout_interface/wrapper_traits.rs b/servo/components/script_layout_interface/wrapper_traits.rs index a3365c3c6e50..a23a417cb6d7 100644 --- a/servo/components/script_layout_interface/wrapper_traits.rs +++ b/servo/components/script_layout_interface/wrapper_traits.rs @@ -347,8 +347,8 @@ pub trait ThreadSafeLayoutElement #[inline] fn get_details_summary_pseudo(&self) -> Option { - if self.get_local_name() == &local_name!("details") && - self.get_namespace() == &ns!(html) { + if self.local_name() == &local_name!("details") && + self.namespace() == &ns!(html) { Some(self.with_pseudo(PseudoElementType::DetailsSummary)) } else { None @@ -357,8 +357,8 @@ pub trait ThreadSafeLayoutElement #[inline] fn get_details_content_pseudo(&self) -> Option { - if self.get_local_name() == &local_name!("details") && - self.get_namespace() == &ns!(html) && + if self.local_name() == &local_name!("details") && + self.namespace() == &ns!(html) && self.get_attr(&ns!(), &local_name!("open")).is_some() { Some(self.with_pseudo(PseudoElementType::DetailsContent)) } else { diff --git a/servo/components/selectors/matching.rs b/servo/components/selectors/matching.rs index a8bf3f689f9d..849edf74a09f 100644 --- a/servo/components/selectors/matching.rs +++ b/servo/components/selectors/matching.rs @@ -553,7 +553,7 @@ where &local_name.name, &local_name.lower_name ).borrow(); - element.get_local_name() == name + element.local_name() == name } /// Determines whether the given element matches the given compound selector. @@ -655,11 +655,11 @@ where } Component::Namespace(_, ref url) | Component::DefaultNamespace(ref url) => { - element.get_namespace() == url.borrow() + element.namespace() == url.borrow() } Component::ExplicitNoNamespace => { let ns = ::parser::namespace_empty_string::(); - element.get_namespace() == ns.borrow() + element.namespace() == ns.borrow() } Component::ID(ref id) => { element.has_id(id, context.shared.classes_and_ids_case_sensitivity()) @@ -860,8 +860,8 @@ where #[inline] fn same_type(a: &E, b: &E) -> bool { - a.get_local_name() == b.get_local_name() && - a.get_namespace() == b.get_namespace() + a.local_name() == b.local_name() && + a.namespace() == b.namespace() } #[inline] diff --git a/servo/components/selectors/tree.rs b/servo/components/selectors/tree.rs index 0b0f65de073a..c9baeb1316d4 100644 --- a/servo/components/selectors/tree.rs +++ b/servo/components/selectors/tree.rs @@ -53,16 +53,17 @@ pub trait Element: Sized + Clone + Debug { fn is_html_element_in_html_document(&self) -> bool; - fn get_local_name(&self) -> &::BorrowedLocalName; + fn local_name(&self) -> &::BorrowedLocalName; /// Empty string for no namespace - fn get_namespace(&self) -> &::BorrowedNamespaceUrl; + fn namespace(&self) -> &::BorrowedNamespaceUrl; - fn attr_matches(&self, - ns: &NamespaceConstraint<&::NamespaceUrl>, - local_name: &::LocalName, - operation: &AttrSelectorOperation<&::AttrValue>) - -> bool; + fn attr_matches( + &self, + ns: &NamespaceConstraint<&::NamespaceUrl>, + local_name: &::LocalName, + operation: &AttrSelectorOperation<&::AttrValue>, + ) -> bool; fn match_non_ts_pseudo_class( &self, @@ -92,15 +93,17 @@ pub trait Element: Sized + Clone + Debug { None } - fn has_id(&self, - id: &::Identifier, - case_sensitivity: CaseSensitivity) - -> bool; + fn has_id( + &self, + id: &::Identifier, + case_sensitivity: CaseSensitivity, + ) -> bool; - fn has_class(&self, - name: &::ClassName, - case_sensitivity: CaseSensitivity) - -> bool; + fn has_class( + &self, + name: &::ClassName, + case_sensitivity: CaseSensitivity, + ) -> bool; /// Returns whether this element matches `:empty`. /// diff --git a/servo/components/style/animation.rs b/servo/components/style/animation.rs index ab482862358b..c934d075245c 100644 --- a/servo/components/style/animation.rs +++ b/servo/components/style/animation.rs @@ -606,7 +606,6 @@ pub fn update_style_for_animation_frame(mut new_style: &mut Arc, true } /// Updates a single animation and associated style based on the current time. -/// If `damage` is provided, inserts the appropriate restyle damage. pub fn update_style_for_animation( context: &SharedStyleContext, animation: &Animation, @@ -796,8 +795,11 @@ where /// Update the style in the node when it finishes. #[cfg(feature = "servo")] -pub fn complete_expired_transitions(node: OpaqueNode, style: &mut Arc, - context: &SharedStyleContext) -> bool { +pub fn complete_expired_transitions( + node: OpaqueNode, + style: &mut Arc, + context: &SharedStyleContext, +) -> bool { let had_animations_to_expire; { let all_expired_animations = context.expired_animations.read(); diff --git a/servo/components/style/bloom.rs b/servo/components/style/bloom.rs index 2e85c61ff735..83063e194c76 100644 --- a/servo/components/style/bloom.rs +++ b/servo/components/style/bloom.rs @@ -98,10 +98,10 @@ where E: TElement, F: FnMut(u32), { - f(element.get_local_name().get_hash()); - f(element.get_namespace().get_hash()); + f(element.local_name().get_hash()); + f(element.namespace().get_hash()); - if let Some(id) = element.get_id() { + if let Some(id) = element.id() { f(id.get_hash()); } diff --git a/servo/components/style/context.rs b/servo/components/style/context.rs index 337afd65ceb0..e6a7b544ab0a 100644 --- a/servo/components/style/context.rs +++ b/servo/components/style/context.rs @@ -282,6 +282,7 @@ pub struct ElementCascadeInputs { impl ElementCascadeInputs { /// Construct inputs from previous cascade results, if any. + #[inline] pub fn new_from_element_data(data: &ElementData) -> Self { debug_assert!(data.has_styles()); ElementCascadeInputs { @@ -383,8 +384,9 @@ impl fmt::Display for TraversalStatistics { impl TraversalStatistics { /// Computes the traversal time given the start time in seconds. pub fn finish(&mut self, traversal: &D, parallel: bool, start: f64) - where E: TElement, - D: DomTraversal, + where + E: TElement, + D: DomTraversal, { let threshold = traversal.shared_context().options.style_statistics_threshold; let stylist = traversal.shared_context().stylist; @@ -447,23 +449,27 @@ pub enum SequentialTask { /// Entry to avoid an unused type parameter error on servo. Unused(SendElement), - /// Performs one of a number of possible tasks related to updating animations based on the - /// |tasks| field. These include updating CSS animations/transitions that changed as part - /// of the non-animation style traversal, and updating the computed effect properties. + /// Performs one of a number of possible tasks related to updating + /// animations based on the |tasks| field. These include updating CSS + /// animations/transitions that changed as part of the non-animation style + /// traversal, and updating the computed effect properties. #[cfg(feature = "gecko")] UpdateAnimations { /// The target element or pseudo-element. el: SendElement, - /// The before-change style for transitions. We use before-change style as the initial - /// value of its Keyframe. Required if |tasks| includes CSSTransitions. + /// The before-change style for transitions. We use before-change style + /// as the initial value of its Keyframe. Required if |tasks| includes + /// CSSTransitions. before_change_style: Option>, /// The tasks which are performed in this SequentialTask. tasks: UpdateAnimationsTasks }, - /// Performs one of a number of possible tasks as a result of animation-only restyle. - /// Currently we do only process for resolving descendant elements that were display:none - /// subtree for SMIL animation. + /// Performs one of a number of possible tasks as a result of animation-only + /// restyle. + /// + /// Currently we do only process for resolving descendant elements that were + /// display:none subtree for SMIL animation. #[cfg(feature = "gecko")] PostAnimation { /// The target element. @@ -491,17 +497,19 @@ impl SequentialTask { } } - /// Creates a task to update various animation-related state on - /// a given (pseudo-)element. + /// Creates a task to update various animation-related state on a given + /// (pseudo-)element. #[cfg(feature = "gecko")] - pub fn update_animations(el: E, - before_change_style: Option>, - tasks: UpdateAnimationsTasks) -> Self { + pub fn update_animations( + el: E, + before_change_style: Option>, + tasks: UpdateAnimationsTasks, + ) -> Self { use self::SequentialTask::*; UpdateAnimations { el: unsafe { SendElement::new(el) }, - before_change_style: before_change_style, - tasks: tasks, + before_change_style, + tasks, } } @@ -512,7 +520,7 @@ impl SequentialTask { use self::SequentialTask::*; PostAnimation { el: unsafe { SendElement::new(el) }, - tasks: tasks, + tasks, } } } diff --git a/servo/components/style/dom.rs b/servo/components/style/dom.rs index 6d4cd23a9341..087e56ac6c10 100644 --- a/servo/components/style/dom.rs +++ b/servo/components/style/dom.rs @@ -7,7 +7,7 @@ #![allow(unsafe_code)] #![deny(missing_docs)] -use {Atom, Namespace, LocalName}; +use {Atom, Namespace, LocalName, WeakAtom}; use applicable_declarations::ApplicableDeclarationBlock; use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; #[cfg(feature = "gecko")] use context::PostAnimationTasks; @@ -17,9 +17,6 @@ use element_state::ElementState; use font_metrics::FontMetricsProvider; use media_queries::Device; use properties::{AnimationRules, ComputedValues, PropertyDeclarationBlock}; -#[cfg(feature = "gecko")] use properties::LonghandId; -#[cfg(feature = "gecko")] use properties::animated_properties::AnimationValue; -use rule_tree::CascadeLevel; use selector_parser::{AttrValue, PseudoClassStringArg, PseudoElement, SelectorImpl}; use selectors::Element as SelectorsElement; use selectors::matching::{ElementSelectorFlags, QuirksMode, VisitedHandlingMode}; @@ -27,7 +24,6 @@ use selectors::sink::Push; use servo_arc::{Arc, ArcBorrow}; use shared_lock::Locked; use std::fmt; -#[cfg(feature = "gecko")] use hash::FnvHashMap; use std::fmt::Debug; use std::hash::Hash; use std::ops::Deref; @@ -439,49 +435,39 @@ pub trait TElement } /// Get this element's SMIL override declarations. - fn get_smil_override(&self) -> Option>> { - None - } - - /// Get this element's animation rule by the cascade level. - fn get_animation_rule_by_cascade(&self, - _cascade_level: CascadeLevel) - -> Option>> { + fn smil_override(&self) -> Option>> { None } /// Get the combined animation and transition rules. - fn get_animation_rules(&self) -> AnimationRules { + /// + /// FIXME(emilio): Is this really useful? + fn animation_rules(&self) -> AnimationRules { if !self.may_have_animations() { return AnimationRules(None, None) } - AnimationRules( - self.get_animation_rule(), - self.get_transition_rule(), - ) + AnimationRules(self.animation_rule(), self.transition_rule()) } /// Get this element's animation rule. - fn get_animation_rule(&self) - -> Option>> { + fn animation_rule(&self) -> Option>> { None } /// Get this element's transition rule. - fn get_transition_rule(&self) - -> Option>> { + fn transition_rule(&self) -> Option>> { None } /// Get this element's state, for non-tree-structural pseudos. - fn get_state(&self) -> ElementState; + fn state(&self) -> ElementState; /// Whether this element has an attribute with a given namespace. fn has_attr(&self, namespace: &Namespace, attr: &LocalName) -> bool; /// The ID for this element. - fn get_id(&self) -> Option; + fn id(&self) -> Option<&WeakAtom>; /// Internal iterator for the classes of this element. fn each_class(&self, callback: F) where F: FnMut(&Atom); @@ -779,12 +765,6 @@ pub trait TElement cut_off_inheritance } - /// Gets the current existing CSS transitions, by |property, end value| pairs in a FnvHashMap. - #[cfg(feature = "gecko")] - fn get_css_transitions_info( - &self, - ) -> FnvHashMap>; - /// Does a rough (and cheap) check for whether or not transitions might need to be updated that /// will quickly return false for the common case of no transitions specified or running. If /// this returns false, we definitely don't need to update transitions but if it returns true @@ -808,17 +788,6 @@ pub trait TElement after_change_style: &ComputedValues ) -> bool; - /// Returns true if we need to update transitions for the specified property on this element. - #[cfg(feature = "gecko")] - fn needs_transitions_update_per_property( - &self, - property: &LonghandId, - combined_duration: f32, - before_change_style: &ComputedValues, - after_change_style: &ComputedValues, - existing_transitions: &FnvHashMap> - ) -> bool; - /// Returns the value of the `xml:lang=""` attribute (or, if appropriate, /// the `lang=""` attribute) on this element. fn lang_attr(&self) -> Option; diff --git a/servo/components/style/dom_apis.rs b/servo/components/style/dom_apis.rs index c1c5c9891935..1a775853f904 100644 --- a/servo/components/style/dom_apis.rs +++ b/servo/components/style/dom_apis.rs @@ -349,9 +349,9 @@ where Component::LocalName(LocalName { ref name, ref lower_name }) => { collect_all_elements::(root, results, |element| { if element.is_html_element_in_html_document() { - element.get_local_name() == lower_name.borrow() + element.local_name() == lower_name.borrow() } else { - element.get_local_name() == name.borrow() + element.local_name() == name.borrow() } }) } diff --git a/servo/components/style/gecko/snapshot.rs b/servo/components/style/gecko/snapshot.rs index c108b9851efd..f51d031a5270 100644 --- a/servo/components/style/gecko/snapshot.rs +++ b/servo/components/style/gecko/snapshot.rs @@ -5,6 +5,7 @@ //! A gecko snapshot, that stores the element attributes and state before they //! change in order to properly calculate restyle hints. +use WeakAtom; use dom::TElement; use element_state::ElementState; use gecko::snapshot_helpers; @@ -80,17 +81,20 @@ impl GeckoElementSnapshot { } /// selectors::Element::attr_matches - pub fn attr_matches(&self, - ns: &NamespaceConstraint<&Namespace>, - local_name: &Atom, - operation: &AttrSelectorOperation<&Atom>) - -> bool { + pub fn attr_matches( + &self, + ns: &NamespaceConstraint<&Namespace>, + local_name: &Atom, + operation: &AttrSelectorOperation<&Atom>, + ) -> bool { unsafe { match *operation { AttrSelectorOperation::Exists => { - bindings:: Gecko_SnapshotHasAttr(self, - ns.atom_or_null(), - local_name.as_ptr()) + bindings:: Gecko_SnapshotHasAttr( + self, + ns.atom_or_null(), + local_name.as_ptr(), + ) } AttrSelectorOperation::WithValue { operator, case_sensitivity, expected_value } => { let ignore_case = match case_sensitivity { @@ -163,7 +167,7 @@ impl ElementSnapshot for GeckoElementSnapshot { } #[inline] - fn id_attr(&self) -> Option { + fn id_attr(&self) -> Option<&WeakAtom> { if !self.has_any(Flags::Id) { return None } @@ -172,10 +176,11 @@ impl ElementSnapshot for GeckoElementSnapshot { bindings::Gecko_SnapshotAtomAttrValue(self, atom!("id").as_ptr()) }; + // FIXME(emilio): This should assert, since this flag is exact. if ptr.is_null() { None } else { - Some(Atom::from(ptr)) + Some(unsafe { WeakAtom::new(ptr) }) } } diff --git a/servo/components/style/gecko/wrapper.rs b/servo/components/style/gecko/wrapper.rs index adac5713cb48..0c5e283cee74 100644 --- a/servo/components/style/gecko/wrapper.rs +++ b/servo/components/style/gecko/wrapper.rs @@ -84,7 +84,6 @@ use std::cell::RefCell; use std::fmt; use std::hash::{Hash, Hasher}; use std::mem; -use std::ops::DerefMut; use std::ptr; use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace}; use stylist::CascadeData; @@ -226,7 +225,7 @@ impl<'ln> GeckoNode<'ln> { } if let Some(parent) = parent_el { - if parent.has_shadow_root() || parent.get_xbl_binding().is_some() { + if parent.shadow_root().is_some() || parent.xbl_binding().is_some() { return false; } } @@ -420,7 +419,7 @@ impl<'lb> GeckoXBLBinding<'lb> { // This duplicates the logic in Gecko's // nsBindingManager::GetBindingWithContent. - fn get_binding_with_content(&self) -> Option { + fn binding_with_content(&self) -> Option { let mut binding = *self; loop { if !binding.anon_content().is_null() { @@ -455,8 +454,8 @@ pub struct GeckoElement<'le>(pub &'le RawGeckoElement); impl<'le> fmt::Debug for GeckoElement<'le> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "<{}", self.get_local_name())?; - if let Some(id) = self.get_id() { + write!(f, "<{}", self.local_name())?; + if let Some(id) = self.id() { write!(f, " id={}", id)?; } @@ -519,22 +518,23 @@ impl<'le> GeckoElement<'le> { } /// Returns true if this element has a shadow root. - fn has_shadow_root(&self) -> bool { - self.get_extended_slots() - .map_or(false, |slots| !slots.mShadowRoot.mRawPtr.is_null()) + #[inline] + fn shadow_root(&self) -> Option<&structs::ShadowRoot> { + let slots = self.extended_slots()?; + unsafe { slots.mShadowRoot.mRawPtr.as_ref() } } /// Returns a reference to the DOM slots for this Element, if they exist. - fn get_dom_slots(&self) -> Option<&structs::FragmentOrElement_nsDOMSlots> { + fn dom_slots(&self) -> Option<&structs::FragmentOrElement_nsDOMSlots> { let slots = self.as_node().0.mSlots as *const structs::FragmentOrElement_nsDOMSlots; unsafe { slots.as_ref() } } /// Returns a reference to the extended DOM slots for this Element. - fn get_extended_slots( + fn extended_slots( &self, ) -> Option<&structs::FragmentOrElement_nsExtendedDOMSlots> { - self.get_dom_slots().and_then(|s| unsafe { + self.dom_slots().and_then(|s| unsafe { (s._base.mExtendedSlots.mPtr as *const structs::FragmentOrElement_nsExtendedDOMSlots).as_ref() }) } @@ -545,7 +545,7 @@ impl<'le> GeckoElement<'le> { } #[inline] - fn get_xbl_binding(&self) -> Option> { + fn xbl_binding(&self) -> Option> { if !self.may_be_in_binding_manager() { return None; } @@ -554,21 +554,20 @@ impl<'le> GeckoElement<'le> { } #[inline] - fn get_xbl_binding_with_content(&self) -> Option> { - self.get_xbl_binding() - .and_then(|b| b.get_binding_with_content()) + fn xbl_binding_with_content(&self) -> Option> { + self.xbl_binding().and_then(|b| b.binding_with_content()) } #[inline] fn has_xbl_binding_with_content(&self) -> bool { - !self.get_xbl_binding_with_content().is_none() + !self.xbl_binding_with_content().is_none() } /// This and has_xbl_binding_parent duplicate the logic in Gecko's virtual /// nsINode::GetBindingParent function, which only has two implementations: /// one for XUL elements, and one for other elements. We just hard code in /// our knowledge of those two implementations here. - fn get_xbl_binding_parent(&self) -> Option { + fn xbl_binding_parent(&self) -> Option { if self.is_xul_element() { // FIXME(heycam): Having trouble with bindgen on nsXULElement, // where the binding parent is stored in a member variable @@ -578,7 +577,7 @@ impl<'le> GeckoElement<'le> { } } else { let binding_parent = unsafe { - self.get_non_xul_xbl_binding_parent_raw_content().as_ref() + self.non_xul_xbl_binding_parent_raw_content().as_ref() }.map(GeckoNode::from_content).and_then(|n| n.as_element()); debug_assert!(binding_parent == unsafe { @@ -588,9 +587,9 @@ impl<'le> GeckoElement<'le> { } } - fn get_non_xul_xbl_binding_parent_raw_content(&self) -> *mut nsIContent { + fn non_xul_xbl_binding_parent_raw_content(&self) -> *mut nsIContent { debug_assert!(!self.is_xul_element()); - self.get_extended_slots() + self.extended_slots() .map_or(ptr::null_mut(), |slots| slots._base.mBindingParent) } @@ -601,7 +600,7 @@ impl<'le> GeckoElement<'le> { // rather than in slots. So just get it through FFI for now. unsafe { bindings::Gecko_GetBindingParent(self.0).is_some() } } else { - !self.get_non_xul_xbl_binding_parent_raw_content().is_null() + !self.non_xul_xbl_binding_parent_raw_content().is_null() } } @@ -615,36 +614,13 @@ impl<'le> GeckoElement<'le> { self.namespace_id() == (structs::root::kNameSpaceID_XUL as i32) } - /// Sets the specified element data, return any existing data. - /// - /// Like `ensure_data`, only safe to call with exclusive access to the - /// element. - pub unsafe fn set_data(&self, replace_data: Option) -> Option { - match (self.get_data(), replace_data) { - (Some(old), Some(replace_data)) => { - Some(mem::replace(old.borrow_mut().deref_mut(), replace_data)) - } - (Some(old), None) => { - let old_data = mem::replace(old.borrow_mut().deref_mut(), ElementData::default()); - self.0.mServoData.set(ptr::null_mut()); - Some(old_data) - } - (None, Some(replace_data)) => { - let ptr = Box::into_raw(Box::new(AtomicRefCell::new(replace_data))); - self.0.mServoData.set(ptr); - None - } - (None, None) => None, - } - } - #[inline] fn has_id(&self) -> bool { self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasID) } #[inline] - fn get_state_internal(&self) -> u64 { + fn state_internal(&self) -> u64 { if !self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasLockedStyleStates) { return self.0.mState.mStates; } @@ -660,8 +636,7 @@ impl<'le> GeckoElement<'le> { #[inline] fn may_have_class(&self) -> bool { - self.as_node() - .get_bool_flag(nsINode_BooleanFlag::ElementMayHaveClass) + self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementMayHaveClass) } #[inline] @@ -672,7 +647,7 @@ impl<'le> GeckoElement<'le> { } #[inline] - fn get_before_or_after_pseudo(&self, is_before: bool) -> Option { + fn before_or_after_pseudo(&self, is_before: bool) -> Option { if !self.has_properties() { return None; } @@ -682,12 +657,11 @@ impl<'le> GeckoElement<'le> { #[inline] fn may_have_style_attribute(&self) -> bool { - self.as_node() - .get_bool_flag(nsINode_BooleanFlag::ElementMayHaveStyle) + self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementMayHaveStyle) } #[inline] - fn get_document_theme(&self) -> DocumentTheme { + fn document_theme(&self) -> DocumentTheme { let node = self.as_node(); unsafe { Gecko_GetDocumentLWTheme(node.owner_doc().0) } } @@ -771,6 +745,76 @@ impl<'le> GeckoElement<'le> { self.is_in_native_anonymous_subtree() || (!self.is_in_shadow_tree() && self.has_xbl_binding_parent()) } + + fn css_transitions_info(&self) -> FnvHashMap> { + use gecko_bindings::bindings::Gecko_ElementTransitions_EndValueAt; + use gecko_bindings::bindings::Gecko_ElementTransitions_Length; + + let collection_length = + unsafe { Gecko_ElementTransitions_Length(self.0) } as usize; + let mut map = FnvHashMap::with_capacity_and_hasher( + collection_length, + Default::default() + ); + + for i in 0..collection_length { + let raw_end_value = unsafe { + Gecko_ElementTransitions_EndValueAt(self.0, i) + }; + + let end_value = AnimationValue::arc_from_borrowed(&raw_end_value) + .expect("AnimationValue not found in ElementTransitions"); + + let property = end_value.id(); + map.insert(property, end_value.clone_arc()); + } + map + } + + fn needs_transitions_update_per_property( + &self, + longhand_id: &LonghandId, + combined_duration: f32, + before_change_style: &ComputedValues, + after_change_style: &ComputedValues, + existing_transitions: &FnvHashMap>, + ) -> bool { + use values::animated::{Animate, Procedure}; + + // If there is an existing transition, update only if the end value + // differs. + // + // If the end value has not changed, we should leave the currently + // running transition as-is since we don't want to interrupt its timing + // function. + if let Some(ref existing) = existing_transitions.get(longhand_id) { + let after_value = + AnimationValue::from_computed_values( + longhand_id, + after_change_style + ).unwrap(); + + return ***existing != after_value + } + + let from = AnimationValue::from_computed_values( + &longhand_id, + before_change_style, + ); + let to = AnimationValue::from_computed_values( + &longhand_id, + after_change_style, + ); + + debug_assert_eq!(to.is_some(), from.is_some()); + + combined_duration > 0.0f32 && + from != to && + from.unwrap().animate( + to.as_ref().unwrap(), + Procedure::Interpolate { progress: 0.5 } + ).is_ok() + } } /// Converts flags from the layout used by rust-selectors to the layout used @@ -928,11 +972,11 @@ impl<'le> TElement for GeckoElement<'le> { } fn before_pseudo_element(&self) -> Option { - self.get_before_or_after_pseudo(/* is_before = */ true) + self.before_or_after_pseudo(/* is_before = */ true) } fn after_pseudo_element(&self) -> Option { - self.get_before_or_after_pseudo(/* is_before = */ false) + self.before_or_after_pseudo(/* is_before = */ false) } /// Ensure this accurately represents the rules that an element may ever @@ -946,11 +990,11 @@ impl<'le> TElement for GeckoElement<'le> { return self.as_node().owner_doc().as_node(); } - if self.get_xbl_binding().is_some() { + if self.xbl_binding().is_some() { return self.as_node(); } - if let Some(parent) = self.get_xbl_binding_parent() { + if let Some(parent) = self.xbl_binding_parent() { return parent.as_node(); } @@ -1059,9 +1103,9 @@ impl<'le> TElement for GeckoElement<'le> { unsafe { Gecko_UnsetDirtyStyleAttr(self.0) }; } - fn get_smil_override(&self) -> Option>> { + fn smil_override(&self) -> Option>> { unsafe { - let slots = self.get_extended_slots()?; + let slots = self.extended_slots()?; let base_declaration: &structs::DeclarationBlock = slots.mSMILOverrideStyleDeclaration.mRawPtr.as_ref()?; @@ -1083,32 +1127,17 @@ impl<'le> TElement for GeckoElement<'le> { } } - fn get_animation_rule_by_cascade( - &self, - cascade_level: ServoCascadeLevel, - ) -> Option>> { - match cascade_level { - ServoCascadeLevel::Animations => self.get_animation_rule(), - ServoCascadeLevel::Transitions => self.get_transition_rule(), - _ => panic!("Unsupported cascade level for getting the animation rule") - } - } - - fn get_animation_rule( - &self, - ) -> Option>> { + fn animation_rule(&self) -> Option>> { get_animation_rule(self, CascadeLevel::Animations) } - fn get_transition_rule( - &self, - ) -> Option>> { + fn transition_rule(&self) -> Option>> { get_animation_rule(self, CascadeLevel::Transitions) } #[inline] - fn get_state(&self) -> ElementState { - ElementState::from_bits_truncate(self.get_state_internal()) + fn state(&self) -> ElementState { + ElementState::from_bits_truncate(self.state_internal()) } #[inline] @@ -1118,7 +1147,9 @@ impl<'le> TElement for GeckoElement<'le> { } } - fn get_id(&self) -> Option { + // FIXME(emilio): we should probably just return a reference to the Atom. + #[inline] + fn id(&self) -> Option<&WeakAtom> { if !self.has_id() { return None; } @@ -1127,10 +1158,12 @@ impl<'le> TElement for GeckoElement<'le> { bindings::Gecko_AtomAttrValue(self.0, atom!("id").as_ptr()) }; + // FIXME(emilio): Pretty sure the has_id flag is exact and we could + // assert here. if ptr.is_null() { None } else { - Some(Atom::from(ptr)) + Some(unsafe { WeakAtom::new(ptr) }) } } @@ -1203,7 +1236,7 @@ impl<'le> TElement for GeckoElement<'le> { } fn is_visited_link(&self) -> bool { - self.get_state().intersects(ElementState::IN_VISITED_STATE) + self.state().intersects(ElementState::IN_VISITED_STATE) } #[inline] @@ -1296,10 +1329,10 @@ impl<'le> TElement for GeckoElement<'le> { if !pseudo.is_before_or_after() { return false; } - return self.parent_element() - .map_or(false, |p| { - p.as_node() - .get_bool_flag(nsINode_BooleanFlag::ElementHasAnimations) + // FIXME(emilio): When would the parent of a ::before / ::after + // pseudo-element be null? + return self.parent_element().map_or(false, |p| { + p.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasAnimations) }); } self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasAnimations) @@ -1377,7 +1410,7 @@ impl<'le> TElement for GeckoElement<'le> { let mut current = Some(self.rule_hash_target()); while let Some(element) = current { - if let Some(binding) = element.get_xbl_binding() { + if let Some(binding) = element.xbl_binding() { binding.each_xbl_cascade_data(&mut f); // If we're not looking at our original element, allow the @@ -1396,7 +1429,7 @@ impl<'le> TElement for GeckoElement<'le> { break; } - current = element.get_xbl_binding_parent(); + current = element.xbl_binding_parent(); } // If current has something, this means we cut off inheritance at some @@ -1405,37 +1438,10 @@ impl<'le> TElement for GeckoElement<'le> { } fn xbl_binding_anonymous_content(&self) -> Option> { - self.get_xbl_binding_with_content() + self.xbl_binding_with_content() .map(|b| unsafe { GeckoNode::from_content(&*b.anon_content()) }) } - fn get_css_transitions_info( - &self, - ) -> FnvHashMap> { - use gecko_bindings::bindings::Gecko_ElementTransitions_EndValueAt; - use gecko_bindings::bindings::Gecko_ElementTransitions_Length; - - let collection_length = - unsafe { Gecko_ElementTransitions_Length(self.0) } as usize; - let mut map = FnvHashMap::with_capacity_and_hasher( - collection_length, - Default::default() - ); - - for i in 0..collection_length { - let raw_end_value = unsafe { - Gecko_ElementTransitions_EndValueAt(self.0, i) - }; - - let end_value = AnimationValue::arc_from_borrowed(&raw_end_value) - .expect("AnimationValue not found in ElementTransitions"); - - let property = end_value.id(); - map.insert(property, end_value.clone_arc()); - } - map - } - fn might_need_transitions_update( &self, old_values: Option<&ComputedValues>, @@ -1473,7 +1479,7 @@ impl<'le> TElement for GeckoElement<'le> { fn needs_transitions_update( &self, before_change_style: &ComputedValues, - after_change_style: &ComputedValues + after_change_style: &ComputedValues, ) -> bool { use gecko_bindings::structs::nsCSSPropertyID; use properties::LonghandIdSet; @@ -1485,7 +1491,7 @@ impl<'le> TElement for GeckoElement<'le> { let after_change_box_style = after_change_style.get_box(); let transitions_count = after_change_box_style.transition_property_count(); - let existing_transitions = self.get_css_transitions_info(); + let existing_transitions = self.css_transitions_info(); // Check if this property is none, custom or unknown. let is_none_or_custom_property = |property: nsCSSPropertyID| -> bool { @@ -1545,51 +1551,6 @@ impl<'le> TElement for GeckoElement<'le> { }) } - fn needs_transitions_update_per_property( - &self, - longhand_id: &LonghandId, - combined_duration: f32, - before_change_style: &ComputedValues, - after_change_style: &ComputedValues, - existing_transitions: &FnvHashMap>, - ) -> bool { - use values::animated::{Animate, Procedure}; - - // If there is an existing transition, update only if the end value - // differs. - // - // If the end value has not changed, we should leave the currently - // running transition as-is since we don't want to interrupt its timing - // function. - if let Some(ref existing) = existing_transitions.get(longhand_id) { - let after_value = - AnimationValue::from_computed_values( - longhand_id, - after_change_style - ).unwrap(); - - return ***existing != after_value - } - - let from = AnimationValue::from_computed_values( - &longhand_id, - before_change_style, - ); - let to = AnimationValue::from_computed_values( - &longhand_id, - after_change_style, - ); - - debug_assert_eq!(to.is_some(), from.is_some()); - - combined_duration > 0.0f32 && - from != to && - from.unwrap().animate( - to.as_ref().unwrap(), - Procedure::Interpolate { progress: 0.5 } - ).is_ok() - } - #[inline] fn lang_attr(&self) -> Option { let ptr = unsafe { bindings::Gecko_LangValue(self.0) }; @@ -1620,7 +1581,7 @@ impl<'le> TElement for GeckoElement<'le> { } fn is_html_document_body_element(&self) -> bool { - if self.get_local_name() != &*local_name!("body") { + if self.local_name() != &*local_name!("body") { return false; } @@ -1686,15 +1647,15 @@ impl<'le> TElement for GeckoElement<'le> { let ns = self.namespace_id(); // elements get a default MozCenterOrInherit which may get overridden if ns == structs::kNameSpaceID_XHTML as i32 { - if self.get_local_name().as_ptr() == atom!("th").as_ptr() { + if self.local_name().as_ptr() == atom!("th").as_ptr() { hints.push(TH_RULE.clone()); - } else if self.get_local_name().as_ptr() == atom!("table").as_ptr() && + } else if self.local_name().as_ptr() == atom!("table").as_ptr() && self.as_node().owner_doc().quirks_mode() == QuirksMode::Quirks { hints.push(TABLE_COLOR_RULE.clone()); } } if ns == structs::kNameSpaceID_SVG as i32 { - if self.get_local_name().as_ptr() == atom!("text").as_ptr() { + if self.local_name().as_ptr() == atom!("text").as_ptr() { hints.push(SVG_TEXT_DISABLE_ZOOM_RULE.clone()); } } @@ -1739,7 +1700,7 @@ impl<'le> TElement for GeckoElement<'le> { ); } - let active = self.get_state().intersects(NonTSPseudoClass::Active.state_flag()); + let active = self.state().intersects(NonTSPseudoClass::Active.state_flag()); if active { let declarations = unsafe { Gecko_GetActiveLinkAttrDeclarationBlock(self.0) }; let declarations: Option<&RawOffsetArc>> = @@ -1771,7 +1732,7 @@ impl<'le> TElement for GeckoElement<'le> { } // MathML's default lang has precedence over both `lang` and `xml:lang` if ns == structs::kNameSpaceID_MathML as i32 { - if self.get_local_name().as_ptr() == atom!("math").as_ptr() { + if self.local_name().as_ptr() == atom!("math").as_ptr() { hints.push(MATHML_LANG_RULE.clone()); } } @@ -1818,7 +1779,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { #[inline] fn assigned_slot(&self) -> Option { - let slot = self.get_extended_slots()?._base.mAssignedSlot.mRawPtr; + let slot = self.extended_slots()?._base.mAssignedSlot.mRawPtr; unsafe { Some(GeckoElement(&slot.as_ref()?._base._base._base._base)) @@ -1964,14 +1925,14 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { } #[inline] - fn get_local_name(&self) -> &WeakAtom { + fn local_name(&self) -> &WeakAtom { unsafe { WeakAtom::new(self.as_node().node_info().mInner.mName) } } #[inline] - fn get_namespace(&self) -> &WeakNamespace { + fn namespace(&self) -> &WeakNamespace { unsafe { let namespace_manager = structs::nsContentUtils_sNameSpaceManager; WeakNamespace::new((*namespace_manager).mURIArray[self.namespace_id() as usize].mRawPtr) @@ -2037,7 +1998,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { NonTSPseudoClass::Active | NonTSPseudoClass::Hover | NonTSPseudoClass::MozAutofillPreview => { - self.get_state().intersects(pseudo_class.state_flag()) + self.state().intersects(pseudo_class.state_flag()) }, NonTSPseudoClass::AnyLink => self.is_link(), NonTSPseudoClass::Link => { @@ -2085,13 +2046,13 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { self.is_html_element_in_html_document() } NonTSPseudoClass::MozLWTheme => { - self.get_document_theme() != DocumentTheme::Doc_Theme_None + self.document_theme() != DocumentTheme::Doc_Theme_None } NonTSPseudoClass::MozLWThemeBrightText => { - self.get_document_theme() == DocumentTheme::Doc_Theme_Bright + self.document_theme() == DocumentTheme::Doc_Theme_Bright } NonTSPseudoClass::MozLWThemeDarkText => { - self.get_document_theme() == DocumentTheme::Doc_Theme_Dark + self.document_theme() == DocumentTheme::Doc_Theme_Dark } NonTSPseudoClass::MozWindowInactive => { let state_bit = DocumentState::NS_DOCUMENT_STATE_WINDOW_INACTIVE; @@ -2130,8 +2091,8 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { } NonTSPseudoClass::Dir(ref dir) => { match **dir { - Direction::Ltr => self.get_state().intersects(ElementState::IN_LTR_STATE), - Direction::Rtl => self.get_state().intersects(ElementState::IN_RTL_STATE), + Direction::Ltr => self.state().intersects(ElementState::IN_LTR_STATE), + Direction::Rtl => self.state().intersects(ElementState::IN_RTL_STATE), Direction::Other(..) => false, } } @@ -2154,7 +2115,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { #[inline] fn is_link(&self) -> bool { - self.get_state().intersects(NonTSPseudoClass::AnyLink.state_flag()) + self.state().intersects(NonTSPseudoClass::AnyLink.state_flag()) } #[inline] @@ -2197,7 +2158,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { #[inline] fn is_html_slot_element(&self) -> bool { self.is_html_element() && - self.get_local_name().as_ptr() == local_name!("slot").as_ptr() + self.local_name().as_ptr() == local_name!("slot").as_ptr() } #[inline] @@ -2216,8 +2177,8 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { // If this element is the shadow root of an use-element shadow // tree, according to the spec, we should not match rules // cross the shadow DOM boundary. - e.get_local_name() == &*local_name!("use") && - e.get_namespace() == &*ns!("http://www.w3.org/2000/svg") + e.local_name() == &*local_name!("use") && + e.namespace() == &*ns!("http://www.w3.org/2000/svg") }, None => false, } diff --git a/servo/components/style/invalidation/element/element_wrapper.rs b/servo/components/style/invalidation/element/element_wrapper.rs index 2fb0a4f84121..5483c2756972 100644 --- a/servo/components/style/invalidation/element/element_wrapper.rs +++ b/servo/components/style/invalidation/element/element_wrapper.rs @@ -5,7 +5,7 @@ //! A wrapper over an element and a snapshot, that allows us to selector-match //! against a past state of the element. -use {Atom, CaseSensitivityExt, LocalName, Namespace}; +use {Atom, CaseSensitivityExt, LocalName, Namespace, WeakAtom}; use dom::TElement; use element_state::ElementState; use selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl, Snapshot, SnapshotMap, AttrValue}; @@ -44,7 +44,7 @@ pub trait ElementSnapshot : Sized { /// The ID attribute per this snapshot. Should only be called if /// `has_attrs()` returns true. - fn id_attr(&self) -> Option; + fn id_attr(&self) -> Option<&WeakAtom>; /// Whether this snapshot contains the class `name`. Should only be called /// if `has_attrs()` returns true. @@ -53,7 +53,8 @@ pub trait ElementSnapshot : Sized { /// A callback that should be called for each class of the snapshot. Should /// only be called if `has_attrs()` returns true. fn each_class(&self, F) - where F: FnMut(&Atom); + where + F: FnMut(&Atom); /// The `xml:lang=""` or `lang=""` attribute value per this snapshot. fn lang_attr(&self) -> Option; @@ -63,7 +64,8 @@ pub trait ElementSnapshot : Sized { /// selector-match against a past state of the element. #[derive(Clone)] pub struct ElementWrapper<'a, E> - where E: TElement, +where + E: TElement, { element: E, cached_snapshot: Cell>, @@ -109,7 +111,7 @@ where }; match snapshot.state() { - Some(state) => state ^ self.element.get_state(), + Some(state) => state ^ self.element.state(), None => ElementState::empty(), } } @@ -133,7 +135,8 @@ where } impl<'a, E> fmt::Debug for ElementWrapper<'a, E> - where E: TElement, +where + E: TElement, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { // Ignore other fields for now, can change later if needed. @@ -142,7 +145,8 @@ impl<'a, E> fmt::Debug for ElementWrapper<'a, E> } impl<'a, E> Element for ElementWrapper<'a, E> - where E: TElement, +where + E: TElement, { type Impl = SelectorImpl; @@ -186,7 +190,7 @@ impl<'a, E> Element for ElementWrapper<'a, E> } let state = match self.snapshot().and_then(|s| s.state()) { Some(snapshot_state) => snapshot_state, - None => self.element.get_state(), + None => self.element.state(), }; return state.contains(selector_flag); } @@ -291,27 +295,32 @@ impl<'a, E> Element for ElementWrapper<'a, E> .map(|e| ElementWrapper::new(e, self.snapshot_map)) } + #[inline] fn is_html_element_in_html_document(&self) -> bool { self.element.is_html_element_in_html_document() } + #[inline] fn is_html_slot_element(&self) -> bool { self.element.is_html_slot_element() } - fn get_local_name(&self) -> &::BorrowedLocalName { - self.element.get_local_name() + #[inline] + fn local_name(&self) -> &::BorrowedLocalName { + self.element.local_name() } - fn get_namespace(&self) -> &::BorrowedNamespaceUrl { - self.element.get_namespace() + #[inline] + fn namespace(&self) -> &::BorrowedNamespaceUrl { + self.element.namespace() } - fn attr_matches(&self, - ns: &NamespaceConstraint<&Namespace>, - local_name: &LocalName, - operation: &AttrSelectorOperation<&AttrValue>) - -> bool { + fn attr_matches( + &self, + ns: &NamespaceConstraint<&Namespace>, + local_name: &LocalName, + operation: &AttrSelectorOperation<&AttrValue>, + ) -> bool { match self.snapshot() { Some(snapshot) if snapshot.has_attrs() => { snapshot.attr_matches(ns, local_name, operation) diff --git a/servo/components/style/invalidation/element/state_and_attributes.rs b/servo/components/style/invalidation/element/state_and_attributes.rs index f272d22a71d8..10a7f07aef4d 100644 --- a/servo/components/style/invalidation/element/state_and_attributes.rs +++ b/servo/components/style/invalidation/element/state_and_attributes.rs @@ -5,7 +5,7 @@ //! An invalidation processor for style changes due to state and attribute //! changes. -use Atom; +use {Atom, WeakAtom}; use context::{QuirksMode, SharedStyleContext}; use data::ElementData; use dom::TElement; @@ -42,8 +42,8 @@ where snapshot: &'a Snapshot, quirks_mode: QuirksMode, lookup_element: E, - removed_id: Option<&'a Atom>, - added_id: Option<&'a Atom>, + removed_id: Option<&'a WeakAtom>, + added_id: Option<&'a WeakAtom>, classes_removed: &'a SmallVec<[Atom; 8]>, classes_added: &'a SmallVec<[Atom; 8]>, state_changes: ElementState, @@ -200,7 +200,7 @@ where let mut id_added = None; if snapshot.id_changed() { let old_id = snapshot.id_attr(); - let current_id = element.get_id(); + let current_id = element.id(); if old_id != current_id { id_removed = old_id; @@ -239,8 +239,8 @@ where snapshot: &snapshot, quirks_mode: self.shared_context.quirks_mode(), nth_index_cache: self.matching_context.nth_index_cache.as_mut().map(|c| &mut **c), - removed_id: id_removed.as_ref(), - added_id: id_added.as_ref(), + removed_id: id_removed, + added_id: id_added, classes_removed: &classes_removed, classes_added: &classes_added, descendant_invalidations, diff --git a/servo/components/style/invalidation/stylesheets.rs b/servo/components/style/invalidation/stylesheets.rs index 2b511ea15696..25dcb68e43aa 100644 --- a/servo/components/style/invalidation/stylesheets.rs +++ b/servo/components/style/invalidation/stylesheets.rs @@ -66,7 +66,7 @@ impl Invalidation { } } Invalidation::ID(ref id) => { - if let Some(ref element_id) = element.get_id() { + if let Some(ref element_id) = element.id() { if case_sensitivity.eq_atom(element_id, id) { return true; } @@ -84,7 +84,7 @@ impl Invalidation { // This could look at the quirks mode of the document, instead // of testing against both names, but it's probably not worth // it. - let local_name = element.get_local_name(); + let local_name = element.local_name(); return *local_name == **name || *local_name == **lower_name } } diff --git a/servo/components/style/matching.rs b/servo/components/style/matching.rs index 594717cc52a3..09b07ee02c6f 100644 --- a/servo/components/style/matching.rs +++ b/servo/components/style/matching.rs @@ -72,7 +72,7 @@ impl ChildCascadeRequirement { /// Determines which styles are being cascaded currently. #[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum CascadeVisitedMode { +enum CascadeVisitedMode { /// Cascade the regular, unvisited styles. Unvisited, /// Cascade the styles used when an element's relevant link is visited. A @@ -81,18 +81,115 @@ pub enum CascadeVisitedMode { Visited, } -impl CascadeVisitedMode { - /// Returns whether the cascade should filter to only visited dependent - /// properties based on the cascade mode. - pub fn visited_dependent_only(&self) -> bool { - *self == CascadeVisitedMode::Visited - } -} - trait PrivateMatchMethods: TElement { + /// Updates the rule nodes without re-running selector matching, using just + /// the rule tree, for a specific visited mode. + /// + /// Returns true if an !important rule was replaced. + fn replace_rules_internal( + &self, + replacements: RestyleHint, + context: &mut StyleContext, + cascade_visited: CascadeVisitedMode, + cascade_inputs: &mut ElementCascadeInputs, + ) -> bool { + use properties::PropertyDeclarationBlock; + use shared_lock::Locked; + + debug_assert!(replacements.intersects(RestyleHint::replacements()) && + (replacements & !RestyleHint::replacements()).is_empty()); + + let stylist = &context.shared.stylist; + let guards = &context.shared.guards; + + let primary_rules = + match cascade_visited { + CascadeVisitedMode::Unvisited => cascade_inputs.primary.rules.as_mut(), + CascadeVisitedMode::Visited => cascade_inputs.primary.visited_rules.as_mut(), + }; + + let primary_rules = match primary_rules { + Some(r) => r, + None => return false, + }; + + let replace_rule_node = |level: CascadeLevel, + pdb: Option>>, + path: &mut StrongRuleNode| -> bool { + let mut important_rules_changed = false; + let new_node = + stylist.rule_tree().update_rule_at_level( + level, + pdb, + path, + guards, + &mut important_rules_changed, + ); + if let Some(n) = new_node { + *path = n; + } + important_rules_changed + }; + + if !context.shared.traversal_flags.for_animation_only() { + let mut result = false; + if replacements.contains(RestyleHint::RESTYLE_STYLE_ATTRIBUTE) { + let style_attribute = self.style_attribute(); + result |= replace_rule_node( + CascadeLevel::StyleAttributeNormal, + style_attribute, + primary_rules, + ); + result |= replace_rule_node( + CascadeLevel::StyleAttributeImportant, + style_attribute, + primary_rules, + ); + // FIXME(emilio): Still a hack! + self.unset_dirty_style_attribute(); + } + return result; + } + + // Animation restyle hints are processed prior to other restyle + // hints in the animation-only traversal. + // + // Non-animation restyle hints will be processed in a subsequent + // normal traversal. + if replacements.intersects(RestyleHint::for_animations()) { + debug_assert!(context.shared.traversal_flags.for_animation_only()); + + if replacements.contains(RestyleHint::RESTYLE_SMIL) { + replace_rule_node( + CascadeLevel::SMILOverride, + self.smil_override(), + primary_rules, + ); + } + + if replacements.contains(RestyleHint::RESTYLE_CSS_TRANSITIONS) { + replace_rule_node( + CascadeLevel::Transitions, + self.transition_rule().as_ref().map(|a| a.borrow_arc()), + primary_rules, + ); + } + + if replacements.contains(RestyleHint::RESTYLE_CSS_ANIMATIONS) { + replace_rule_node( + CascadeLevel::Animations, + self.animation_rule().as_ref().map(|a| a.borrow_arc()), + primary_rules, + ); + } + } + + false + } + /// If there is no transition rule in the ComputedValues, it returns None. #[cfg(feature = "gecko")] - fn get_after_change_style( + fn after_change_style( &self, context: &mut StyleContext, primary_style: &Arc @@ -130,34 +227,54 @@ trait PrivateMatchMethods: TElement { fn needs_animations_update( &self, context: &mut StyleContext, - old_values: Option<&Arc>, + old_values: Option<&ComputedValues>, new_values: &ComputedValues, ) -> bool { let new_box_style = new_values.get_box(); let has_new_animation_style = new_box_style.specifies_animations(); - let has_animations = self.has_css_animations(); - old_values.map_or(has_new_animation_style, |old| { - let old_box_style = old.get_box(); - let old_display_style = old_box_style.clone_display(); - let new_display_style = new_box_style.clone_display(); + let old = match old_values { + Some(old) => old, + None => return has_new_animation_style, + }; - // If the traverse is triggered by CSS rule changes, we need to - // try to update all CSS animations on the element if the element - // has or will have CSS animation style regardless of whether the - // animation is running or not. - // TODO: We should check which @keyframes changed/added/deleted - // and update only animations corresponding to those @keyframes. - (context.shared.traversal_flags.contains(TraversalFlags::ForCSSRuleChanges) && - (has_new_animation_style || has_animations)) || - !old_box_style.animations_equals(new_box_style) || - (old_display_style == Display::None && - new_display_style != Display::None && - has_new_animation_style) || - (old_display_style != Display::None && - new_display_style == Display::None && - has_animations) - }) + let old_box_style = old.get_box(); + + let keyframes_could_have_changed = + context.shared.traversal_flags.contains(TraversalFlags::ForCSSRuleChanges); + + // If the traversal is triggered due to changes in CSS rules changes, we + // need to try to update all CSS animations on the element if the + // element has or will have CSS animation style regardless of whether + // the animation is running or not. + // + // TODO: We should check which @keyframes were added/changed/deleted and + // update only animations corresponding to those @keyframes. + if keyframes_could_have_changed && + (has_new_animation_style || self.has_css_animations()) + { + return true; + } + + // If the animations changed, well... + if !old_box_style.animations_equals(new_box_style) { + return true; + } + + let old_display = old_box_style.clone_display(); + let new_display = new_box_style.clone_display(); + + // If we were display: none, we may need to trigger animations. + if old_display == Display::None && new_display != Display::None { + return has_new_animation_style; + } + + // If we are becoming display: none, we may need to stop animations. + if old_display != Display::None && new_display == Display::None { + return self.has_css_animations(); + } + + false } /// Create a SequentialTask for resolving descendants in a SMIL display property @@ -192,12 +309,14 @@ trait PrivateMatchMethods: TElement { } #[cfg(feature = "gecko")] - fn process_animations(&self, - context: &mut StyleContext, - old_values: &mut Option>, - new_values: &mut Arc, - restyle_hint: RestyleHint, - important_rules_changed: bool) { + fn process_animations( + &self, + context: &mut StyleContext, + old_values: &mut Option>, + new_values: &mut Arc, + restyle_hint: RestyleHint, + important_rules_changed: bool, + ) { use context::UpdateAnimationsTasks; if context.shared.traversal_flags.for_animation_only() { @@ -214,14 +333,14 @@ trait PrivateMatchMethods: TElement { // in addition to the unvisited styles. let mut tasks = UpdateAnimationsTasks::empty(); - if self.needs_animations_update(context, old_values.as_ref(), new_values) { + if self.needs_animations_update(context, old_values.as_ref().map(|s| &**s), new_values) { tasks.insert(UpdateAnimationsTasks::CSS_ANIMATIONS); } let before_change_style = if self.might_need_transitions_update(old_values.as_ref().map(|s| &**s), new_values) { let after_change_style = if self.has_css_transitions() { - self.get_after_change_style(context, new_values) + self.after_change_style(context, new_values) } else { None }; @@ -235,8 +354,10 @@ trait PrivateMatchMethods: TElement { let after_change_style_ref = after_change_style.as_ref().unwrap_or(&new_values); - self.needs_transitions_update(old_values.as_ref().unwrap(), - after_change_style_ref) + self.needs_transitions_update( + old_values.as_ref().unwrap(), + after_change_style_ref, + ) }; if needs_transitions_update { @@ -245,7 +366,8 @@ trait PrivateMatchMethods: TElement { } tasks.insert(UpdateAnimationsTasks::CSS_TRANSITIONS); - // We need to clone old_values into SequentialTask, so we can use it later. + // We need to clone old_values into SequentialTask, so we can + // use it later. old_values.clone() } else { None @@ -273,29 +395,38 @@ trait PrivateMatchMethods: TElement { } #[cfg(feature = "servo")] - fn process_animations(&self, - context: &mut StyleContext, - old_values: &mut Option>, - new_values: &mut Arc, - _restyle_hint: RestyleHint, - _important_rules_changed: bool) { + fn process_animations( + &self, + context: &mut StyleContext, + old_values: &mut Option>, + new_values: &mut Arc, + _restyle_hint: RestyleHint, + _important_rules_changed: bool, + ) { use animation; use dom::TNode; let mut possibly_expired_animations = vec![]; let shared_context = context.shared; if let Some(ref mut old) = *old_values { - self.update_animations_for_cascade(shared_context, old, - &mut possibly_expired_animations, - &context.thread_local.font_metrics_provider); + // FIXME(emilio, #20116): This makes no sense. + self.update_animations_for_cascade( + shared_context, + old, + &mut possibly_expired_animations, + &context.thread_local.font_metrics_provider, + ); } let new_animations_sender = &context.thread_local.new_animations_sender; let this_opaque = self.as_node().opaque(); // Trigger any present animations if necessary. - animation::maybe_start_animations(&shared_context, - new_animations_sender, - this_opaque, &new_values); + animation::maybe_start_animations( + &shared_context, + new_animations_sender, + this_opaque, + &new_values, + ); // Trigger transitions if necessary. This will reset `new_values` back // to its old value if it did trigger a transition. @@ -303,10 +434,11 @@ trait PrivateMatchMethods: TElement { animation::start_transitions_if_applicable( new_animations_sender, this_opaque, - &**values, + &values, new_values, &shared_context.timer, - &possibly_expired_animations); + &possibly_expired_animations, + ); } } @@ -423,12 +555,20 @@ trait PrivateMatchMethods: TElement { ChildCascadeRequirement::MustCascadeChildrenIfInheritResetStyle } + // FIXME(emilio, #20116): It's not clear to me that the name of this method + // represents anything of what it does. + // + // Also, this function gets the old style, for some reason I don't really + // get, but the functions called (mainly update_style_for_animation) expects + // the new style, wtf? #[cfg(feature = "servo")] - fn update_animations_for_cascade(&self, - context: &SharedStyleContext, - style: &mut Arc, - possibly_expired_animations: &mut Vec<::animation::PropertyAnimation>, - font_metrics: &::font_metrics::FontMetricsProvider) { + fn update_animations_for_cascade( + &self, + context: &SharedStyleContext, + style: &mut Arc, + possibly_expired_animations: &mut Vec<::animation::PropertyAnimation>, + font_metrics: &::font_metrics::FontMetricsProvider, + ) { use animation::{self, Animation}; use dom::TNode; @@ -436,36 +576,38 @@ trait PrivateMatchMethods: TElement { let this_opaque = self.as_node().opaque(); animation::complete_expired_transitions(this_opaque, style, context); - // Merge any running transitions into the current style, and cancel them. - let had_running_animations = context.running_animations - .read() - .get(&this_opaque) - .is_some(); - if had_running_animations { - let mut all_running_animations = context.running_animations.write(); - for running_animation in all_running_animations.get_mut(&this_opaque).unwrap() { - // This shouldn't happen frequently, but under some - // circumstances mainly huge load or debug builds, the - // constellation might be delayed in sending the - // `TickAllAnimations` message to layout. - // - // Thus, we can't assume all the animations have been already - // updated by layout, because other restyle due to script might - // be triggered by layout before the animation tick. - // - // See #12171 and the associated PR for an example where this - // happened while debugging other release panic. - if !running_animation.is_expired() { - animation::update_style_for_animation::( - context, - running_animation, - style, - font_metrics, - ); - if let Animation::Transition(_, _, ref frame, _) = *running_animation { - possibly_expired_animations.push(frame.property_animation.clone()) - } - } + // Merge any running animations into the current style, and cancel them. + let had_running_animations = + context.running_animations.read().get(&this_opaque).is_some(); + if !had_running_animations { + return; + } + + let mut all_running_animations = context.running_animations.write(); + for running_animation in all_running_animations.get_mut(&this_opaque).unwrap() { + // This shouldn't happen frequently, but under some circumstances + // mainly huge load or debug builds, the constellation might be + // delayed in sending the `TickAllAnimations` message to layout. + // + // Thus, we can't assume all the animations have been already + // updated by layout, because other restyle due to script might be + // triggered by layout before the animation tick. + // + // See #12171 and the associated PR for an example where this + // happened while debugging other release panic. + if running_animation.is_expired() { + continue; + } + + animation::update_style_for_animation::( + context, + running_animation, + style, + font_metrics, + ); + + if let Animation::Transition(_, _, ref frame, _) = *running_animation { + possibly_expired_animations.push(frame.property_animation.clone()) } } } @@ -484,7 +626,7 @@ pub trait MatchMethods : TElement { /// Returns itself if the element has no parent. In practice this doesn't /// happen because the root element is blockified per spec, but it could /// happen if we decide to not blockify for roots of disconnected subtrees, - /// which is a kind of dubious beahavior. + /// which is a kind of dubious behavior. fn layout_parent(&self) -> Self { let mut current = self.clone(); loop { @@ -608,11 +750,9 @@ pub trait MatchMethods : TElement { // case. let pseudo = PseudoElement::from_eager_index(i); let new_pseudo_should_exist = - new.as_ref().map_or(false, - |s| pseudo.should_exist(s)); + new.as_ref().map_or(false, |s| pseudo.should_exist(s)); let old_pseudo_should_exist = - old.as_ref().map_or(false, - |s| pseudo.should_exist(s)); + old.as_ref().map_or(false, |s| pseudo.should_exist(s)); if new_pseudo_should_exist != old_pseudo_should_exist { data.damage |= RestyleDamage::reconstruct(); return cascade_requirement; @@ -630,10 +770,12 @@ pub trait MatchMethods : TElement { /// /// TODO(emilio): This is somewhat inefficient, because it doesn't take /// advantage of us knowing that the traversal is sequential. - fn apply_selector_flags(&self, - map: &mut SelectorFlagsMap, - element: &Self, - flags: ElementSelectorFlags) { + fn apply_selector_flags( + &self, + map: &mut SelectorFlagsMap, + element: &Self, + flags: ElementSelectorFlags, + ) { // Handle flags that apply to the element. let self_flags = flags.for_self(); if !self_flags.is_empty() { @@ -692,107 +834,6 @@ pub trait MatchMethods : TElement { result } - /// Updates the rule nodes without re-running selector matching, using just - /// the rule tree, for a specific visited mode. - /// - /// Returns true if an !important rule was replaced. - fn replace_rules_internal( - &self, - replacements: RestyleHint, - context: &mut StyleContext, - cascade_visited: CascadeVisitedMode, - cascade_inputs: &mut ElementCascadeInputs, - ) -> bool { - use properties::PropertyDeclarationBlock; - use shared_lock::Locked; - - debug_assert!(replacements.intersects(RestyleHint::replacements()) && - (replacements & !RestyleHint::replacements()).is_empty()); - - let stylist = &context.shared.stylist; - let guards = &context.shared.guards; - - let primary_rules = - match cascade_visited { - CascadeVisitedMode::Unvisited => cascade_inputs.primary.rules.as_mut(), - CascadeVisitedMode::Visited => cascade_inputs.primary.visited_rules.as_mut(), - }; - - let primary_rules = match primary_rules { - Some(r) => r, - None => return false, - }; - - let replace_rule_node = |level: CascadeLevel, - pdb: Option>>, - path: &mut StrongRuleNode| -> bool { - let mut important_rules_changed = false; - let new_node = stylist.rule_tree() - .update_rule_at_level(level, - pdb, - path, - guards, - &mut important_rules_changed); - if let Some(n) = new_node { - *path = n; - } - important_rules_changed - }; - - if !context.shared.traversal_flags.for_animation_only() { - let mut result = false; - if replacements.contains(RestyleHint::RESTYLE_STYLE_ATTRIBUTE) { - let style_attribute = self.style_attribute(); - result |= replace_rule_node(CascadeLevel::StyleAttributeNormal, - style_attribute, - primary_rules); - result |= replace_rule_node(CascadeLevel::StyleAttributeImportant, - style_attribute, - primary_rules); - // FIXME(emilio): Still a hack! - self.unset_dirty_style_attribute(); - } - return result; - } - - // Animation restyle hints are processed prior to other restyle - // hints in the animation-only traversal. - // - // Non-animation restyle hints will be processed in a subsequent - // normal traversal. - if replacements.intersects(RestyleHint::for_animations()) { - debug_assert!(context.shared.traversal_flags.for_animation_only()); - - if replacements.contains(RestyleHint::RESTYLE_SMIL) { - replace_rule_node(CascadeLevel::SMILOverride, - self.get_smil_override(), - primary_rules); - } - - let replace_rule_node_for_animation = |level: CascadeLevel, - primary_rules: &mut StrongRuleNode| { - let animation_rule = self.get_animation_rule_by_cascade(level); - replace_rule_node(level, - animation_rule.as_ref().map(|a| a.borrow_arc()), - primary_rules); - }; - - // Apply Transition rules and Animation rules if the corresponding restyle hint - // is contained. - if replacements.contains(RestyleHint::RESTYLE_CSS_TRANSITIONS) { - replace_rule_node_for_animation(CascadeLevel::Transitions, - primary_rules); - } - - if replacements.contains(RestyleHint::RESTYLE_CSS_ANIMATIONS) { - replace_rule_node_for_animation(CascadeLevel::Animations, - primary_rules); - } - } - - false - } - /// Given the old and new style of this element, and whether it's a /// pseudo-element, compute the restyle damage used to determine which /// kind of layout or painting operations we'll need. diff --git a/servo/components/style/parallel.rs b/servo/components/style/parallel.rs index d7604a6a5cf5..fd2ca7bc3a94 100644 --- a/servo/components/style/parallel.rs +++ b/servo/components/style/parallel.rs @@ -77,9 +77,11 @@ type WorkUnit = ArrayVec<[SendNode; WORK_UNIT_MAX]>; #[inline(never)] fn create_thread_local_context<'scope, E, D>( traversal: &'scope D, - slot: &mut Option>) - where E: TElement + 'scope, - D: DomTraversal + slot: &mut Option>, +) +where + E: TElement + 'scope, + D: DomTraversal, { *slot = Some(ThreadLocalStyleContext::new(traversal.shared_context())); } @@ -99,15 +101,18 @@ fn create_thread_local_context<'scope, E, D>( /// a thread-local cache to share styles between siblings. #[inline(always)] #[allow(unsafe_code)] -fn top_down_dom<'a, 'scope, E, D>(nodes: &'a [SendNode], - root: OpaqueNode, - mut traversal_data: PerLevelTraversalData, - scope: &'a rayon::Scope<'scope>, - pool: &'scope rayon::ThreadPool, - traversal: &'scope D, - tls: &'scope ScopedTLS<'scope, ThreadLocalStyleContext>) - where E: TElement + 'scope, - D: DomTraversal, +fn top_down_dom<'a, 'scope, E, D>( + nodes: &'a [SendNode], + root: OpaqueNode, + mut traversal_data: PerLevelTraversalData, + scope: &'a rayon::Scope<'scope>, + pool: &'scope rayon::ThreadPool, + traversal: &'scope D, + tls: &'scope ScopedTLS<'scope, ThreadLocalStyleContext>, +) +where + E: TElement + 'scope, + D: DomTraversal, { debug_assert!(nodes.len() <= WORK_UNIT_MAX); diff --git a/servo/components/style/rule_tree/mod.rs b/servo/components/style/rule_tree/mod.rs index 0ff2dbf28dcd..040a93d9a456 100644 --- a/servo/components/style/rule_tree/mod.rs +++ b/servo/components/style/rule_tree/mod.rs @@ -1096,14 +1096,16 @@ impl StrongRuleNode { /// Returns true if any properties specified by `rule_type_mask` was set by /// an author rule. #[cfg(feature = "gecko")] - pub fn has_author_specified_rules(&self, - mut element: E, - mut pseudo: Option, - guards: &StylesheetGuards, - rule_type_mask: u32, - author_colors_allowed: bool) - -> bool - where E: ::dom::TElement + pub fn has_author_specified_rules( + &self, + mut element: E, + mut pseudo: Option, + guards: &StylesheetGuards, + rule_type_mask: u32, + author_colors_allowed: bool, + ) -> bool + where + E: ::dom::TElement { use gecko_bindings::structs::NS_AUTHOR_SPECIFIED_BACKGROUND; use gecko_bindings::structs::NS_AUTHOR_SPECIFIED_BORDER; diff --git a/servo/components/style/selector_map.rs b/servo/components/style/selector_map.rs index b10a54f8124b..c1f02b5797f1 100644 --- a/servo/components/style/selector_map.rs +++ b/servo/components/style/selector_map.rs @@ -5,7 +5,7 @@ //! A data structure to efficiently index structs containing selectors by local //! name, ids and hash. -use {Atom, LocalName}; +use {Atom, LocalName, WeakAtom}; use applicable_declarations::ApplicableDeclarationList; use context::QuirksMode; use dom::TElement; @@ -175,8 +175,8 @@ impl SelectorMap { // At the end, we're going to sort the rules that we added, so remember // where we began. let init_len = matching_rules_list.len(); - if let Some(id) = rule_hash_target.get_id() { - if let Some(rules) = self.id_hash.get(&id, quirks_mode) { + if let Some(id) = rule_hash_target.id() { + if let Some(rules) = self.id_hash.get(id, quirks_mode) { SelectorMap::get_matching_rules( element, rules, @@ -201,7 +201,7 @@ impl SelectorMap { } }); - if let Some(rules) = self.local_name_hash.get(rule_hash_target.get_local_name()) { + if let Some(rules) = self.local_name_hash.get(rule_hash_target.local_name()) { SelectorMap::get_matching_rules( element, rules, @@ -315,8 +315,8 @@ impl SelectorMap { F: FnMut(&'a T) -> bool { // Id. - if let Some(id) = element.get_id() { - if let Some(v) = self.id_hash.get(&id, quirks_mode) { + if let Some(id) = element.id() { + if let Some(v) = self.id_hash.get(id, quirks_mode) { for entry in v.iter() { if !f(&entry) { return false; @@ -344,7 +344,7 @@ impl SelectorMap { } // Local name. - if let Some(v) = self.local_name_hash.get(element.get_local_name()) { + if let Some(v) = self.local_name_hash.get(element.local_name()) { for entry in v.iter() { if !f(&entry) { return false; @@ -374,7 +374,7 @@ impl SelectorMap { &'a self, element: E, quirks_mode: QuirksMode, - additional_id: Option<&Atom>, + additional_id: Option<&WeakAtom>, additional_classes: &[Atom], mut f: F, ) -> bool @@ -535,7 +535,7 @@ impl MaybeCaseInsensitiveHashMap { } /// HashMap::get - pub fn get(&self, key: &Atom, quirks_mode: QuirksMode) -> Option<&V> { + pub fn get(&self, key: &WeakAtom, quirks_mode: QuirksMode) -> Option<&V> { if quirks_mode == QuirksMode::Quirks { self.0.get(&key.to_ascii_lowercase()) } else { diff --git a/servo/components/style/servo/selector_parser.rs b/servo/components/style/servo/selector_parser.rs index 04c164e290d7..08d3401cebd3 100644 --- a/servo/components/style/servo/selector_parser.rs +++ b/servo/components/style/servo/selector_parser.rs @@ -696,8 +696,8 @@ impl ElementSnapshot for ServoElementSnapshot { self.attrs.is_some() } - fn id_attr(&self) -> Option { - self.get_attr(&ns!(), &local_name!("id")).map(|v| v.as_atom().clone()) + fn id_attr(&self) -> Option<&Atom> { + self.get_attr(&ns!(), &local_name!("id")).map(|v| v.as_atom()) } fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { diff --git a/servo/components/style/sharing/checks.rs b/servo/components/style/sharing/checks.rs index ac434245c690..546b99e63108 100644 --- a/servo/components/style/sharing/checks.rs +++ b/servo/components/style/sharing/checks.rs @@ -145,8 +145,8 @@ pub fn may_match_different_id_rules( where E: TElement, { - let element_id = element.get_id(); - let candidate_id = candidate.get_id(); + let element_id = element.id(); + let candidate_id = candidate.id(); if element_id == candidate_id { return false; @@ -155,7 +155,7 @@ where 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, element), None => false }; @@ -164,7 +164,7 @@ where } match candidate_id { - Some(ref id) => stylist.may_have_rules_for_id(id, candidate), + Some(id) => stylist.may_have_rules_for_id(id, candidate), None => false } } diff --git a/servo/components/style/sharing/mod.rs b/servo/components/style/sharing/mod.rs index 729a8592187b..de815d115209 100644 --- a/servo/components/style/sharing/mod.rs +++ b/servo/components/style/sharing/mod.rs @@ -676,12 +676,12 @@ impl StyleSharingCache { return None; } - if *target.get_local_name() != *candidate.element.get_local_name() { + if target.local_name() != candidate.element.local_name() { trace!("Miss: Local Name"); return None; } - if *target.get_namespace() != *candidate.element.get_namespace() { + if target.namespace() != candidate.element.namespace() { trace!("Miss: Namespace"); return None; } @@ -700,7 +700,7 @@ impl StyleSharingCache { // 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() { + if target.element.state() != candidate.state() { trace!("Miss: User and Author State"); return None; } diff --git a/servo/components/style/style_resolver.rs b/servo/components/style/style_resolver.rs index b07dfb487b9a..99cc0ac33638 100644 --- a/servo/components/style/style_resolver.rs +++ b/servo/components/style/style_resolver.rs @@ -439,8 +439,8 @@ where self.element, implemented_pseudo.as_ref(), self.element.style_attribute(), - self.element.get_smil_override(), - self.element.get_animation_rules(), + self.element.smil_override(), + self.element.animation_rules(), self.rule_inclusion, &mut applicable_declarations, &mut matching_context, diff --git a/servo/components/style/stylist.rs b/servo/components/style/stylist.rs index b8056f828138..cd4ff265009f 100644 --- a/servo/components/style/stylist.rs +++ b/servo/components/style/stylist.rs @@ -4,7 +4,7 @@ //! Selector matching. -use {Atom, LocalName, Namespace}; +use {Atom, LocalName, Namespace, WeakAtom}; use applicable_declarations::{ApplicableDeclarationBlock, ApplicableDeclarationList}; use context::{CascadeInputs, QuirksMode}; use dom::TElement; @@ -1385,7 +1385,7 @@ impl Stylist { #[inline] pub fn may_have_rules_for_id( &self, - id: &Atom, + id: &WeakAtom, element: E, ) -> bool where