Fix handling of Heap<T>

components/plugins/reflector.rs

@@ -23,6 +23,9 @@ pub fn expand_reflector(cx: &mut ExtCtxt, span: Span, _: &MetaItem, item: &Item,
                         fn reflector<'a>(&'a self) -> &'a ::dom::bindings::utils::Reflector {
                             &self.$field_name
                         }
+                        fn init_reflector(&mut self, obj: *mut ::js::jsapi::JSObject) {
+                            self.$field_name.set_jsobject(obj);
+                        }
                     }
                 );
                 impl_item.map(|it| push(it))
@@ -35,6 +38,9 @@ pub fn expand_reflector(cx: &mut ExtCtxt, span: Span, _: &MetaItem, item: &Item,
                         fn reflector<'a>(&'a self) -> &'a ::dom::bindings::utils::Reflector {
                             self.$field_name.reflector()
                         }
+                        fn init_reflector(&mut self, obj: *mut ::js::jsapi::JSObject) {
+                            self.$field_name.init_reflector(obj);
+                        }
                     }
                 );
                 impl_item.map(|it| push(it))

components/script/dom/bindings/callback.rs

@@ -18,6 +18,7 @@ use js::jsval::{JSVal, UndefinedValue};
 
 use std::ffi::CString;
 use std::ptr;
+use std::rc::Rc;
 
 /// The exception handling used for a call.
 #[derive(Copy, Clone, PartialEq)]
@@ -29,25 +30,31 @@ pub enum ExceptionHandling {
 }
 
 /// A common base class for representing IDL callback function types.
-#[derive(Copy, Clone,PartialEq)]
+#[derive(PartialEq)]
 #[jstraceable]
 pub struct CallbackFunction {
     object: CallbackObject
 }
 
 impl CallbackFunction {
     /// Create a new `CallbackFunction` for this object.
-    pub fn new(callback: *mut JSObject) -> CallbackFunction {
+    pub fn new() -> CallbackFunction {
         CallbackFunction {
             object: CallbackObject {
-                callback: Heap::<*mut JSObject>::new(callback)
+                callback: Heap { ptr: ptr::null_mut() }
             }
         }
     }
+
+    /// Initialize the callback function with a value.
+    /// Should be called once this object is done moving.
+    pub fn init(&mut self, callback: *mut JSObject) {
+        self.object.callback.set(callback);
+    }
 }
 
 /// A common base class for representing IDL callback interface types.
-#[derive(Copy, Clone,PartialEq)]
+#[derive(PartialEq)]
 #[jstraceable]
 pub struct CallbackInterface {
     object: CallbackObject
@@ -56,7 +63,7 @@ pub struct CallbackInterface {
 /// A common base class for representing IDL callback function and
 /// callback interface types.
 #[allow(raw_pointer_derive)]
-#[derive(Copy, Clone, PartialEq)]
+#[derive(PartialEq)]
 #[jstraceable]
 struct CallbackObject {
     /// The underlying `JSObject`.
@@ -67,7 +74,7 @@ struct CallbackObject {
 /// callback interface types.
 pub trait CallbackContainer {
     /// Create a new CallbackContainer object for the given `JSObject`.
-    fn new(callback: *mut JSObject) -> Self;
+    fn new(callback: *mut JSObject) -> Rc<Self>;
     /// Returns the underlying `JSObject`.
     fn callback(&self) -> *mut JSObject;
 }
@@ -88,14 +95,20 @@ impl CallbackFunction {
 
 impl CallbackInterface {
     /// Create a new CallbackInterface object for the given `JSObject`.
-    pub fn new(callback: *mut JSObject) -> CallbackInterface {
+    pub fn new() -> CallbackInterface {
         CallbackInterface {
             object: CallbackObject {
-                callback: Heap::<*mut JSObject>::new(callback)
+                callback: Heap { ptr: ptr::null_mut() }
             }
         }
     }
 
+    /// Initialize the callback function with a value.
+    /// Should be called once this object is done moving.
+    pub fn init(&mut self, callback: *mut JSObject) {
+        self.object.callback.set(callback);
+    }
+
     /// Returns the property with the given `name`, if it is a callable object,
     /// or `Err(())` otherwise. If it returns `Err(())`, a JSAPI exception is
     /// pending.
@@ -152,7 +165,7 @@ pub struct CallSetup {
 impl CallSetup {
     /// Performs the setup needed to make a call.
     #[allow(unrooted_must_root)]
-    pub fn new<T: CallbackContainer>(callback: T, handling: ExceptionHandling) -> CallSetup {
+    pub fn new<T: CallbackContainer>(callback: &T, handling: ExceptionHandling) -> CallSetup {
         let global = global_object_for_js_object(callback.callback());
         let cx = global.r().get_cx();
         unsafe { JS_BeginRequest(cx); }

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

@@ -761,7 +761,7 @@ def wrapObjectTemplate(templateBody, nullValue, isDefinitelyObject, type,
 
         if descriptor.interface.isCallback():
             name = descriptor.nativeType
-            declType = CGGeneric(name)
+            declType = CGWrapper(CGGeneric(name), pre="Rc<", post=">")
             template = "%s::new(${val}.get().to_object())" % name
             if type.nullable():
                 declType = CGWrapper(declType, pre="Option<", post=">")
@@ -932,11 +932,13 @@ def wrapObjectTemplate(templateBody, nullValue, isDefinitelyObject, type,
         assert not type.treatNonObjectAsNull() or not type.treatNonCallableAsNull()
 
         declType = CGGeneric('%s::%s' % (type.unroll().module(), type.unroll().identifier.name))
+        finalDeclType = CGTemplatedType("Rc", declType)
 
         conversion = CGCallbackTempRoot(declType.define())
 
         if type.nullable():
             declType = CGTemplatedType("Option", declType)
+            finalDeclType = CGTemplatedType("Option", finalDeclType)
             conversion = CGWrapper(conversion, pre="Some(", post=")")
 
         if allowTreatNonObjectAsNull and type.treatNonObjectAsNull():
@@ -967,7 +969,7 @@ def wrapObjectTemplate(templateBody, nullValue, isDefinitelyObject, type,
         else:
             default = None
 
-        return JSToNativeConversionInfo(template, default, declType, needsRooting=needsRooting)
+        return JSToNativeConversionInfo(template, default, finalDeclType, needsRooting=needsRooting)
 
     if type.isAny():
         assert not isEnforceRange and not isClamp
@@ -1275,8 +1277,8 @@ def getRetvalDeclarationForType(returnType, descriptorProvider):
             result = CGWrapper(result, pre="Option<", post=">")
         return result
     if returnType.isCallback():
-        result = CGGeneric('%s::%s' % (returnType.unroll().module(),
-                                       returnType.unroll().identifier.name))
+        result = CGGeneric('Rc<%s::%s>' % (returnType.unroll().module(),
+                                           returnType.unroll().identifier.name))
         if returnType.nullable():
             result = CGWrapper(result, pre="Option<", post=">")
         return result
@@ -2119,7 +2121,7 @@ def definition_body(self):
 
 %s
 
-(*raw).reflector().set_jsobject(obj.ptr);
+(*raw).init_reflector(obj.ptr);
 
 Root::from_ref(&*raw)""" % CreateBindingJSObject(self.descriptor, "scope"))
         else:
@@ -2131,7 +2133,7 @@ def definition_body(self):
 let proto = RootedObject::new(cx, GetProtoObject(cx, obj.handle(), obj.handle()));
 JS_SetPrototype(cx, obj.handle(), proto.handle());
 
-(*raw).reflector().set_jsobject(obj.ptr);
+(*raw).init_reflector(obj.ptr);
 
 let ret = Root::from_ref(&*raw);
 
@@ -3735,9 +3737,14 @@ def getInitializationList(self, cgClass):
     def getBody(self, cgClass):
         initializers = ["    parent: %s" % str(self.baseConstructors[0])]
         return (self.body + (
-                "%s {\n"
+                "let mut ret = Rc::new(%s {\n"
                 "%s\n"
-                "}") % (cgClass.name, '\n'.join(initializers)))
+                "});\n"
+                "match rc::get_mut(&mut ret) {\n"
+                "    Some(ref mut callback) => callback.parent.init(%s),\n"
+                "    None => unreachable!(),\n"
+                "};\n"
+                "ret") % (cgClass.name, '\n'.join(initializers), self.args[0].name))
 
     def declare(self, cgClass):
         args = ', '.join([a.declare() for a in self.args])
@@ -3748,7 +3755,7 @@ def declare(self, cgClass):
         body = ' {\n' + body + '}'
 
         return string.Template("""\
-pub fn ${decorators}new(${args}) -> ${className}${body}
+pub fn ${decorators}new(${args}) -> Rc<${className}>${body}
 """).substitute({ 'decorators': self.getDecorators(True),
                   'className': cgClass.getNameString(),
                   'args': args,
@@ -5116,6 +5123,8 @@ def __init__(self, config, prefix, webIDLFile):
             'std::num',
             'std::ptr',
             'std::str',
+            'std::rc',
+            'std::rc::Rc',
             'std::default::Default',
             'std::ffi::CString',
         ])
@@ -5224,7 +5233,7 @@ def __init__(self, idlObject, descriptorProvider, baseName, methods,
                          bases=[ClassBase(baseName)],
                          constructors=self.getConstructors(),
                          methods=realMethods+getters+setters,
-                         decorators="#[derive(PartialEq,Copy,Clone)]#[jstraceable]")
+                         decorators="#[derive(PartialEq)]#[jstraceable]")
 
     def getConstructors(self):
         return [ClassConstructor(
@@ -5233,7 +5242,7 @@ def getConstructors(self):
             visibility="pub",
             explicit=False,
             baseConstructors=[
-                "%s::new(aCallback)" % self.baseName
+                "%s::new()" % self.baseName
                 ])]
 
     def getMethodImpls(self, method):
@@ -5260,9 +5269,9 @@ def getMethodImpls(self, method):
         args.insert(0, Argument("&T",  "thisObj"))
 
         # And the self argument
-        method.args.insert(0, Argument(None, "self"))
-        args.insert(0, Argument(None, "self"))
-        argsWithoutThis.insert(0, Argument(None, "self"))
+        method.args.insert(0, Argument(None, "&self"))
+        args.insert(0, Argument(None, "&self"))
+        argsWithoutThis.insert(0, Argument(None, "&self"))
 
         setupCall = ("let s = CallSetup::new(self, aExceptionHandling);\n"
                      "if s.get_context().is_null() {\n"
@@ -5323,7 +5332,7 @@ class CGCallbackFunctionImpl(CGGeneric):
     def __init__(self, callback):
         impl = string.Template("""\
 impl CallbackContainer for ${type} {
-    fn new(callback: *mut JSObject) -> ${type} {
+    fn new(callback: *mut JSObject) -> Rc<${type}> {
         ${type}::new(callback)
     }
 

components/script/dom/bindings/codegen/Configuration.py

@@ -148,7 +148,7 @@ def __init__(self, config, interface, desc):
         if self.interface.isCallback():
             self.needsRooting = False
             ty = "%sBinding::%s" % (ifaceName, ifaceName)
-            self.returnType = ty
+            self.returnType = "Rc<%s>"% ty
             self.argumentType = "???"
             self.memberType = "???"
             self.nativeType = ty

components/script/dom/bindings/conversions.rs

@@ -62,6 +62,7 @@ use std::borrow::ToOwned;
 use std::default;
 use std::slice;
 use std::ptr;
+use std::rc::Rc;
 use core::nonzero::NonZero;
 
 /// A trait to retrieve the constants necessary to check if a `JSObject`
@@ -625,6 +626,15 @@ impl<T: ToJSValConvertible> ToJSValConvertible for Option<T> {
     }
 }
 
+impl<T: ToJSValConvertible> ToJSValConvertible for Option<Rc<T>> {
+    fn to_jsval(&self, cx: *mut JSContext) -> JSVal {
+        match self {
+            &Some(ref value) => (**value).to_jsval(cx),
+            &None => NullValue(),
+        }
+    }
+}
+
 impl<X: default::Default, T: FromJSValConvertible<Config=X>> FromJSValConvertible for Option<T> {
     type Config = ();
     fn from_jsval(cx: *mut JSContext, value: HandleValue, _: ()) -> Result<Option<T>, ()> {

components/script/dom/bindings/trace.rs

@@ -58,7 +58,7 @@ use msg::constellation_msg::ConstellationChan;
 use net_traits::image::base::Image;
 use util::smallvec::SmallVec1;
 use util::str::{LengthOrPercentageOrAuto};
-use std::cell::{Cell, RefCell};
+use std::cell::{Cell, UnsafeCell, RefCell};
 use std::collections::{HashMap, HashSet};
 use std::collections::hash_state::HashState;
 use std::ffi::CString;
@@ -170,6 +170,12 @@ impl<T: JSTraceable+Copy> JSTraceable for Cell<T> {
     }
 }
 
+impl<T: JSTraceable> JSTraceable for UnsafeCell<T> {
+    fn trace(&self, trc: *mut JSTracer) {
+        unsafe { (*self.get()).trace(trc) }
+    }
+}
+
 
 impl JSTraceable for Heap<*mut JSObject> {
     fn trace(&self, trc: *mut JSTracer) {

components/script/dom/bindings/utils.rs

@@ -18,7 +18,6 @@ use util::str::DOMString;
 use libc;
 use libc::c_uint;
 use std::boxed;
-use std::cell::Cell;
 use std::ffi::CString;
 use std::ptr;
 use std::cmp::PartialEq;
@@ -362,6 +361,10 @@ pub fn initialize_global(global: *mut JSObject) {
 pub trait Reflectable {
     /// Returns the receiver's reflector.
     fn reflector<'a>(&'a self) -> &'a Reflector;
+    /// Initializes the Reflector
+    fn init_reflector(&mut self, _obj: *mut JSObject) {
+        panic!("Cannot call init on this Reflectable");
+    }
 }
 
 /// Create the reflector for a new DOM object and yield ownership to the
@@ -381,44 +384,41 @@ pub fn reflect_dom_object<T: Reflectable>
 #[servo_lang = "reflector"]
 // If you're renaming or moving this field, update the path in plugins::reflector as well
 pub struct Reflector {
-    object: Cell<Heap<*mut JSObject>>,
+    object: Heap<*mut JSObject>,
 }
 
 #[allow(unrooted_must_root)]
 impl PartialEq for Reflector {
     fn eq(&self, other: &Reflector) -> bool {
-        self.object.get().ptr == other.object.get().ptr
+        self.object.ptr == other.object.ptr
     }
 }
 
 impl Reflector {
     /// Get the reflector.
     #[inline]
     pub fn get_jsobject(&self) -> *mut JSObject {
-        self.object.get().ptr
+        self.object.ptr
     }
 
     /// Initialize the reflector. (May be called only once.)
-    pub fn set_jsobject(&self, object: *mut JSObject) {
-        assert!(self.object.get().ptr.is_null());
+    pub fn set_jsobject(&mut self, object: *mut JSObject) {
+        assert!(self.object.ptr.is_null());
         assert!(!object.is_null());
-        unsafe {
-            let cell = self.object.as_unsafe_cell().get();
-            (*cell).set(object);
-        }
+        self.object.set(object);
     }
 
     /// Return a pointer to the memory location at which the JS reflector
     /// object is stored. Used to root the reflector, as
     /// required by the JSAPI rooting APIs.
     pub unsafe fn rootable(&self) -> *mut Heap<*mut JSObject> {
-        self.object.as_unsafe_cell().get()
+        &self.object as *const Heap<*mut JSObject> as *mut Heap<*mut JSObject>
     }
 
     /// Create an uninitialized `Reflector`.
     pub fn new() -> Reflector {
         Reflector {
-            object: Cell::new(Heap { ptr: ptr::null_mut() })
+            object: Heap { ptr: ptr::null_mut() }
         }
     }
 }