From 2babc8dde192a045e9ab8e46a299d0c16876b507 Mon Sep 17 00:00:00 2001 From: Keegan McAllister Date: Thu, 5 Sep 2013 12:56:18 -0700 Subject: [PATCH 1/6] Provide a useful error message when we fail to create the GLFW window --- src/components/main/platform/common/glfw_windowing.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/main/platform/common/glfw_windowing.rs b/src/components/main/platform/common/glfw_windowing.rs index de02a9b861c..0a7f255665c 100644 --- a/src/components/main/platform/common/glfw_windowing.rs +++ b/src/components/main/platform/common/glfw_windowing.rs @@ -59,7 +59,8 @@ impl WindowMethods for Window { /// Creates a new window. fn new(_: &Application) -> @mut Window { // Create the GLFW window. - let glfw_window = glfw::Window::create(800, 600, "Servo", glfw::Windowed).unwrap(); + let glfw_window = glfw::Window::create(800, 600, "Servo", glfw::Windowed) + .expect("Failed to create GLFW window"); glfw_window.make_context_current(); // Create our window object. From 1e4d3e26613e28f32085ba5932acfc3e91efa4ea Mon Sep 17 00:00:00 2001 From: Keegan McAllister Date: Thu, 17 Oct 2013 18:06:00 -0700 Subject: [PATCH 2/6] Print GLFW errors --- src/components/main/platform/common/glfw_windowing.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/components/main/platform/common/glfw_windowing.rs b/src/components/main/platform/common/glfw_windowing.rs index 0a7f255665c..60eaa421c6c 100644 --- a/src/components/main/platform/common/glfw_windowing.rs +++ b/src/components/main/platform/common/glfw_windowing.rs @@ -27,6 +27,11 @@ pub struct Application; impl ApplicationMethods for Application { fn new() -> Application { + // Per GLFW docs it's safe to set the error callback before calling + // glfwInit(), and this way we notice errors from init too. + do glfw::set_error_callback |_error_code, description| { + error!("GLFW error: %s", description); + }; glfw::init(); Application } From 377a76ab1b1eacec707c360f41ea2408ce4a5c4b Mon Sep 17 00:00:00 2001 From: Keegan McAllister Date: Thu, 17 Oct 2013 18:58:41 -0700 Subject: [PATCH 3/6] Enable DOMParser test case Now that #1071 is fixed. --- src/test/html/content/test_DOMParser.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/html/content/test_DOMParser.html b/src/test/html/content/test_DOMParser.html index 50c3b4f4f5f..c773116e3f6 100644 --- a/src/test/html/content/test_DOMParser.html +++ b/src/test/html/content/test_DOMParser.html @@ -5,7 +5,7 @@ is_function(DOMParser, "DOMParser"); let parser = new DOMParser(); is_a(parser, DOMParser); -//is_a(parser.parseFromString("", "text/html"), Document); +is_a(parser.parseFromString("", "text/html"), Document); finish(); From 1cd5d9179d88a55f0f49da3cdf42e687115b0a0d Mon Sep 17 00:00:00 2001 From: Keegan McAllister Date: Wed, 16 Oct 2013 16:32:08 -0700 Subject: [PATCH 4/6] Remove special-casing of URLs ending in ".js" This was a very old (May 2012) testing feature which used std::io::read_whole_file rather than our normal resource-loader mechanism. We can implement javascript: URLs later. --- src/components/main/constellation.rs | 58 +++++++++++----------------- src/components/main/pipeline.rs | 7 +--- src/components/script/script_task.rs | 27 ------------- 3 files changed, 24 insertions(+), 68 deletions(-) diff --git a/src/components/main/constellation.rs b/src/components/main/constellation.rs index a7e06dd4a17..3e3b2e969ab 100644 --- a/src/components/main/constellation.rs +++ b/src/components/main/constellation.rs @@ -16,7 +16,7 @@ use servo_msg::constellation_msg::{IFrameSandboxState, InitLoadUrlMsg, LoadIfram use servo_msg::constellation_msg::{Msg, NavigateMsg, NavigationType, IFrameUnsandboxed}; use servo_msg::constellation_msg::{PipelineId, RendererReadyMsg, ResizedWindowMsg, SubpageId}; use servo_msg::constellation_msg; -use script::script_task::{ResizeMsg, ResizeInactiveMsg, ExecuteMsg}; +use script::script_task::{ResizeMsg, ResizeInactiveMsg}; use servo_net::image_cache_task::{ImageCacheTask, ImageCacheTaskClient}; use servo_net::resource_task::ResourceTask; use servo_net::resource_task; @@ -404,21 +404,17 @@ impl Constellation { let size = self.compositor_chan.get_size(); from_value(Size2D(size.width as uint, size.height as uint)) }); - if url.path.ends_with(".js") { - pipeline.script_chan.send(ExecuteMsg(pipeline.id, url)); - } else { - pipeline.load(url); + pipeline.load(url); - self.pending_frames.push(FrameChange{ - before: None, - after: @mut FrameTree { - pipeline: pipeline, - parent: None, - children: ~[], - }, - navigation_type: constellation_msg::Load, - }); - } + self.pending_frames.push(FrameChange{ + before: None, + after: @mut FrameTree { + pipeline: pipeline, + parent: None, + children: ~[], + }, + navigation_type: constellation_msg::Load, + }); self.pipelines.insert(pipeline.id, pipeline); } @@ -549,12 +545,8 @@ impl Constellation { size_future) }; - if url.path.ends_with(".js") { - pipeline.execute(url); - } else { - debug!("Constellation: sending load msg to pipeline %?", pipeline.id); - pipeline.load(url); - } + debug!("Constellation: sending load msg to pipeline %?", pipeline.id); + pipeline.load(url); let rect = self.pending_sizes.pop(&(source_pipeline_id, subpage_id)); for frame_tree in frame_trees.iter() { frame_tree.children.push(ChildFrameTree { @@ -606,21 +598,17 @@ impl Constellation { self.opts.clone(), size_future); - if url.path.ends_with(".js") { - pipeline.script_chan.send(ExecuteMsg(pipeline.id, url)); - } else { - pipeline.load(url); + pipeline.load(url); - self.pending_frames.push(FrameChange{ - before: Some(source_id), - after: @mut FrameTree { - pipeline: pipeline, - parent: parent, - children: ~[], - }, - navigation_type: constellation_msg::Load, - }); - } + self.pending_frames.push(FrameChange{ + before: Some(source_id), + after: @mut FrameTree { + pipeline: pipeline, + parent: parent, + children: ~[], + }, + navigation_type: constellation_msg::Load, + }); self.pipelines.insert(pipeline.id, pipeline); } diff --git a/src/components/main/pipeline.rs b/src/components/main/pipeline.rs index 1f7dbb8eef9..12b301a4c68 100644 --- a/src/components/main/pipeline.rs +++ b/src/components/main/pipeline.rs @@ -9,7 +9,7 @@ use gfx::render_task::{PaintPermissionGranted, PaintPermissionRevoked}; use gfx::opts::Opts; use layout::layout_task::LayoutTask; use script::layout_interface::LayoutChan; -use script::script_task::{ExecuteMsg, LoadMsg}; +use script::script_task::LoadMsg; use servo_msg::constellation_msg::{ConstellationChan, FailureMsg, PipelineId, SubpageId}; use script::dom::node::AbstractNode; use script::script_task::{AttachLayoutMsg, NewLayoutInfo, ScriptTask, ScriptChan}; @@ -194,11 +194,6 @@ impl Pipeline { self.script_chan.send(LoadMsg(self.id, url)); } - pub fn execute(&mut self, url: Url) { - self.url = Some(url.clone()); - self.script_chan.send(ExecuteMsg(self.id, url)); - } - pub fn grant_paint_permission(&self) { self.render_chan.try_send(PaintPermissionGranted); } diff --git a/src/components/script/script_task.rs b/src/components/script/script_task.rs index 9a729ba2970..e025fdfc5d5 100644 --- a/src/components/script/script_task.rs +++ b/src/components/script/script_task.rs @@ -28,9 +28,7 @@ use servo_msg::constellation_msg; use std::cell::Cell; use std::comm; use std::comm::{Port, SharedChan}; -use std::io::read_whole_file; use std::ptr; -use std::str; use std::task::{spawn_sched, SingleThreaded}; use std::util::replace; use dom::window::TimerData; @@ -58,8 +56,6 @@ pub enum ScriptMsg { LoadMsg(PipelineId, Url), /// Gives a channel and ID to a layout task, as well as the ID of that layout's parent AttachLayoutMsg(NewLayoutInfo), - /// Executes a standalone script. - ExecuteMsg(PipelineId, Url), /// Instructs the script task to send a navigate message to the constellation. NavigateMsg(NavigationDirection), /// Sends a DOM event. @@ -533,7 +529,6 @@ impl ScriptTask { // TODO(tkuehn) need to handle auxiliary layouts for iframes AttachLayoutMsg(new_layout_info) => self.handle_new_layout(new_layout_info), LoadMsg(id, url) => self.load(id, url), - ExecuteMsg(id, url) => self.handle_execute_msg(id, url), SendEventMsg(id, event) => self.handle_event(id, event), FireTimerMsg(id, timer_data) => self.handle_fire_timer_msg(id, timer_data), NavigateMsg(direction) => self.handle_navigate_msg(direction), @@ -564,28 +559,6 @@ impl ScriptTask { parent_page_tree.inner.push(new_page_tree); } - /// Handles a request to execute a script. - fn handle_execute_msg(&mut self, id: PipelineId, url: Url) { - debug!("script: Received url `%s` to execute", url.to_str()); - - let page_tree = self.page_tree.find(id).expect("ScriptTask: received fire timer msg for a - pipeline ID not associated with this script task. This is a bug."); - let compartment = page_tree.page.js_info.get_ref().js_compartment; - let cx = page_tree.page.js_info.get_ref().js_context; - - match read_whole_file(&Path(url.path)) { - Err(msg) => println(fmt!("Error opening %s: %s", url.to_str(), msg)), - - Ok(bytes) => { - compartment.define_functions(debug_fns); - cx.evaluate_script(compartment.global_obj, - str::from_utf8(bytes), - url.path.clone(), - 1); - } - } - } - /// Handles a timer that fired. #[fixed_stack_segment] fn handle_fire_timer_msg(&mut self, id: PipelineId, timer_data: ~TimerData) { From 5b1fede3943191b3ec12d726eae8a947b0310728 Mon Sep 17 00:00:00 2001 From: Keegan McAllister Date: Wed, 16 Oct 2013 17:09:54 -0700 Subject: [PATCH 5/6] Factor out a convenience function load_whole_resource Also remove an unnecessary spawn in js_script_listener, and remember the final script URL after redirects. --- src/components/net/resource_task.rs | 17 ++++++++ .../script/html/hubbub_html_parser.rs | 42 +++++-------------- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/src/components/net/resource_task.rs b/src/components/net/resource_task.rs index ce1e02e0820..02ae1c715d9 100644 --- a/src/components/net/resource_task.rs +++ b/src/components/net/resource_task.rs @@ -96,6 +96,23 @@ pub fn start_sending(start_chan: Chan, progress_chan } +/// Convenience function for synchronously loading a whole resource. +pub fn load_whole_resource(resource_task: &ResourceTask, url: Url) + -> Result<(Metadata, ~[u8]), ()> { + let (start_port, start_chan) = comm::stream(); + resource_task.send(Load(url, start_chan)); + let response = start_port.recv(); + + let mut buf = ~[]; + loop { + match response.progress_port.recv() { + Payload(data) => buf.push_all(data), + Done(Ok(())) => return Ok((response.metadata, buf)), + Done(Err(e)) => return Err(e) + } + } +} + /// Handle to a resource task pub type ResourceTask = SharedChan; diff --git a/src/components/script/html/hubbub_html_parser.rs b/src/components/script/html/hubbub_html_parser.rs index 4a503f1c5da..a1b600c2c35 100644 --- a/src/components/script/html/hubbub_html_parser.rs +++ b/src/components/script/html/hubbub_html_parser.rs @@ -21,12 +21,11 @@ use std::comm; use std::comm::{Port, SharedChan}; use std::str; use std::str::eq_slice; -use std::task; use std::from_str::FromStr; use hubbub::hubbub; use servo_msg::constellation_msg::{ConstellationChan, SubpageId}; use servo_net::image_cache_task::ImageCacheTask; -use servo_net::resource_task::{Load, Payload, Done, ResourceTask}; +use servo_net::resource_task::{Load, Payload, Done, ResourceTask, load_whole_resource}; use servo_util::tree::{TreeNodeRef, ElementLike}; use servo_util::url::make_url; use extra::url::Url; @@ -193,37 +192,16 @@ fn js_script_listener(to_parent: SharedChan, loop { match from_parent.recv() { JSTaskNewFile(url) => { - let (result_port, result_chan) = comm::stream(); - let resource_task = resource_task.clone(); - let url_clone = url.clone(); - do task::spawn { - let (input_port, input_chan) = comm::stream(); - // TODO: change copy to move once we can move into closures - resource_task.send(Load(url.clone(), input_chan)); - - let progress_port = input_port.recv().progress_port; - let mut buf = ~[]; - loop { - match progress_port.recv() { - Payload(data) => { - buf.push_all(data); - } - Done(Ok(*)) => { - result_chan.send(Some(buf)); - break; - } - Done(Err(*)) => { - error!("error loading script %s", url.to_str()); - result_chan.send(None); - break; - } - } + match load_whole_resource(&resource_task, url.clone()) { + Err(_) => { + error!("error loading script %s", url.to_str()); + } + Ok((metadata, bytes)) => { + result_vec.push(JSFile { + data: str::from_utf8(bytes), + url: metadata.final_url, + }); } - } - - let bytes = result_port.recv(); - if bytes.is_some() { - result_vec.push(JSFile { data: str::from_utf8(bytes.unwrap()), url: url_clone }); } } JSTaskNewInlineScript(data, url) => { From 8bd9be7240faf751b7d23e3d9a65a3db8adf3496 Mon Sep 17 00:00:00 2001 From: Keegan McAllister Date: Thu, 17 Oct 2013 12:01:35 -0700 Subject: [PATCH 6/6] Add a spawn_with! macro and clean up some spawns --- src/components/main/layout/layout_task.rs | 26 +++++------ src/components/main/macros.rs | 12 +++++ src/components/main/pipeline.rs | 53 +++++++---------------- 3 files changed, 37 insertions(+), 54 deletions(-) diff --git a/src/components/main/layout/layout_task.rs b/src/components/main/layout/layout_task.rs index b91e22369f0..1feb7338c62 100644 --- a/src/components/main/layout/layout_task.rs +++ b/src/components/main/layout/layout_task.rs @@ -17,6 +17,7 @@ use layout::incremental::{RestyleDamage, BubbleWidths}; use std::cast::transmute; use std::cell::Cell; use std::comm::{Port}; +use std::task; use extra::arc::Arc; use geom::point::Point2D; use geom::rect::Rect; @@ -75,25 +76,18 @@ impl LayoutTask { img_cache_task: ImageCacheTask, opts: Opts, profiler_chan: ProfilerChan) { - - let port = Cell::new(port); - let constellation_chan = Cell::new(constellation_chan); - let script_chan = Cell::new(script_chan); - let render_chan = Cell::new(render_chan); - let img_cache_task = Cell::new(img_cache_task); - let profiler_chan = Cell::new(profiler_chan); - - do spawn { + spawn_with!(task::task(), [port, constellation_chan, script_chan, + render_chan, img_cache_task, profiler_chan], { let mut layout = LayoutTask::new(id, - port.take(), - constellation_chan.take(), - script_chan.take(), - render_chan.take(), - img_cache_task.take(), + port, + constellation_chan, + script_chan, + render_chan, + img_cache_task, &opts, - profiler_chan.take()); + profiler_chan); layout.start(); - }; + }); } fn new(id: PipelineId, diff --git a/src/components/main/macros.rs b/src/components/main/macros.rs index 9f364c48f74..144d57d40c4 100644 --- a/src/components/main/macros.rs +++ b/src/components/main/macros.rs @@ -10,3 +10,15 @@ macro_rules! special_stream( } ); ) + +// Spawn a task, capturing the listed variables in a way that avoids the +// move-from-closure error. This is sugar around the function spawn_with, +// taking care of building a tuple and a lambda. +// +// FIXME: Once cross-crate macros work, there are a few places outside of +// the main crate which could benefit from this macro. +macro_rules! spawn_with( + ($task:expr, [ $($var:ident),+ ], $body:block) => ( + do ($task).spawn_with(( $($var),+ , () )) |( $($var),+ , () )| $body + ) +) diff --git a/src/components/main/pipeline.rs b/src/components/main/pipeline.rs index 12b301a4c68..da9848c0181 100644 --- a/src/components/main/pipeline.rs +++ b/src/components/main/pipeline.rs @@ -19,7 +19,6 @@ use servo_net::resource_task::ResourceTask; use servo_util::time::ProfilerChan; use geom::size::Size2D; use extra::future::Future; -use std::cell::Cell; use std::task; /// A uniquely-identifiable pipeline of script task, layout task, and render task. @@ -100,28 +99,18 @@ impl Pipeline { script_chan.clone(), layout_chan.clone(), render_chan.clone()); - let (port, chan) = stream::(); - - let script_port = Cell::new(script_port); - let resource_task = Cell::new(resource_task); - let size = Cell::new(size); - let render_port = Cell::new(render_port); - let layout_port = Cell::new(layout_port); - let constellation_chan_handler = Cell::new(constellation_chan.clone()); - let constellation_chan = Cell::new(constellation_chan); - let image_cache_task = Cell::new(image_cache_task); - let profiler_chan = Cell::new(profiler_chan); - do Pipeline::spawn(chan) { - let script_port = script_port.take(); - let resource_task = resource_task.take(); - let size = size.take(); - let render_port = render_port.take(); - let layout_port = layout_port.take(); - let constellation_chan = constellation_chan.take(); - let image_cache_task = image_cache_task.take(); - let profiler_chan = profiler_chan.take(); + // Wrap task creation within a supervised task so that failure will + // only tear down those tasks instead of ours. + let failure_chan = constellation_chan.clone(); + let (task_port, task_chan) = stream::(); + let mut supervised_task = task::task(); + supervised_task.opts.notify_chan = Some(task_chan); + supervised_task.supervised(); + spawn_with!(supervised_task, [script_port, resource_task, size, render_port, + layout_port, constellation_chan, image_cache_task, + profiler_chan], { ScriptTask::create(id, compositor_chan.clone(), layout_chan.clone(), @@ -147,32 +136,20 @@ impl Pipeline { image_cache_task, opts.clone(), profiler_chan); - }; + }); - do spawn { - match port.recv() { + spawn_with!(task::task(), [failure_chan], { + match task_port.recv() { task::Success => (), task::Failure => { - let constellation_chan = constellation_chan_handler.take(); - constellation_chan.send(FailureMsg(id, subpage_id)); + failure_chan.send(FailureMsg(id, subpage_id)); } } - }; + }); pipeline } - /// This function wraps the task creation within a supervised task - /// so that failure will only tear down those tasks instead of ours. - pub fn spawn(chan: Chan, f: ~fn()) { - let mut task = task::task(); - task.opts.notify_chan = Some(chan); - task.supervised(); - do task.spawn { - f(); - }; - } - pub fn new(id: PipelineId, subpage_id: Option, script_chan: ScriptChan,