Auto merge of #8768 - vegayours:8616_intermittent_option_unwrap_in_timers, r=jdm

fix intermittent Option::unwrap in timers

fixes intermittent #8616

This intermittent indicates real problem in code.
Lets consider such code:
```javascript
// timer 1
setTimeout(function() {
     //timer 2
     setTimeout(function() {}, 0);
}, 0);
```
When we receive event to fire timer 1 it will be selected and extracted from active timers list in fire_timer function. During timer 1 handler execution we will schedule timer 2 and request timer event for it. But it will be executed during same fire_timer call because of 0 timeout. And as a result we will have empty timers list and expecting event for timer 2 that will crash in assert.

I'm not sure that all I've written is clear, but we have something like this:
```
install timer 1 -> [1] in timers list
push and expect timer event 1 -> expected_event=1
received timer event 1
    fire_timer()
         select timer 1 to execute -> [] in timers list
         execute timer 1 handler
             install timer 2 -> [2] in timers list
             push and expect timer event 2 -> expected_event=2
         select timer 2 to execute (because of 0 timeout) -> [] in tiemrs list
         execute timer 2 handler
expected_event=2 is dangling
received timer event 2
    fire_timer() -> BOOM
```

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8768)
<!-- Reviewable:end -->

4 files changed, 55 insertions, 8 deletions

diff --git a/components/script/timers.rs b/components/script/timers.rs
index 4b8a2b6d272..7e164bbe5eb 100644
--- a/components/script/timers.rs
+++ b/components/script/timers.rs
@@ -23,7 +23,7 @@ use std::rc::Rc;
 use util::mem::HeapSizeOf;
 use util::str::DOMString;
 
-#[derive(JSTraceable, PartialEq, Eq, Copy, Clone, HeapSizeOf, Hash, PartialOrd, Ord)]
+#[derive(JSTraceable, PartialEq, Eq, Copy, Clone, HeapSizeOf, Hash, PartialOrd, Ord, Debug)]
 pub struct TimerHandle(i32);
 
 #[derive(JSTraceable, HeapSizeOf)]
@@ -253,16 +253,22 @@ impl ActiveTimers {
         // Since the event id was the expected one, at least one timer should be due.
         assert!(base_time >= self.timers.borrow().last().unwrap().next_call);
 
+        // select timers to run to prevent firing timers
+        // that were installed during fire of another timer
+        let mut timers_to_run: Vec<Timer> = Vec::new();
+
         loop {
-            let timer = {
-                let mut timers = self.timers.borrow_mut();
+            let mut timers = self.timers.borrow_mut();
 
-                if timers.is_empty() || timers.last().unwrap().next_call > base_time {
-                    break;
-                }
+            if timers.is_empty() || timers.last().unwrap().next_call > base_time {
+                break;
+            }
+
+            timers_to_run.push(timers.pop().unwrap());
+        }
+
+        for timer in timers_to_run {
 
-                timers.pop().unwrap()
-            };
             let callback = timer.callback.clone();
 
             // prep for step 6 in nested set_timeout_or_interval calls
diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json
index 84453ef490e..5fa510c5fe0 100644
--- a/tests/wpt/mozilla/meta/MANIFEST.json
+++ b/tests/wpt/mozilla/meta/MANIFEST.json
@@ -5757,6 +5757,12 @@
             "url": "/_mozilla/mozilla/textcontent.html"
           }
         ],
+        "mozilla/timer_eventInvalidation.html": [
+          {
+            "path": "mozilla/timer_eventInvalidation.html",
+            "url": "/_mozilla/mozilla/timer_eventInvalidation.html"
+          }
+        ],
         "mozilla/title.html": [
           {
             "path": "mozilla/title.html",
diff --git a/tests/wpt/mozilla/tests/mozilla/timer_eventInvalidation.html b/tests/wpt/mozilla/tests/mozilla/timer_eventInvalidation.html
new file mode 100644
index 00000000000..4a182a62f75
--- /dev/null
+++ b/tests/wpt/mozilla/tests/mozilla/timer_eventInvalidation.html
@@ -0,0 +1,21 @@
+<html>
+  <head>
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+  </head>
+  <body>
+    <script>
+    async_test(function(test) {
+        // couldn't reproduce this behaviour without iframe
+        var iframe = document.createElement('iframe');
+        window.addEventListener('test_done', function() {
+            // if event invalidation is not properly implemented
+            // servo will crash during a short period of time
+            setTimeout(() => test.done(), 50);
+        }, false);
+        iframe.src = 'timer_eventInvalidation_test.html';
+        document.body.appendChild(iframe);
+    });
+    </script>
+  </body>
+</html>
diff --git a/tests/wpt/mozilla/tests/mozilla/timer_eventInvalidation_test.html b/tests/wpt/mozilla/tests/mozilla/timer_eventInvalidation_test.html
new file mode 100644
index 00000000000..9656e578605
--- /dev/null
+++ b/tests/wpt/mozilla/tests/mozilla/timer_eventInvalidation_test.html
@@ -0,0 +1,14 @@
+<html>
+  <head>
+  </head>
+  <body>
+    <script>
+    setTimeout(function() {
+        setTimeout(function() {
+            var e = new Event('test_done');
+            window.parent.dispatchEvent(e);
+        }, 0);
+    }, 0);
+    </script>
+  </body>
+</html>