auto merge of #2589 : bjz/servo/cleanups, r=pcwalton

@pcwalton r?

Might conflict with the rust upgrade - I can rebase later.

7 files changed, 72 insertions, 193 deletions

diff --git a/src/components/gfx/text/glyph.rs b/src/components/gfx/text/glyph.rs
index 545811cfee8..2597b65cd1a 100644
--- a/src/components/gfx/text/glyph.rs
+++ b/src/components/gfx/text/glyph.rs
@@ -562,7 +562,7 @@ impl<'a> GlyphStore {
         }
 
         assert!(data.ligature_start); // can't compress ligature continuation glyphs.
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
 
         let entry = match (data.is_missing, glyph_is_compressible(data)) {
             (true, _) => GlyphEntry::missing(1),
@@ -578,7 +578,7 @@ impl<'a> GlyphStore {
     }
 
     pub fn add_glyphs_for_char_index(&mut self, i: CharIndex, data_for_glyphs: &[GlyphData]) {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         assert!(data_for_glyphs.len() > 0);
 
         let glyph_count = data_for_glyphs.len() as int;
@@ -607,7 +607,7 @@ impl<'a> GlyphStore {
 
     // used when a character index has no associated glyph---for example, a ligature continuation.
     pub fn add_nonglyph_for_char_index(&mut self, i: CharIndex, cluster_start: bool, ligature_start: bool) {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
 
         let entry = GlyphEntry::complex(cluster_start, ligature_start, 0);
         debug!("adding spacer for chracter without associated glyph[idx={}]", i);
@@ -621,10 +621,10 @@ impl<'a> GlyphStore {
 
     #[inline]
     pub fn iter_glyphs_for_char_range(&'a self, rang: &Range<CharIndex>) -> GlyphIterator<'a> {
-        if rang.begin() >= CharIndex(self.entry_buffer.len() as int) {
+        if rang.begin() >= self.char_len() {
             fail!("iter_glyphs_for_range: range.begin beyond length!");
         }
-        if rang.end() > CharIndex(self.entry_buffer.len() as int) {
+        if rang.end() > self.char_len() {
             fail!("iter_glyphs_for_range: range.end beyond length!");
         }
 
@@ -644,56 +644,56 @@ impl<'a> GlyphStore {
 
     // getter methods
     pub fn char_is_space(&self, i: CharIndex) -> bool {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         self.entry_buffer.get(i.to_uint()).char_is_space()
     }
 
     pub fn char_is_tab(&self, i: CharIndex) -> bool {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         self.entry_buffer.get(i.to_uint()).char_is_tab()
     }
 
     pub fn char_is_newline(&self, i: CharIndex) -> bool {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         self.entry_buffer.get(i.to_uint()).char_is_newline()
     }
 
     pub fn is_ligature_start(&self, i: CharIndex) -> bool {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         self.entry_buffer.get(i.to_uint()).is_ligature_start()
     }
 
     pub fn is_cluster_start(&self, i: CharIndex) -> bool {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         self.entry_buffer.get(i.to_uint()).is_cluster_start()
     }
 
     pub fn can_break_before(&self, i: CharIndex) -> BreakType {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         self.entry_buffer.get(i.to_uint()).can_break_before()
     }
 
     // setter methods
     pub fn set_char_is_space(&mut self, i: CharIndex) {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         let entry = *self.entry_buffer.get(i.to_uint());
         *self.entry_buffer.get_mut(i.to_uint()) = entry.set_char_is_space();
     }
 
     pub fn set_char_is_tab(&mut self, i: CharIndex) {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         let entry = *self.entry_buffer.get(i.to_uint());
         *self.entry_buffer.get_mut(i.to_uint()) = entry.set_char_is_tab();
     }
 
     pub fn set_char_is_newline(&mut self, i: CharIndex) {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         let entry = *self.entry_buffer.get(i.to_uint());
         *self.entry_buffer.get_mut(i.to_uint()) = entry.set_char_is_newline();
     }
 
     pub fn set_can_break_before(&mut self, i: CharIndex, t: BreakType) {
-        assert!(i < CharIndex(self.entry_buffer.len() as int));
+        assert!(i < self.char_len());
         let entry = *self.entry_buffer.get(i.to_uint());
         *self.entry_buffer.get_mut(i.to_uint()) = entry.set_can_break_before(t);
     }
@@ -750,7 +750,7 @@ impl<'a> Iterator<(CharIndex, GlyphInfo<'a>)> for GlyphIterator<'a> {
             match self.char_range.next() {
                 Some(i) => {
                     self.char_index = i;
-                    assert!(i < CharIndex(self.store.entry_buffer.len() as int));
+                    assert!(i < self.store.char_len());
                     let entry = self.store.entry_buffer.get(i.to_uint());
                     if entry.is_simple() {
                         Some((self.char_index, SimpleGlyphInfo(self.store, i)))
diff --git a/src/components/main/css/node_util.rs b/src/components/main/css/node_util.rs
index b67098984e8..4fd5fa5d1b2 100644
--- a/src/components/main/css/node_util.rs
+++ b/src/components/main/css/node_util.rs
@@ -68,7 +68,7 @@ impl<'ln> NodeUtil for ThreadSafeLayoutNode<'ln> {
         let default = if self.node_is_element() {
             RestyleDamage::all()
         } else {
-            RestyleDamage::none()
+            RestyleDamage::empty()
         };
 
         let layout_data_ref = self.borrow_layout_data();
@@ -76,7 +76,6 @@ impl<'ln> NodeUtil for ThreadSafeLayoutNode<'ln> {
             .get_ref()
             .data
             .restyle_damage
-            .map(|x| RestyleDamage::from_int(x))
             .unwrap_or(default)
     }
 
@@ -84,7 +83,7 @@ impl<'ln> NodeUtil for ThreadSafeLayoutNode<'ln> {
     fn set_restyle_damage(&self, damage: RestyleDamage) {
         let mut layout_data_ref = self.mutate_layout_data();
         match &mut *layout_data_ref {
-            &Some(ref mut layout_data) => layout_data.data.restyle_damage = Some(damage.to_int()),
+            &Some(ref mut layout_data) => layout_data.data.restyle_damage = Some(damage),
             _ => fail!("no layout data for this node"),
         }
     }
diff --git a/src/components/main/layout/incremental.rs b/src/components/main/layout/incremental.rs
index e050418886e..710e3979e0b 100644
--- a/src/components/main/layout/incremental.rs
+++ b/src/components/main/layout/incremental.rs
@@ -4,100 +4,34 @@
 
 use style::ComputedValues;
 
-/// Individual layout actions that may be necessary after restyling.
-///
-/// If you add to this enum, also add the value to RestyleDamage::all below.
-/// (FIXME: do this automatically)
-pub enum RestyleEffect {
-    /// Repaint the node itself.
-    /// Currently unused; need to decide how this propagates.
-    Repaint = 0x01,
-
-    /// Recompute intrinsic widths (minimum and preferred).
-    /// Propagates down the flow tree because the computation is
-    /// bottom-up.
-    BubbleWidths = 0x02,
-
-    /// Recompute actual widths and heights.
-    /// Propagates up the flow tree because the computation is
-    /// top-down.
-    Reflow = 0x04,
-}
-
-/// A set of RestyleEffects.
-// FIXME: Switch to librustc/util/enum_set.rs if that gets moved into
-// libextra (Rust #8054)
-pub struct RestyleDamage {
-    bits: int
+bitflags! {
+    #[doc = "Individual layout actions that may be necessary after restyling."]
+    flags RestyleDamage: int {
+        #[doc = "Repaint the node itself."]
+        #[doc = "Currently unused; need to decide how this propagates."]
+        static Repaint = 0x01,
+
+        #[doc = "Recompute intrinsic widths (minimum and preferred)."]
+        #[doc = "Propagates down the flow tree because the computation is"]
+        #[doc = "bottom-up."]
+        static BubbleWidths = 0x02,
+
+        #[doc = "Recompute actual widths and heights."]
+        #[doc = "Propagates up the flow tree because the computation is"]
+        #[doc = "top-down."]
+        static Reflow = 0x04
+    }
 }
 
-// Provide literal syntax of the form restyle_damage!(Repaint, Reflow)
-macro_rules! restyle_damage(
-    ( $($damage:ident),* ) => (
-        RestyleDamage::none() $( .add($damage) )*
-    )
-)
-
 impl RestyleDamage {
-    pub fn none() -> RestyleDamage {
-        RestyleDamage { bits: 0 }
-    }
-
-    pub fn all() -> RestyleDamage {
-        restyle_damage!(Repaint, BubbleWidths, Reflow)
-    }
-
-    /// Create a RestyleDamage from the underlying bit field.
-    /// We would rather not allow this, but some types in script
-    /// need to store RestyleDamage without depending on this crate.
-    pub fn from_int(n: int) -> RestyleDamage {
-        RestyleDamage { bits: n }
-    }
-
-    pub fn to_int(self) -> int {
-        self.bits
-    }
-
-    pub fn is_empty(self) -> bool {
-        self.bits == 0
-    }
-
-    pub fn is_nonempty(self) -> bool {
-        self.bits != 0
-    }
-
-    pub fn add(self, effect: RestyleEffect) -> RestyleDamage {
-        RestyleDamage { bits: self.bits | (effect as int) }
-    }
-
-    pub fn has(self, effect: RestyleEffect) -> bool {
-        (self.bits & (effect as int)) != 0
-    }
-
-    pub fn lacks(self, effect: RestyleEffect) -> bool {
-        (self.bits & (effect as int)) == 0
-    }
-
-    pub fn union(self, other: RestyleDamage) -> RestyleDamage {
-        RestyleDamage { bits: self.bits | other.bits }
-    }
-
-    pub fn union_in_place(&mut self, other: RestyleDamage) {
-        self.bits = self.bits | other.bits;
-    }
-
-    pub fn intersect(self, other: RestyleDamage) -> RestyleDamage {
-        RestyleDamage { bits: self.bits & other.bits }
-    }
-
     /// Elements of self which should also get set on any ancestor flow.
     pub fn propagate_up(self) -> RestyleDamage {
-        self.intersect(restyle_damage!(Reflow))
+        self & Reflow
     }
 
     /// Elements of self which should also get set on any child flows.
     pub fn propagate_down(self) -> RestyleDamage {
-        self.intersect(restyle_damage!(BubbleWidths))
+        self & BubbleWidths
     }
 }
 
@@ -108,13 +42,13 @@ macro_rules! add_if_not_equal(
     ($old:ident, $new:ident, $damage:ident,
      [ $($effect:ident),* ], [ $($style_struct_getter:ident.$name:ident),* ]) => ({
         if $( ($old.$style_struct_getter().$name != $new.$style_struct_getter().$name) )||* {
-            $damage.union_in_place( restyle_damage!( $($effect),* ) );
+            $damage.insert($($effect)|*);
         }
     })
 )
 
 pub fn compute_damage(old: &ComputedValues, new: &ComputedValues) -> RestyleDamage {
-    let mut damage = RestyleDamage::none();
+    let mut damage = RestyleDamage::empty();
 
     // This checks every CSS property, as enumerated in
     // impl<'self> CssComputedStyle<'self>
@@ -142,57 +76,3 @@ pub fn compute_damage(old: &ComputedValues, new: &ComputedValues) -> RestyleDama
 
     damage
 }
-
-
-#[cfg(test)]
-mod restyle_damage_tests {
-    use super::*;
-
-    #[test]
-    fn none_is_empty() {
-        let d = RestyleDamage::none();
-        assert!(!d.has(Repaint));
-        assert!(!d.has(BubbleWidths));
-        assert!(d.lacks(Repaint));
-        assert!(d.lacks(BubbleWidths));
-    }
-
-    #[test]
-    fn all_is_full() {
-        let d = RestyleDamage::all();
-        assert!(d.has(Repaint));
-        assert!(d.has(BubbleWidths));
-        assert!(!d.lacks(Repaint));
-        assert!(!d.lacks(BubbleWidths));
-    }
-
-    #[test]
-    fn can_add() {
-        assert!(RestyleDamage::none().add(BubbleWidths).has(BubbleWidths));
-    }
-
-    #[test]
-    fn can_union() {
-        let d = restyle_damage!(Repaint).union(restyle_damage!(BubbleWidths));
-        assert!(d.has(Repaint));
-        assert!(d.has(BubbleWidths));
-    }
-
-    #[test]
-    fn can_union_in_place() {
-        let mut d = restyle_damage!(Repaint);
-        d.union_in_place(restyle_damage!(BubbleWidths));
-        assert!(d.has(Repaint));
-        assert!(d.has(BubbleWidths));
-    }
-
-    #[test]
-    fn can_intersect() {
-        let x = restyle_damage!(Repaint, BubbleWidths);
-        let y = restyle_damage!(Repaint, Reflow);
-        let d = x.intersect(y);
-        assert!(d.has(Repaint));
-        assert!(d.lacks(BubbleWidths));
-        assert!(d.lacks(Reflow));
-    }
-}
diff --git a/src/components/main/layout/inline.rs b/src/components/main/layout/inline.rs
index 1df0e898401..f1911b47226 100644
--- a/src/components/main/layout/inline.rs
+++ b/src/components/main/layout/inline.rs
@@ -750,7 +750,7 @@ impl InlineFragments {
                 style: other_style,
                 range: mut other_range
             } = other_range;
- 
+
             other_range.shift_by(adjustment);
             self.push_range(other_style, other_range)
         }
diff --git a/src/components/main/layout/layout_task.rs b/src/components/main/layout/layout_task.rs
index fe9ebc2aa65..b5aa76a1511 100644
--- a/src/components/main/layout/layout_task.rs
+++ b/src/components/main/layout/layout_task.rs
@@ -118,7 +118,7 @@ impl PostorderFlowTraversal for ComputeDamageTraversal {
     fn process(&mut self, flow: &mut Flow) -> bool {
         let mut damage = flow::base(flow).restyle_damage;
         for child in flow::child_iter(flow) {
-            damage.union_in_place(flow::base(child).restyle_damage.propagate_up())
+            damage.insert(flow::base(child).restyle_damage.propagate_up())
         }
         flow::mut_base(flow).restyle_damage = damage;
         true
@@ -136,14 +136,14 @@ impl PreorderFlowTraversal for PropagateDamageTraversal {
     #[inline]
     fn process(&mut self, flow: &mut Flow) -> bool {
         if self.all_style_damage {
-            flow::mut_base(flow).restyle_damage.union_in_place(RestyleDamage::all())
+            flow::mut_base(flow).restyle_damage.insert(RestyleDamage::all())
         }
         debug!("restyle damage = {:?}", flow::base(flow).restyle_damage);
 
         let prop = flow::base(flow).restyle_damage.propagate_down();
-        if prop.is_nonempty() {
+        if !prop.is_empty() {
             for kid_ctx in flow::child_iter(flow) {
-                flow::mut_base(kid_ctx).restyle_damage.union_in_place(prop)
+                flow::mut_base(kid_ctx).restyle_damage.insert(prop)
             }
         }
         true
diff --git a/src/components/main/layout/util.rs b/src/components/main/layout/util.rs
index 3a37cdd0d03..486cb204804 100644
--- a/src/components/main/layout/util.rs
+++ b/src/components/main/layout/util.rs
@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use layout::construct::{ConstructionResult, NoConstructionResult};
+use layout::incremental::RestyleDamage;
 use layout::parallel::DomParallelInfo;
 use layout::wrapper::{LayoutNode, TLayoutNode, ThreadSafeLayoutNode};
 
@@ -28,7 +29,7 @@ pub struct PrivateLayoutData {
     pub after_style: Option<Arc<ComputedValues>>,
 
     /// Description of how to account for recent style changes.
-    pub restyle_damage: Option<int>,
+    pub restyle_damage: Option<RestyleDamage>,
 
     /// The current results of flow construction for this node. This is either a flow or a
     /// `ConstructionItem`. See comments in `construct.rs` for more details.
diff --git a/src/components/script/dom/node.rs b/src/components/script/dom/node.rs
index c7cde126e42..ba3ea37f936 100644
--- a/src/components/script/dom/node.rs
+++ b/src/components/script/dom/node.rs
@@ -106,31 +106,22 @@ impl NodeDerived for EventTarget {
     }
 }
 
-/// Flags for node items.
-#[deriving(Encodable)]
-pub struct NodeFlags(pub u8);
+bitflags! {
+    #[doc = "Flags for node items."]
+    #[deriving(Encodable)]
+    flags NodeFlags: u8 {
+        #[doc = "Specifies whether this node is in a document."]
+        static IsInDoc = 0x01,
+        #[doc = "Specifies whether this node is hover state for this node"]
+        static InHoverState = 0x02
+    }
+}
 
 impl NodeFlags {
     pub fn new(type_id: NodeTypeId) -> NodeFlags {
-        let mut flags = NodeFlags(0);
         match type_id {
-            DocumentNodeTypeId => { flags.set_is_in_doc(true); }
-            _ => {}
-        }
-        flags
-    }
-}
-
-/// Specifies whether this node is in a document.
-bitfield!(NodeFlags, is_in_doc, set_is_in_doc, 0x01)
-/// Specifies whether this node is hover state for this node
-bitfield!(NodeFlags, get_in_hover_state, set_is_in_hover_state, 0x02)
-
-#[unsafe_destructor]
-impl Drop for Node {
-    fn drop(&mut self) {
-        unsafe {
-            self.reap_layout_data()
+            DocumentNodeTypeId => IsInDoc,
+            _                  => NodeFlags::empty(),
         }
     }
 }
@@ -424,7 +415,7 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
     }
 
     fn is_in_doc(&self) -> bool {
-        self.deref().flags.is_in_doc()
+        self.deref().flags.contains(IsInDoc)
     }
 
     /// Returns the type ID of this node. Fails if this node is borrowed mutably.
@@ -483,11 +474,15 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
     }
 
     fn get_hover_state(&self) -> bool {
-        self.flags.get_in_hover_state()
+        self.flags.contains(InHoverState)
     }
 
     fn set_hover_state(&mut self, state: bool) {
-        self.flags.set_is_in_hover_state(state);
+        if state {
+            self.flags.insert(InHoverState);
+        } else {
+            self.flags.remove(InHoverState);
+        }
     }
 
     /// Iterates over this node and all its descendants, in preorder.
@@ -668,7 +663,7 @@ pub trait RawLayoutNodeHelpers {
 
 impl RawLayoutNodeHelpers for Node {
     unsafe fn get_hover_state_for_layout(&self) -> bool {
-        self.flags.get_in_hover_state()
+        self.flags.contains(InHoverState)
     }
 }
 
@@ -1071,7 +1066,11 @@ impl Node {
         // Step 8.
         for node in nodes.mut_iter() {
             parent.add_child(node, child);
-            node.deref_mut().flags.set_is_in_doc(parent.is_in_doc());
+            if parent.is_in_doc() {
+                node.flags.insert(IsInDoc);
+            } else {
+                node.flags.remove(IsInDoc);
+            }
         }
 
         // Step 9.
@@ -1157,7 +1156,7 @@ impl Node {
 
         // FIXME(2513): remove this `node_alias` when in fix mozilla#2513
         let mut node_alias = node.clone();
-        node_alias.deref_mut().flags.set_is_in_doc(false);
+        node_alias.deref_mut().flags.remove(IsInDoc);
 
         // Step 9.
         match suppress_observers {