Address review comments

components/plugins/jstraceable.rs

@@ -31,7 +31,7 @@ use syntax::ext::deriving::generic::{combine_substructure, EnumMatching, FieldIn
             }
         )
     };
-    trait_def.expand(cx, mitem, item, push)    
+    trait_def.expand(cx, mitem, item, push)
 }
 
 // Mostly copied from syntax::ext::deriving::hash
@@ -40,23 +40,20 @@ fn jstraceable_substructure(cx: &mut ExtCtxt, trait_span: Span, substr: &Substru
         [ref state_expr] => state_expr,
         _ => cx.span_bug(trait_span, "incorrect number of arguments in `jstraceable`")
     };
-    let hash_ident = substr.method_ident;
+    let trace_ident = substr.method_ident;
-    let call_hash = |span, thing_expr| {
+    let call_trace = |span, thing_expr| {
-        let expr = cx.expr_method_call(span, thing_expr, hash_ident, vec!(state_expr.clone()));
+        let expr = cx.expr_method_call(span, thing_expr, trace_ident, vec!(state_expr.clone()));
         cx.stmt_expr(expr)
     };
     let mut stmts = Vec::new();
 
     let fields = match *substr.fields {
-        Struct(ref fs) => fs,
+        Struct(ref fs) | EnumMatching(_, _, ref fs) => fs,
-        EnumMatching(_, _, ref fs) => {
-            fs
-        }
         _ => cx.span_bug(trait_span, "impossible substructure in `jstraceable`")
     };
 
     for &FieldInfo { ref self_, span, .. } in fields.iter() {
-        stmts.push(call_hash(span, self_.clone()));
+        stmts.push(call_trace(span, self_.clone()));
     }
 
     cx.expr_block(cx.block(trait_span, stmts, None))

components/script/dom/bindings/trace.rs

@@ -21,7 +21,7 @@
 //!    reflector.
 //! 5. `trace_object()` calls `JS_CallTracer()` to notify the GC, which will
 //!    add the object to the graph, and will trace that object as well.
-//! 
+//!
 //! The untraceable!() macro adds an empty implementation of JSTraceable to 
 //! a datatype.
 
@@ -158,13 +158,13 @@ impl<T: JSTraceable> JSTraceable for RefCell<T> {
 
 impl<T: JSTraceable> JSTraceable for Rc<T> {
     fn trace(&self, trc: *mut JSTracer) {
-        self.trace(trc)
+        self.deref().trace(trc)
     }
 }
 
 impl<T: JSTraceable> JSTraceable for Box<T> {
     fn trace(&self, trc: *mut JSTracer) {
-        self.trace(trc)
+        (**self).trace(trc)
     }
 }
 
@@ -221,13 +221,15 @@ impl<K: Eq+Hash, V: JSTraceable> JSTraceable for HashMap<K, V> {
 }
 
 untraceable!(bool, f32, f64, String, Url)
-untraceable!(SubpageId, WindowSizeData, PipelineId)
 untraceable!(uint, u8, u16, u32, u64)
 untraceable!(int, i8, i16, i32, i64)
 untraceable!(Untraceable<T>)
 untraceable!(ImageCacheTask, ScriptControlChan)
 untraceable!(Atom, Namespace)
 untraceable!(PropertyDeclarationBlock)
+// These three are interdependent, if you plan to put jsmanaged data
+// in one of these make sure it is propagated properly to containing structs
+untraceable!(SubpageId, WindowSizeData, PipelineId)
 
 impl<'a> JSTraceable for &'a str {
     #[inline]

components/script/dom/htmlcollection.rs

@@ -60,7 +60,7 @@ impl HTMLCollection {
     fn all_elements(window: JSRef<Window>, root: JSRef<Node>,
                     namespace_filter: Option<Namespace>) -> Temporary<HTMLCollection> {
         #[jstraceable]
-struct AllElementFilter {
+        struct AllElementFilter {
             namespace_filter: Option<Namespace>
         }
         impl CollectionFilter for AllElementFilter {
@@ -82,7 +82,7 @@ struct AllElementFilter {
         }
 
         #[jstraceable]
-struct TagNameFilter {
+        struct TagNameFilter {
             tag: Atom,
             ascii_lower_tag: Atom,
         }
@@ -118,7 +118,7 @@ struct TagNameFilter {
             return HTMLCollection::all_elements(window, root, namespace_filter);
         }
         #[jstraceable]
-struct TagNameNSFilter {
+        struct TagNameNSFilter {
             tag: Atom,
             namespace_filter: Option<Namespace>
         }
@@ -143,7 +143,7 @@ struct TagNameNSFilter {
     pub fn by_class_name(window: JSRef<Window>, root: JSRef<Node>, classes: DOMString)
                          -> Temporary<HTMLCollection> {
         #[jstraceable]
-struct ClassNameFilter {
+        struct ClassNameFilter {
             classes: Vec<DOMString>
         }
         impl CollectionFilter for ClassNameFilter {
@@ -159,7 +159,7 @@ struct ClassNameFilter {
 
     pub fn children(window: JSRef<Window>, root: JSRef<Node>) -> Temporary<HTMLCollection> {
         #[jstraceable]
-struct ElementChildFilter;
+        struct ElementChildFilter;
         impl CollectionFilter for ElementChildFilter {
             fn filter(&self, elem: JSRef<Element>, root: JSRef<Node>) -> bool {
                 root.is_parent_of(NodeCast::from_ref(elem))

components/script/dom/htmldatalistelement.rs

@@ -45,7 +45,7 @@ impl HTMLDataListElement {
 impl<'a> HTMLDataListElementMethods for JSRef<'a, HTMLDataListElement> {
     fn Options(self) -> Temporary<HTMLCollection> {
         #[jstraceable]
-struct HTMLDataListOptionsFilter;
+        struct HTMLDataListOptionsFilter;
         impl CollectionFilter for HTMLDataListOptionsFilter {
             fn filter(&self, elem: JSRef<Element>, _root: JSRef<Node>) -> bool {
                 elem.is_htmloptionelement()

components/script/dom/htmlfieldsetelement.rs

@@ -51,7 +51,7 @@ impl<'a> HTMLFieldSetElementMethods for JSRef<'a, HTMLFieldSetElement> {
     // http://www.whatwg.org/html/#dom-fieldset-elements
     fn Elements(self) -> Temporary<HTMLCollection> {
         #[jstraceable]
-struct ElementsFilter;
+        struct ElementsFilter;
         impl CollectionFilter for ElementsFilter {
             fn filter(&self, elem: JSRef<Element>, root: JSRef<Node>) -> bool {
                 static tag_names: StaticStringVec = &["button", "fieldset", "input",

components/script/dom/macros.rs

@@ -46,7 +46,7 @@ macro_rules! make_uint_getter(
 /// Use #[jstraceable] on JS managed types
 macro_rules! untraceable(
     ($($ty:ident),+) => (
-        $(  
+        $(
             impl JSTraceable for $ty {
                 #[inline]
                 fn trace(&self, _: *mut JSTracer) {

components/script/dom/node.rs

@@ -64,8 +64,6 @@ use style::ComputedValues;
 use sync::Arc;
 use uuid;
 
-
-
 //
 // The basic Node structure
 //

components/script/dom/treewalker.rs

@@ -19,7 +19,6 @@ use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
 use dom::document::Document;
 use dom::node::{Node, NodeHelpers};
 
-
 use std::cell::Cell;
 
 // http://dom.spec.whatwg.org/#interface-treewalker

components/script/page.rs

@@ -35,8 +35,6 @@ use std::mem::replace;
 use std::rc::Rc;
 use url::Url;
 
-
-
 /// Encapsulates a handle to a frame and its associated layout information.
 #[jstraceable]
 pub struct Page {