Auto merge of #12404 - ConnorGBrewster:task_source_cleanup, r=asajeffrey

Clean up task sources and make all tasks cancellable

<!-- Please describe your changes on the following line: -->
This makes it so each task is a thin wrapper over a runnable and whenever a task is queued, it is automatically wrapped by the window's `runnable_wrapper`.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix  #11703 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12404)
<!-- Reviewable:end -->

components/script/dom/document.rs

@@ -1488,11 +1488,12 @@ impl Document {
 
         update_with_current_time_ms(&self.dom_content_loaded_event_start);
 
-        self.window().dom_manipulation_task_source().queue_event(self.upcast(), atom!("DOMContentLoaded"),
-            EventBubbles::Bubbles, EventCancelable::NotCancelable);
-        self.window().reflow(ReflowGoal::ForDisplay,
-                             ReflowQueryType::NoQuery,
-                             ReflowReason::DOMContentLoaded);
+        let window = self.window();
+        window.dom_manipulation_task_source().queue_event(self.upcast(), atom!("DOMContentLoaded"),
+            EventBubbles::Bubbles, EventCancelable::NotCancelable, window);
+        window.reflow(ReflowGoal::ForDisplay,
+                      ReflowQueryType::NoQuery,
+                      ReflowReason::DOMContentLoaded);
 
         update_with_current_time_ms(&self.dom_content_loaded_event_end);
     }

components/script/dom/event.rs

@@ -8,9 +8,11 @@ use dom::bindings::codegen::Bindings::EventBinding::{EventConstants, EventMethod
 use dom::bindings::error::Fallible;
 use dom::bindings::global::GlobalRef;
 use dom::bindings::js::{JS, MutNullableHeap, Root};
+use dom::bindings::refcounted::Trusted;
 use dom::bindings::reflector::{Reflector, reflect_dom_object};
 use dom::bindings::str::DOMString;
 use dom::eventtarget::EventTarget;
+use script_thread::Runnable;
 use std::cell::Cell;
 use std::default::Default;
 use string_cache::Atom;
@@ -26,7 +28,7 @@ pub enum EventPhase {
     Bubbling  = EventConstants::BUBBLING_PHASE,
 }
 
-#[derive(PartialEq, HeapSizeOf)]
+#[derive(PartialEq, HeapSizeOf, Copy, Clone)]
 pub enum EventBubbles {
     Bubbles,
     DoesNotBubble
@@ -50,7 +52,7 @@ impl From<bool> for EventBubbles {
     }
 }
 
-#[derive(PartialEq, HeapSizeOf)]
+#[derive(PartialEq, HeapSizeOf, Copy, Clone)]
 pub enum EventCancelable {
     Cancelable,
     NotCancelable
@@ -297,3 +299,35 @@ impl Event {
         target.dispatch_event(self)
     }
 }
+
+// https://dom.spec.whatwg.org/#concept-event-fire
+pub struct EventRunnable {
+    pub target: Trusted<EventTarget>,
+    pub name: Atom,
+    pub bubbles: EventBubbles,
+    pub cancelable: EventCancelable,
+}
+
+impl Runnable for EventRunnable {
+    fn name(&self) -> &'static str { "EventRunnable" }
+
+    fn handler(self: Box<EventRunnable>) {
+        let target = self.target.root();
+        target.fire_event(&*self.name, self.bubbles, self.cancelable);
+    }
+}
+
+// https://html.spec.whatwg.org/multipage/#fire-a-simple-event
+pub struct SimpleEventRunnable {
+    pub target: Trusted<EventTarget>,
+    pub name: Atom,
+}
+
+impl Runnable for SimpleEventRunnable {
+    fn name(&self) -> &'static str { "SimpleEventRunnable" }
+
+    fn handler(self: Box<SimpleEventRunnable>) {
+        let target = self.target.root();
+        target.fire_simple_event(&*self.name);
+    }
+}

components/script/dom/htmldetailselement.rs

@@ -19,7 +19,6 @@ use script_thread::Runnable;
 use std::cell::Cell;
 use string_cache::Atom;
 use task_source::TaskSource;
-use task_source::dom_manipulation::DOMManipulationTask;
 
 #[dom_struct]
 pub struct HTMLDetailsElement {
@@ -72,14 +71,13 @@ impl VirtualMethods for HTMLDetailsElement {
             self.toggle_counter.set(counter);
 
             let window = window_from_node(self);
-            let window = window.r();
             let task_source = window.dom_manipulation_task_source();
             let details = Trusted::new(self);
             let runnable = box DetailsNotificationRunnable {
                 element: details,
                 toggle_number: counter
             };
-            let _ = task_source.queue(DOMManipulationTask::Runnable(runnable));
+            let _ = task_source.queue(runnable, window.r());
         }
     }
 }

components/script/dom/htmlformelement.rs

@@ -53,7 +53,6 @@ use string_cache::Atom;
 use style::attr::AttrValue;
 use style::str::split_html_space_chars;
 use task_source::TaskSource;
-use task_source::dom_manipulation::DOMManipulationTask;
 use url::form_urlencoded;
 
 #[derive(JSTraceable, PartialEq, Clone, Copy, HeapSizeOf)]
@@ -484,7 +483,7 @@ impl HTMLFormElement {
         };
 
         // Step 3
-        window.dom_manipulation_task_source().queue(DOMManipulationTask::Runnable(nav)).unwrap();
+        window.dom_manipulation_task_source().queue(nav, window).unwrap();
     }
 
     /// Interactively validate the constraints of form elements

components/script/dom/htmlimageelement.rs

@@ -32,7 +32,6 @@ use std::sync::Arc;
 use string_cache::Atom;
 use style::attr::{AttrValue, LengthOrPercentageOrAuto};
 use task_source::TaskSource;
-use task_source::dom_manipulation::DOMManipulationTask;
 use url::Url;
 
 #[derive(JSTraceable, HeapSizeOf)]
@@ -141,7 +140,7 @@ impl HTMLImageElement {
                         // Return the image via a message to the script thread, which marks the element
                         // as dirty and triggers a reflow.
                         let image_response = message.to().unwrap();
-                        let runnable = ImageResponseHandlerRunnable::new(
+                        let runnable = box ImageResponseHandlerRunnable::new(
                             trusted_node.clone(), image_response);
                         let runnable = wrapper.wrap_runnable(runnable);
                         let _ = script_chan.send(CommonScriptMsg::RunnableMsg(
@@ -180,12 +179,12 @@ impl HTMLImageElement {
                         }
                     }
 
-                    let runnable = Box::new(ImgParseErrorRunnable {
+                    let runnable = box ImgParseErrorRunnable {
                         img: Trusted::new(self),
                         src: src.into(),
-                    });
+                    };
                     let task = window.dom_manipulation_task_source();
-                    let _ = task.queue(DOMManipulationTask::Runnable(runnable));
+                    let _ = task.queue(runnable, window);
                 }
             }
         }

components/script/dom/htmlinputelement.rs

@@ -577,7 +577,8 @@ impl HTMLInputElementMethods for HTMLInputElement {
             &self.upcast(),
             atom!("select"),
             EventBubbles::Bubbles,
-            EventCancelable::NotCancelable);
+            EventCancelable::NotCancelable,
+            window.r());
         self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
     }
 
@@ -1070,7 +1071,8 @@ impl VirtualMethods for HTMLInputElement {
                                     &self.upcast(),
                                     atom!("input"),
                                     EventBubbles::Bubbles,
-                                    EventCancelable::NotCancelable);
+                                    EventCancelable::NotCancelable,
+                                    window.r());
                             }
 
                             self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);

components/script/dom/htmlmediaelement.rs

@@ -33,7 +33,6 @@ use std::cell::Cell;
 use std::sync::{Arc, Mutex};
 use string_cache::Atom;
 use task_source::TaskSource;
-use task_source::dom_manipulation::DOMManipulationTask;
 use time::{self, Timespec, Duration};
 use url::Url;
 
@@ -238,11 +237,11 @@ impl HTMLMediaElement {
             }
         }
 
-        let task = Task {
+        let task = box Task {
             elem: Trusted::new(self),
         };
         let win = window_from_node(self);
-        let _ = win.dom_manipulation_task_source().queue(DOMManipulationTask::Runnable(box task));
+        let _ = win.dom_manipulation_task_source().queue(task, win.r());
     }
 
     // https://html.spec.whatwg.org/multipage/#internal-pause-steps step 2.2
@@ -262,17 +261,17 @@ impl HTMLMediaElement {
             }
         }
 
-        let task = Task {
+        let task = box Task {
             elem: Trusted::new(self),
         };
         let win = window_from_node(self);
-        let _ = win.dom_manipulation_task_source().queue(DOMManipulationTask::Runnable(box task));
+        let _ = win.dom_manipulation_task_source().queue(task, win.r());
     }
 
     fn queue_fire_simple_event(&self, type_: &'static str) {
         let win = window_from_node(self);
-        let task = FireSimpleEventTask::new(self, type_);
-        let _ = win.dom_manipulation_task_source().queue(DOMManipulationTask::Runnable(box task));
+        let task = box FireSimpleEventTask::new(self, type_);
+        let _ = win.dom_manipulation_task_source().queue(task, win.r());
     }
 
     fn fire_simple_event(&self, type_: &str) {
@@ -498,8 +497,8 @@ impl HTMLMediaElement {
     }
 
     fn queue_dedicated_media_source_failure_steps(&self) {
-        let _ = window_from_node(self).dom_manipulation_task_source().queue(
-            DOMManipulationTask::Runnable(box DedicatedMediaSourceFailureTask::new(self)));
+        let window = window_from_node(self);
+        let _ = window.dom_manipulation_task_source().queue(box DedicatedMediaSourceFailureTask::new(self), window.r());
     }
 
     // https://html.spec.whatwg.org/multipage/#dedicated-media-source-failure-steps

components/script/dom/htmlscriptelement.rs

@@ -460,12 +460,13 @@ impl HTMLScriptElement {
         if external {
             self.dispatch_load_event();
         } else {
-            window.dom_manipulation_task_source().queue_simple_event(self.upcast(), atom!("load"));
+            window.dom_manipulation_task_source().queue_simple_event(self.upcast(), atom!("load"), window.r());
         }
     }
 
     pub fn queue_error_event(&self) {
-        window_from_node(self).dom_manipulation_task_source().queue_simple_event(self.upcast(), atom!("error"));
+        let window = window_from_node(self);
+        window.dom_manipulation_task_source().queue_simple_event(self.upcast(), atom!("error"), window.r());
     }
 
     pub fn dispatch_before_script_execute_event(&self) -> bool {

components/script/dom/htmltextareaelement.rs

@@ -260,7 +260,8 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement {
             &self.upcast(),
             atom!("select"),
             EventBubbles::Bubbles,
-            EventCancelable::NotCancelable);
+            EventCancelable::NotCancelable,
+            window.r());
         self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
     }
 }
@@ -383,7 +384,8 @@ impl VirtualMethods for HTMLTextAreaElement {
                                 &self.upcast(),
                                 atom!("input"),
                                 EventBubbles::Bubbles,
-                                EventCancelable::NotCancelable);
+                                EventCancelable::NotCancelable,
+                                window.r());
                         }
 
                         self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);

components/script/dom/storage.rs

@@ -19,7 +19,6 @@ use net_traits::IpcSend;
 use net_traits::storage_thread::{StorageThreadMsg, StorageType};
 use script_thread::{Runnable, ScriptThread};
 use task_source::TaskSource;
-use task_source::dom_manipulation::DOMManipulationTask;
 use url::Url;
 
 #[dom_struct]
@@ -159,10 +158,10 @@ impl Storage {
                                      new_value: Option<String>) {
         let global_root = self.global();
         let global_ref = global_root.r();
-        let task_source = global_ref.as_window().dom_manipulation_task_source();
+        let window = global_ref.as_window();
+        let task_source = window.dom_manipulation_task_source();
         let trusted_storage = Trusted::new(self);
-        task_source.queue(DOMManipulationTask::Runnable(
-            box StorageEventRunnable::new(trusted_storage, key, old_value, new_value))).unwrap();
+        task_source.queue(box StorageEventRunnable::new(trusted_storage, key, old_value, new_value), window).unwrap();
     }
 }
 

components/script/network_listener.rs

@@ -19,20 +19,17 @@ pub struct NetworkListener<Listener: PreInvoke + Send + 'static> {
 
 impl<Listener: PreInvoke + Send + 'static> NetworkListener<Listener> {
     pub fn notify<A: Action<Listener> + Send + 'static>(&self, action: A) {
-        let runnable = ListenerRunnable {
+        let runnable = box ListenerRunnable {
             context: self.context.clone(),
             action: action,
         };
-        if let Some(ref wrapper) = self.wrapper {
-            if let Err(err) = self.script_chan.send(
-                CommonScriptMsg::RunnableMsg(NetworkEvent, wrapper.wrap_runnable(runnable))) {
-                warn!("failed to deliver network data: {:?}", err);
-            }
+        let result = if let Some(ref wrapper) = self.wrapper {
+            self.script_chan.send(CommonScriptMsg::RunnableMsg(NetworkEvent, wrapper.wrap_runnable(runnable)))
         } else {
-            if let Err(err) = self.script_chan.send(
-                CommonScriptMsg::RunnableMsg(NetworkEvent, box runnable)) {
-                warn!("failed to deliver network data: {:?}", err);
-            }
+            self.script_chan.send(CommonScriptMsg::RunnableMsg(NetworkEvent, runnable))
+        };
+        if let Err(err) = result {
+            warn!("failed to deliver network data: {:?}", err);
         }
     }
 }

components/script/script_thread.rs

@@ -174,10 +174,10 @@ pub struct RunnableWrapper {
 }
 
 impl RunnableWrapper {
-    pub fn wrap_runnable<T: Runnable + Send + 'static>(&self, runnable: T) -> Box<Runnable + Send> {
+    pub fn wrap_runnable<T: Runnable + Send + 'static>(&self, runnable: Box<T>) -> Box<Runnable + Send> {
         box CancellableRunnable {
             cancelled: self.cancelled.clone(),
-            inner: box runnable,
+            inner: runnable,
         }
     }
 }
@@ -189,10 +189,16 @@ pub struct CancellableRunnable<T: Runnable + Send> {
 }
 
 impl<T: Runnable + Send> Runnable for CancellableRunnable<T> {
+    fn name(&self) -> &'static str { self.inner.name() }
+
     fn is_cancelled(&self) -> bool {
         self.cancelled.load(Ordering::SeqCst)
     }
 
+    fn main_thread_handler(self: Box<CancellableRunnable<T>>, script_thread: &ScriptThread) {
+        self.inner.main_thread_handler(script_thread);
+    }
+
     fn handler(self: Box<CancellableRunnable<T>>) {
         self.inner.handler()
     }
@@ -982,7 +988,7 @@ impl ScriptThread {
             MainThreadScriptMsg::DOMManipulation(task) =>
                 task.handle_task(self),
             MainThreadScriptMsg::UserInteraction(task) =>
-                task.handle_task(),
+                task.handle_task(self),
         }
     }
 
@@ -1221,7 +1227,7 @@ impl ScriptThread {
 
         // https://html.spec.whatwg.org/multipage/#the-end step 7
         let handler = box DocumentProgressHandler::new(Trusted::new(doc));
-        self.dom_manipulation_task_source.queue(DOMManipulationTask::Runnable(handler)).unwrap();
+        self.dom_manipulation_task_source.queue(handler, doc.window()).unwrap();
 
         self.constellation_chan.send(ConstellationMsg::LoadComplete(pipeline)).unwrap();
     }

components/script/task_source/dom_manipulation.rs

@@ -3,8 +3,9 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use dom::bindings::refcounted::Trusted;
-use dom::event::{EventBubbles, EventCancelable};
+use dom::event::{EventBubbles, EventCancelable, EventRunnable, SimpleEventRunnable};
 use dom::eventtarget::EventTarget;
+use dom::window::Window;
 use script_thread::{MainThreadScriptMsg, Runnable, ScriptThread};
 use std::result::Result;
 use std::sync::mpsc::Sender;
@@ -14,8 +15,9 @@ use task_source::TaskSource;
 #[derive(JSTraceable, Clone)]
 pub struct DOMManipulationTaskSource(pub Sender<MainThreadScriptMsg>);
 
-impl TaskSource<DOMManipulationTask> for DOMManipulationTaskSource {
-    fn queue(&self, msg: DOMManipulationTask) -> Result<(), ()> {
+impl TaskSource for DOMManipulationTaskSource {
+    fn queue<T: Runnable + Send + 'static>(&self, msg: Box<T>, window: &Window) -> Result<(), ()> {
+        let msg = DOMManipulationTask(window.get_runnable_wrapper().wrap_runnable(msg));
         self.0.send(MainThreadScriptMsg::DOMManipulation(msg)).map_err(|_| ())
     }
 }
@@ -25,46 +27,34 @@ impl DOMManipulationTaskSource {
                        target: &EventTarget,
                        name: Atom,
                        bubbles: EventBubbles,
-                       cancelable: EventCancelable) {
+                       cancelable: EventCancelable,
+                       window: &Window) {
         let target = Trusted::new(target);
-        let _ = self.0.send(MainThreadScriptMsg::DOMManipulation(DOMManipulationTask::FireEvent(
-            target, name, bubbles, cancelable)));
+        let runnable = box EventRunnable {
+            target: target,
+            name: name,
+            bubbles: bubbles,
+            cancelable: cancelable,
+        };
+        let _ = self.queue(runnable, window);
     }
 
-    pub fn queue_simple_event(&self, target: &EventTarget, name: Atom) {
+    pub fn queue_simple_event(&self, target: &EventTarget, name: Atom, window: &Window) {
         let target = Trusted::new(target);
-        let _ = self.0.send(MainThreadScriptMsg::DOMManipulation(DOMManipulationTask::FireSimpleEvent(
-            target, name)));
+        let runnable = box SimpleEventRunnable {
+            target: target,
+            name: name,
+        };
+        let _ = self.queue(runnable, window);
     }
 }
 
-pub enum DOMManipulationTask {
-    // https://dom.spec.whatwg.org/#concept-event-fire
-    FireEvent(Trusted<EventTarget>, Atom, EventBubbles, EventCancelable),
-    // https://html.spec.whatwg.org/multipage/#fire-a-simple-event
-    FireSimpleEvent(Trusted<EventTarget>, Atom),
-
-    Runnable(Box<Runnable + Send>),
-}
+pub struct DOMManipulationTask(pub Box<Runnable + Send>);
 
 impl DOMManipulationTask {
     pub fn handle_task(self, script_thread: &ScriptThread) {
-        use self::DOMManipulationTask::*;
-
-        match self {
-            FireEvent(element, name, bubbles, cancelable) => {
-                let target = element.root();
-                target.fire_event(&*name, bubbles, cancelable);
-            }
-            FireSimpleEvent(element, name) => {
-                let target = element.root();
-                target.fire_simple_event(&*name);
-            }
-            Runnable(runnable) => {
-                if !runnable.is_cancelled() {
-                    runnable.main_thread_handler(script_thread);
-                }
-            }
+        if !self.0.is_cancelled() {
+            self.0.main_thread_handler(script_thread);
         }
     }
 }

components/script/task_source/mod.rs

@@ -8,8 +8,10 @@ pub mod history_traversal;
 pub mod networking;
 pub mod user_interaction;
 
+use dom::window::Window;
+use script_thread::Runnable;
 use std::result::Result;
 
-pub trait TaskSource<T> {
-    fn queue(&self, msg: T) -> Result<(), ()>;
+pub trait TaskSource {
+    fn queue<T: Runnable + Send + 'static>(&self, msg: Box<T>, window: &Window) -> Result<(), ()>;
 }

components/script/task_source/user_interaction.rs

@@ -3,9 +3,10 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use dom::bindings::refcounted::Trusted;
-use dom::event::{EventBubbles, EventCancelable};
+use dom::event::{EventBubbles, EventCancelable, EventRunnable};
 use dom::eventtarget::EventTarget;
-use script_thread::MainThreadScriptMsg;
+use dom::window::Window;
+use script_thread::{MainThreadScriptMsg, Runnable, ScriptThread};
 use std::result::Result;
 use std::sync::mpsc::Sender;
 use string_cache::Atom;
@@ -14,38 +15,37 @@ use task_source::TaskSource;
 #[derive(JSTraceable, Clone)]
 pub struct UserInteractionTaskSource(pub Sender<MainThreadScriptMsg>);
 
-impl TaskSource<UserInteractionTask> for UserInteractionTaskSource {
-    fn queue(&self, msg: UserInteractionTask) -> Result<(), ()> {
+impl TaskSource for UserInteractionTaskSource {
+    fn queue<T: Runnable + Send + 'static>(&self, msg: Box<T>, window: &Window) -> Result<(), ()> {
+        let msg = UserInteractionTask(window.get_runnable_wrapper().wrap_runnable(msg));
         self.0.send(MainThreadScriptMsg::UserInteraction(msg)).map_err(|_| ())
     }
 }
 
 impl UserInteractionTaskSource {
     pub fn queue_event(&self,
-                   target: &EventTarget,
-                   name: Atom,
-                   bubbles: EventBubbles,
-                   cancelable: EventCancelable) {
+                       target: &EventTarget,
+                       name: Atom,
+                       bubbles: EventBubbles,
+                       cancelable: EventCancelable,
+                       window: &Window) {
         let target = Trusted::new(target);
-        let _ = self.0.send(MainThreadScriptMsg::UserInteraction(UserInteractionTask::FireEvent(
-            target, name, bubbles, cancelable)));
+        let runnable = box EventRunnable {
+            target: target,
+            name: name,
+            bubbles: bubbles,
+            cancelable: cancelable,
+        };
+        let _ = self.queue(runnable, window);
     }
 }
 
-pub enum UserInteractionTask {
-    // https://dom.spec.whatwg.org/#concept-event-fire
-    FireEvent(Trusted<EventTarget>, Atom, EventBubbles, EventCancelable),
-}
+pub struct UserInteractionTask(pub Box<Runnable + Send>);
 
 impl UserInteractionTask {
-    pub fn handle_task(self) {
-        use self::UserInteractionTask::*;
-
-        match self {
-            FireEvent(element, name, bubbles, cancelable) => {
-                let target = element.root();
-                target.fire_event(&*name, bubbles, cancelable);
-            }
+    pub fn handle_task(self, script_thread: &ScriptThread) {
+        if !self.0.is_cancelled() {
+            self.0.main_thread_handler(script_thread);
         }
     }
 }

tests/wpt/metadata/html/semantics/embedded-content/the-iframe-element/change_parentage.html.ini

@@ -1,3 +0,0 @@
-[change_parentage.html]
-  type: testharness
-  disabled: https://github.com/servo/servo/issues/11703