auto merge of #2198 : Ms2ger/servo/js_compartment, r=jdm

A js::rust::Compartment is little more than a glorified pointer to the
reflector of a window, so there's no good reason to use it. Instead, this
commit passes a JS<Window> directly when it's necessary.

This also means that we now have to use JS_DefineFunctions rather than
Compartment::define_functions; I believe the former is clearer to the reader
than the extra indirection involved in the latter calling through three
reopsitories.

This commit also simplifies ScriptTask::load to reuse the 'cx' local that is
in scope already, rather than refetching it through js_info.

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

@@ -2118,7 +2118,10 @@ class CGDefineDOMInterfaceMethod(CGAbstractMethod):
     a given interface.
     """
     def __init__(self, descriptor):
-        args = [Argument('&mut JSPageInfo', 'js_info')]
+        args = [
+            Argument('&JS<Window>', 'window'),
+            Argument('&mut JSPageInfo', 'js_info'),
+        ]
         CGAbstractMethod.__init__(self, descriptor, 'DefineDOMInterface', 'void', args, pub=True)
 
     def define(self):
@@ -2173,10 +2176,10 @@ class CGDefineDOMInterfaceMethod(CGAbstractMethod):
        ('Some(%s)' % TRACE_HOOK_NAME),
        self.descriptor.name)
 
-        return (body + """  let cx = (*js_info.js_context).deref().ptr;
+        return (body + """  let cx = (**js_info.js_context).ptr;
-  let receiver = js_info.js_compartment.global_obj;
+  let global = window.reflector().get_jsobject();
-  let global: *JSObject = JS_GetGlobalForObject(cx, receiver);
+  assert!(global.is_not_null());
-  assert!(%s(cx, global, receiver).is_not_null());""" % (getter))
+  assert!(%s(cx, global, global).is_not_null());""" % (getter))
 
 def needCx(returnType, arguments, extendedAttributes, considerTypes):
     return (considerTypes and
@@ -4305,13 +4308,16 @@ class CGDictionary(CGThing):
 
 class CGRegisterProtos(CGAbstractMethod):
     def __init__(self, config):
-        CGAbstractMethod.__init__(self, None, 'Register', 'void',
+        arguments = [
-                                  [Argument('&mut JSPageInfo', 'js_info')],
+            Argument('&JS<Window>', 'window'),
+            Argument('&mut JSPageInfo', 'js_info'),
+        ]
+        CGAbstractMethod.__init__(self, None, 'Register', 'void', arguments,
                                   unsafe=False, pub=True)
         self.config = config
 
     def _registerProtos(self):
-        lines = ["  codegen::%sBinding::DefineDOMInterface(js_info);" % desc.name
+        lines = ["  codegen::%sBinding::DefineDOMInterface(window, js_info);" % desc.name
                  for desc in self.config.getDescriptors(hasInterfaceObject=True,
                                                         register=True)]
         return '\n'.join(lines) + '\n'
@@ -5349,6 +5355,8 @@ class GlobalGenRoots():
         # TODO - Generate the methods we want
         return CGImports(CGRegisterProtos(config), [
             'dom::bindings::codegen',
+            'dom::bindings::js::JS',
+            'dom::window::Window',
             'script_task::JSPageInfo',
         ])
 

src/components/script/dom/window.rs

@@ -20,7 +20,7 @@ use servo_net::image_cache_task::ImageCacheTask;
 use servo_util::str::DOMString;
 use servo_util::task::{spawn_named};
 
-use js::jsapi::{JSObject, JSContext, JS_DefineProperty, JS_PropertyStub, JS_StrictPropertyStub};
+use js::jsapi::{JSContext, JS_DefineProperty, JS_PropertyStub, JS_StrictPropertyStub};
 use js::jsval::{NullValue, ObjectValue, JSVal};
 use js::JSPROP_ENUMERATE;
 
@@ -91,9 +91,9 @@ pub struct Window {
 }
 
 impl Window {
-    pub fn get_cx(&self) -> *JSObject {
+    pub fn get_cx(&self) -> *JSContext {
         let js_info = self.page().js_info();
-        js_info.get_ref().js_compartment.deref().cx.deref().ptr
+        (**js_info.get_ref().js_context).ptr
     }
 
     pub fn page<'a>(&'a self) -> &'a Page {

src/components/script/script_task.rs

@@ -33,9 +33,9 @@ use layout_interface;
 use geom::point::Point2D;
 use geom::size::Size2D;
 use js::global::DEBUG_FNS;
-use js::jsapi::{JSObject, JS_InhibitGC, JS_AllowGC, JS_CallFunctionValue};
+use js::jsapi::{JSObject, JS_InhibitGC, JS_AllowGC, JS_CallFunctionValue, JS_DefineFunctions};
 use js::jsval::NullValue;
-use js::rust::{Compartment, Cx, CxUtils, RtUtils};
+use js::rust::{Cx, RtUtils};
 use js;
 use servo_msg::compositor_msg::{FinishedLoading, LayerId, Loading, PerformingLayout};
 use servo_msg::compositor_msg::{ScriptListener};
@@ -426,11 +426,6 @@ impl Page {
         js_context.set_default_options_and_version();
         js_context.set_logging_error_reporter();
 
-        let compartment = match js_context.new_compartment_with_global(global) {
-              Ok(c) => c,
-              Err(()) => fail!("Failed to create a compartment"),
-        };
-
         unsafe {
             JS_InhibitGC(js_context.deref().ptr);
         }
@@ -438,7 +433,6 @@ impl Page {
         let mut js_info = self.mut_js_info();
         *js_info = Some(JSPageInfo {
             dom_static: GlobalStaticData(),
-            js_compartment: Untraceable::new(compartment),
             js_context: Untraceable::new(js_context),
         });
     }
@@ -458,8 +452,6 @@ pub struct Frame {
 pub struct JSPageInfo {
     /// Global static data related to the DOM.
     dom_static: GlobalStaticData,
-    /// The JavaScript compartment for the origin associated with the script task.
-    js_compartment: Untraceable<Rc<Compartment>>,
     /// The JavaScript context.
     js_context: Untraceable<Rc<Cx>>,
 }
@@ -667,15 +659,15 @@ impl ScriptTask {
             window.get_mut().active_timers.remove(&timer_data.handle);
         }
 
-        let js_info = page.js_info();
         let this_value = if timer_data.args.len() > 0 {
             fail!("NYI")
         } else {
-            js_info.get_ref().js_compartment.global_obj
+            window.reflector().get_jsobject()
         };
 
         // TODO: Support extra arguments. This requires passing a `*JSVal` array as `argv`.
         let rval = NullValue();
+        let js_info = page.js_info();
         let cx = js_info.get_ref().js_context.deref().deref().ptr;
         with_gc_enabled(cx, || {
             unsafe {
@@ -802,7 +794,7 @@ impl ScriptTask {
 
         {
             let mut js_info = page.mut_js_info();
-            RegisterBindings::Register(js_info.get_mut_ref());
+            RegisterBindings::Register(&window, js_info.get_mut_ref());
         }
 
         self.compositor.set_ready_state(Loading);
@@ -879,22 +871,16 @@ impl ScriptTask {
         debug!("js_scripts: {:?}", js_scripts);
 
         // Define debug functions.
-        let cx = {
+        unsafe {
-            let js_info = page.js_info();
+            assert!(JS_DefineFunctions((*cx).ptr,
-            let js_info = js_info.get_ref();
+                                       window.reflector().get_jsobject(),
-            assert!(js_info.js_compartment.deref().define_functions(DEBUG_FNS).is_ok());
+                                       DEBUG_FNS.as_ptr()) != 0);
-
+        }
-            js_info.js_context.deref().deref().ptr
-        };
 
         // Evaluate every script in the document.
         for file in js_scripts.iter() {
-            with_gc_enabled(cx, || {
+            with_gc_enabled((*cx).ptr, || {
-                let (cx, global_obj) = {
+                let global_obj = window.reflector().get_jsobject();
-                    let js_info = page.js_info();
-                    (js_info.get_ref().js_context.deref().clone(),
-                     js_info.get_ref().js_compartment.global_obj)
-                };
                 //FIXME: this should have some kind of error handling, or explicitly
                 //       drop an exception on the floor.
                 match cx.evaluate_script(global_obj, file.data.clone(), file.url.to_str(), 1) {