servo: Merge #8056 - Fix the implementation of JSTraceable for RefCell (from eefriedman:trace-refcell); r=jdm

The existing implementation could panic; make sure that doesn't
happen by requiring that the contents of a RefCell are trivially
traceable (i.e. the value don't contain any traceable objects).

I'm not sure whether the TriviallyJSTraceable trait is actually
worthwhile; maybe we should just never use RefCell in the DOM.

Source-Repo: https://github.com/servo/servo
Source-Revision: 4f51710ed387baa1ad0a6e4cdb0fc5eee44093d5

servo/components/script/dom/bindings/trace.rs

@@ -66,7 +66,7 @@ use selectors::states::*;
 use serde::{Deserialize, Serialize};
 use smallvec::SmallVec;
 use std::boxed::FnBox;
-use std::cell::{Cell, RefCell, UnsafeCell};
+use std::cell::{Cell, UnsafeCell};
 use std::collections::hash_state::HashState;
 use std::collections::{HashMap, HashSet};
 use std::ffi::CString;
@@ -139,12 +139,6 @@ pub fn trace_object(tracer: *mut JSTracer, description: &str, obj: &Heap<*mut JS
     }
 }
 
-impl<T: JSTraceable> JSTraceable for RefCell<T> {
-    fn trace(&self, trc: *mut JSTracer) {
-        self.borrow().trace(trc)
-    }
-}
-
 impl<T: JSTraceable> JSTraceable for Rc<T> {
     fn trace(&self, trc: *mut JSTracer) {
         (**self).trace(trc)
@@ -157,26 +151,6 @@ impl<T: JSTraceable> JSTraceable for Box<T> {
     }
 }
 
-impl<T: JSTraceable> JSTraceable for *const T {
-    fn trace(&self, trc: *mut JSTracer) {
-        if !self.is_null() {
-            unsafe {
-                (**self).trace(trc)
-            }
-        }
-    }
-}
-
-impl<T: JSTraceable> JSTraceable for *mut T {
-    fn trace(&self, trc: *mut JSTracer) {
-        if !self.is_null() {
-            unsafe {
-                (**self).trace(trc)
-            }
-        }
-    }
-}
-
 impl<T: JSTraceable + Copy> JSTraceable for Cell<T> {
     fn trace(&self, trc: *mut JSTracer) {
         self.get().trace(trc)

servo/components/script/dom/canvasrenderingcontext2d.rs

@@ -8,6 +8,7 @@ use canvas_traits::{CompositionOrBlending, LineCapStyle, LineJoinStyle};
 use canvas_traits::{FillOrStrokeStyle, LinearGradientStyle, RadialGradientStyle, RepetitionStyle};
 use cssparser::Color as CSSColor;
 use cssparser::{Parser, RGBA};
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::CanvasRenderingContext2DBinding;
 use dom::bindings::codegen::Bindings::CanvasRenderingContext2DBinding::CanvasRenderingContext2DMethods;
 use dom::bindings::codegen::Bindings::CanvasRenderingContext2DBinding::CanvasWindingRule;
@@ -37,7 +38,6 @@ use net_traits::image::base::PixelFormat;
 use net_traits::image_cache_task::ImageResponse;
 use num::{Float, ToPrimitive};
 use std::borrow::ToOwned;
-use std::cell::RefCell;
 use std::str::FromStr;
 use std::sync::mpsc::channel;
 use std::{cmp, fmt};
@@ -63,8 +63,8 @@ pub struct CanvasRenderingContext2D {
     #[ignore_heap_size_of = "Defined in ipc-channel"]
     ipc_renderer: IpcSender<CanvasMsg>,
     canvas: JS<HTMLCanvasElement>,
-    state: RefCell<CanvasContextState>,
-    saved_states: RefCell<Vec<CanvasContextState>>,
+    state: DOMRefCell<CanvasContextState>,
+    saved_states: DOMRefCell<Vec<CanvasContextState>>,
 }
 
 #[must_root]
@@ -126,8 +126,8 @@ impl CanvasRenderingContext2D {
             renderer_id: renderer_id,
             ipc_renderer: ipc_renderer,
             canvas: JS::from_ref(canvas),
-            state: RefCell::new(CanvasContextState::new()),
-            saved_states: RefCell::new(Vec::new()),
+            state: DOMRefCell::new(CanvasContextState::new()),
+            saved_states: DOMRefCell::new(Vec::new()),
         }
     }
 

servo/components/script/dom/document.rs

@@ -89,7 +89,7 @@ use selectors::states::*;
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
 use std::boxed::FnBox;
-use std::cell::{Cell, Ref, RefCell, RefMut};
+use std::cell::{Cell, Ref, RefMut};
 use std::collections::HashMap;
 use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::default::Default;
@@ -163,7 +163,7 @@ pub struct Document {
     /// https://html.spec.whatwg.org/multipage/#list-of-animation-frame-callbacks
     /// List of animation frame callbacks
     #[ignore_heap_size_of = "closures are hard"]
-    animation_frame_list: RefCell<HashMap<u32, Box<FnBox(f64)>>>,
+    animation_frame_list: DOMRefCell<HashMap<u32, Box<FnBox(f64)>>>,
     /// Tracks all outstanding loads related to this document.
     loader: DOMRefCell<DocumentLoader>,
     /// The current active HTML parser, to allow resuming after interruptions.
@@ -1262,7 +1262,7 @@ impl Document {
             asap_scripts_set: DOMRefCell::new(vec!()),
             scripting_enabled: Cell::new(true),
             animation_frame_ident: Cell::new(0),
-            animation_frame_list: RefCell::new(HashMap::new()),
+            animation_frame_list: DOMRefCell::new(HashMap::new()),
             loader: DOMRefCell::new(doc_loader),
             current_parser: Default::default(),
             reflow_timeout: Cell::new(None),

servo/components/script/dom/filereader.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::BlobBinding::BlobMethods;
 use dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull;
 use dom::bindings::codegen::Bindings::FileReaderBinding::{self, FileReaderConstants, FileReaderMethods};
@@ -23,7 +24,7 @@ use hyper::mime::{Attr, Mime};
 use rustc_serialize::base64::{CharacterSet, Config, Newline, ToBase64};
 use script_task::ScriptTaskEventCategory::FileRead;
 use script_task::{CommonScriptMsg, Runnable, ScriptChan, ScriptPort};
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::sync::mpsc;
 use std::sync::mpsc::Receiver;
 use util::str::DOMString;
@@ -72,7 +73,7 @@ pub struct FileReader {
     global: GlobalField,
     ready_state: Cell<FileReaderReadyState>,
     error: MutNullableHeap<JS<DOMException>>,
-    result: RefCell<Option<DOMString>>,
+    result: DOMRefCell<Option<DOMString>>,
     generation_id: Cell<GenerationId>,
 }
 
@@ -83,7 +84,7 @@ impl FileReader {
             global: GlobalField::from_rooted(&global),
             ready_state: Cell::new(FileReaderReadyState::Empty),
             error: MutNullableHeap::new(None),
-            result: RefCell::new(None),
+            result: DOMRefCell::new(None),
             generation_id: Cell::new(GenerationId(0)),
         }
     }

servo/components/script/dom/htmlscriptelement.rs

@@ -37,7 +37,7 @@ use network_listener::{NetworkListener, PreInvoke};
 use script_task::ScriptTaskEventCategory::ScriptEvent;
 use script_task::{CommonScriptMsg, Runnable, ScriptChan};
 use std::ascii::AsciiExt;
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::mem;
 use std::sync::{Arc, Mutex};
 use url::{Url, UrlParser};
@@ -65,7 +65,7 @@ pub struct HTMLScriptElement {
     parser_document: JS<Document>,
 
     /// The source this script was loaded from
-    load: RefCell<Option<ScriptOrigin>>,
+    load: DOMRefCell<Option<ScriptOrigin>>,
 
     #[ignore_heap_size_of = "Defined in rust-encoding"]
     /// https://html.spec.whatwg.org/multipage/#concept-script-encoding
@@ -83,7 +83,7 @@ impl HTMLScriptElement {
             non_blocking: Cell::new(creator != ElementCreator::ParserCreated),
             ready_to_be_parser_executed: Cell::new(false),
             parser_document: JS::from_ref(document),
-            load: RefCell::new(None),
+            load: DOMRefCell::new(None),
             block_character_encoding: DOMRefCell::new(UTF_8 as EncodingRef),
         }
     }

servo/components/script/dom/keyboardevent.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::KeyboardEventBinding;
 use dom::bindings::codegen::Bindings::KeyboardEventBinding::{KeyboardEventConstants, KeyboardEventMethods};
 use dom::bindings::codegen::Bindings::UIEventBinding::UIEventMethods;
@@ -17,7 +18,7 @@ use msg::constellation_msg;
 use msg::constellation_msg::{ALT, CONTROL, SHIFT, SUPER};
 use msg::constellation_msg::{Key, KeyModifiers};
 use std::borrow::ToOwned;
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use util::str::DOMString;
 
 no_jsmanaged_fields!(Key);
@@ -26,8 +27,8 @@ no_jsmanaged_fields!(Key);
 pub struct KeyboardEvent {
     uievent: UIEvent,
     key: Cell<Option<Key>>,
-    key_string: RefCell<DOMString>,
-    code: RefCell<DOMString>,
+    key_string: DOMRefCell<DOMString>,
+    code: DOMRefCell<DOMString>,
     location: Cell<u32>,
     ctrl: Cell<bool>,
     alt: Cell<bool>,
@@ -44,8 +45,8 @@ impl KeyboardEvent {
         KeyboardEvent {
             uievent: UIEvent::new_inherited(),
             key: Cell::new(None),
-            key_string: RefCell::new("".to_owned()),
-            code: RefCell::new("".to_owned()),
+            key_string: DOMRefCell::new("".to_owned()),
+            code: DOMRefCell::new("".to_owned()),
             location: Cell::new(0),
             ctrl: Cell::new(false),
             alt: Cell::new(false),

servo/components/script/dom/url.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::URLBinding::{self, URLMethods};
 use dom::bindings::error::{Error, ErrorResult, Fallible};
 use dom::bindings::global::GlobalRef;
@@ -10,7 +11,6 @@ use dom::bindings::str::USVString;
 use dom::bindings::utils::{Reflector, reflect_dom_object};
 use dom::urlhelper::UrlHelper;
 use std::borrow::ToOwned;
-use std::cell::RefCell;
 use url::{Host, ParseResult, Url, UrlParser};
 use util::str::DOMString;
 
@@ -20,7 +20,7 @@ pub struct URL {
     reflector_: Reflector,
 
     // https://url.spec.whatwg.org/#concept-urlutils-url
-    url: RefCell<Url>,
+    url: DOMRefCell<Url>,
 
     // https://url.spec.whatwg.org/#concept-urlutils-get-the-base
     base: Option<Url>,
@@ -30,7 +30,7 @@ impl URL {
     fn new_inherited(url: Url, base: Option<Url>) -> URL {
         URL {
             reflector_: Reflector::new(),
-            url: RefCell::new(url),
+            url: DOMRefCell::new(url),
             base: base,
         }
     }

servo/components/script/dom/webglshader.rs

@@ -5,6 +5,7 @@
 // https://www.khronos.org/registry/webgl/specs/latest/1.0/webgl.idl
 use angle::hl::{BuiltInResources, Output, ShaderValidator};
 use canvas_traits::{CanvasMsg, CanvasWebGLMsg, WebGLError, WebGLResult, WebGLShaderParameter};
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants;
 use dom::bindings::codegen::Bindings::WebGLShaderBinding;
 use dom::bindings::global::GlobalRef;
@@ -12,7 +13,7 @@ use dom::bindings::js::Root;
 use dom::bindings::utils::reflect_dom_object;
 use dom::webglobject::WebGLObject;
 use ipc_channel::ipc::{self, IpcSender};
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::sync::{ONCE_INIT, Once};
 
 #[derive(Clone, Copy, PartialEq, Debug, JSTraceable, HeapSizeOf)]
@@ -27,8 +28,8 @@ pub struct WebGLShader {
     webgl_object: WebGLObject,
     id: u32,
     gl_type: u32,
-    source: RefCell<Option<String>>,
-    info_log: RefCell<Option<String>>,
+    source: DOMRefCell<Option<String>>,
+    info_log: DOMRefCell<Option<String>>,
     is_deleted: Cell<bool>,
     compilation_status: Cell<ShaderCompilationStatus>,
     #[ignore_heap_size_of = "Defined in ipc-channel"]
@@ -50,8 +51,8 @@ impl WebGLShader {
             webgl_object: WebGLObject::new_inherited(),
             id: id,
             gl_type: shader_type,
-            source: RefCell::new(None),
-            info_log: RefCell::new(None),
+            source: DOMRefCell::new(None),
+            info_log: DOMRefCell::new(None),
             is_deleted: Cell::new(false),
             compilation_status: Cell::new(ShaderCompilationStatus::NotCompiled),
             renderer: renderer,

servo/components/script/dom/websocket.rs

@@ -28,7 +28,7 @@ use net_traits::hosts::replace_hosts;
 use script_task::ScriptTaskEventCategory::WebSocketEvent;
 use script_task::{CommonScriptMsg, Runnable};
 use std::borrow::ToOwned;
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::sync::{Arc, Mutex};
 use std::{ptr, slice};
 use util::str::DOMString;
@@ -133,7 +133,7 @@ pub struct WebSocket {
     buffered_amount: Cell<u32>,
     clearing_buffer: Cell<bool>, //Flag to tell if there is a running task to clear buffered_amount
     #[ignore_heap_size_of = "Defined in std"]
-    sender: RefCell<Option<Arc<Mutex<Sender<WebSocketStream>>>>>,
+    sender: DOMRefCell<Option<Arc<Mutex<Sender<WebSocketStream>>>>>,
     failed: Cell<bool>, //Flag to tell if websocket was closed due to failure
     full: Cell<bool>, //Flag to tell if websocket queue is full
     clean_close: Cell<bool>, //Flag to tell if the websocket closed cleanly (not due to full or fail)
@@ -175,7 +175,7 @@ impl WebSocket {
             buffered_amount: Cell::new(0),
             clearing_buffer: Cell::new(false),
             failed: Cell::new(false),
-            sender: RefCell::new(None),
+            sender: DOMRefCell::new(None),
             full: Cell::new(false),
             clean_close: Cell::new(true),
             code: Cell::new(0),

servo/components/script/dom/window.rs

@@ -58,7 +58,7 @@ use script_traits::{ConstellationControlMsg, TimerEventChan, TimerEventId, Timer
 use selectors::parser::PseudoElement;
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
-use std::cell::{Cell, Ref, RefCell};
+use std::cell::{Cell, Ref};
 use std::collections::HashSet;
 use std::default::Default;
 use std::ffi::CString;
@@ -144,9 +144,9 @@ pub struct Window {
     /// For sending timeline markers. Will be ignored if
     /// no devtools server
     #[ignore_heap_size_of = "TODO(#6909) need to measure HashSet"]
-    devtools_markers: RefCell<HashSet<TimelineMarkerType>>,
+    devtools_markers: DOMRefCell<HashSet<TimelineMarkerType>>,
     #[ignore_heap_size_of = "channels are hard"]
-    devtools_marker_sender: RefCell<Option<IpcSender<TimelineMarker>>>,
+    devtools_marker_sender: DOMRefCell<Option<IpcSender<TimelineMarker>>>,
 
     /// A flag to indicate whether the developer tools have requested live updates of
     /// page changes.
@@ -208,7 +208,7 @@ pub struct Window {
 
     /// A channel for communicating results of async scripts back to the webdriver server
     #[ignore_heap_size_of = "channels are hard"]
-    webdriver_script_chan: RefCell<Option<IpcSender<WebDriverJSResult>>>,
+    webdriver_script_chan: DOMRefCell<Option<IpcSender<WebDriverJSResult>>>,
 
     /// The current state of the window object
     current_state: Cell<WindowState>,
@@ -1276,10 +1276,10 @@ impl Window {
             pending_reflow_count: Cell::new(0),
             current_state: Cell::new(WindowState::Alive),
 
-            devtools_marker_sender: RefCell::new(None),
-            devtools_markers: RefCell::new(HashSet::new()),
+            devtools_marker_sender: DOMRefCell::new(None),
+            devtools_markers: DOMRefCell::new(HashSet::new()),
             devtools_wants_updates: Cell::new(false),
-            webdriver_script_chan: RefCell::new(None),
+            webdriver_script_chan: DOMRefCell::new(None),
         };
 
         WindowBinding::Wrap(runtime.cx(), win)

servo/components/script/script_task.rs

@@ -414,7 +414,7 @@ pub struct ScriptTask {
     mouse_over_targets: DOMRefCell<Vec<JS<Element>>>,
 
     /// List of pipelines that have been owned and closed by this script task.
-    closed_pipelines: RefCell<HashSet<PipelineId>>,
+    closed_pipelines: DOMRefCell<HashSet<PipelineId>>,
 
     scheduler_chan: Sender<TimerEventRequest>,
     timer_event_chan: Sender<TimerEvent>,
@@ -643,7 +643,7 @@ impl ScriptTask {
 
             js_runtime: Rc::new(runtime),
             mouse_over_targets: DOMRefCell::new(vec!()),
-            closed_pipelines: RefCell::new(HashSet::new()),
+            closed_pipelines: DOMRefCell::new(HashSet::new()),
 
             scheduler_chan: state.scheduler_chan,
             timer_event_chan: timer_event_chan,