Allow passing arguments to setTimeout/setInterval callbacks

components/script/dom/dedicatedworkerglobalscope.rs

@@ -146,7 +146,7 @@ impl DedicatedWorkerGlobalScope {
                         Worker::handle_release(addr)
                     },
                     Ok(FireTimerMsg(FromWorker, timer_id)) => {
-                        scope.handle_fire_timer(timer_id, js_context.ptr);
+                        scope.handle_fire_timer(timer_id);
                     }
                     Ok(_) => panic!("Unexpected message"),
                     Err(_) => break,

components/script/dom/webidls/Function.webidl

@@ -0,0 +1,14 @@
+/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/.
+ *
+ * The origin of this IDL file is
+ * http://www.whatwg.org/specs/web-apps/current-work/#functiocn
+ *
+ * © Copyright 2004-2011 Apple Computer, Inc., Mozilla Foundation, and
+ * Opera Software ASA. You are granted a license to use, reproduce
+ * and create derivative works of this document.
+ */
+
+callback Function = any(any... arguments);

components/script/dom/webidls/Window.webidl

@@ -64,13 +64,11 @@ Window implements WindowEventHandlers;
 // http://www.whatwg.org/html/#windowtimers
 [NoInterfaceObject/*, Exposed=Window,Worker*/]
 interface WindowTimers {
-  //long setTimeout(Function handler, optional long timeout = 0, any... arguments);
+  long setTimeout(Function handler, optional long timeout = 0, any... arguments);
   //long setTimeout(DOMString handler, optional long timeout = 0, any... arguments);
-  long setTimeout(any handler, optional long timeout = 0);
   void clearTimeout(optional long handle = 0);
-  //long setInterval(Function handler, optional long timeout = 0, any... arguments);
+  long setInterval(Function handler, optional long timeout = 0, any... arguments);
   //long setInterval(DOMString handler, optional long timeout = 0, any... arguments);
-  long setInterval(any handler, optional long timeout = 0);
   void clearInterval(optional long handle = 0);
 };
 Window implements WindowTimers;

components/script/dom/window.rs

@@ -4,6 +4,7 @@
 
 use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::EventHandlerBinding::{OnErrorEventHandlerNonNull, EventHandlerNonNull};
+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;
@@ -223,8 +224,9 @@ impl<'a> WindowMethods for JSRef<'a, Window> {
         self.navigator.get().unwrap()
     }
 
-    fn SetTimeout(self, _cx: *mut JSContext, callback: JSVal, timeout: i32) -> i32 {
+    fn SetTimeout(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec<JSVal>) -> i32 {
         self.timers.set_timeout_or_interval(callback,
+                                            args,
                                             timeout,
                                             false, // is_interval
                                             FromWindow(self.page.id.clone()),
@@ -235,8 +237,9 @@ impl<'a> WindowMethods for JSRef<'a, Window> {
         self.timers.clear_timeout_or_interval(handle);
     }
 
-    fn SetInterval(self, _cx: *mut JSContext, callback: JSVal, timeout: i32) -> i32 {
+    fn SetInterval(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec<JSVal>) -> i32 {
         self.timers.set_timeout_or_interval(callback,
+                                            args,
                                             timeout,
                                             true, // is_interval
                                             FromWindow(self.page.id.clone()),
@@ -317,7 +320,7 @@ pub trait WindowHelpers {
     fn wait_until_safe_to_modify_dom(self);
     fn init_browser_context(self, doc: JSRef<Document>);
     fn load_url(self, href: DOMString);
-    fn handle_fire_timer(self, timer_id: TimerId, cx: *mut JSContext);
+    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;
 }
@@ -380,9 +383,8 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
         }
     }
 
-    fn handle_fire_timer(self, timer_id: TimerId, cx: *mut JSContext) {
+    fn handle_fire_timer(self, timer_id: TimerId) {
-        let this_value = self.reflector().get_jsobject();
+        self.timers.fire_timer(timer_id, self.clone());
-        self.timers.fire_timer(timer_id, this_value, cx);
         self.flush_layout();
     }
 }

components/script/dom/workerglobalscope.rs

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use dom::bindings::codegen::Bindings::WorkerGlobalScopeBinding::WorkerGlobalScopeMethods;
+use dom::bindings::codegen::Bindings::FunctionBinding::Function;
 use dom::bindings::error::{ErrorResult, Fallible, Syntax, Network, FailureUnknown};
 use dom::bindings::global;
 use dom::bindings::js::{MutNullableJS, JSRef, Temporary, OptionalSettable};
@@ -155,8 +156,9 @@ impl<'a> WorkerGlobalScopeMethods for JSRef<'a, WorkerGlobalScope> {
         base64_atob(atob)
     }
 
-    fn SetTimeout(self, _cx: *mut JSContext, handler: JSVal, timeout: i32) -> i32 {
+    fn SetTimeout(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec<JSVal>) -> i32 {
-        self.timers.set_timeout_or_interval(handler,
+        self.timers.set_timeout_or_interval(callback,
+                                            args,
                                             timeout,
                                             false, // is_interval
                                             FromWorker,
@@ -167,8 +169,9 @@ impl<'a> WorkerGlobalScopeMethods for JSRef<'a, WorkerGlobalScope> {
         self.timers.clear_timeout_or_interval(handle);
     }
 
-    fn SetInterval(self, _cx: *mut JSContext, handler: JSVal, timeout: i32) -> i32 {
+    fn SetInterval(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec<JSVal>) -> i32 {
-        self.timers.set_timeout_or_interval(handler,
+        self.timers.set_timeout_or_interval(callback,
+                                            args,
                                             timeout,
                                             true, // is_interval
                                             FromWorker,
@@ -181,14 +184,13 @@ impl<'a> WorkerGlobalScopeMethods for JSRef<'a, WorkerGlobalScope> {
 }
 
 pub trait WorkerGlobalScopeHelpers {
-    fn handle_fire_timer(self, timer_id: TimerId, cx: *mut JSContext);
+    fn handle_fire_timer(self, timer_id: TimerId);
 }
 
 impl<'a> WorkerGlobalScopeHelpers for JSRef<'a, WorkerGlobalScope> {
 
-    fn handle_fire_timer(self, timer_id: TimerId, cx: *mut JSContext) {
+    fn handle_fire_timer(self, timer_id: TimerId) {
-        let this_value = self.reflector().get_jsobject();
+        self.timers.fire_timer(timer_id, self.clone());
-        self.timers.fire_timer(timer_id, this_value, cx);
     }
 
 }

components/script/script_task.rs

@@ -665,7 +665,7 @@ impl ScriptTask {
             pipeline ID not associated with this script task. This is a bug.");
         let frame = page.frame();
         let window = frame.as_ref().unwrap().window.root();
-        window.handle_fire_timer(timer_id, self.get_cx());
+        window.handle_fire_timer(timer_id);
     }
 
     /// Handles a notification that reflow completed.

components/script/timers.rs

@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use dom::bindings::cell::DOMRefCell;
+use dom::bindings::callback::ReportExceptions;
+use dom::bindings::codegen::Bindings::FunctionBinding::Function;
+use dom::bindings::js::JSRef;
+use dom::bindings::utils::Reflectable;
 
 use script_task::{FireTimerMsg, ScriptChan};
 use script_task::{TimerSource, FromWindow, FromWorker};
 
 use servo_util::task::spawn_named;
 
-use js::jsapi::JS_CallFunctionValue;
+use js::jsval::JSVal;
-use js::jsapi::{JSContext, JSObject};
-use js::jsval::{JSVal, NullValue};
-use js::rust::with_compartment;
 
 use std::cell::Cell;
 use std::cmp;
@@ -21,7 +22,6 @@ use std::comm::{channel, Sender};
 use std::comm::Select;
 use std::hash::{Hash, sip};
 use std::io::timer::Timer;
-use std::ptr;
 use std::time::duration::Duration;
 
 #[deriving(PartialEq, Eq)]
@@ -69,11 +69,14 @@ impl Drop for TimerManager {
 // Holder for the various JS values associated with setTimeout
 // (ie. function value to invoke and all arguments to pass
 //      to the function when calling it)
+// TODO: Handle rooting during fire_timer when movable GC is turned on
 #[jstraceable]
 #[privatize]
+#[deriving(Clone)]
 struct TimerData {
     is_interval: bool,
-    funval: JSVal,
+    funval: Function,
+    args: Vec<JSVal>
 }
 
 impl TimerManager {
@@ -85,7 +88,8 @@ impl TimerManager {
     }
 
     pub fn set_timeout_or_interval(&self,
-                                  callback: JSVal,
+                                  callback: Function,
+                                  arguments: Vec<JSVal>,
                                   timeout: i32,
                                   is_interval: bool,
                                   source: TimerSource,
@@ -142,6 +146,7 @@ impl TimerManager {
             data: TimerData {
                 is_interval: is_interval,
                 funval: callback,
+                args: arguments
             }
         };
         self.active_timers.borrow_mut().insert(timer_id, timer);
@@ -156,21 +161,15 @@ impl TimerManager {
         }
     }
 
-    pub fn fire_timer(&self, timer_id: TimerId, this: *mut JSObject, cx: *mut JSContext) {
+    pub fn fire_timer<T: Reflectable>(&self, timer_id: TimerId, this: JSRef<T>) {
 
         let data = match self.active_timers.borrow().get(&timer_id) {
             None => return,
-            Some(timer_handle) => timer_handle.data,
+            Some(timer_handle) => timer_handle.data.clone(),
         };
 
-        // TODO: Support extra arguments. This requires passing a `*JSVal` array as `argv`.
+        // TODO: Must handle rooting of funval and args when movable GC is turned on
-        with_compartment(cx, this, || {
+        let _ = data.funval.Call_(this, data.args, ReportExceptions);
-            let mut rval = NullValue();
-            unsafe {
-                JS_CallFunctionValue(cx, this, data.funval,
-                                     0, ptr::null_mut(), &mut rval);
-            }
-        });
 
         if !data.is_interval {
             self.active_timers.borrow_mut().remove(&timer_id);

tests/wpt/metadata/html/dom/interfaces.html.ini

@@ -9948,3 +9948,27 @@
   [HTMLFontElement interface: document.createElement("font") must inherit property "size" with the proper type (2)]
     expected: FAIL
 
+  [Window interface: operation setTimeout(Function,long,any)]
+    expected: FAIL
+
+  [Window interface: operation setTimeout(DOMString,long,any)]
+    expected: FAIL
+
+  [Window interface: operation setInterval(Function,long,any)]
+    expected: FAIL
+
+  [Window interface: operation setInterval(DOMString,long,any)]
+    expected: FAIL
+
+  [WorkerGlobalScope interface: operation setTimeout(Function,long,any)]
+    expected: FAIL
+
+  [WorkerGlobalScope interface: operation setTimeout(DOMString,long,any)]
+    expected: FAIL
+
+  [WorkerGlobalScope interface: operation setInterval(Function,long,any)]
+    expected: FAIL
+
+  [WorkerGlobalScope interface: operation setInterval(DOMString,long,any)]
+    expected: FAIL
+

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

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

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

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

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

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

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

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