auto merge of #1456 : Ms2ger/servo/remove-attrs-hashmap, r=jdm

src/components/script/dom/attr.rs

@@ -8,6 +8,8 @@ use dom::bindings::utils::reflect_dom_object;
 use dom::namespace::{Namespace, Null};
 use dom::window::Window;
 
+use std::util;
+
 pub struct Attr {
     reflector_: Reflector,
     local_name: DOMString,
@@ -58,6 +60,13 @@ impl Attr {
         reflect_dom_object(@mut attr, window, AttrBinding::Wrap)
     }
 
+    pub fn set_value(&mut self, mut value: DOMString) -> DOMString {
+        util::swap(&mut self.value, &mut value);
+        value
+    }
+}
+
+impl Attr {
     pub fn LocalName(&self) -> DOMString {
         self.local_name.clone()
     }

src/components/script/dom/attrlist.rs

@@ -29,22 +29,13 @@ impl AttrList {
     }
 
     pub fn Length(&self) -> u32 {
-        self.owner.with_imm_element(|elem| elem.attrs_insert_order.len() as u32)
+        self.owner.with_imm_element(|elem| elem.attrs.len() as u32)
     }
 
     pub fn Item(&self, index: u32) -> Option<@mut Attr> {
-        if index >= self.Length() {
-            None
-        } else {
-            do self.owner.with_imm_element |elem| {
-                let insert_order = &elem.attrs_insert_order[index];
-                do elem.attrs.find_equiv(&insert_order.first()).and_then |attrs| {
-                    attrs.iter()
-                         .find(|attr| attr.namespace == insert_order.second())
-                         .map(|attr| *attr)
-                }
-            }
-        }
+        self.owner.with_imm_element(|elem| {
+            elem.attrs.get_opt(index as uint).map(|&x| x)
+        })
     }
 
     pub fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<@mut Attr> {

src/components/script/dom/element.rs

@@ -23,16 +23,14 @@ use layout_interface::{MatchSelectorsDocumentDamage};
 use style;
 
 use std::comm;
-use std::hashmap::HashMap;
-use std::str::{eq, eq_slice};
+use std::str::eq;
 use std::ascii::StrAsciiExt;
 
 pub struct Element {
     node: Node,
     tag_name: ~str,     // TODO: This should be an atom, not a ~str.
     namespace: Namespace,
-    attrs: HashMap<~str, ~[@mut Attr]>,
-    attrs_insert_order: ~[(~str, Namespace)], // store an order of attributes.
+    attrs: ~[@mut Attr],
     style_attribute: Option<style::PropertyDeclarationBlock>,
     attr_list: Option<@mut AttrList>
 }
@@ -131,8 +129,7 @@ impl<'self> Element {
             node: Node::new_inherited(ElementNodeTypeId(type_id), document),
             tag_name: tag_name,
             namespace: namespace,
-            attrs: HashMap::new(),
-            attrs_insert_order: ~[],
+            attrs: ~[],
             attr_list: None,
             style_attribute: None,
         }
@@ -153,11 +150,9 @@ impl<'self> Element {
                          name: &str) -> Option<@mut Attr> {
         // FIXME: only case-insensitive in the HTML namespace (as opposed to SVG, etc.)
         let name = name.to_ascii_lower();
-        self.attrs.find_equiv(&name).and_then(|attrs| {
-            do attrs.iter().find |attr| {
-                eq_slice(attr.local_name, name) && attr.namespace == namespace
-            }.map(|x| *x)
-        })
+        self.attrs.iter().find(|attr| {
+            name == attr.local_name && attr.namespace == namespace
+        }).map(|&x| x)
     }
 
     // FIXME(pcwalton): This is kind of confusingly named relative to the above...
@@ -198,36 +193,21 @@ impl<'self> Element {
         self.node.wait_until_safe_to_modify_dom();
 
         // FIXME: reduce the time of `value.clone()`.
-        let win = self.node.owner_doc().document().window;
-        let new_attr = Attr::new_ns(win, local_name.clone(), value.clone(),
-                                    name.clone(), namespace.clone(), prefix);
         let mut old_raw_value: Option<DOMString> = None;
-        self.attrs.mangle(local_name.clone(), new_attr,
-                          |new_name: &~str, new_value: @mut Attr| {
-                              // register to the ordered list.
-                              let order_value = (new_name.clone(), new_value.namespace.clone());
-                              self.attrs_insert_order.push(order_value);
-                              ~[new_value]
-                          },
-                          |name, old_value: &mut ~[@mut Attr], new_value: @mut Attr| {
-                              // update value.
-                              let mut found = false;
-                              for attr in old_value.mut_iter() {
-                                  if eq_slice(attr.local_name, *name) &&
-                                     attr.namespace == new_value.namespace {
-                                      old_raw_value = Some(attr.Value());
-                                      *attr = new_value;
-                                      found = true;
-                                      break;
-                                  }
+        for attr in self.attrs.iter() {
+            if attr.local_name == local_name {
+                old_raw_value = Some(attr.set_value(value.clone()));
+                break;
+            }
+        }
 
-                              }
-                              if !found {
-                                  old_value.push(new_value);
-                                  let order_value = (name.clone(), new_value.namespace.clone());
-                                  self.attrs_insert_order.push(order_value);
-                              }
-                          });
+        if old_raw_value.is_none() {
+            let win = self.node.owner_doc().document().window;
+            let new_attr = Attr::new_ns(win, local_name.clone(), value.clone(),
+                                        name.clone(), namespace.clone(),
+                                        prefix);
+            self.attrs.push(new_attr);
+        }
 
         if namespace == namespace::Null {
             self.after_set_attr(abstract_self, local_name, value, old_raw_value);