Fix #3936 – {Window,WorkerGlobalScope}.set{Timeout,Interval}(DOMString)

components/script/devtools.rs

@@ -12,7 +12,7 @@ use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods;
 use dom::bindings::codegen::Bindings::DOMRectBinding::{DOMRectMethods};
 use dom::bindings::codegen::Bindings::ElementBinding::{ElementMethods};
 use dom::node::{Node, NodeHelpers};
-use dom::window::{WindowHelpers};
+use dom::window::{ScriptHelpers};
 use dom::element::Element;
 use dom::document::DocumentHelpers;
 use page::Page;
@@ -26,7 +26,7 @@ pub fn handle_evaluate_js(page: &Rc<Page>, pipeline: PipelineId, eval: String, r
     let frame = page.frame();
     let window = frame.as_ref().unwrap().window.root();
     let cx = window.r().get_cx();
-    let rval = window.r().evaluate_js_with_result(eval.as_slice());
+    let rval = window.r().evaluate_js_on_global_with_result(eval.as_slice());
 
     reply.send(if rval.is_undefined() {
         devtools_traits::VoidValue

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

@@ -336,7 +336,7 @@ class CGMethodCall(CGThing):
 
             # Check for vanilla JS objects
             # XXXbz Do we need to worry about security wrappers?
-            pickFirstSignature("%s.isObject() && !IsPlatformObject(cx, &%s.toObject())" %
+            pickFirstSignature("%s.is_object() && !IsPlatformObject(%s.to_object())" %
                                (distinguishingArg, distinguishingArg),
                                lambda s: (s[1][distinguishingIndex].type.isCallback() or
                                           s[1][distinguishingIndex].type.isCallbackInterface() or
@@ -4552,6 +4552,7 @@ class CGBindingRoot(CGThing):
             'dom::bindings::utils::{FindEnumStringIndex, GetArrayIndexFromId}',
             'dom::bindings::utils::{GetPropertyOnPrototype, GetProtoOrIfaceArray}',
             'dom::bindings::utils::HasPropertyOnPrototype',
+            'dom::bindings::utils::IsPlatformObject',
             'dom::bindings::utils::{Reflectable}',
             'dom::bindings::utils::{squirrel_away_unique}',
             'dom::bindings::utils::{ThrowingConstructor}',

components/script/dom/bindings/conversions.rs

@@ -353,7 +353,7 @@ impl ToJSValConvertible for Reflector {
 }
 
 /// Returns whether the given `clasp` is one for a DOM object.
-fn is_dom_class(clasp: *const JSClass) -> bool {
+pub fn is_dom_class(clasp: *const JSClass) -> bool {
     unsafe {
         ((*clasp).flags & js::JSCLASS_IS_DOMJSCLASS) != 0
     }

components/script/dom/bindings/utils.rs

@@ -8,7 +8,7 @@
 
 use dom::bindings::codegen::PrototypeList;
 use dom::bindings::codegen::PrototypeList::MAX_PROTO_CHAIN_LENGTH;
-use dom::bindings::conversions::unwrap_jsmanaged;
+use dom::bindings::conversions::{unwrap_jsmanaged, is_dom_class};
 use dom::bindings::error::throw_type_error;
 use dom::bindings::global::GlobalRef;
 use dom::bindings::js::{Temporary, Root};
@@ -20,7 +20,8 @@ use libc::c_uint;
 use std::cell::Cell;
 use std::mem;
 use std::ptr;
-use js::glue::{RUST_JSID_IS_INT, RUST_JSID_TO_INT};
+use js::glue::UnwrapObject;
+use js::glue::{IsWrapper, RUST_JSID_IS_INT, RUST_JSID_TO_INT};
 use js::jsapi::{JS_AlreadyHasOwnProperty, JS_NewFunction};
 use js::jsapi::{JS_DefineProperties, JS_ForwardGetPropertyTo};
 use js::jsapi::{JS_GetClass, JS_LinkConstructorAndPrototype, JS_GetStringCharsAndLength};
@@ -464,6 +465,28 @@ pub fn FindEnumStringIndex(cx: *mut JSContext,
     }
 }
 
+/// Returns wether `obj` is a platform object
+/// http://heycam.github.io/webidl/#dfn-platform-object
+pub fn IsPlatformObject(obj: *mut JSObject) -> bool {
+    unsafe {
+        // Fast-path the common case
+        let mut clasp = JS_GetClass(obj);
+        if is_dom_class(&*clasp) {
+            return true;
+        }
+        // Now for simplicity check for security wrappers before anything else
+        if IsWrapper(obj) == 1 {
+            let unwrapped_obj = UnwrapObject(obj, /* stopAtOuter = */ 0, ptr::null_mut());
+            if unwrapped_obj.is_null() {
+                return false;
+            }
+            clasp = js::jsapi::JS_GetClass(obj);
+        }
+        // TODO also check if JS_IsArrayBufferObject
+        return is_dom_class(&*clasp);
+    }
+}
+
 /// Get the property with name `property` from `object`.
 /// Returns `Err(())` on JSAPI failure (there is a pending exception), and
 /// `Ok(None)` if there was no property with the given name.

components/script/dom/htmlscriptelement.rs

@@ -23,7 +23,7 @@ use dom::element::ElementTypeId;
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{Node, NodeHelpers, NodeTypeId, window_from_node, CloneChildrenFlag};
 use dom::virtualmethods::VirtualMethods;
-use dom::window::WindowHelpers;
+use dom::window::ScriptHelpers;
 
 use encoding::all::UTF_8;
 use encoding::types::{Encoding, DecoderTrap};
@@ -209,7 +209,7 @@ impl<'a> HTMLScriptElementHelpers for JSRef<'a, HTMLScriptElement> {
             None => (text, base_url)
         };
 
-        window.evaluate_script_with_result(source.as_slice(), url.serialize().as_slice());
+        window.evaluate_script_on_global_with_result(source.as_slice(), url.serialize().as_slice());
 
         let event = Event::new(GlobalRef::Window(window),
                                "load".into_string(),

components/script/dom/webidls/Window.webidl

@@ -65,10 +65,10 @@ Window implements WindowEventHandlers;
 [NoInterfaceObject/*, Exposed=Window,Worker*/]
 interface WindowTimers {
   long setTimeout(Function handler, optional long timeout = 0, any... arguments);
-  //long setTimeout(DOMString handler, optional long timeout = 0, any... arguments);
+  long setTimeout(DOMString handler, optional long timeout = 0, any... arguments);
   void clearTimeout(optional long handle = 0);
   long setInterval(Function handler, optional long timeout = 0, any... arguments);
-  //long setInterval(DOMString handler, optional long timeout = 0, any... arguments);
+  long setInterval(DOMString handler, optional long timeout = 0, any... arguments);
   void clearInterval(optional long handle = 0);
 };
 Window implements WindowTimers;

components/script/dom/window.rs

@@ -8,6 +8,7 @@ use dom::bindings::codegen::Bindings::FunctionBinding::Function;
 use dom::bindings::codegen::Bindings::WindowBinding;
 use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
 use dom::bindings::codegen::InheritTypes::EventTargetCast;
+use dom::bindings::global::global_object_for_js_object;
 use dom::bindings::error::Fallible;
 use dom::bindings::error::Error::InvalidCharacter;
 use dom::bindings::global::GlobalRef;
@@ -27,7 +28,7 @@ use page::Page;
 use script_task::{TimerSource, ScriptChan};
 use script_task::ScriptMsg;
 use script_traits::ScriptControlChan;
-use timers::{IsInterval, TimerId, TimerManager};
+use timers::{IsInterval, TimerId, TimerManager, TimerCallback};
 
 use servo_msg::compositor_msg::ScriptListener;
 use servo_msg::constellation_msg::LoadData;
@@ -214,7 +215,16 @@ impl<'a> WindowMethods for JSRef<'a, Window> {
     }
 
     fn SetTimeout(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec<JSVal>) -> i32 {
-        self.timers.set_timeout_or_interval(callback,
+        self.timers.set_timeout_or_interval(TimerCallback::FunctionTimerCallback(callback),
+                                            args,
+                                            timeout,
+                                            IsInterval::NonInterval,
+                                            TimerSource::FromWindow(self.page.id.clone()),
+                                            self.script_chan.clone())
+    }
+
+    fn SetTimeout_(self, _cx: *mut JSContext, callback: DOMString, timeout: i32, args: Vec<JSVal>) -> i32 {
+        self.timers.set_timeout_or_interval(TimerCallback::StringTimerCallback(callback),
                                             args,
                                             timeout,
                                             IsInterval::NonInterval,
@@ -227,7 +237,16 @@ impl<'a> WindowMethods for JSRef<'a, Window> {
     }
 
     fn SetInterval(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec<JSVal>) -> i32 {
-        self.timers.set_timeout_or_interval(callback,
+        self.timers.set_timeout_or_interval(TimerCallback::FunctionTimerCallback(callback),
+                                            args,
+                                            timeout,
+                                            IsInterval::Interval,
+                                            TimerSource::FromWindow(self.page.id.clone()),
+                                            self.script_chan.clone())
+    }
+
+    fn SetInterval_(self, _cx: *mut JSContext, callback: DOMString, timeout: i32, args: Vec<JSVal>) -> i32 {
+        self.timers.set_timeout_or_interval(TimerCallback::StringTimerCallback(callback),
                                             args,
                                             timeout,
                                             IsInterval::Interval,
@@ -297,22 +316,25 @@ pub trait WindowHelpers {
     fn init_browser_context(self, doc: JSRef<Document>);
     fn load_url(self, href: DOMString);
     fn handle_fire_timer(self, timer_id: TimerId);
-    fn evaluate_js_with_result(self, code: &str) -> JSVal;
-    fn evaluate_script_with_result(self, code: &str, filename: &str) -> JSVal;
 }
 
+pub trait ScriptHelpers {
+    fn evaluate_js_on_global_with_result(self, code: &str) -> JSVal;
+    fn evaluate_script_on_global_with_result(self, code: &str, filename: &str) -> JSVal;
+}
 
-impl<'a> WindowHelpers for JSRef<'a, Window> {
+impl<'a, T: Reflectable> ScriptHelpers for JSRef<'a, T> {
-    fn evaluate_js_with_result(self, code: &str) -> JSVal {
+    fn evaluate_js_on_global_with_result(self, code: &str) -> JSVal {
-        self.evaluate_script_with_result(code, "")
+        self.evaluate_script_on_global_with_result(code, "")
     }
 
-    fn evaluate_script_with_result(self, code: &str, filename: &str) -> JSVal {
+    fn evaluate_script_on_global_with_result(self, code: &str, filename: &str) -> JSVal {
-        let global = self.reflector().get_jsobject();
+        let this = self.reflector().get_jsobject();
+        let cx = global_object_for_js_object(this).root().r().get_cx();
+        let global = global_object_for_js_object(this).root().r().reflector().get_jsobject();
         let code: Vec<u16> = code.as_slice().utf16_units().collect();
         let mut rval = UndefinedValue();
         let filename = filename.to_c_str();
-        let cx = self.get_cx();
 
         with_compartment(cx, global, || {
             unsafe {
@@ -325,7 +347,9 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
             }
         })
     }
+}
 
+impl<'a> WindowHelpers for JSRef<'a, Window> {
     fn flush_layout(self, goal: ReflowGoal, query: ReflowQueryType) {
         self.page().flush_layout(goal, query);
     }

components/script/dom/workerglobalscope.rs

@@ -17,7 +17,7 @@ use dom::workerlocation::WorkerLocation;
 use dom::workernavigator::WorkerNavigator;
 use dom::window::{base64_atob, base64_btoa};
 use script_task::{ScriptChan, TimerSource};
-use timers::{IsInterval, TimerId, TimerManager};
+use timers::{IsInterval, TimerId, TimerManager, TimerCallback};
 
 use servo_net::resource_task::{ResourceTask, load_whole_resource};
 use servo_util::str::DOMString;
@@ -143,7 +143,16 @@ impl<'a> WorkerGlobalScopeMethods for JSRef<'a, WorkerGlobalScope> {
     }
 
     fn SetTimeout(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec<JSVal>) -> i32 {
-        self.timers.set_timeout_or_interval(callback,
+        self.timers.set_timeout_or_interval(TimerCallback::FunctionTimerCallback(callback),
+                                            args,
+                                            timeout,
+                                            IsInterval::NonInterval,
+                                            TimerSource::FromWorker,
+                                            self.script_chan())
+    }
+
+    fn SetTimeout_(self, _cx: *mut JSContext, callback: DOMString, timeout: i32, args: Vec<JSVal>) -> i32 {
+        self.timers.set_timeout_or_interval(TimerCallback::StringTimerCallback(callback),
                                             args,
                                             timeout,
                                             IsInterval::NonInterval,
@@ -156,7 +165,16 @@ impl<'a> WorkerGlobalScopeMethods for JSRef<'a, WorkerGlobalScope> {
     }
 
     fn SetInterval(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec<JSVal>) -> i32 {
-        self.timers.set_timeout_or_interval(callback,
+        self.timers.set_timeout_or_interval(TimerCallback::FunctionTimerCallback(callback),
+                                            args,
+                                            timeout,
+                                            IsInterval::Interval,
+                                            TimerSource::FromWorker,
+                                            self.script_chan())
+    }
+
+    fn SetInterval_(self, _cx: *mut JSContext, callback: DOMString, timeout: i32, args: Vec<JSVal>) -> i32 {
+        self.timers.set_timeout_or_interval(TimerCallback::StringTimerCallback(callback),
                                             args,
                                             timeout,
                                             IsInterval::Interval,

components/script/script_task.rs

@@ -27,7 +27,7 @@ use dom::htmlelement::HTMLElementTypeId;
 use dom::keyboardevent::KeyboardEvent;
 use dom::mouseevent::MouseEvent;
 use dom::node::{mod, Node, NodeHelpers, NodeDamage, NodeTypeId};
-use dom::window::{Window, WindowHelpers};
+use dom::window::{Window, WindowHelpers, ScriptHelpers};
 use parse::html::{HTMLInput, parse_html};
 use layout_interface::{ScriptLayoutChan, LayoutChan, ReflowGoal, ReflowQueryType};
 use layout_interface;
@@ -836,7 +836,7 @@ impl ScriptTask {
             (HTMLInput::InputUrl(load_response), final_url)
         } else {
             let evalstr = load_data.url.non_relative_scheme_data().unwrap();
-            let jsval = window.r().evaluate_js_with_result(evalstr);
+            let jsval = window.r().evaluate_js_on_global_with_result(evalstr);
             let strval = FromJSValConvertible::from_jsval(self.get_cx(), jsval,
                                                           StringificationBehavior::Empty);
             (HTMLInput::InputString(strval.unwrap_or("".into_string())), doc_url)

components/script/timers.rs

@@ -8,9 +8,12 @@ use dom::bindings::codegen::Bindings::FunctionBinding::Function;
 use dom::bindings::js::JSRef;
 use dom::bindings::utils::Reflectable;
 
+use dom::window::ScriptHelpers;
+
 use script_task::{ScriptChan, ScriptMsg, TimerSource};
 
 use servo_util::task::spawn_named;
+use servo_util::str::DOMString;
 
 use js::jsval::JSVal;
 
@@ -35,6 +38,13 @@ struct TimerHandle {
     cancel_chan: Option<Sender<()>>,
 }
 
+#[jstraceable]
+#[deriving(Clone)]
+pub enum TimerCallback {
+    StringTimerCallback(DOMString),
+    FunctionTimerCallback(Function)
+}
+
 impl Hash for TimerId {
     fn hash(&self, state: &mut sip::SipState) {
         let TimerId(id) = *self;
@@ -82,7 +92,7 @@ pub enum IsInterval {
 #[deriving(Clone)]
 struct TimerData {
     is_interval: IsInterval,
-    funval: Function,
+    callback: TimerCallback,
     args: Vec<JSVal>
 }
 
@@ -95,7 +105,7 @@ impl TimerManager {
     }
 
     pub fn set_timeout_or_interval(&self,
-                                  callback: Function,
+                                  callback: TimerCallback,
                                   arguments: Vec<JSVal>,
                                   timeout: i32,
                                   is_interval: IsInterval,
@@ -151,7 +161,7 @@ impl TimerManager {
             cancel_chan: Some(cancel_chan),
             data: TimerData {
                 is_interval: is_interval,
-                funval: callback,
+                callback: callback,
                 args: arguments
             }
         };
@@ -163,7 +173,7 @@ impl TimerManager {
         let mut timer_handle = self.active_timers.borrow_mut().remove(&TimerId(handle));
         match timer_handle {
             Some(ref mut handle) => handle.cancel(),
-            None => { }
+            None => {}
         }
     }
 
@@ -175,7 +185,14 @@ impl TimerManager {
         };
 
         // TODO: Must handle rooting of funval and args when movable GC is turned on
-        let _ = data.funval.Call_(this, data.args, ReportExceptions);
+        match data.callback {
+            TimerCallback::FunctionTimerCallback(function) => {
+                let _ = function.Call_(this, data.args, ReportExceptions);
+            }
+            TimerCallback::StringTimerCallback(code_str) => {
+                this.evaluate_js_on_global_with_result(code_str.as_slice());
+            }
+        };
 
         if data.is_interval == IsInterval::NonInterval {
             self.active_timers.borrow_mut().remove(&timer_id);

tests/wpt/metadata/html/webappapis/scripting/processing-model-2/compile-error-in-setInterval.html.ini

@@ -1,9 +1,9 @@
 [compile-error-in-setInterval.html]
   type: testharness
-  expected: TIMEOUT
+  expected: OK
   [window.onerror - compile error in setInterval]
-    expected: NOTRUN
+    expected: FAIL
 
   [window.onerror - compile error in setInterval (column)]
-    expected: NOTRUN
+    expected: FAIL
 

tests/wpt/metadata/html/webappapis/scripting/processing-model-2/compile-error-in-setTimeout.html.ini

@@ -1,9 +1,9 @@
 [compile-error-in-setTimeout.html]
   type: testharness
-  expected: TIMEOUT
+  expected: OK
   [window.onerror - compile error in setTimeout]
-    expected: NOTRUN
+    expected: FAIL
 
   [window.onerror - compile error in setTimeout (column)]
-    expected: NOTRUN
+    expected: FAIL
 

tests/wpt/metadata/html/webappapis/scripting/processing-model-2/runtime-error-in-setInterval.html.ini

@@ -1,9 +1,9 @@
 [runtime-error-in-setInterval.html]
   type: testharness
-  expected: TIMEOUT
+  expected: OK
   [window.onerror - runtime error in setInterval]
-    expected: NOTRUN
+    expected: FAIL
 
   [window.onerror - runtime error in setInterval (column)]
-    expected: NOTRUN
+    expected: FAIL
 

tests/wpt/metadata/html/webappapis/scripting/processing-model-2/runtime-error-in-setTimeout.html.ini

@@ -1,9 +1,9 @@
 [runtime-error-in-setTimeout.html]
   type: testharness
-  expected: TIMEOUT
+  expected: OK
   [window.onerror - runtime error in setTimeout]
-    expected: NOTRUN
+    expected: FAIL
 
   [window.onerror - runtime error in setTimeout (column)]
-    expected: NOTRUN
+    expected: FAIL
 

tests/wpt/metadata/workers/WorkerGlobalScope_setInterval.htm.ini

@@ -1,6 +0,0 @@
-[WorkerGlobalScope_setInterval.htm]
-  type: testharness
-  expected: TIMEOUT
-  [ WorkerGlobalScope API: setInterval() ]
-    expected: TIMEOUT
-

tests/wpt/metadata/workers/WorkerGlobalScope_setTimeout.htm.ini

@@ -1,6 +0,0 @@
-[WorkerGlobalScope_setTimeout.htm]
-  type: testharness
-  expected: TIMEOUT
-  [ WorkerGlobalScope API: setTimeout() ]
-    expected: TIMEOUT
-