auto merge of #1082 : kmcallister/servo/misc, r=jdm

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.load(url);
-            pipeline.script_chan.send(ExecuteMsg(pipeline.id, url));
-        } else {
-            pipeline.load(url);
 
-            self.pending_frames.push(FrameChange{
+        self.pending_frames.push(FrameChange{
-                before: None,
+            before: None,
-                after: @mut FrameTree {
+            after: @mut FrameTree {
-                    pipeline: pipeline, 
+                pipeline: pipeline,
-                    parent: None,
+                parent: None,
-                    children: ~[],
+                children: ~[],
-                },
+            },
-                navigation_type: constellation_msg::Load,
+            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") {
+        debug!("Constellation: sending load msg to pipeline %?", pipeline.id);
-            pipeline.execute(url);
+        pipeline.load(url);
-        } else {
-            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.load(url);
-            pipeline.script_chan.send(ExecuteMsg(pipeline.id, url));
-        } else {
-            pipeline.load(url);
 
-            self.pending_frames.push(FrameChange{
+        self.pending_frames.push(FrameChange{
-                before: Some(source_id),
+            before: Some(source_id),
-                after: @mut FrameTree {
+            after: @mut FrameTree {
-                    pipeline: pipeline, 
+                pipeline: pipeline,
-                    parent: parent,
+                parent: parent,
-                    children: ~[],
+                children: ~[],
-                },
+            },
-                navigation_type: constellation_msg::Load,
+            navigation_type: constellation_msg::Load,
-            });
+        });
-        }
         self.pipelines.insert(pipeline.id, pipeline);
     }
     

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) {
-
+        spawn_with!(task::task(), [port, constellation_chan, script_chan,
-        let port = Cell::new(port);
+                                   render_chan, img_cache_task, profiler_chan], {
-        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 {
             let mut layout = LayoutTask::new(id,
-                                             port.take(),
+                                             port,
-                                             constellation_chan.take(),
+                                             constellation_chan,
-                                             script_chan.take(),
+                                             script_chan,
-                                             render_chan.take(),
+                                             render_chan,
-                                             img_cache_task.take(),
+                                             img_cache_task,
                                              &opts,
-                                             profiler_chan.take());
+                                             profiler_chan);
             layout.start();
-        };
+        });
     }
 
     fn new(id: PipelineId,

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
+    )
+)

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};
@@ -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::<task::TaskResult>();
-        
-        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) {
+        // Wrap task creation within a supervised task so that failure will
-            let script_port = script_port.take();
+        // only tear down those tasks instead of ours.
-            let resource_task = resource_task.take();
+        let failure_chan = constellation_chan.clone();
-            let size = size.take();
+        let (task_port, task_chan) = stream::<task::TaskResult>();
-            let render_port = render_port.take();
+        let mut supervised_task = task::task();
-            let layout_port = layout_port.take();
+        supervised_task.opts.notify_chan = Some(task_chan);
-            let constellation_chan = constellation_chan.take();
+        supervised_task.supervised();
-            let image_cache_task = image_cache_task.take();
-            let profiler_chan = profiler_chan.take();
 
+        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 {
+        spawn_with!(task::task(), [failure_chan], {
-            match port.recv() {
+            match task_port.recv() {
                 task::Success => (),
                 task::Failure => {
-                    let constellation_chan = constellation_chan_handler.take();
+                    failure_chan.send(FailureMsg(id, subpage_id));
-                    constellation_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<task::TaskResult>, 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<SubpageId>,
                script_chan: ScriptChan,
@@ -194,11 +171,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);
     }

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
     }
@@ -59,7 +64,8 @@ impl WindowMethods<Application> 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.

src/components/net/resource_task.rs

@@ -96,6 +96,23 @@ pub fn start_sending(start_chan: Chan<LoadResponse>,
     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<ControlMsg>;
 

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<HtmlDiscoveryMessage>,
     loop {
         match from_parent.recv() {
             JSTaskNewFile(url) => {
-                let (result_port, result_chan) = comm::stream();
+                match load_whole_resource(&resource_task, url.clone()) {
-                let resource_task = resource_task.clone();
+                    Err(_) => {
-                let url_clone = url.clone();
+                        error!("error loading script %s", url.to_str());
-                do task::spawn {
+                    }
-                    let (input_port, input_chan) = comm::stream();
+                    Ok((metadata, bytes)) => {
-                    // TODO: change copy to move once we can move into closures
+                        result_vec.push(JSFile {
-                    resource_task.send(Load(url.clone(), input_chan));
+                            data: str::from_utf8(bytes),
-
+                            url: metadata.final_url,
-                    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;
-                            }
-                        }
                     }
-                }
-
-                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) => {

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) {

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();
 </script>
 </head>