auto merge of #2129 : lpy/servo/issue2116, r=jdm

see #2116
I add an `is_interval` field, so that when the `TimerData` is passed by `SetInterval`, we will not delete it from `active_timers`.
Also I think maybe we can extract the code in `ClearTimeout` and `ClearInterval` into another method to avoid duplicate.

src/components/script/dom/webidls/Window.webidl

@@ -71,6 +71,8 @@ interface WindowTimers {
   //XXXjdm No support for Function or variadic arguments yet
   long setTimeout(any handler, optional long timeout = 0/*, any... arguments*/);
   void clearTimeout(optional long handle = 0);
+  long setInterval(any handler, optional long timeout = 0/*, any... arguments*/);
+  void clearInterval(optional long handler = 0);
   /*long setTimeout(DOMString 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);

src/components/script/dom/window.rs

@@ -128,6 +128,7 @@ impl Drop for Window {
 //      to the function when calling it)
 pub struct TimerData {
     handle: i32,
+    is_interval: bool,
     funval: JSVal,
     args: ~[JSVal],
 }
@@ -226,7 +227,7 @@ impl Reflectable for Window {
 }
 
 impl Window {
-    pub fn SetTimeout(&mut self, _cx: *JSContext, callback: JSVal, timeout: i32) -> i32 {
+    fn set_timeout_or_interval(&mut self, callback: JSVal, timeout: i32, is_interval: bool) -> i32 {
         let timeout = cmp::max(0, timeout) as u64;
         let handle = self.next_timer_handle;
         self.next_timer_handle += 1;
@@ -236,9 +237,18 @@ impl Window {
         let tm = Timer::new().unwrap();
         let (cancel_chan, cancel_port) = channel();
         let chan = self.extra.timer_chan.clone();
-        spawn_named("Window:SetTimeout", proc() {
+        let spawn_name = if is_interval {
+            "Window:SetInterval"
+        } else {
+            "Window:SetTimeout"
+        };
+        spawn_named(spawn_name, proc() {
             let mut tm = tm;
-            let timeout_port = tm.oneshot(timeout);
+            let timeout_port = if is_interval {
+                tm.periodic(timeout)
+            } else {
+                tm.oneshot(timeout)
+            };
             let cancel_port = cancel_port;
 
             let select = Select::new();
@@ -246,19 +256,33 @@ impl Window {
             unsafe { timeout_handle.add() };
             let mut cancel_handle = select.handle(&cancel_port);
             unsafe { cancel_handle.add() };
+
+            loop {
                 let id = select.wait();
                 if id == timeout_handle.id() {
+                    timeout_port.recv();
                     chan.send(TimerMessageFire(~TimerData {
                         handle: handle,
+                        is_interval: is_interval,
                         funval: callback,
                         args: ~[],
                     }));
+                    if !is_interval {
+                        break;
+                    }
+                } else if id == cancel_handle.id() {
+                    break;
+                }
             }
         });
         self.active_timers.insert(handle, TimerHandle { handle: handle, cancel_chan: Some(cancel_chan) });
         handle
     }
 
+    pub fn SetTimeout(&mut self, _cx: *JSContext, callback: JSVal, timeout: i32) -> i32 {
+        self.set_timeout_or_interval(callback, timeout, false)
+    }
+
     pub fn ClearTimeout(&mut self, handle: i32) {
         let timer_handle = self.active_timers.pop(&handle);
         match timer_handle {
@@ -267,6 +291,14 @@ impl Window {
         }
     }
 
+    pub fn SetInterval(&mut self, _cx: *JSContext, callback: JSVal, timeout: i32) -> i32 {
+        self.set_timeout_or_interval(callback, timeout, true)
+    }
+
+    pub fn ClearInterval(&mut self, handle: i32) {
+        self.ClearTimeout(handle);
+    }
+
     pub fn damage_and_reflow(&self, damage: DocumentDamageLevel) {
         // FIXME This should probably be ReflowForQuery, not Display. All queries currently
         // currently rely on the display list, which means we can't destroy it by

src/components/script/script_task.rs

@@ -660,10 +660,16 @@ impl ScriptTask {
         let frame = page.frame();
         let mut window = frame.get_ref().window.clone();
 
-        let timer_handle = window.get_mut().active_timers.pop(&timer_data.handle);
+        {
+            let timer_handle = window.get().active_timers.find(&timer_data.handle);
             if timer_handle.is_none() {
                 return;
             }
+        }
+
+        if !timer_data.is_interval {
+            window.get_mut().active_timers.remove(&timer_data.handle);
+        }
 
         let js_info = page.js_info();
         let this_value = if timer_data.args.len() > 0 {

src/test/content/test_window_setInterval.html

@@ -0,0 +1,20 @@
+<html>
+  <head>
+    <script src="harness.js"></script>
+  </head>
+  <body>
+    <script>
+      var x = 0;
+      var intervalID = setInterval(function() {
+        x += 1;
+        if (x == 2) {
+          clearInterval(intervalID);
+          setTimeout(function() {
+            is(x, 2);
+            finish();
+          }, 300);
+        }
+      }, 10);
+    </script>
+  </body>
+</html>

src/test/html/test_interval.html

@@ -0,0 +1,19 @@
+<html>
+  <head>
+  </head>
+  <body>
+    <script>
+      var x = 0;
+      alert("Interval begin");
+      var intervalID = setInterval(function() {
+        if (x < 10) {
+          alert("interval " + x);
+          x += 1;
+        } else {
+          clearInterval(intervalID);
+          alert("Interval deleted");
+        }
+      }, 300);
+    </script>
+  </body>
+</html>