From 9252f2b8a2b310993530b083d1ee321a34804f09 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 8 Sep 2017 10:03:35 +0200 Subject: [PATCH 1/4] Kill DOMManipulationTask Just use a bare Runnable value. --- components/script/script_thread.rs | 6 +---- .../script/task_source/dom_manipulation.rs | 24 ++++++------------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index a576af4407e..3062f57e8b5 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -114,7 +114,7 @@ use std::sync::mpsc::{Receiver, Select, Sender, channel}; use std::thread; use style::context::ReflowGoal; use style::thread_state; -use task_source::dom_manipulation::{DOMManipulationTask, DOMManipulationTaskSource}; +use task_source::dom_manipulation::DOMManipulationTaskSource; use task_source::file_reading::FileReadingTaskSource; use task_source::history_traversal::HistoryTraversalTaskSource; use task_source::networking::NetworkingTaskSource; @@ -276,8 +276,6 @@ pub enum MainThreadScriptMsg { /// dispatched to ScriptThread). Allows for a replace bool to be passed. If true, /// the current entry will be replaced instead of a new entry being added. Navigate(PipelineId, LoadData, bool), - /// Tasks that originate from the DOM manipulation task source - DOMManipulation(DOMManipulationTask), /// Tasks that originate from the user interaction task source UserInteraction(UserInteractionTask), /// Notifies the script thread that a new worklet has been loaded, and thus the page should be @@ -1298,8 +1296,6 @@ impl ScriptThread { self.collect_reports(reports_chan), MainThreadScriptMsg::WorkletLoaded(pipeline_id) => self.handle_worklet_loaded(pipeline_id), - MainThreadScriptMsg::DOMManipulation(task) => - task.handle_task(self), MainThreadScriptMsg::UserInteraction(task) => task.handle_task(self), } diff --git a/components/script/task_source/dom_manipulation.rs b/components/script/task_source/dom_manipulation.rs index 2ba5e297ade..18721539693 100644 --- a/components/script/task_source/dom_manipulation.rs +++ b/components/script/task_source/dom_manipulation.rs @@ -7,7 +7,8 @@ use dom::bindings::refcounted::Trusted; use dom::event::{EventBubbles, EventCancelable, EventRunnable, SimpleEventRunnable}; use dom::eventtarget::EventTarget; use dom::window::Window; -use script_thread::{MainThreadScriptMsg, Runnable, RunnableWrapper, ScriptThread}; +use script_runtime::{CommonScriptMsg, ScriptThreadEventCategory}; +use script_thread::{MainThreadScriptMsg, Runnable, RunnableWrapper}; use servo_atoms::Atom; use std::fmt; use std::result::Result; @@ -29,8 +30,11 @@ impl TaskSource for DOMManipulationTaskSource { wrapper: &RunnableWrapper) -> Result<(), ()> where T: Runnable + Send + 'static { - let msg = DOMManipulationTask(wrapper.wrap_runnable(msg)); - self.0.send(MainThreadScriptMsg::DOMManipulation(msg)).map_err(|_| ()) + let msg = MainThreadScriptMsg::Common(CommonScriptMsg::RunnableMsg( + ScriptThreadEventCategory::ScriptEvent, + wrapper.wrap_runnable(msg), + )); + self.0.send(msg).map_err(|_| ()) } } @@ -60,17 +64,3 @@ impl DOMManipulationTaskSource { let _ = self.queue(runnable, window.upcast()); } } - -pub struct DOMManipulationTask(pub Box); - -impl fmt::Debug for DOMManipulationTask { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "DOMManipulationTask(...)") - } -} - -impl DOMManipulationTask { - pub fn handle_task(self, script_thread: &ScriptThread) { - self.0.main_thread_handler(script_thread); - } -} From 35a725225468631517b993afbcde4f58c88288c6 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 8 Sep 2017 10:03:35 +0200 Subject: [PATCH 2/4] Kill UserInteractionTask Just use a bare Runnable value. --- components/script/script_thread.rs | 6 +---- .../script/task_source/user_interaction.rs | 24 ++++++------------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 3062f57e8b5..8ef650ecee9 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -119,7 +119,7 @@ use task_source::file_reading::FileReadingTaskSource; use task_source::history_traversal::HistoryTraversalTaskSource; use task_source::networking::NetworkingTaskSource; use task_source::performance_timeline::PerformanceTimelineTaskSource; -use task_source::user_interaction::{UserInteractionTask, UserInteractionTaskSource}; +use task_source::user_interaction::UserInteractionTaskSource; use time::{get_time, precise_time_ns, Tm}; use url::Position; use webdriver_handlers; @@ -276,8 +276,6 @@ pub enum MainThreadScriptMsg { /// dispatched to ScriptThread). Allows for a replace bool to be passed. If true, /// the current entry will be replaced instead of a new entry being added. Navigate(PipelineId, LoadData, bool), - /// Tasks that originate from the user interaction task source - UserInteraction(UserInteractionTask), /// Notifies the script thread that a new worklet has been loaded, and thus the page should be /// reflowed. WorkletLoaded(PipelineId), @@ -1296,8 +1294,6 @@ impl ScriptThread { self.collect_reports(reports_chan), MainThreadScriptMsg::WorkletLoaded(pipeline_id) => self.handle_worklet_loaded(pipeline_id), - MainThreadScriptMsg::UserInteraction(task) => - task.handle_task(self), } } diff --git a/components/script/task_source/user_interaction.rs b/components/script/task_source/user_interaction.rs index 1969ee0a41a..753c090df9e 100644 --- a/components/script/task_source/user_interaction.rs +++ b/components/script/task_source/user_interaction.rs @@ -7,7 +7,8 @@ use dom::bindings::refcounted::Trusted; use dom::event::{EventBubbles, EventCancelable, EventRunnable}; use dom::eventtarget::EventTarget; use dom::window::Window; -use script_thread::{MainThreadScriptMsg, Runnable, RunnableWrapper, ScriptThread}; +use script_runtime::{CommonScriptMsg, ScriptThreadEventCategory}; +use script_thread::{MainThreadScriptMsg, Runnable, RunnableWrapper}; use servo_atoms::Atom; use std::fmt; use std::result::Result; @@ -29,8 +30,11 @@ impl TaskSource for UserInteractionTaskSource { wrapper: &RunnableWrapper) -> Result<(), ()> where T: Runnable + Send + 'static { - let msg = UserInteractionTask(wrapper.wrap_runnable(msg)); - self.0.send(MainThreadScriptMsg::UserInteraction(msg)).map_err(|_| ()) + let msg = MainThreadScriptMsg::Common(CommonScriptMsg::RunnableMsg( + ScriptThreadEventCategory::InputEvent, + wrapper.wrap_runnable(msg), + )); + self.0.send(msg).map_err(|_| ()) } } @@ -51,17 +55,3 @@ impl UserInteractionTaskSource { let _ = self.queue(runnable, window.upcast()); } } - -pub struct UserInteractionTask(pub Box); - -impl fmt::Debug for UserInteractionTask { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "UserInteractionTask(...)") - } -} - -impl UserInteractionTask { - pub fn handle_task(self, script_thread: &ScriptThread) { - self.0.main_thread_handler(script_thread); - } -} From 53d30d85b3489b78370c25d404c1095cd6104119 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 8 Sep 2017 10:19:06 +0200 Subject: [PATCH 3/4] Reformat some task-related functions --- components/script/script_thread.rs | 22 ++++++++++--------- .../script/task_source/dom_manipulation.rs | 13 ++++++----- .../script/task_source/user_interaction.rs | 13 ++++++----- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 8ef650ecee9..1a69c10dcfa 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1281,19 +1281,21 @@ impl ScriptThread { fn handle_msg_from_script(&self, msg: MainThreadScriptMsg) { match msg { - MainThreadScriptMsg::Navigate(parent_pipeline_id, load_data, replace) => - self.handle_navigate(parent_pipeline_id, None, load_data, replace), - MainThreadScriptMsg::ExitWindow(id) => - self.handle_exit_window_msg(id), + MainThreadScriptMsg::Navigate(parent_pipeline_id, load_data, replace) => { + self.handle_navigate(parent_pipeline_id, None, load_data, replace) + }, + MainThreadScriptMsg::ExitWindow(id) => { + self.handle_exit_window_msg(id) + }, MainThreadScriptMsg::Common(CommonScriptMsg::RunnableMsg(_, runnable)) => { - // The category of the runnable is ignored by the pattern, however - // it is still respected by profiling (see categorize_msg). runnable.main_thread_handler(self) } - MainThreadScriptMsg::Common(CommonScriptMsg::CollectReports(reports_chan)) => - self.collect_reports(reports_chan), - MainThreadScriptMsg::WorkletLoaded(pipeline_id) => - self.handle_worklet_loaded(pipeline_id), + MainThreadScriptMsg::Common(CommonScriptMsg::CollectReports(chan)) => { + self.collect_reports(chan) + }, + MainThreadScriptMsg::WorkletLoaded(pipeline_id) => { + self.handle_worklet_loaded(pipeline_id) + }, } } diff --git a/components/script/task_source/dom_manipulation.rs b/components/script/task_source/dom_manipulation.rs index 18721539693..636b99f87bf 100644 --- a/components/script/task_source/dom_manipulation.rs +++ b/components/script/task_source/dom_manipulation.rs @@ -25,11 +25,14 @@ impl fmt::Debug for DOMManipulationTaskSource { } impl TaskSource for DOMManipulationTaskSource { - fn queue_with_wrapper(&self, - msg: Box, - wrapper: &RunnableWrapper) - -> Result<(), ()> - where T: Runnable + Send + 'static { + fn queue_with_wrapper( + &self, + msg: Box, + wrapper: &RunnableWrapper, + ) -> Result<(), ()> + where + T: Runnable + Send + 'static, + { let msg = MainThreadScriptMsg::Common(CommonScriptMsg::RunnableMsg( ScriptThreadEventCategory::ScriptEvent, wrapper.wrap_runnable(msg), diff --git a/components/script/task_source/user_interaction.rs b/components/script/task_source/user_interaction.rs index 753c090df9e..9456d940416 100644 --- a/components/script/task_source/user_interaction.rs +++ b/components/script/task_source/user_interaction.rs @@ -25,11 +25,14 @@ impl fmt::Debug for UserInteractionTaskSource { } impl TaskSource for UserInteractionTaskSource { - fn queue_with_wrapper(&self, - msg: Box, - wrapper: &RunnableWrapper) - -> Result<(), ()> - where T: Runnable + Send + 'static { + fn queue_with_wrapper( + &self, + msg: Box, + wrapper: &RunnableWrapper, + ) -> Result<(), ()> + where + T: Runnable + Send + 'static, + { let msg = MainThreadScriptMsg::Common(CommonScriptMsg::RunnableMsg( ScriptThreadEventCategory::InputEvent, wrapper.wrap_runnable(msg), From 0c33d6168e1eda13ee260ee71fa03365ab2cad52 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 8 Sep 2017 10:21:28 +0200 Subject: [PATCH 4/4] Panic from the default impl of Runnable::handler This will allow us to make sure that tasks using main_thread_handler don't actually get consumed through a call to the bare handler method. --- components/script/script_thread.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 1a69c10dcfa..207e47572e7 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -251,7 +251,9 @@ where pub trait Runnable { fn name(&self) -> &'static str { unsafe { intrinsics::type_name::() } } - fn handler(self: Box) {} + fn handler(self: Box) { + panic!("This should probably be redefined.") + } fn main_thread_handler(self: Box, _script_thread: &ScriptThread) { self.handler(); } }