Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Heap instead of UnsafeCell in DOM reflectors #15118

Merged
merged 1 commit into from Jan 25, 2017
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -7,9 +7,8 @@
use dom::bindings::conversions::DerivedFrom;
use dom::bindings::js::Root;
use dom::globalscope::GlobalScope;
use js::jsapi::{HandleObject, JSContext, JSObject};
use std::cell::UnsafeCell;
use std::ptr;
use js::jsapi::{HandleObject, JSContext, JSObject, Heap};
use std::default::Default;

/// Create the reflector for a new DOM object and yield ownership to the
/// reflector.
@@ -34,44 +33,41 @@ pub fn reflect_dom_object<T, U>(
// If you're renaming or moving this field, update the path in plugins::reflector as well
pub struct Reflector {
#[ignore_heap_size_of = "defined and measured in rust-mozjs"]
object: UnsafeCell<*mut JSObject>,
object: Heap<*mut JSObject>,
}

#[allow(unrooted_must_root)]
impl PartialEq for Reflector {
fn eq(&self, other: &Reflector) -> bool {
unsafe { *self.object.get() == *other.object.get() }
self.object.get() == other.object.get()
}
}

impl Reflector {
/// Get the reflector.
#[inline]
pub fn get_jsobject(&self) -> HandleObject {
unsafe { HandleObject::from_marked_location(self.object.get()) }
self.object.handle()
}

/// Initialize the reflector. (May be called only once.)
pub fn set_jsobject(&mut self, object: *mut JSObject) {
unsafe {
let obj = self.object.get();
assert!((*obj).is_null());
assert!(!object.is_null());
*obj = object;
}
assert!(self.object.get().is_null());
assert!(!object.is_null());
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 fn rootable(&self) -> *mut *mut JSObject {
self.object.get()
pub fn rootable(&self) -> &Heap<*mut JSObject> {

This comment has been minimized.

Copy link
@KiChjang

KiChjang Jan 23, 2017

Member

Should this be &mut instead?

This comment has been minimized.

Copy link
@jdm

jdm Jan 23, 2017

Author Member

I argue that we can discuss that separately, since the changes here achieve the same effect as previously existed.

&self.object
}

/// Create an uninitialized `Reflector`.
pub fn new() -> Reflector {
Reflector {
object: UnsafeCell::new(ptr::null_mut()),
object: Heap::default(),
}
}
}
@@ -20,7 +20,7 @@
//! calls `trace()` on the field.
//! For example, for fields of type `JS<T>`, `JS<T>::trace()` calls
//! `trace_reflector()`.
//! 4. `trace_reflector()` calls `JS_CallUnbarrieredObjectTracer()` with a
//! 4. `trace_reflector()` calls `JS::TraceEdge()` with a
//! pointer to the `JSObject` for the reflector. This notifies the GC, which
//! will add the object to the graph, and will trace that object as well.
//! 5. When the GC finishes tracing, it [`finalizes`](../index.html#destruction)
@@ -54,7 +54,7 @@ use hyper::method::Method;
use hyper::mime::Mime;
use hyper::status::StatusCode;
use ipc_channel::ipc::{IpcReceiver, IpcSender};
use js::glue::{CallObjectTracer, CallUnbarrieredObjectTracer, CallValueTracer};
use js::glue::{CallObjectTracer, CallValueTracer};
use js::jsapi::{GCTraceKindToAscii, Heap, JSObject, JSTracer, TraceKind};
use js::jsval::JSVal;
use js::rust::Runtime;
@@ -139,12 +139,8 @@ pub fn trace_jsval(tracer: *mut JSTracer, description: &str, val: &Heap<JSVal>)
/// Trace the `JSObject` held by `reflector`.
#[allow(unrooted_must_root)]
pub fn trace_reflector(tracer: *mut JSTracer, description: &str, reflector: &Reflector) {
unsafe {
trace!("tracing reflector {}", description);
CallUnbarrieredObjectTracer(tracer,
reflector.rootable(),
GCTraceKindToAscii(TraceKind::Object));
}
trace!("tracing reflector {}", description);
trace_object(tracer, description, reflector.rootable())
}

/// Trace a `JSObject`.
@@ -90,12 +90,12 @@ impl Promise {
#[allow(unsafe_code, unrooted_must_root)]
unsafe fn new_with_js_promise(obj: HandleObject, cx: *mut JSContext) -> Rc<Promise> {
assert!(IsPromiseObject(obj));
let mut promise = Promise {
let promise = Promise {
reflector: Reflector::new(),
permanent_js_root: MutHeapJSVal::new(),
};
promise.init_reflector(obj.get());
let promise = Rc::new(promise);
let mut promise = Rc::new(promise);
Rc::get_mut(&mut promise).unwrap().init_reflector(obj.get());
promise.initialize(cx);
promise
}
@@ -58,6 +58,7 @@
var p = new Promise(function() {});
var start = Date.now();
t.resolvePromiseDelayed(p, 'success', 100);
test.step_timeout(function() { window.gc() }, 0);
return p.then(function(v) {
var end = Date.now();
assert_greater_than_equal(end - start, 100);
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.