auto merge of #961 : tikue/servo/master, r=kmcallister

* Profiler is now close to a no-op when ```-p``` is not passed in
* The profiler's printing ```Timer``` now stops looping when the profiler is closed
* Most task ```Chans``` are now newtype ```structs```
* Some more ```Cell``` removals in places where ```spawn_with``` is appropriate

src/components/gfx/render_task.rs

@@ -9,7 +9,7 @@ use azure::azure_hl::{B8G8R8A8, DrawTarget};
 use display_list::DisplayList;
 use servo_msg::compositor_msg::{RenderListener, IdleRenderState, RenderingRenderState, LayerBuffer};
 use servo_msg::compositor_msg::{LayerBufferSet, Epoch};
-use servo_msg::constellation_msg::PipelineId;
+use servo_msg::constellation_msg::{ConstellationChan, PipelineId, RendererReadyMsg};
 use font_context::FontContext;
 use geom::matrix2d::Matrix2D;
 use geom::size::Size2D;
@@ -17,8 +17,8 @@ use geom::rect::Rect;
 use opts::Opts;
 use render_context::RenderContext;
 
-use std::cell::Cell;
 use std::comm::{Chan, Port, SharedChan};
+use std::task::spawn_with;
 use extra::arc::Arc;
 
 use servo_util::time::{ProfilerChan, profile};
@@ -58,19 +58,27 @@ pub fn BufferRequest(screen_rect: Rect<uint>, page_rect: Rect<f32>) -> BufferReq
     }
 }
 
+// FIXME(rust#9155): this should be a newtype struct, but
+// generic newtypes ICE when compiled cross-crate
 #[deriving(Clone)]
 pub struct RenderChan<T> {
     chan: SharedChan<Msg<T>>,
 }
-
+impl<T: Send> RenderChan<T> {
-impl<T> RenderChan<T> {
     pub fn new(chan: Chan<Msg<T>>) -> RenderChan<T> {
         RenderChan {
             chan: SharedChan::new(chan),
         }
     }
-    pub fn send(&self, msg: Msg<T>) {
+}
-        self.chan.send(msg);
+impl<T: Send> GenericChan<Msg<T>> for RenderChan<T> {
+    fn send(&self, msg: Msg<T>) {
+        assert!(self.try_send(msg), "RenderChan.send: render port closed")
+    }
+}
+impl<T: Send> GenericSmartChan<Msg<T>> for RenderChan<T> {
+    fn try_send(&self, msg: Msg<T>) -> bool {
+        self.chan.try_send(msg)
     }
 }
 
@@ -78,6 +86,7 @@ struct RenderTask<C,T> {
     id: PipelineId,
     port: Port<Msg<T>>,
     compositor: C,
+    constellation_chan: ConstellationChan,
     font_ctx: @mut FontContext,
     opts: Opts,
 
@@ -102,24 +111,21 @@ impl<C: RenderListener + Send,T:Send+Freeze> RenderTask<C,T> {
     pub fn create(id: PipelineId,
                   port: Port<Msg<T>>,
                   compositor: C,
+                  constellation_chan: ConstellationChan,
                   opts: Opts,
                   profiler_chan: ProfilerChan) {
-        let compositor = Cell::new(compositor);
-        let opts = Cell::new(opts);
-        let port = Cell::new(port);
-        let profiler_chan = Cell::new(profiler_chan);
 
-        do spawn {
+        do spawn_with((port, compositor, constellation_chan, opts, profiler_chan))
-            let compositor = compositor.take();
+            |(port, compositor, constellation_chan, opts, profiler_chan)| {
+
             let share_gl_context = compositor.get_gl_context();
-            let opts = opts.take();
-            let profiler_chan = profiler_chan.take();
 
             // FIXME: rust/#5967
             let mut render_task = RenderTask {
                 id: id,
-                port: port.take(),
+                port: port,
                 compositor: compositor,
+                constellation_chan: constellation_chan,
                 font_ctx: @mut FontContext::new(opts.render_backend.clone(),
                                                 false,
                                                 profiler_chan.clone()),
@@ -147,6 +153,8 @@ impl<C: RenderListener + Send,T:Send+Freeze> RenderTask<C,T> {
                     if self.paint_permission {
                         self.epoch.next();
                         self.compositor.set_layer_page_size(self.id, render_layer.size, self.epoch);
+                    } else {
+                        self.constellation_chan.send(RendererReadyMsg(self.id));
                     }
                     self.render_layer = Some(render_layer);
                     self.last_paint_msg = None;
@@ -276,6 +284,8 @@ impl<C: RenderListener + Send,T:Send+Freeze> RenderTask<C,T> {
             debug!("render_task: returning surface");
             if self.paint_permission {
                 self.compositor.paint(self.id, layer_buffer_set.clone(), self.epoch);
+            } else {
+                self.constellation_chan.send(RendererReadyMsg(self.id));
             }
             debug!("caching paint msg");
             self.last_paint_msg = Some(layer_buffer_set);

src/components/main/constellation.rs

@@ -201,20 +201,18 @@ impl NavigationContext {
     pub fn back(&mut self) -> @mut FrameTree {
         self.next.push(self.current.take_unwrap());
         self.current = Some(self.previous.pop());
-        debug!("previous: %? next: %? current: %?", self.previous, self.next, *self.current.get_ref());
         self.current.unwrap()
     }
 
     pub fn forward(&mut self) -> @mut FrameTree {
         self.previous.push(self.current.take_unwrap());
         self.current = Some(self.next.pop());
-        debug!("previous: %? next: %? current: %?", self.previous, self.next, *self.current.get_ref());
         self.current.unwrap()
     }
 
     /// Loads a new set of page frames, returning all evicted frame trees
     pub fn load(&mut self, frame_tree: @mut FrameTree) -> ~[@mut FrameTree] {
-        debug!("navigating to %?", frame_tree);
+        debug!("navigating to %?", frame_tree.pipeline.id);
         let evicted = replace(&mut self.next, ~[]);
         if self.current.is_some() {
             self.previous.push(self.current.take_unwrap());
@@ -428,45 +426,52 @@ impl Constellation {
         debug!("Received frame rect %? from %?, %?", rect, pipeline_id, subpage_id);
         let mut already_sent = HashSet::new();
 
+        // Returns true if a child frame tree's subpage id matches the given subpage id
+        let subpage_eq = |child_frame_tree: & &mut ChildFrameTree| {
+            child_frame_tree.frame_tree.pipeline.subpage_id.expect("Constellation:
+                child frame does not have a subpage id. This should not be possible.")
+                == subpage_id
+        };
+
+        // Update a child's frame rect and inform its script task of the change,
+        // if it hasn't been already. Optionally inform the compositor if 
+        // resize happens immediately.
+        let update_child_rect = |child_frame_tree: &mut ChildFrameTree, is_active: bool| {
+            child_frame_tree.rect = Some(rect.clone());
+            let pipeline = &child_frame_tree.frame_tree.pipeline;
+            if !already_sent.contains(&pipeline.id) {
+                let Size2D { width, height } = rect.size;
+                if is_active {
+                    pipeline.script_chan.send(ResizeMsg(pipeline.id, Size2D {
+                        width:  width  as uint,
+                        height: height as uint
+                    }));
+                    self.compositor_chan.send(SetLayerClipRect(pipeline.id, rect));
+                } else {
+                    pipeline.script_chan.send(ResizeInactiveMsg(pipeline.id,
+                                                                Size2D(width as uint, height as uint)));
+                }
+                already_sent.insert(pipeline.id);
+            }
+        };
+
         // If the subframe is in the current frame tree, the compositor needs the new size
         for current_frame in self.current_frame().iter() {
             debug!("Constellation: Sending size for frame in current frame tree.");
             let source_frame = current_frame.find_mut(pipeline_id);
             for source_frame in source_frame.iter() {
-                for child_frame_tree in source_frame.children.mut_iter() {
+                let found_child = source_frame.children.mut_iter()
-                    let pipeline = &child_frame_tree.frame_tree.pipeline;
+                    .find(|child| subpage_eq(child));
-                    if pipeline.subpage_id.expect("Constellation: child frame does not have a
+                found_child.map_move(|child| update_child_rect(child, true));
-                        subpage id. This should not be possible.") == subpage_id {
-                        child_frame_tree.rect = Some(rect.clone());
-                        let Rect { size: Size2D { width, height }, _ } = rect;
-                        pipeline.script_chan.send(ResizeMsg(pipeline.id.clone(), Size2D {
-                            width:  width  as uint,
-                            height: height as uint
-                        }));
-                        self.compositor_chan.send(SetLayerClipRect(pipeline.id, rect));
-                        already_sent.insert(pipeline.id.clone());
-                        break;
-                    }
-                } 
             }
         }
-        // Traverse the navigation context and pending frames and tell each associated pipeline to resize.
+
+        // Update all frames with matching pipeline- and subpage-ids
         let frames = self.find_all(pipeline_id);
         for frame_tree in frames.iter() {
-            for child_frame_tree in frame_tree.children.mut_iter() {
+            let found_child = frame_tree.children.mut_iter()
-                let pipeline = &child_frame_tree.frame_tree.pipeline;
+                .find(|child| subpage_eq(child));
-                if pipeline.subpage_id.expect("Constellation: child frame does not have a
+            found_child.map_move(|child| update_child_rect(child, false));
-                    subpage id. This should not be possible.") == subpage_id {
-                    child_frame_tree.rect = Some(rect.clone());
-                    if !already_sent.contains(&pipeline.id) {
-                        let Size2D { width, height } = rect.size;
-                        pipeline.script_chan.send(ResizeInactiveMsg(pipeline.id.clone(),
-                                                                    Size2D(width as uint, height as uint)));
-                        already_sent.insert(pipeline.id.clone());
-                    }
-                    break;
-                }
-            }
         }
 
         // At this point, if no pipelines were sent a resize msg, then this subpage id
@@ -547,7 +552,7 @@ impl Constellation {
         if url.path.ends_with(".js") {
             pipeline.execute(url);
         } else {
-            debug!("Constellation: sending load msg to %?", pipeline);
+            debug!("Constellation: sending load msg to pipeline %?", pipeline.id);
             pipeline.load(url);
         }
         let rect = self.pending_sizes.pop(&(source_pipeline_id, subpage_id));
@@ -588,7 +593,7 @@ impl Constellation {
         // changes would be overriden by changing the subframe associated with source_id.
 
         let parent = source_frame.parent.clone();
-        let subpage_id = source_frame.pipeline.subpage_id.clone();
+        let subpage_id = source_frame.pipeline.subpage_id;
         let next_pipeline_id = self.get_next_pipeline_id();
 
         let pipeline = @mut Pipeline::create(next_pipeline_id,
@@ -711,12 +716,11 @@ impl Constellation {
 
                     // If to_add is not the root frame, then replace revoked_frame with it.
                     // This conveniently keeps scissor rect size intact.
-                    debug!("Constellation: replacing %? with %? in %?", revoke_id, to_add, next_frame_tree);
+                    debug!("Constellation: replacing %? with %? in %?",
+                        revoke_id, to_add.pipeline.id, next_frame_tree.pipeline.id);
                     if to_add.parent.is_some() {
-                        let replaced = next_frame_tree.replace_child(revoke_id, to_add);
+                        next_frame_tree.replace_child(revoke_id, to_add);
-                        debug!("Replaced child: %?", replaced);
                     }
-                    debug!("Constellation: frame tree after replacing: %?", next_frame_tree);
                 }
 
                 None => {
@@ -743,15 +747,15 @@ impl Constellation {
     fn handle_resized_window_msg(&mut self, new_size: Size2D<uint>) {
         let mut already_seen = HashSet::new();
         for &@FrameTree { pipeline: pipeline, _ } in self.current_frame().iter() {
-            pipeline.script_chan.send(ResizeMsg(pipeline.id.clone(), new_size));
+            pipeline.script_chan.send(ResizeMsg(pipeline.id, new_size));
-            already_seen.insert(pipeline.id.clone());
+            already_seen.insert(pipeline.id);
         }
         for frame_tree in self.navigation_context.previous.iter()
             .chain(self.navigation_context.next.iter()) {
             let pipeline = &frame_tree.pipeline;
             if !already_seen.contains(&pipeline.id) {
-                pipeline.script_chan.send(ResizeInactiveMsg(pipeline.id.clone(), new_size));
+                pipeline.script_chan.send(ResizeInactiveMsg(pipeline.id, new_size));
-                already_seen.insert(pipeline.id.clone());
+                already_seen.insert(pipeline.id);
             }
         }
     }

src/components/main/pipeline.rs

@@ -54,6 +54,7 @@ impl Pipeline {
         RenderTask::create(id,
                            render_port,
                            compositor_chan.clone(),
+                           constellation_chan.clone(),
                            opts.clone(),
                            profiler_chan.clone());
 
@@ -136,6 +137,7 @@ impl Pipeline {
             RenderTask::create(id,
                                render_port,
                                compositor_chan.clone(),
+                               constellation_chan.clone(),
                                opts.clone(),
                                profiler_chan.clone());
 

src/components/main/servo.rc

@@ -47,7 +47,7 @@ use gfx::opts;
 
 use servo_net::image_cache_task::ImageCacheTask;
 use servo_net::resource_task::ResourceTask;
-use servo_util::time::{Profiler, ProfilerChan, PrintMsg};
+use servo_util::time::{Profiler, ProfilerChan};
 
 pub use gfx::opts::Opts;
 pub use gfx::text;
@@ -56,7 +56,7 @@ use std::comm;
 #[cfg(not(test))]
 use std::os;
 use std::rt::rtio::RtioTimer;
-use std::rt::io::timer::Timer;
+use std::task::spawn_with;
 
 #[path="compositing/mod.rs"]
 pub mod compositing;
@@ -127,38 +127,19 @@ fn start(argc: int, argv: **u8, crate_map: *u8) -> int {
 
 fn run(opts: Opts) {
     let (shutdown_port, shutdown_chan) = comm::stream();
-    let (profiler_port, profiler_chan) = comm::stream();
+    let (profiler_port, profiler_chan) = special_stream!(ProfilerChan);
-    let (compositor_port, compositor_chan) = comm::stream();
+    let (compositor_port, compositor_chan) = special_stream!(CompositorChan);
 
-    let profiler_chan = ProfilerChan::new(profiler_chan);
+    Profiler::create(profiler_port, profiler_chan.clone(), opts.profiler_period);
-    Profiler::create(profiler_port);
-    do opts.profiler_period.map |&period| {
-        let profiler_chan = profiler_chan.clone();
-        let period = (period * 1000f) as u64;
-        do spawn {
-            let mut tm = Timer::new().unwrap();
-            loop {
-                tm.sleep(period);
-                profiler_chan.send(PrintMsg);
-            }
-        }
-    };
-    let compositor_chan = CompositorChan::new(compositor_chan);
-    let profiler_chan_clone = profiler_chan.clone();
 
-    let opts_clone = opts.clone();
+    do spawn_with((profiler_chan.clone(), compositor_chan, opts.clone()))
-
+        |(profiler_chan, compositor_chan, opts)| {
-    do spawn {
-        let profiler_chan = profiler_chan_clone.clone();
-        let compositor_chan = compositor_chan.clone();
-
-        let opts = &opts_clone.clone();
 
+        let opts = &opts;
         // Create a Servo instance.
-
         let resource_task = ResourceTask();
         let image_cache_task = ImageCacheTask(resource_task.clone());
-        let constellation_chan = Constellation::start(compositor_chan.clone(),
+        let constellation_chan = Constellation::start(compositor_chan,
                                                       opts,
                                                       resource_task,
                                                       image_cache_task,

src/components/msg/constellation_msg.rs

@@ -12,18 +12,10 @@ use geom::size::Size2D;
 use geom::rect::Rect;
 
 #[deriving(Clone)]
-pub struct ConstellationChan {
+pub struct ConstellationChan(SharedChan<Msg>);
-    chan: SharedChan<Msg>,
-}
-
 impl ConstellationChan {
     pub fn new(chan: Chan<Msg>) -> ConstellationChan {
-        ConstellationChan {
+        ConstellationChan(SharedChan::new(chan))
-            chan: SharedChan::new(chan),
-        }
-    }
-    pub fn send(&self, msg: Msg) {
-        self.chan.send(msg);
     }
 }
 

src/components/script/dom/window.rs

@@ -30,6 +30,7 @@ use std::int;
 use std::libc;
 use std::rt::rtio::RtioTimer;
 use std::rt::io::timer::Timer;
+use std::task::spawn_with;
 use js::jsapi::JSVal;
 
 pub enum TimerControlMsg {
@@ -198,21 +199,20 @@ impl Window {
                compositor: @ScriptListener,
                image_cache_task: ImageCacheTask)
                -> @mut Window {
-        let script_chan_clone = script_chan.clone();
         let win = @mut Window {
             page: page,
-            script_chan: script_chan,
+            script_chan: script_chan.clone(),
             compositor: compositor,
             wrapper: WrapperCache::new(),
             timer_chan: {
                 let (timer_port, timer_chan) = comm::stream::<TimerControlMsg>();
                 let id = page.id.clone();
-                do spawn {
+                do spawn_with(script_chan) |script_chan| {
                     loop {
                         match timer_port.recv() {
                             TimerMessage_Close => break,
-                            TimerMessage_Fire(td) => script_chan_clone.chan.send(FireTimerMsg(id, td)),
+                            TimerMessage_Fire(td) => script_chan.send(FireTimerMsg(id, td)),
-                            TimerMessage_TriggerExit => script_chan_clone.chan.send(ExitMsg),
+                            TimerMessage_TriggerExit => script_chan.send(ExitMsg),
                         }
                     }
                 }

src/components/script/layout_interface.rs

@@ -111,17 +111,9 @@ pub struct Reflow {
 
 /// Encapsulates a channel to the layout task.
 #[deriving(Clone)]
-pub struct LayoutChan {
+pub struct LayoutChan(SharedChan<Msg>);
-    chan: SharedChan<Msg>,
-}
-
 impl LayoutChan {
     pub fn new(chan: Chan<Msg>) -> LayoutChan {
-        LayoutChan {
+        LayoutChan(SharedChan::new(chan))
-            chan: SharedChan::new(chan),
-        }
-    }
-    pub fn send(&self, msg: Msg) {
-        self.chan.send(msg);
     }
 }

src/components/script/script_task.rs

@@ -21,7 +21,7 @@ use layout_interface::{ReflowDocumentDamage, ReflowForDisplay, ReflowGoal};
 use layout_interface::ReflowMsg;
 use layout_interface;
 use servo_msg::constellation_msg::{ConstellationChan, LoadUrlMsg, NavigationDirection};
-use servo_msg::constellation_msg::{PipelineId, SubpageId, RendererReadyMsg};
+use servo_msg::constellation_msg::{PipelineId, SubpageId};
 use servo_msg::constellation_msg::{LoadIframeUrlMsg, IFrameSandboxed, IFrameUnsandboxed};
 use servo_msg::constellation_msg;
 
@@ -83,20 +83,11 @@ pub struct NewLayoutInfo {
 
 /// Encapsulates external communication with the script task.
 #[deriving(Clone)]
-pub struct ScriptChan {
+pub struct ScriptChan(SharedChan<ScriptMsg>);
-    /// The channel used to send messages to the script task.
-    chan: SharedChan<ScriptMsg>,
-}
-
 impl ScriptChan {
     /// Creates a new script chan.
     pub fn new(chan: Chan<ScriptMsg>) -> ScriptChan {
-        ScriptChan {
+        ScriptChan(SharedChan::new(chan))
-            chan: SharedChan::new(chan)
-        }
-    }
-    pub fn send(&self, msg: ScriptMsg) {
-        self.chan.send(msg);
     }
 }
 
@@ -591,7 +582,6 @@ impl ScriptTask {
         if page_tree.page.last_reflow_id == reflow_id {
             page_tree.page.layout_join_port = None;
         }
-        self.constellation_chan.send(RendererReadyMsg(pipeline_id));
         self.compositor.set_ready_state(FinishedLoading);
     }
 

src/components/util/time.rs

@@ -3,27 +3,21 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // Timing functions.
-use extra::time::precise_time_ns;
-use std::cell::Cell;
 use std::comm::{Port, SharedChan};
-use extra::sort::tim_sort;
 use std::iterator::AdditiveIterator;
+use std::rt::io::timer::Timer;
+use std::task::spawn_with;
+
+use extra::sort::tim_sort;
+use extra::time::precise_time_ns;
 use extra::treemap::TreeMap;
 
 // front-end representation of the profiler used to communicate with the profiler
 #[deriving(Clone)]
-pub struct ProfilerChan {
+pub struct ProfilerChan(SharedChan<ProfilerMsg>);
-    chan: SharedChan<ProfilerMsg>,
-}
-
 impl ProfilerChan {
     pub fn new(chan: Chan<ProfilerMsg>) -> ProfilerChan {
-        ProfilerChan {
+        ProfilerChan(SharedChan::new(chan))
-            chan: SharedChan::new(chan),
-        }
-    }
-    pub fn send(&self, msg: ProfilerMsg) {
-        self.chan.send(msg);
     }
 }
 
@@ -101,11 +95,31 @@ pub struct Profiler {
 }
 
 impl Profiler {
-    pub fn create(port: Port<ProfilerMsg>) {
+    pub fn create(port: Port<ProfilerMsg>, chan: ProfilerChan, period: Option<float>) {
-        let port = Cell::new(port);
+        match period {
-        do spawn {
+            Some(period) => {
-            let mut profiler = Profiler::new(port.take());
+                let period = (period * 1000f) as u64;
-            profiler.start();
+                do spawn {
+                    let mut timer = Timer::new().unwrap();
+                    loop {
+                        timer.sleep(period);
+                        if !chan.try_send(PrintMsg) {
+                            break;
+                        }
+                    }
+                }
+                // Spawn the profiler
+                do spawn_with(port) |port| {
+                    let mut profiler = Profiler::new(port);
+                    profiler.start();
+                }
+            }
+            None => {
+                // no-op to handle profiler messages when the profiler is inactive
+                do spawn_with(port) |port| {
+                    while port.try_recv().is_some() {}
+                }
+            }
         }
     }