Skip to content

Commit

Permalink
Unify the task source and task canceller API
Browse files Browse the repository at this point in the history
I moved away from the `Window` struct all the logic to handle task
sources, into a new struct called `TaskManager`. In a happy world, I'd
be able to just have there two functions, of the types:

```rust
fn task_source<T: TaskSource>(&self, name: TaskSourceName) -> Box<T>
fn task_source_with_canceller<T: TaskSource>(&self, name: TaskSourceName)
  -> (Box<T>, TaskSourceCanceller)
```

And not so much duplicated code. However, because TaskSource can't be a
trait object (because it has generic type parameters), that's not
possible. Instead, I decided to reduce duplicated logic through macros.

For reasons[1], I have to pass both the name of the function with
canceller and the name of the function without, as I'm not able to
concatenate them in the macro itself. I could probably use
`concat_idents` to create both types already defined and reduce the
amount of arguments by one, but that macro is nightly only. At the same
time, not being able to declare macros inside `impl` forces me to pass
`self` as an argument.

All this makes this solution more verbose than it would be ideally. It
does reduce duplication, but it doesn't reduce the size of the file.

[1](rust-lang/rust#29599)
  • Loading branch information
Agustin Chiappe Berrini committed Nov 14, 2018
1 parent 14bc8ab commit 75eb94a
Show file tree
Hide file tree
Showing 26 changed files with 359 additions and 249 deletions.
7 changes: 4 additions & 3 deletions components/script/dom/analysernode.rs
Expand Up @@ -17,7 +17,7 @@ use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::reflector::reflect_dom_object;
use crate::dom::bindings::root::DomRoot;
use crate::dom::window::Window;
use crate::task_source::{TaskSource, TaskSourceName};
use crate::task_source::TaskSource;
use dom_struct::dom_struct;
use ipc_channel::ipc::{self, IpcReceiver};
use ipc_channel::router::ROUTER;
Expand Down Expand Up @@ -97,8 +97,9 @@ impl AnalyserNode {
) -> Fallible<DomRoot<AnalyserNode>> {
let (node, recv) = AnalyserNode::new_inherited(window, context, options)?;
let object = reflect_dom_object(Box::new(node), window, AnalyserNodeBinding::Wrap);
let source = window.dom_manipulation_task_source();
let canceller = window.task_canceller(TaskSourceName::DOMManipulation);
let (source, canceller) = window
.task_manager()
.dom_manipulation_task_source_with_canceller();
let this = Trusted::new(&*object);

ROUTER.add_route(
Expand Down
8 changes: 4 additions & 4 deletions components/script/dom/audiocontext.rs
Expand Up @@ -126,7 +126,7 @@ impl AudioContextMethods for AudioContext {

// Steps 4 and 5.
let window = DomRoot::downcast::<Window>(self.global()).unwrap();
let task_source = window.dom_manipulation_task_source();
let task_source = window.task_manager().dom_manipulation_task_source();
let trusted_promise = TrustedPromise::new(promise.clone());
match self.context.audio_context_impl().suspend() {
Ok(_) => {
Expand All @@ -141,7 +141,7 @@ impl AudioContextMethods for AudioContext {
if base_context.State() != AudioContextState::Suspended {
base_context.set_state_attribute(AudioContextState::Suspended);
let window = DomRoot::downcast::<Window>(context.global()).unwrap();
window.dom_manipulation_task_source().queue_simple_event(
window.task_manager().dom_manipulation_task_source().queue_simple_event(
context.upcast(),
atom!("statechange"),
&window
Expand Down Expand Up @@ -188,7 +188,7 @@ impl AudioContextMethods for AudioContext {

// Steps 4 and 5.
let window = DomRoot::downcast::<Window>(self.global()).unwrap();
let task_source = window.dom_manipulation_task_source();
let task_source = window.task_manager().dom_manipulation_task_source();
let trusted_promise = TrustedPromise::new(promise.clone());
match self.context.audio_context_impl().close() {
Ok(_) => {
Expand All @@ -203,7 +203,7 @@ impl AudioContextMethods for AudioContext {
if base_context.State() != AudioContextState::Closed {
base_context.set_state_attribute(AudioContextState::Closed);
let window = DomRoot::downcast::<Window>(context.global()).unwrap();
window.dom_manipulation_task_source().queue_simple_event(
window.task_manager().dom_manipulation_task_source().queue_simple_event(
context.upcast(),
atom!("statechange"),
&window
Expand Down
9 changes: 5 additions & 4 deletions components/script/dom/audioscheduledsourcenode.rs
Expand Up @@ -10,7 +10,7 @@ use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::num::Finite;
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::reflector::DomObject;
use crate::task_source::{TaskSource, TaskSourceName};
use crate::task_source::TaskSource;
use dom_struct::dom_struct;
use servo_media::audio::node::OnEndedCallback;
use servo_media::audio::node::{AudioNodeInit, AudioNodeMessage, AudioScheduledSourceNodeMessage};
Expand Down Expand Up @@ -71,15 +71,16 @@ impl AudioScheduledSourceNodeMethods for AudioScheduledSourceNode {
let this = Trusted::new(self);
let global = self.global();
let window = global.as_window();
let task_source = window.dom_manipulation_task_source();
let canceller = window.task_canceller(TaskSourceName::DOMManipulation);
let (task_source, canceller) = window
.task_manager()
.dom_manipulation_task_source_with_canceller();
let callback = OnEndedCallback::new(move || {
let _ = task_source.queue_with_canceller(
task!(ended: move || {
let this = this.root();
let global = this.global();
let window = global.as_window();
window.dom_manipulation_task_source().queue_simple_event(
window.task_manager().dom_manipulation_task_source().queue_simple_event(
this.upcast(),
atom!("ended"),
&window
Expand Down
16 changes: 9 additions & 7 deletions components/script/dom/baseaudiocontext.rs
Expand Up @@ -40,7 +40,7 @@ use crate::dom::oscillatornode::OscillatorNode;
use crate::dom::pannernode::PannerNode;
use crate::dom::promise::Promise;
use crate::dom::window::Window;
use crate::task_source::{TaskSource, TaskSourceName};
use crate::task_source::TaskSource;
use dom_struct::dom_struct;
use js::rust::CustomAutoRooterGuard;
use js::typedarray::ArrayBuffer;
Expand Down Expand Up @@ -213,7 +213,7 @@ impl BaseAudioContext {
pub fn resume(&self) {
let global = self.global();
let window = global.as_window();
let task_source = window.dom_manipulation_task_source();
let task_source = window.task_manager().dom_manipulation_task_source();
let this = Trusted::new(self);
// Set the rendering thread state to 'running' and start
// rendering the audio graph.
Expand All @@ -227,7 +227,7 @@ impl BaseAudioContext {
if this.state.get() != AudioContextState::Running {
this.state.set(AudioContextState::Running);
let window = DomRoot::downcast::<Window>(this.global()).unwrap();
window.dom_manipulation_task_source().queue_simple_event(
window.task_manager().dom_manipulation_task_source().queue_simple_event(
this.upcast(),
atom!("statechange"),
&window
Expand Down Expand Up @@ -428,10 +428,12 @@ impl BaseAudioContextMethods for BaseAudioContext {
let decoded_audio__ = decoded_audio.clone();
let this = Trusted::new(self);
let this_ = this.clone();
let task_source = window.dom_manipulation_task_source();
let task_source_ = window.dom_manipulation_task_source();
let canceller = window.task_canceller(TaskSourceName::DOMManipulation);
let canceller_ = window.task_canceller(TaskSourceName::DOMManipulation);
let (task_source, canceller) = window
.task_manager()
.dom_manipulation_task_source_with_canceller();
let (task_source_, canceller_) = window
.task_manager()
.dom_manipulation_task_source_with_canceller();
let callbacks = AudioDecoderCallbacks::new()
.ready(move |channel_count| {
decoded_audio
Expand Down
20 changes: 13 additions & 7 deletions components/script/dom/document.rs
Expand Up @@ -521,6 +521,7 @@ impl Document {
if self.ready_state.get() == DocumentReadyState::Complete {
let document = Trusted::new(self);
self.window
.task_manager()
.dom_manipulation_task_source()
.queue(
task!(fire_pageshow_event: move || {
Expand Down Expand Up @@ -1869,6 +1870,7 @@ impl Document {
debug!("Document loads are complete.");
let document = Trusted::new(self);
self.window
.task_manager()
.dom_manipulation_task_source()
.queue(
task!(fire_load_event: move || {
Expand Down Expand Up @@ -1922,6 +1924,7 @@ impl Document {
let document = Trusted::new(self);
if document.root().browsing_context().is_some() {
self.window
.task_manager()
.dom_manipulation_task_source()
.queue(
task!(fire_pageshow_event: move || {
Expand Down Expand Up @@ -2104,13 +2107,16 @@ impl Document {

// Step 4.1.
let window = self.window();
window.dom_manipulation_task_source().queue_event(
self.upcast(),
atom!("DOMContentLoaded"),
EventBubbles::Bubbles,
EventCancelable::NotCancelable,
window,
);
window
.task_manager()
.dom_manipulation_task_source()
.queue_event(
self.upcast(),
atom!("DOMContentLoaded"),
EventBubbles::Bubbles,
EventCancelable::NotCancelable,
window,
);

window.reflow(ReflowGoal::Full, ReflowReason::DOMContentLoaded);
update_with_current_time_ms(&self.dom_content_loaded_event_end);
Expand Down
14 changes: 7 additions & 7 deletions components/script/dom/globalscope.rs
Expand Up @@ -476,7 +476,7 @@ impl GlobalScope {
/// this global scope.
pub fn networking_task_source(&self) -> NetworkingTaskSource {
if let Some(window) = self.downcast::<Window>() {
return window.networking_task_source();
return window.task_manager().networking_task_source();
}
if let Some(worker) = self.downcast::<WorkerGlobalScope>() {
return worker.networking_task_source();
Expand All @@ -488,7 +488,7 @@ impl GlobalScope {
/// this global scope.
pub fn remote_event_task_source(&self) -> RemoteEventTaskSource {
if let Some(window) = self.downcast::<Window>() {
return window.remote_event_task_source();
return window.task_manager().remote_event_task_source();
}
if let Some(worker) = self.downcast::<WorkerGlobalScope>() {
return worker.remote_event_task_source();
Expand All @@ -500,7 +500,7 @@ impl GlobalScope {
/// this global scope.
pub fn websocket_task_source(&self) -> WebsocketTaskSource {
if let Some(window) = self.downcast::<Window>() {
return window.websocket_task_source();
return window.task_manager().websocket_task_source();
}
if let Some(worker) = self.downcast::<WorkerGlobalScope>() {
return worker.websocket_task_source();
Expand Down Expand Up @@ -635,7 +635,7 @@ impl GlobalScope {
/// properly cancelled when the global scope is destroyed.
pub fn task_canceller(&self, name: TaskSourceName) -> TaskCanceller {
if let Some(window) = self.downcast::<Window>() {
return window.task_canceller(name);
return window.task_manager().task_canceller(name);
}
if let Some(worker) = self.downcast::<WorkerGlobalScope>() {
// Note: the "name" is not passed to the worker,
Expand Down Expand Up @@ -691,7 +691,7 @@ impl GlobalScope {

pub fn dom_manipulation_task_source(&self) -> DOMManipulationTaskSource {
if let Some(window) = self.downcast::<Window>() {
return window.dom_manipulation_task_source();
return window.task_manager().dom_manipulation_task_source();
}
if let Some(worker) = self.downcast::<WorkerGlobalScope>() {
return worker.dom_manipulation_task_source();
Expand All @@ -703,7 +703,7 @@ impl GlobalScope {
/// this of this global scope.
pub fn file_reading_task_source(&self) -> FileReadingTaskSource {
if let Some(window) = self.downcast::<Window>() {
return window.file_reading_task_source();
return window.task_manager().file_reading_task_source();
}
if let Some(worker) = self.downcast::<WorkerGlobalScope>() {
return worker.file_reading_task_source();
Expand Down Expand Up @@ -756,7 +756,7 @@ impl GlobalScope {
/// of this global scope.
pub fn performance_timeline_task_source(&self) -> PerformanceTimelineTaskSource {
if let Some(window) = self.downcast::<Window>() {
return window.performance_timeline_task_source();
return window.task_manager().performance_timeline_task_source();
}
if let Some(worker) = self.downcast::<WorkerGlobalScope>() {
return worker.performance_timeline_task_source();
Expand Down
2 changes: 1 addition & 1 deletion components/script/dom/htmldetailselement.rs
Expand Up @@ -76,7 +76,7 @@ impl VirtualMethods for HTMLDetailsElement {
let window = window_from_node(self);
let this = Trusted::new(self);
// FIXME(nox): Why are errors silenced here?
let _ = window.dom_manipulation_task_source().queue(
let _ = window.task_manager().dom_manipulation_task_source().queue(
task!(details_notification_task_steps: move || {
let this = this.root();
if counter == this.toggle_counter.get() {
Expand Down
3 changes: 2 additions & 1 deletion components/script/dom/htmldialogelement.rs
Expand Up @@ -90,7 +90,8 @@ impl HTMLDialogElementMethods for HTMLDialogElement {
// TODO: Step 4 implement pending dialog stack removal

// Step 5
win.dom_manipulation_task_source()
win.task_manager()
.dom_manipulation_task_source()
.queue_simple_event(target, atom!("close"), &win);
}
}
1 change: 1 addition & 0 deletions components/script/dom/htmlformelement.rs
Expand Up @@ -523,6 +523,7 @@ impl HTMLFormElement {

// Step 3.
target
.task_manager()
.dom_manipulation_task_source()
.queue(task, target.upcast())
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion components/script/dom/htmliframeelement.rs
Expand Up @@ -237,7 +237,7 @@ impl HTMLIFrameElement {
let this = Trusted::new(self);
let pipeline_id = self.pipeline_id().unwrap();
// FIXME(nox): Why are errors silenced here?
let _ = window.dom_manipulation_task_source().queue(
let _ = window.task_manager().dom_manipulation_task_source().queue(
task!(iframe_load_event_steps: move || {
this.root().iframe_load_event_steps(pipeline_id);
}),
Expand Down
32 changes: 19 additions & 13 deletions components/script/dom/htmlimageelement.rs
Expand Up @@ -40,7 +40,7 @@ use crate::dom::window::Window;
use crate::microtask::{Microtask, MicrotaskRunnable};
use crate::network_listener::{NetworkListener, PreInvoke};
use crate::script_thread::ScriptThread;
use crate::task_source::{TaskSource, TaskSourceName};
use crate::task_source::TaskSource;
use cssparser::{Parser, ParserInput};
use dom_struct::dom_struct;
use euclid::Point2D;
Expand Down Expand Up @@ -237,8 +237,9 @@ impl HTMLImageElement {
let (responder_sender, responder_receiver) = ipc::channel().unwrap();

let window = window_from_node(elem);
let task_source = window.networking_task_source();
let task_canceller = window.task_canceller(TaskSourceName::Networking);
let (task_source, canceller) = window
.task_manager()
.networking_task_source_with_canceller();
let generation = elem.generation.get();
ROUTER.add_route(
responder_receiver.to_opaque(),
Expand All @@ -257,7 +258,7 @@ impl HTMLImageElement {
element.process_image_response(image);
}
}),
&task_canceller,
&canceller,
);
}),
);
Expand Down Expand Up @@ -308,10 +309,14 @@ impl HTMLImageElement {
}));

let (action_sender, action_receiver) = ipc::channel().unwrap();
let (task_source, canceller) = document
.window()
.task_manager()
.networking_task_source_with_canceller();
let listener = NetworkListener {
context: context,
task_source: window.networking_task_source(),
canceller: Some(window.task_canceller(TaskSourceName::Networking)),
context,
task_source,
canceller: Some(canceller),
};
ROUTER.add_route(
action_receiver.to_opaque(),
Expand Down Expand Up @@ -780,7 +785,7 @@ impl HTMLImageElement {
fn update_the_image_data_sync_steps(&self) {
let document = document_from_node(self);
let window = document.window();
let task_source = window.dom_manipulation_task_source();
let task_source = window.task_manager().dom_manipulation_task_source();
let this = Trusted::new(self);
let (src, pixel_density) = match self.select_image_source() {
// Step 8
Expand Down Expand Up @@ -938,7 +943,7 @@ impl HTMLImageElement {
current_request.current_pixel_density = pixel_density;
let this = Trusted::new(self);
let src = String::from(src);
let _ = window.dom_manipulation_task_source().queue(
let _ = window.task_manager().dom_manipulation_task_source().queue(
task!(image_load_event: move || {
let this = this.root();
{
Expand Down Expand Up @@ -989,8 +994,9 @@ impl HTMLImageElement {
let (responder_sender, responder_receiver) = ipc::channel().unwrap();

let window = window_from_node(elem);
let task_source = window.networking_task_source();
let task_canceller = window.task_canceller(TaskSourceName::Networking);
let (task_source, canceller) = window
.task_manager()
.networking_task_source_with_canceller();
let generation = elem.generation.get();
ROUTER.add_route(responder_receiver.to_opaque(), Box::new(move |message| {
debug!("Got image {:?}", message);
Expand All @@ -1008,7 +1014,7 @@ impl HTMLImageElement {
DOMString::from_string(selected_source_clone), generation, selected_pixel_density);
}
}),
&task_canceller,
&canceller,
);
}));

Expand Down Expand Up @@ -1133,7 +1139,7 @@ impl HTMLImageElement {
let this = Trusted::new(self);
let window = window_from_node(self);
let src = src.to_string();
let _ = window.dom_manipulation_task_source().queue(
let _ = window.task_manager().dom_manipulation_task_source().queue(
task!(image_load_event: move || {
let this = this.root();
let relevant_mutation = this.generation.get() != generation;
Expand Down
17 changes: 10 additions & 7 deletions components/script/dom/htmlinputelement.rs
Expand Up @@ -1515,13 +1515,16 @@ impl VirtualMethods for HTMLInputElement {
{
if event.IsTrusted() {
let window = window_from_node(self);
let _ = window.user_interaction_task_source().queue_event(
&self.upcast(),
atom!("input"),
EventBubbles::Bubbles,
EventCancelable::NotCancelable,
&window,
);
let _ = window
.task_manager()
.user_interaction_task_source()
.queue_event(
&self.upcast(),
atom!("input"),
EventBubbles::Bubbles,
EventCancelable::NotCancelable,
&window,
);
}
}
}
Expand Down

0 comments on commit 75eb94a

Please sign in to comment.