Auto merge of #5777 - 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

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5777)
<!-- Reviewable:end -->

components/script/dom/attr.rs

@@ -101,7 +101,7 @@ pub struct Attr {
     value: DOMRefCell<AttrValue>,
     name: Atom,
     namespace: Namespace,
-    prefix: Option<DOMString>,
+    prefix: Option<Atom>,
 
     /// the element that owns this attribute.
     owner: MutNullableHeap<JS<Element>>,
@@ -109,7 +109,7 @@ pub struct Attr {
 
 impl Attr {
     fn new_inherited(local_name: Atom, value: AttrValue, name: Atom, namespace: Namespace,
-                     prefix: Option<DOMString>, owner: Option<JSRef<Element>>) -> Attr {
+                     prefix: Option<Atom>, owner: Option<JSRef<Element>>) -> Attr {
         Attr {
             reflector_: Reflector::new(),
             local_name: local_name,
@@ -123,7 +123,7 @@ impl Attr {
 
     pub fn new(window: JSRef<Window>, local_name: Atom, value: AttrValue,
                name: Atom, namespace: Namespace,
-               prefix: Option<DOMString>, owner: Option<JSRef<Element>>) -> Temporary<Attr> {
+               prefix: Option<Atom>, owner: Option<JSRef<Element>>) -> Temporary<Attr> {
         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<DOMString> {
+    pub fn prefix<'a>(&'a self) -> &'a Option<Atom> {
         &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<DOMString> {
-        self.prefix.clone()
+        self.prefix().as_ref().map(|p| (**p).to_owned())
     }
 
     // https://dom.spec.whatwg.org/#dom-attr-ownerelement

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 {
 /// Validate a namespace and qualified name and extract their parts.
 /// See https://dom.spec.whatwg.org/#validate-and-extract for details.
 pub fn validate_and_extract(namespace: Option<DOMString>, qualified_name: &str)
-                            -> Fallible<(Namespace, Option<DOMString>, Atom)> {
+                            -> Fallible<(Namespace, Option<Atom>, Atom)> {
     // Step 1.
     let namespace = namespace::from_domstring(namespace);
 
@@ -667,7 +666,7 @@ pub fn validate_and_extract(namespace: Option<DOMString>, 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)))
         }
     }
 }

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::{Atom, QualName};
-
-use string_cache::QualName;
 
 use std::borrow::ToOwned;
 
-pub fn create_element(name: QualName, prefix: Option<DOMString>,
+pub fn create_element(name: QualName, prefix: Option<Atom>,
                       document: JSRef<Document>, creator: ElementCreator)
                       -> Temporary<Element> {
+    let prefix = prefix.map(|p| (*p).to_owned());
+
     if name.ns != ns!(HTML) {
         return Element::new((*name.local).to_owned(), name.ns, prefix, document);
     }

components/script/dom/element.rs

@@ -120,7 +120,7 @@ pub enum ElementCreator {
 // Element methods
 //
 impl Element {
-    pub fn create(name: QualName, prefix: Option<DOMString>,
+    pub fn create(name: QualName, prefix: Option<Atom>,
                   document: JSRef<Document>, creator: ElementCreator)
                   -> Temporary<Element> {
         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<DOMString>);
+                                 prefix: Option<Atom>);
     fn set_attribute(self, name: &Atom, value: AttrValue);
     fn set_custom_attribute(self, name: DOMString, value: DOMString) -> ErrorResult;
     fn do_set_attribute<F>(self, local_name: Atom, value: AttrValue,
                            name: Atom, namespace: Namespace,
-                           prefix: Option<DOMString>, cb: F)
+                           prefix: Option<Atom>, cb: F)
         where F: Fn(JSRef<Attr>) -> 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<DOMString>) {
+                                 prefix: Option<Atom>) {
         // 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<DOMString>,
+                           prefix: Option<Atom>,
                            cb: F)
         where F: Fn(JSRef<Attr>) -> 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()),
+                              namespace.clone(), prefix, |attr| {
-                              |attr| {
             *attr.local_name() == local_name &&
             *attr.namespace() == namespace
         });

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};
 
 //
 // The basic Node structure
@@ -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)
             },