Stop implementing Copy for JS<T>.

A copy of a JS<T> doesn't have the rooting properties of the original,
so it makes no sense for it to implement Copy.
This commit is contained in:
Eli Friedman 2015-10-14 16:20:47 -07:00
parent 9d5f09e09c
commit 88a1cbb28b
7 changed files with 47 additions and 35 deletions

View file

@ -32,9 +32,10 @@ use js::jsapi::{Heap, JSObject, JSTracer};
use js::jsval::JSVal; use js::jsval::JSVal;
use layout_interface::TrustedNodeAddress; use layout_interface::TrustedNodeAddress;
use script_task::STACK_ROOTS; use script_task::STACK_ROOTS;
use std::cell::{Cell, UnsafeCell}; use std::cell::UnsafeCell;
use std::default::Default; use std::default::Default;
use std::ops::Deref; use std::ops::Deref;
use std::ptr;
use util::mem::HeapSizeOf; use util::mem::HeapSizeOf;
/// A traced reference to a DOM object. Must only be used as a field in other /// A traced reference to a DOM object. Must only be used as a field in other
@ -54,7 +55,7 @@ impl<T> HeapSizeOf for JS<T> {
impl<T> JS<T> { impl<T> JS<T> {
/// Returns `LayoutJS<T>` containing the same pointer. /// Returns `LayoutJS<T>` containing the same pointer.
pub unsafe fn to_layout(self) -> LayoutJS<T> { pub unsafe fn to_layout(&self) -> LayoutJS<T> {
LayoutJS { LayoutJS {
ptr: self.ptr.clone() ptr: self.ptr.clone()
} }
@ -109,8 +110,6 @@ impl<T: Reflectable> LayoutJS<T> {
} }
} }
impl<T> Copy for JS<T> {}
impl<T> Copy for LayoutJS<T> {} impl<T> Copy for LayoutJS<T> {}
impl<T> PartialEq for JS<T> { impl<T> PartialEq for JS<T> {
@ -208,56 +207,62 @@ impl MutHeapJSVal {
/// `JS<T>`. /// `JS<T>`.
#[must_root] #[must_root]
#[derive(JSTraceable)] #[derive(JSTraceable)]
#[derive(HeapSizeOf)] pub struct MutHeap<T: HeapGCValue> {
pub struct MutHeap<T: HeapGCValue + Copy> { val: UnsafeCell<T>,
val: Cell<T>,
} }
impl<T: HeapGCValue + Copy> MutHeap<T> { impl<T: HeapGCValue> MutHeap<T> {
/// Create a new `MutHeap`. /// Create a new `MutHeap`.
pub fn new(initial: T) -> MutHeap<T> { pub fn new(initial: T) -> MutHeap<T> {
MutHeap { MutHeap {
val: Cell::new(initial), val: UnsafeCell::new(initial),
} }
} }
/// Set this `MutHeap` to the given value. /// Set this `MutHeap` to the given value.
pub fn set(&self, val: T) { pub fn set(&self, val: T) {
self.val.set(val) unsafe { *self.val.get() = val; }
} }
/// Set the value in this `MutHeap`. /// Set the value in this `MutHeap`.
pub fn get(&self) -> T { pub fn get(&self) -> T {
self.val.get() unsafe { ptr::read(self.val.get()) }
}
}
impl<T: HeapGCValue> HeapSizeOf for MutHeap<T> {
fn heap_size_of_children(&self) -> usize {
// See comment on HeapSizeOf for JS<T>.
0
} }
} }
/// A mutable holder for GC-managed values such as `JSval` and `JS<T>`, with /// A mutable holder for GC-managed values such as `JSval` and `JS<T>`, with
/// nullability represented by an enclosing Option wrapper. Must be used in /// nullability represented by an enclosing Option wrapper. Roughly equivalent
/// place of traditional internal mutability to ensure that the proper GC /// to a DOMRefCell<Option<JS<T>>>, but smaller; the cost is that values which
/// barriers are enforced. /// are read must be immediately rooted.
#[must_root] #[must_root]
#[derive(JSTraceable, HeapSizeOf)] #[derive(JSTraceable)]
pub struct MutNullableHeap<T: HeapGCValue + Copy> { pub struct MutNullableHeap<T: HeapGCValue> {
ptr: Cell<Option<T>> ptr: UnsafeCell<Option<T>>
} }
impl<T: HeapGCValue + Copy> MutNullableHeap<T> { impl<T: HeapGCValue> MutNullableHeap<T> {
/// Create a new `MutNullableHeap`. /// Create a new `MutNullableHeap`.
pub fn new(initial: Option<T>) -> MutNullableHeap<T> { pub fn new(initial: Option<T>) -> MutNullableHeap<T> {
MutNullableHeap { MutNullableHeap {
ptr: Cell::new(initial) ptr: UnsafeCell::new(initial)
} }
} }
/// Set this `MutNullableHeap` to the given value. /// Set this `MutNullableHeap` to the given value.
pub fn set(&self, val: Option<T>) { pub fn set(&self, val: Option<T>) {
self.ptr.set(val); unsafe { *self.ptr.get() = val; }
} }
/// Retrieve a copy of the current optional inner value. /// Retrieve a copy of the current optional inner value.
pub fn get(&self) -> Option<T> { pub fn get(&self) -> Option<T> {
self.ptr.get() unsafe { ptr::read(self.ptr.get()) }
} }
} }
@ -280,7 +285,7 @@ impl<T: Reflectable> MutNullableHeap<JS<T>> {
/// Retrieve a copy of the inner optional `JS<T>` as `LayoutJS<T>`. /// Retrieve a copy of the inner optional `JS<T>` as `LayoutJS<T>`.
/// For use by layout, which can't use safe types like Temporary. /// For use by layout, which can't use safe types like Temporary.
pub unsafe fn get_inner_as_layout(&self) -> Option<LayoutJS<T>> { pub unsafe fn get_inner_as_layout(&self) -> Option<LayoutJS<T>> {
self.ptr.get().map(|js| js.to_layout()) ptr::read(self.ptr.get()).map(|js| js.to_layout())
} }
/// Get a rooted value out of this object /// Get a rooted value out of this object
@ -290,15 +295,22 @@ impl<T: Reflectable> MutNullableHeap<JS<T>> {
} }
} }
impl<T: HeapGCValue + Copy> Default for MutNullableHeap<T> { impl<T: HeapGCValue> Default for MutNullableHeap<T> {
#[allow(unrooted_must_root)] #[allow(unrooted_must_root)]
fn default() -> MutNullableHeap<T> { fn default() -> MutNullableHeap<T> {
MutNullableHeap { MutNullableHeap {
ptr: Cell::new(None) ptr: UnsafeCell::new(None)
} }
} }
} }
impl<T: HeapGCValue> HeapSizeOf for MutNullableHeap<T> {
fn heap_size_of_children(&self) -> usize {
// See comment on HeapSizeOf for JS<T>.
0
}
}
impl<T: Reflectable> LayoutJS<T> { impl<T: Reflectable> LayoutJS<T> {
/// Returns an unsafe pointer to the interior of this JS object. This is /// Returns an unsafe pointer to the interior of this JS object. This is
/// the only method that be safely accessed from layout. (The fact that /// the only method that be safely accessed from layout. (The fact that

View file

@ -54,7 +54,7 @@ impl BrowsingContext {
} }
pub fn frame_element(&self) -> Option<Root<Element>> { pub fn frame_element(&self) -> Option<Root<Element>> {
self.frame_element.map(Root::from_rooted) self.frame_element.as_ref().map(JS::root)
} }
pub fn window_proxy(&self) -> *mut JSObject { pub fn window_proxy(&self) -> *mut JSObject {

View file

@ -763,7 +763,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
serialize(rgba, &mut result).unwrap(); serialize(rgba, &mut result).unwrap();
StringOrCanvasGradientOrCanvasPattern::eString(result) StringOrCanvasGradientOrCanvasPattern::eString(result)
}, },
CanvasFillOrStrokeStyle::Gradient(gradient) => { CanvasFillOrStrokeStyle::Gradient(ref gradient) => {
StringOrCanvasGradientOrCanvasPattern::eCanvasGradient(gradient.root()) StringOrCanvasGradientOrCanvasPattern::eCanvasGradient(gradient.root())
}, },
} }
@ -803,7 +803,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
serialize(rgba, &mut result).unwrap(); serialize(rgba, &mut result).unwrap();
StringOrCanvasGradientOrCanvasPattern::eString(result) StringOrCanvasGradientOrCanvasPattern::eString(result)
}, },
CanvasFillOrStrokeStyle::Gradient(gradient) => { CanvasFillOrStrokeStyle::Gradient(ref gradient) => {
StringOrCanvasGradientOrCanvasPattern::eCanvasGradient(gradient.root()) StringOrCanvasGradientOrCanvasPattern::eCanvasGradient(gradient.root())
}, },
} }

View file

@ -32,7 +32,7 @@ const DEFAULT_WIDTH: u32 = 300;
const DEFAULT_HEIGHT: u32 = 150; const DEFAULT_HEIGHT: u32 = 150;
#[must_root] #[must_root]
#[derive(JSTraceable, Clone, Copy, HeapSizeOf)] #[derive(JSTraceable, Clone, HeapSizeOf)]
pub enum CanvasContext { pub enum CanvasContext {
Context2d(JS<CanvasRenderingContext2D>), Context2d(JS<CanvasRenderingContext2D>),
WebGL(JS<WebGLRenderingContext>), WebGL(JS<WebGLRenderingContext>),

View file

@ -758,7 +758,7 @@ impl Activatable for HTMLInputElement {
InputType::InputRadio => { InputType::InputRadio => {
// We want to restore state only if the element had been changed in the first place // We want to restore state only if the element had been changed in the first place
if cache.was_mutable { if cache.was_mutable {
let old_checked: Option<Root<HTMLInputElement>> = cache.checked_radio.map(|t| t.root()); let old_checked = cache.checked_radio.as_ref().map(|t| t.root());
let name = self.get_radio_group_name(); let name = self.get_radio_group_name();
match old_checked { match old_checked {
Some(ref o) => { Some(ref o) => {

View file

@ -64,7 +64,7 @@ impl NodeListMethods for NodeList {
fn Item(&self, index: u32) -> Option<Root<Node>> { fn Item(&self, index: u32) -> Option<Root<Node>> {
match self.list_type { match self.list_type {
NodeListType::Simple(ref elems) => { NodeListType::Simple(ref elems) => {
elems.get(index as usize).map(|node| Root::from_rooted(*node)) elems.get(index as usize).map(|node| node.root())
}, },
NodeListType::Children(ref list) => list.item(index), NodeListType::Children(ref list) => list.item(index),
} }

View file

@ -12,7 +12,7 @@ use dom::bindings::codegen::InheritTypes::{EventCast, EventTargetCast, NodeCast}
use dom::bindings::codegen::UnionTypes::ImageDataOrHTMLImageElementOrHTMLCanvasElementOrHTMLVideoElement; use dom::bindings::codegen::UnionTypes::ImageDataOrHTMLImageElementOrHTMLCanvasElementOrHTMLVideoElement;
use dom::bindings::conversions::ToJSValConvertible; use dom::bindings::conversions::ToJSValConvertible;
use dom::bindings::global::{GlobalField, GlobalRef}; use dom::bindings::global::{GlobalField, GlobalRef};
use dom::bindings::js::{JS, LayoutJS, Root}; use dom::bindings::js::{JS, LayoutJS, MutNullableHeap, Root};
use dom::bindings::utils::{Reflector, reflect_dom_object}; use dom::bindings::utils::{Reflector, reflect_dom_object};
use dom::event::{EventBubbles, EventCancelable}; use dom::event::{EventBubbles, EventCancelable};
use dom::htmlcanvaselement::HTMLCanvasElement; use dom::htmlcanvaselement::HTMLCanvasElement;
@ -78,8 +78,8 @@ pub struct WebGLRenderingContext {
canvas: JS<HTMLCanvasElement>, canvas: JS<HTMLCanvasElement>,
last_error: Cell<Option<WebGLError>>, last_error: Cell<Option<WebGLError>>,
texture_unpacking_settings: Cell<TextureUnpacking>, texture_unpacking_settings: Cell<TextureUnpacking>,
bound_texture_2d: Cell<Option<JS<WebGLTexture>>>, bound_texture_2d: MutNullableHeap<JS<WebGLTexture>>,
bound_texture_cube_map: Cell<Option<JS<WebGLTexture>>>, bound_texture_cube_map: MutNullableHeap<JS<WebGLTexture>>,
} }
impl WebGLRenderingContext { impl WebGLRenderingContext {
@ -104,8 +104,8 @@ impl WebGLRenderingContext {
canvas: JS::from_ref(canvas), canvas: JS::from_ref(canvas),
last_error: Cell::new(None), last_error: Cell::new(None),
texture_unpacking_settings: Cell::new(CONVERT_COLORSPACE), texture_unpacking_settings: Cell::new(CONVERT_COLORSPACE),
bound_texture_2d: Cell::new(None), bound_texture_2d: MutNullableHeap::new(None),
bound_texture_cube_map: Cell::new(None), bound_texture_cube_map: MutNullableHeap::new(None),
} }
}) })
} }