Address easy feedback.

src/components/script/dom/bindings/callback.rs

@@ -34,9 +34,15 @@ impl<S: Encoder<E>, E> Encodable<S, E> for CallbackInterface {
         unsafe {
             let tracer: *mut JSTracer = cast::transmute(s);
             "callback".to_c_str().with_ref(|name| {
-                //XXXjdm the arg can be moved
+                // FIXME: JS_CallObjectTracer could theoretically do something that can
+                //        cause pointers to shuffle around. We need to pass a *mut *mut JSObject
+                //        to JS_CallObjectTracer, but the Encodable trait doesn't allow us
+                //        to obtain a mutable reference to self (and thereby self.cb);
+                //        All we can do right now is scream loudly if this actually causes
+                //        a problem in practice.
                 let mut cb = self.callback;
                 JS_CallObjectTracer(tracer, &mut cb, name);
+                assert!(cb == self.callback);
             });
         };
         Ok(())

src/components/script/dom/bindings/codegen/CodegenRust.py

@@ -5204,7 +5204,7 @@ def getCall(self):
         return string.Template("${getCallable}"
                 "let ok = unsafe {\n"
                 "  CallFunctionValue(cx, object_handle(&${thisObj}), value_handle(&callable),\n"
-                "                    argc as libc::size_t, &argv[0], rval)\n"
+                "                    ${argc} as libc::size_t, ${argv} as *mut JSVal as *JSVal, rval)\n"
                 "};\n"
                 "if !ok {\n"
                 "  return${errorReturn};\n"

src/components/script/dom/bindings/js.rs

@@ -41,9 +41,7 @@
 
 use dom::bindings::utils::{Reflector, Reflectable, /*cx_for_dom_object*/};
 use dom::node::Node;
-//use js::glue::{AddObjectRoot, RemoveObjectRoot};
 use js::jsapi::JSObject;
-//use js::rust::JSAutoRequest;
 use layout_interface::TrustedNodeAddress;
 use script_task::StackRoots;
 
@@ -57,7 +55,7 @@ use std::local_data;
 /// `JS<T>::assign` method or `OptionalSettable::assign` (for Option<JS<T>> fields).
 pub struct Temporary<T> {
     inner: JS<T>,
-    root: *mut JSObject,
+    root: *mut JSObject, //XXXjdm not sure that this does anything anymore
 }
 
 impl<T> Eq for Temporary<T> {
@@ -69,22 +67,14 @@ impl<T> Eq for Temporary<T> {
 #[unsafe_destructor]
 impl<T: Reflectable> Drop for Temporary<T> {
     fn drop(&mut self) {
-        /*let cx = cx_for_dom_object(&self.inner);
-        let _ar = JSAutoRequest::new(cx);
-        unsafe {
-            RemoveObjectRoot(cx, self.inner.mut_reflector().rootable());
-        }*/
+        //XXXjdm use the JS::PersistentRooted mechanism to ensure proper rooting
     }
 }
 
 impl<T: Reflectable> Temporary<T> {
     /// Create a new Temporary value from a JS-owned value.
     pub fn new(mut inner: JS<T>) -> Temporary<T> {
-        //let cx = cx_for_dom_object(&inner);
-        //let _ar = JSAutoRequest::new(cx);
-        /*unsafe {
-            AddObjectRoot(cx, inner.mut_reflector().rootable());
-        }*/
+        //XXXjdm use the JS::PersistentRooted mechanism to ensure proper rooting
         Temporary {
             root: inner.reflector().get_jsobject(),
             inner: inner,

src/components/script/dom/bindings/utils.rs

@@ -490,32 +490,6 @@ pub fn GetArrayIndexFromId(_cx: *mut JSContext, id: jsid) -> Option<u32> {
     }*/
 }
 
-/*fn InternJSString(cx: *mut JSContext, chars: *libc::c_char) -> Option<JSHandleId> {
-    unsafe {
-        let s = JS_InternString(cx, chars);
-        if s.is_not_null() {
-            Some(RUST_INTERNED_STRING_TO_JSID(cx, s))
-        } else {
-            None
-        }
-    }
-}*/
-
-pub fn InitIds(_cx: *mut JSContext, _specs: &[JSPropertySpec], _ids: &mut [jsid]) -> bool {
-    /*for (i, spec) in specs.iter().enumerate() {
-        if spec.name.is_null() == true {
-            return true;
-        }
-        match InternJSString(cx, spec.name) {
-            Some(id) => ids[i] = id,
-            None => {
-                return false;
-            }
-        }
-    }*/
-    true
-}
-
 pub fn FindEnumStringIndex(cx: *mut JSContext,
                            v: JSVal,
                            values: &[&'static str]) -> Result<Option<uint>, ()> {
@@ -612,6 +586,7 @@ pub fn IsConvertibleToCallbackInterface(cx: *mut JSContext, obj: *mut JSObject)
 
 pub fn CreateDOMGlobal(cx: *mut JSContext, class: *JSClass) -> *mut JSObject {
     unsafe {
+        //XXXjdm need to trace the protoiface cache, too
         let obj = NewGlobalObject(cx, class, ptr::mut_null(), 0 /*FireOnNewGlobalHook*/);
         if obj.is_null() {
             return ptr::mut_null();
@@ -629,24 +604,6 @@ pub fn CreateDOMGlobal(cx: *mut JSContext, class: *JSClass) -> *mut JSObject {
     }
 }
 
-pub extern fn wrap_for_same_compartment(cx: *mut JSContext, obj: *mut JSObject) -> *mut JSObject {
-    unsafe {
-        let clasp = JS_GetClass(obj);
-        let clasp = clasp as *js::Class;
-        match (*clasp).ext.outerObject {
-            Some(outerize) => {
-                debug!("found an outerize hook");
-                let obj = JSHandleObject { unnamed_field1: &obj };
-                outerize(cx, obj)
-            }
-            None => {
-                debug!("no outerize hook found");
-                obj
-            }
-        }
-    }
-}
-
 pub extern fn outerize_global(_cx: *mut JSContext, obj: JSHandleObject) -> *mut JSObject {
     unsafe {
         debug!("outerizing");

src/components/script/dom/window.rs

@@ -5,7 +5,7 @@
 use dom::bindings::codegen::BindingDeclarations::WindowBinding;
 use dom::bindings::js::{JS, JSRef, Temporary, OptionalSettable};
 use dom::bindings::trace::{Traceable, Untraceable};
-use dom::bindings::utils::{Reflectable, Reflector, /*object_handle, value_handle*/};
+use dom::bindings::utils::{Reflectable, Reflector};
 use dom::browsercontext::BrowserContext;
 use dom::document::Document;
 use dom::element::Element;

src/components/script/script_task.rs

@@ -36,7 +36,7 @@ use layout_interface;
 use geom::point::Point2D;
 use geom::size::Size2D;
 use js::glue::CallFunctionValue;
-use js::jsapi::{/*JS_SetWrapObjectCallbacks,*/ JS_SetGCZeal, JS_GC};
+use js::jsapi::{JS_SetGCZeal, JS_GC};
 use js::jsapi::{JSContext, JSRuntime, JSMutableHandleValue};
 use js::jsval::NullValue;
 use js::rust::{Cx, RtUtils};
@@ -635,24 +635,6 @@ impl ScriptTask {
             let ptr: *mut JSRuntime = (*js_runtime).ptr;
             ptr.is_not_null()
         });
-        /*unsafe {
-            // JS_SetWrapObjectCallbacks clobbers the existing wrap callback,
-            // and JSCompartment::wrap crashes if that happens. The only way
-            // to retrieve the default callback is as the result of
-            // JS_SetWrapObjectCallbacks, which is why we call it twice.
-            let callbacks = Struct_JSWrapObjectCallbacks {
-                wrap: None,
-                preWrap: Some(wrap_for_same_compartment),
-            };
-            let callback = JS_SetWrapObjectCallbacks((*js_runtime).ptr,
-                                                     ptr::mut_null(),
-                                                     wrap_for_same_compartment,
-                                                     ptr::mut_null());
-            JS_SetWrapObjectCallbacks((*js_runtime).ptr,
-                                      callback,
-                                      wrap_for_same_compartment,
-                                      ptr::mut_null());
-        }*/
 
         let js_context = js_runtime.cx();
         assert!({

src/test/content/test_global.html

@@ -2,6 +2,7 @@
 <head>
 <script src="harness.js"></script>
 <script>
+gc();
 is(window, window.window);
 is(window, this);
 for (var key in this) {