Use internal mutability for Attr::value.

8 files changed, 35 insertions, 25 deletions

diff --git a/src/components/script/dom/attr.rs b/src/components/script/dom/attr.rs
index 95d03571414..e9acd46356a 100644
--- a/src/components/script/dom/attr.rs
+++ b/src/components/script/dom/attr.rs
@@ -5,6 +5,7 @@
 use dom::bindings::codegen::Bindings::AttrBinding;
 use dom::bindings::codegen::InheritTypes::NodeCast;
 use dom::bindings::js::{JS, JSRef, Temporary};
+use dom::bindings::trace::Traceable;
 use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
 use dom::element::{Element, AttributeHandlers};
 use dom::node::Node;
@@ -13,7 +14,8 @@ use dom::virtualmethods::vtable_for;
 use servo_util::namespace;
 use servo_util::namespace::Namespace;
 use servo_util::str::{DOMString, HTML_SPACE_CHARACTERS};
-use std::cell::Cell;
+use std::cell::{Ref, Cell, RefCell};
+use std::mem;
 
 pub enum AttrSettingType {
     FirstSetAttr,
@@ -58,7 +60,7 @@ impl AttrValue {
 pub struct Attr {
     pub reflector_: Reflector,
     pub local_name: DOMString,
-    value: AttrValue,
+    value: Traceable<RefCell<AttrValue>>,
     pub name: DOMString,
     pub namespace: Namespace,
     pub prefix: Option<DOMString>,
@@ -80,7 +82,7 @@ impl Attr {
         Attr {
             reflector_: Reflector::new(),
             local_name: local_name,
-            value: value,
+            value: Traceable::new(RefCell::new(value)),
             name: name, //TODO: Intern attribute names
             namespace: namespace,
             prefix: prefix,
@@ -95,7 +97,7 @@ impl Attr {
         reflect_dom_object(box attr, window, AttrBinding::Wrap)
     }
 
-    pub fn set_value(&mut self, set_type: AttrSettingType, value: AttrValue) {
+    pub fn set_value(&self, set_type: AttrSettingType, value: AttrValue) {
         let owner = self.owner.get().root();
         let node: &JSRef<Node> = NodeCast::from_ref(&*owner);
         let namespace_is_null = self.namespace == namespace::Null;
@@ -103,32 +105,28 @@ impl Attr {
         match set_type {
             ReplacedAttr => {
                 if namespace_is_null {
-                    vtable_for(node).before_remove_attr(self.local_name.clone(), self.value.as_slice().to_string());
+                    vtable_for(node).before_remove_attr(self.local_name.clone(), self.value.deref().borrow().as_slice().to_string());
                 }
             }
             FirstSetAttr => {}
         }
 
-        self.value = value;
+        *self.value.deref().borrow_mut() = value;
 
         if namespace_is_null {
-            vtable_for(node).after_set_attr(self.local_name.clone(), self.value.as_slice().to_string());
+            vtable_for(node).after_set_attr(self.local_name.clone(), self.value.deref().borrow().as_slice().to_string());
         }
     }
 
-    pub fn value<'a>(&'a self) -> &'a AttrValue {
-        &self.value
-    }
-
-    pub fn value_ref<'a>(&'a self) -> &'a str {
-        self.value.as_slice()
+    pub fn value<'a>(&'a self) -> Ref<'a, AttrValue> {
+        self.value.deref().borrow()
     }
 }
 
 pub trait AttrMethods {
     fn LocalName(&self) -> DOMString;
     fn Value(&self) -> DOMString;
-    fn SetValue(&mut self, value: DOMString);
+    fn SetValue(&self, value: DOMString);
     fn Name(&self) -> DOMString;
     fn GetNamespaceURI(&self) -> Option<DOMString>;
     fn GetPrefix(&self) -> Option<DOMString>;
@@ -140,10 +138,10 @@ impl<'a> AttrMethods for JSRef<'a, Attr> {
     }
 
     fn Value(&self) -> DOMString {
-        self.value.as_slice().to_string()
+        self.value.deref().borrow().as_slice().to_string()
     }
 
-    fn SetValue(&mut self, value: DOMString) {
+    fn SetValue(&self, value: DOMString) {
         let owner = self.owner.get().root();
         let value = owner.deref().parse_attribute(
             &self.namespace, self.deref().local_name.as_slice(), value);
@@ -165,3 +163,15 @@ impl<'a> AttrMethods for JSRef<'a, Attr> {
         self.prefix.clone()
     }
 }
+
+pub trait AttrHelpersForLayout {
+    unsafe fn value_ref_forever(&self) -> &'static str;
+}
+
+impl AttrHelpersForLayout for Attr {
+    unsafe fn value_ref_forever(&self) -> &'static str {
+        // cast to point to T in RefCell<T> directly
+        let value = mem::transmute::<&RefCell<AttrValue>, &AttrValue>(self.value.deref());
+        value.as_slice()
+    }
+}
diff --git a/src/components/script/dom/document.rs b/src/components/script/dom/document.rs
index 137cd15e58e..ece415698c9 100644
--- a/src/components/script/dom/document.rs
+++ b/src/components/script/dom/document.rs
@@ -694,7 +694,7 @@ impl<'a> DocumentMethods for JSRef<'a, Document> {
 
             let element: &JSRef<Element> = ElementCast::to_ref(node).unwrap();
             element.get_attribute(Null, "name").root().map_or(false, |mut attr| {
-                attr.value_ref() == name.as_slice()
+                attr.value().as_slice() == name.as_slice()
             })
         })
     }
diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs
index 2be875e6d40..abe286184de 100644
--- a/src/components/script/dom/element.rs
+++ b/src/components/script/dom/element.rs
@@ -4,7 +4,7 @@
 
 //! Element nodes.
 
-use dom::attr::{Attr, ReplacedAttr, FirstSetAttr, AttrMethods};
+use dom::attr::{Attr, ReplacedAttr, FirstSetAttr, AttrMethods, AttrHelpersForLayout};
 use dom::attr::{AttrValue, StringAttrValue, UIntAttrValue};
 use dom::attrlist::AttrList;
 use dom::bindings::codegen::Bindings::ElementBinding;
@@ -176,7 +176,7 @@ impl RawLayoutElementHelpers for Element {
             name == (*attr).local_name.as_slice() && (*attr).namespace == *namespace
         }).map(|attr| {
             let attr = attr.unsafe_get();
-            mem::transmute((*attr).value_ref())
+            (*attr).value_ref_forever()
         })
     }
 }
@@ -868,7 +868,7 @@ impl<'a> VirtualMethods for JSRef<'a, Element> {
 impl<'a> style::TElement for JSRef<'a, Element> {
     fn get_attr(&self, namespace: &Namespace, attr: &str) -> Option<&'static str> {
         self.get_attribute(namespace.clone(), attr).root().map(|attr| {
-            unsafe { mem::transmute(attr.deref().value_ref()) }
+            unsafe { mem::transmute(attr.deref().value().as_slice()) }
         })
     }
     fn get_link(&self) -> Option<&'static str> {
diff --git a/src/components/script/dom/htmliframeelement.rs b/src/components/script/dom/htmliframeelement.rs
index a61b81a42e3..dd97c8f48b5 100644
--- a/src/components/script/dom/htmliframeelement.rs
+++ b/src/components/script/dom/htmliframeelement.rs
@@ -69,7 +69,7 @@ impl<'a> HTMLIFrameElementHelpers for JSRef<'a, HTMLIFrameElement> {
         let element: &JSRef<Element> = ElementCast::from_ref(self);
         element.get_attribute(Null, "src").root().and_then(|src| {
             let window = window_from_node(self).root();
-            try_parse_url(src.deref().value_ref(),
+            try_parse_url(src.deref().value().as_slice(),
                           Some(window.deref().page().get_url())).ok()
         })
     }
diff --git a/src/components/script/dom/htmlserializer.rs b/src/components/script/dom/htmlserializer.rs
index e5cb06cb15f..2eca215e89b 100644
--- a/src/components/script/dom/htmlserializer.rs
+++ b/src/components/script/dom/htmlserializer.rs
@@ -151,7 +151,7 @@ fn serialize_attr(attr: &JSRef<Attr>, html: &mut String) {
         html.push_str(attr.deref().name.as_slice());
     };
     html.push_str("=\"");
-    escape(attr.deref().value_ref(), true, html);
+    escape(attr.deref().value().as_slice(), true, html);
     html.push_char('"');
 }
 
diff --git a/src/components/script/dom/node.rs b/src/components/script/dom/node.rs
index c673e174a3b..cb2030f7c43 100644
--- a/src/components/script/dom/node.rs
+++ b/src/components/script/dom/node.rs
@@ -1814,7 +1814,7 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
                 other_element.attrs.borrow().iter().map(|attr| attr.root()).any(|other_attr| {
                     (attr.namespace == other_attr.namespace) &&
                     (attr.local_name == other_attr.local_name) &&
-                    (attr.deref().value_ref() == other_attr.deref().value_ref())
+                    (attr.deref().value().as_slice() == other_attr.deref().value().as_slice())
                 })
             })
         }
diff --git a/src/components/script/html/hubbub_html_parser.rs b/src/components/script/html/hubbub_html_parser.rs
index 8909ba690bb..de396368206 100644
--- a/src/components/script/html/hubbub_html_parser.rs
+++ b/src/components/script/html/hubbub_html_parser.rs
@@ -502,7 +502,7 @@ pub fn parse_html(page: &Page,
                 match script.get_attribute(Null, "src").root() {
                     Some(src) => {
                         debug!("found script: {:s}", src.deref().Value());
-                        let new_url = parse_url(src.deref().value_ref(), Some(url3.clone()));
+                        let new_url = parse_url(src.deref().value().as_slice(), Some(url3.clone()));
                         js_chan2.send(JSTaskNewFile(new_url));
                     }
                     None => {
diff --git a/src/components/script/page.rs b/src/components/script/page.rs
index c8f884e6b08..ebdeb324819 100644
--- a/src/components/script/page.rs
+++ b/src/components/script/page.rs
@@ -365,7 +365,7 @@ impl Page {
                 anchors.find(|node| {
                     let elem: &JSRef<Element> = ElementCast::to_ref(node).unwrap();
                     elem.get_attribute(Null, "name").root().map_or(false, |attr| {
-                        attr.deref().value_ref() == fragid.as_slice()
+                        attr.deref().value().as_slice() == fragid.as_slice()
                     })
                 }).map(|node| Temporary::from_rooted(ElementCast::to_ref(&node).unwrap()))
             }