From 61f558dffc98aeb4cf8910c30f19fb7e76dbb8d4 Mon Sep 17 00:00:00 2001
From: Patrick Walton <pcwalton@mimiga.net>
Date: Sun, 18 Nov 2012 19:12:34 -0800
Subject: [PATCH] css, dom, text: Stop copying so much. Also stop asserting.

---
 src/servo-gfx/text/glyph.rs     | 18 ++++++++++++++----
 src/servo/css/select_handler.rs |  8 +++++---
 src/servo/dom/element.rs        | 14 ++++++++++++++
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/src/servo-gfx/text/glyph.rs b/src/servo-gfx/text/glyph.rs
index fc65b5e04b8..f9e34eaa75b 100644
--- a/src/servo-gfx/text/glyph.rs
+++ b/src/servo-gfx/text/glyph.rs
@@ -583,8 +583,14 @@ impl GlyphStore {
     }
 
     fn iter_glyphs_for_range(&self, range: &const Range, cb: fn&(uint, GlyphInfo/&) -> bool) {
-        assert range.begin() < self.entry_buffer.len();
-        assert range.end() <= self.entry_buffer.len();
+        if range.begin() >= self.entry_buffer.len() {
+            error!("iter_glyphs_for_range: range.begin beyond length!");
+            return;
+        }
+        if range.end() > self.entry_buffer.len() {
+            error!("iter_glyphs_for_range: range.end beyond length!");
+            return;
+        }
 
         for range.eachi |i| {
 	    if !self.iter_glyphs_for_index(i, cb) { break; }
@@ -609,8 +615,12 @@ impl GlyphStore {
     }
 
     fn char_is_newline(i: uint) -> bool {
-        assert i < self.entry_buffer.len();
-        self.entry_buffer[i].char_is_newline()
+        if i >= self.entry_buffer.len() {
+            error!("char_is_newline: beyond entry buffer!");
+            false
+        } else {
+            self.entry_buffer[i].char_is_newline()
+        }
     }
 
     fn is_ligature_start(i: uint) -> bool {
diff --git a/src/servo/css/select_handler.rs b/src/servo/css/select_handler.rs
index c1f92085731..179b7cc9501 100644
--- a/src/servo/css/select_handler.rs
+++ b/src/servo/css/select_handler.rs
@@ -84,9 +84,11 @@ impl NodeSelectHandler: SelectHandler<Node> {
         do node.read |data| {
             match *data.kind {
                 Element(ref data) => {
-                    match data.get_attr("id") {
-                        None => false,
-                        Some(existing_id) => str::eq_slice(id, existing_id)
+                    do data.with_attr("id") |existing_id_opt| {
+                        match existing_id_opt {
+                            None => false,
+                            Some(existing_id) => str::eq_slice(id, existing_id)
+                        }
                     }
                 }
                 _ => fail ~"attempting to style non-element node"
diff --git a/src/servo/dom/element.rs b/src/servo/dom/element.rs
index 7ad2e755793..272f1c54b88 100644
--- a/src/servo/dom/element.rs
+++ b/src/servo/dom/element.rs
@@ -20,6 +20,20 @@ impl ElementData {
         }
     }
 
+    // Gets an attribute without copying.
+    //
+    // FIXME: Should not take a closure, but we need covariant type parameters for
+    // that.
+    fn with_attr<R>(name: &str, f: &fn(Option<&str>) -> R) -> R {
+        for self.attrs.each |attr| {
+            if name == attr.name {
+                let value: &str = attr.value;
+                return f(Some(value));
+            }
+        }
+        f(None)
+    }
+
     fn set_attr(name: &str, value: ~str) {
         let idx = do self.attrs.position |attr| { name == attr.name };
         match idx {