diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 2494f8955f5..1eaf74e4e9d 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/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) @@ -382,9 +382,9 @@ impl<'le> TElement for ServoLayoutElement<'le> { } #[inline] - fn id(&self) -> Option { + fn id(&self) -> Option<&Atom> { unsafe { - (*self.element.id_attribute()).clone() + (*self.element.id_attribute()).as_ref() } } diff --git a/components/style/dom.rs b/components/style/dom.rs index 6d307d80845..087e56ac6c1 100644 --- a/components/style/dom.rs +++ b/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; @@ -467,7 +467,7 @@ pub trait TElement fn has_attr(&self, namespace: &Namespace, attr: &LocalName) -> bool; /// The ID for this element. - fn 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); diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index c108b9851ef..f51d031a527 100644 --- a/components/style/gecko/snapshot.rs +++ b/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/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 80772db1caf..e1dc9794a04 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1176,7 +1176,7 @@ impl<'le> TElement for GeckoElement<'le> { // FIXME(emilio): we should probably just return a reference to the Atom. #[inline] - fn id(&self) -> Option { + fn id(&self) -> Option<&WeakAtom> { if !self.has_id() { return None; } @@ -1185,10 +1185,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) }) } } diff --git a/components/style/invalidation/element/element_wrapper.rs b/components/style/invalidation/element/element_wrapper.rs index 88798f82970..6e649588bb2 100644 --- a/components/style/invalidation/element/element_wrapper.rs +++ b/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>, diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index 6d345e06679..10a7f07aef4 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/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, @@ -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/components/style/selector_map.rs b/components/style/selector_map.rs index fbec863d598..c1f02b5797f 100644 --- a/components/style/selector_map.rs +++ b/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; @@ -176,7 +176,7 @@ impl SelectorMap { // where we began. let init_len = matching_rules_list.len(); if let Some(id) = rule_hash_target.id() { - if let Some(rules) = self.id_hash.get(&id, quirks_mode) { + if let Some(rules) = self.id_hash.get(id, quirks_mode) { SelectorMap::get_matching_rules( element, rules, @@ -316,7 +316,7 @@ impl SelectorMap { { // Id. if let Some(id) = element.id() { - if let Some(v) = self.id_hash.get(&id, quirks_mode) { + if let Some(v) = self.id_hash.get(id, quirks_mode) { 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/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index 04c164e290d..08d3401cebd 100644 --- a/components/style/servo/selector_parser.rs +++ b/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/components/style/sharing/checks.rs b/components/style/sharing/checks.rs index df467de1197..546b99e6310 100644 --- a/components/style/sharing/checks.rs +++ b/components/style/sharing/checks.rs @@ -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/components/style/stylist.rs b/components/style/stylist.rs index b8056f82813..cd4ff265009 100644 --- a/components/style/stylist.rs +++ b/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