From fed70753e41a33c7551dae6f49c87b206eca6e20 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Mon, 30 Sep 2019 20:47:20 +0000 Subject: [PATCH] servo: Merge #5777 - Make Attr::prefix an Atom (from tamird:ICE-attr-prefix-atom); r=jdm Rebase of https://github.com/Ms2ger/servo/commits/ICE-attr-prefix-atom Some of the changes weren't necessary since the internals had been refactored some in the interim. In any case, I was unable to reproduce the ICE reported in https://github.com/rust-lang/rust/issues/18957. Ms2ger Source-Repo: https://github.com/servo/servo Source-Revision: b0a7d1bf865eff7b6ca3bae874004a61c19b3c27 UltraBlame original commit: 4af16b10e9d304b2651a66dcca09f497c38fa55d --- servo/components/script/dom/attr.rs | 10 +++++----- servo/components/script/dom/bindings/utils.rs | 5 ++--- servo/components/script/dom/create.rs | 8 ++++---- servo/components/script/dom/element.rs | 15 +++++++-------- servo/components/script/dom/node.rs | 4 ++-- 5 files changed, 20 insertions(+), 22 deletions(-) diff --git a/servo/components/script/dom/attr.rs b/servo/components/script/dom/attr.rs index 42d64f907af10..7792309c1a7e9 100644 --- a/servo/components/script/dom/attr.rs +++ b/servo/components/script/dom/attr.rs @@ -101,7 +101,7 @@ pub struct Attr { value: DOMRefCell, name: Atom, namespace: Namespace, - prefix: Option, + prefix: Option, owner: MutNullableHeap>, @@ -109,7 +109,7 @@ pub struct Attr { impl Attr { fn new_inherited(local_name: Atom, value: AttrValue, name: Atom, namespace: Namespace, - prefix: Option, owner: Option>) -> Attr { + prefix: Option, owner: Option>) -> Attr { Attr { reflector_: Reflector::new(), local_name: local_name, @@ -123,7 +123,7 @@ impl Attr { pub fn new(window: JSRef, local_name: Atom, value: AttrValue, name: Atom, namespace: Namespace, - prefix: Option, owner: Option>) -> Temporary { + prefix: Option, owner: Option>) -> Temporary { reflect_dom_object( box Attr::new_inherited(local_name, value, name, namespace, prefix, owner), GlobalRef::Window(window), @@ -141,7 +141,7 @@ impl Attr { } #[inline] - pub fn prefix<'a>(&'a self) -> &'a Option { + pub fn prefix<'a>(&'a self) -> &'a Option { &self.prefix } } @@ -205,7 +205,7 @@ impl<'a> AttrMethods for JSRef<'a, Attr> { // https://dom.spec.whatwg.org/#dom-attr-prefix fn GetPrefix(self) -> Option { - self.prefix.clone() + self.prefix().as_ref().map(|p| (**p).to_owned()) } // https://dom.spec.whatwg.org/#dom-attr-ownerelement diff --git a/servo/components/script/dom/bindings/utils.rs b/servo/components/script/dom/bindings/utils.rs index 0e6ae3cb08415..e68666ad4a2d7 100644 --- a/servo/components/script/dom/bindings/utils.rs +++ b/servo/components/script/dom/bindings/utils.rs @@ -17,7 +17,6 @@ use util::str::DOMString; use libc; use libc::c_uint; -use std::borrow::ToOwned; use std::boxed; use std::cell::Cell; use std::ffi::CString; @@ -628,7 +627,7 @@ pub fn validate_qualified_name(qualified_name: &str) -> ErrorResult { pub fn validate_and_extract(namespace: Option, qualified_name: &str) - -> Fallible<(Namespace, Option, Atom)> { + -> Fallible<(Namespace, Option, Atom)> { let namespace = namespace::from_domstring(namespace); @@ -667,7 +666,7 @@ pub fn validate_and_extract(namespace: Option, qualified_name: &str) }, (ns, p) => { // Step 10. - Ok((ns, p.map(|s| s.to_owned()), Atom::from_slice(local_name))) + Ok((ns, p.map(Atom::from_slice), Atom::from_slice(local_name))) } } } diff --git a/servo/components/script/dom/create.rs b/servo/components/script/dom/create.rs index ce47b2d9bb664..c7b48bbfb6fed 100644 --- a/servo/components/script/dom/create.rs +++ b/servo/components/script/dom/create.rs @@ -76,15 +76,15 @@ use dom::htmlulistelement::HTMLUListElement; use dom::htmlunknownelement::HTMLUnknownElement; use dom::htmlvideoelement::HTMLVideoElement; -use util::str::DOMString; - -use string_cache::QualName; +use string_cache::{Atom, QualName}; use std::borrow::ToOwned; -pub fn create_element(name: QualName, prefix: Option, +pub fn create_element(name: QualName, prefix: Option, document: JSRef, creator: ElementCreator) -> Temporary { + let prefix = prefix.map(|p| (*p).to_owned()); + if name.ns != ns!(HTML) { return Element::new((*name.local).to_owned(), name.ns, prefix, document); } diff --git a/servo/components/script/dom/element.rs b/servo/components/script/dom/element.rs index 2e47f3c06b91c..b2a690c8fed16 100644 --- a/servo/components/script/dom/element.rs +++ b/servo/components/script/dom/element.rs @@ -120,7 +120,7 @@ pub enum ElementCreator { impl Element { - pub fn create(name: QualName, prefix: Option, + pub fn create(name: QualName, prefix: Option, document: JSRef, creator: ElementCreator) -> Temporary { create_element(name, prefix, document, creator) @@ -670,12 +670,12 @@ pub trait AttributeHandlers { fn set_attribute_from_parser(self, name: QualName, value: DOMString, - prefix: Option); + prefix: Option); fn set_attribute(self, name: &Atom, value: AttrValue); fn set_custom_attribute(self, name: DOMString, value: DOMString) -> ErrorResult; fn do_set_attribute(self, local_name: Atom, value: AttrValue, name: Atom, namespace: Namespace, - prefix: Option, cb: F) + prefix: Option, cb: F) where F: Fn(JSRef) -> bool; fn parse_attribute(self, namespace: &Namespace, local_name: &Atom, value: DOMString) -> AttrValue; @@ -743,7 +743,7 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> { fn set_attribute_from_parser(self, qname: QualName, value: DOMString, - prefix: Option) { + prefix: Option) { // Don't set if the attribute already exists, so we can handle add_attrs_if_missing if self.attrs.borrow().iter().map(|attr| attr.root()) .any(|a| *a.r().local_name() == qname.local && *a.r().namespace() == qname.ns) { @@ -753,7 +753,7 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> { let name = match prefix { None => qname.local.clone(), Some(ref prefix) => { - let name = format!("{}:{}", *prefix, &*qname.local); + let name = format!("{}:{}", &**prefix, &*qname.local); Atom::from_slice(&name) }, }; @@ -791,7 +791,7 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> { value: AttrValue, name: Atom, namespace: Namespace, - prefix: Option, + prefix: Option, cb: F) where F: Fn(JSRef) -> bool { @@ -1099,8 +1099,7 @@ impl<'a> ElementMethods for JSRef<'a, Element> { let qualified_name = Atom::from_slice(&qualified_name); let value = self.parse_attribute(&namespace, &local_name, value); self.do_set_attribute(local_name.clone(), value, qualified_name, - namespace.clone(), prefix.map(|s| s.to_owned()), - |attr| { + namespace.clone(), prefix, |attr| { *attr.local_name() == local_name && *attr.namespace() == namespace }); diff --git a/servo/components/script/dom/node.rs b/servo/components/script/dom/node.rs index e3ed4b662197e..4f976eefdb5cb 100644 --- a/servo/components/script/dom/node.rs +++ b/servo/components/script/dom/node.rs @@ -70,7 +70,7 @@ use std::iter::{FilterMap, Peekable}; use std::mem; use std::sync::Arc; use uuid; -use string_cache::QualName; +use string_cache::{Atom, QualName}; @@ -1735,7 +1735,7 @@ impl Node { local: element.local_name().clone() }; let element = Element::create(name, - element.prefix().as_ref().map(|p| (**p).to_owned()), + element.prefix().as_ref().map(|p| Atom::from_slice(&p)), document.r(), ElementCreator::ScriptCreated); NodeCast::from_temporary(element) },