Remove some duplication in compositing

src/components/compositing/constellation.rs

@@ -31,7 +31,6 @@ use servo_util::opts::Opts;
 use servo_util::time::TimeProfilerChan;
 use servo_util::task::spawn_named;
 use std::cell::RefCell;
-use std::kinds::marker;
 use std::mem::replace;
 use std::io;
 use std::rc::Rc;
@@ -50,7 +49,6 @@ pub struct Constellation<LTF> {
     next_pipeline_id: PipelineId,
     pending_frames: Vec<FrameChange>,
     pending_sizes: HashMap<(PipelineId, SubpageId), TypedRect<PagePx, f32>>,
-    layout_task_factory: marker::CovariantType<LTF>,
     pub time_profiler_chan: TimeProfilerChan,
     pub window_size: WindowSizeData,
     pub opts: Opts,
@@ -266,7 +264,6 @@ impl<LTF: LayoutTaskFactory> Constellation<LTF> {
                 next_pipeline_id: PipelineId(0),
                 pending_frames: vec!(),
                 pending_sizes: HashMap::new(),
-                layout_task_factory: marker::CovariantType,
                 time_profiler_chan: time_profiler_chan,
                 window_size: WindowSizeData {
                     visible_viewport: TypedSize2D(800_f32, 600_f32),
@@ -293,9 +290,10 @@ impl<LTF: LayoutTaskFactory> Constellation<LTF> {
     fn new_pipeline(&self,
                     id: PipelineId,
                     subpage_id: Option<SubpageId>,
+                    script_pipeline: Option<Rc<Pipeline>>,
                     url: Url)
-                    -> Pipeline {
+                    -> Rc<Pipeline> {
-            Pipeline::create::<LTF>(id,
+            let pipe = Pipeline::create::<LTF>(id,
                                                subpage_id,
                                                self.chan.clone(),
                                                self.compositor_chan.clone(),
@@ -305,7 +303,10 @@ impl<LTF: LayoutTaskFactory> Constellation<LTF> {
                                                self.time_profiler_chan.clone(),
                                                self.window_size,
                                                self.opts.clone(),
-                                    url)
+                                               script_pipeline,
+                                               url);
+            pipe.load();
+            Rc::new(pipe)
     }
 
 
@@ -445,39 +446,36 @@ impl<LTF: LayoutTaskFactory> Constellation<LTF> {
         debug!("creating replacement pipeline for about:failure");
 
         let new_id = self.get_next_pipeline_id();
-        let pipeline = self.new_pipeline(new_id, subpage_id, Url::parse("about:failure").unwrap());
+        let pipeline = self.new_pipeline(new_id, subpage_id, None,
-        pipeline.load();
+                                         Url::parse("about:failure").unwrap());
 
-        let pipeline_wrapped = Rc::new(pipeline);
         self.pending_frames.push(FrameChange{
             before: Some(pipeline_id),
             after: Rc::new(FrameTree {
-                pipeline: pipeline_wrapped.clone(),
+                pipeline: pipeline.clone(),
                 parent: RefCell::new(None),
                 children: RefCell::new(vec!()),
             }),
             navigation_type: constellation_msg::Load,
         });
 
-        self.pipelines.insert(new_id, pipeline_wrapped);
+        self.pipelines.insert(new_id, pipeline);
     }
 
     fn handle_init_load(&mut self, url: Url) {
         let next_pipeline_id = self.get_next_pipeline_id();
-        let pipeline = self.new_pipeline(next_pipeline_id, None, url);
+        let pipeline = self.new_pipeline(next_pipeline_id, None, None, url);
-        pipeline.load();
-        let pipeline_wrapped = Rc::new(pipeline);
 
         self.pending_frames.push(FrameChange {
             before: None,
             after: Rc::new(FrameTree {
-                pipeline: pipeline_wrapped.clone(),
+                pipeline: pipeline.clone(),
                 parent: RefCell::new(None),
                 children: RefCell::new(vec!()),
             }),
             navigation_type: constellation_msg::Load,
         });
-        self.pipelines.insert(pipeline_wrapped.id, pipeline_wrapped);
+        self.pipelines.insert(pipeline.id, pipeline);
     }
 
     fn handle_frame_rect_msg(&mut self, pipeline_id: PipelineId, subpage_id: SubpageId,
@@ -578,44 +576,38 @@ impl<LTF: LayoutTaskFactory> Constellation<LTF> {
         let same_script = (source_url.host() == url.host() &&
                            source_url.port() == url.port()) && sandbox == IFrameUnsandboxed;
         // FIXME(tkuehn): Need to follow the standardized spec for checking same-origin
-        let pipeline = if same_script {
+        // Reuse the script task if the URL is same-origin
-            debug!("Constellation: loading same-origin iframe at {}", url.serialize());
+        let new_pipeline = if same_script {
-            // Reuse the script task if same-origin url's
+            debug!("Constellation: loading same-origin iframe at {:?}", url);
-            Pipeline::with_script::<LTF>(next_pipeline_id,
+            Some(source_pipeline.clone())
-                                         subpage_id,
-                                         self.chan.clone(),
-                                         self.compositor_chan.clone(),
-                                         self.image_cache_task.clone(),
-                                         self.font_cache_task.clone(),
-                                         self.time_profiler_chan.clone(),
-                                         self.opts.clone(),
-                                         source_pipeline.clone(),
-                                         url)
         } else {
             debug!("Constellation: loading cross-origin iframe at {:?}", url);
-            // Create a new script task if not same-origin url's
+            None
-            self.new_pipeline(next_pipeline_id, Some(subpage_id), url)
         };
 
-        debug!("Constellation: sending load msg to pipeline {:?}", pipeline.id);
+        let pipeline = self.new_pipeline(
-        pipeline.load();
+            next_pipeline_id,
-        let pipeline_wrapped = Rc::new(pipeline);
+            Some(subpage_id),
+            new_pipeline,
+            url
+        );
+
         let rect = self.pending_sizes.pop(&(source_pipeline_id, subpage_id));
         for frame_tree in frame_trees.iter() {
             frame_tree.children.borrow_mut().push(ChildFrameTree {
                 frame_tree: Rc::new(FrameTree {
-                    pipeline: pipeline_wrapped.clone(),
+                    pipeline: pipeline.clone(),
                     parent: RefCell::new(Some(source_pipeline.clone())),
                     children: RefCell::new(vec!()),
                 }),
                 rect: rect,
             });
         }
-        self.pipelines.insert(pipeline_wrapped.id, pipeline_wrapped);
+        self.pipelines.insert(pipeline.id, pipeline);
     }
 
     fn handle_load_url_msg(&mut self, source_id: PipelineId, url: Url) {
-        debug!("Constellation: received message to load {:s}", url.serialize());
+        debug!("Constellation: received message to load {:s}", url.to_str());
         // Make sure no pending page would be overridden.
         let source_frame = self.current_frame().get_ref().find(source_id).expect(
             "Constellation: received a LoadUrlMsg from a pipeline_id associated
@@ -641,20 +633,18 @@ impl<LTF: LayoutTaskFactory> Constellation<LTF> {
         let subpage_id = source_frame.pipeline.subpage_id;
         let next_pipeline_id = self.get_next_pipeline_id();
 
-        let pipeline = self.new_pipeline(next_pipeline_id, subpage_id, url);
+        let pipeline = self.new_pipeline(next_pipeline_id, subpage_id, None, url);
-        pipeline.load();
-        let pipeline_wrapped = Rc::new(pipeline);
 
         self.pending_frames.push(FrameChange{
             before: Some(source_id),
             after: Rc::new(FrameTree {
-                pipeline: pipeline_wrapped.clone(),
+                pipeline: pipeline.clone(),
                 parent: parent,
                 children: RefCell::new(vec!()),
             }),
             navigation_type: constellation_msg::Load,
         });
-        self.pipelines.insert(pipeline_wrapped.id, pipeline_wrapped);
+        self.pipelines.insert(pipeline.id, pipeline);
     }
 
     fn handle_navigate_msg(&mut self, direction: constellation_msg::NavigationDirection) {

src/components/compositing/pipeline.rs

@@ -43,74 +43,9 @@ pub struct CompositionPipeline {
 }
 
 impl Pipeline {
-    /// Starts a render task, layout task, and script task. Returns the channels wrapped in a
+    /// Starts a render task, layout task, and possibly a script task.
-    /// struct.
+    /// Returns the channels wrapped in a struct.
-    pub fn with_script<LTF:LayoutTaskFactory>(
+    /// If script_pipeline is not None, then subpage_id must also be not None.
-                           id: PipelineId,
-                           subpage_id: SubpageId,
-                           constellation_chan: ConstellationChan,
-                           compositor_chan: CompositorChan,
-                           image_cache_task: ImageCacheTask,
-                           font_cache_task: FontCacheTask,
-                           time_profiler_chan: TimeProfilerChan,
-                           opts: Opts,
-                           script_pipeline: Rc<Pipeline>,
-                           url: Url)
-                           -> Pipeline {
-        let (layout_port, layout_chan) = LayoutChan::new();
-        let (render_port, render_chan) = RenderChan::new();
-        let (render_shutdown_chan, render_shutdown_port) = channel();
-        let (layout_shutdown_chan, layout_shutdown_port) = channel();
-
-        let failure = Failure {
-            pipeline_id: id,
-            subpage_id: Some(subpage_id),
-        };
-
-        RenderTask::create(id,
-                           render_port,
-                           compositor_chan.clone(),
-                           constellation_chan.clone(),
-                           font_cache_task.clone(),
-                           failure.clone(),
-                           opts.clone(),
-                           time_profiler_chan.clone(),
-                           render_shutdown_chan);
-
-        LayoutTaskFactory::create(None::<&mut LTF>,
-                                  id,
-                                  layout_port,
-                                  layout_chan.clone(),
-                                  constellation_chan,
-                                  failure,
-                                  script_pipeline.script_chan.clone(),
-                                  render_chan.clone(),
-                                  image_cache_task.clone(),
-                                  font_cache_task.clone(),
-                                  opts.clone(),
-                                  time_profiler_chan,
-                                  layout_shutdown_chan);
-
-        let new_layout_info = NewLayoutInfo {
-            old_pipeline_id: script_pipeline.id.clone(),
-            new_pipeline_id: id,
-            subpage_id: subpage_id,
-            layout_chan: layout_chan.clone(),
-        };
-
-        let ScriptChan(ref chan) = script_pipeline.script_chan;
-        chan.send(AttachLayoutMsg(new_layout_info));
-
-        Pipeline::new(id,
-                      Some(subpage_id),
-                      script_pipeline.script_chan.clone(),
-                      layout_chan,
-                      render_chan,
-                      layout_shutdown_port,
-                      render_shutdown_port,
-                      url)
-    }
-
     pub fn create<LTF:LayoutTaskFactory>(
                       id: PipelineId,
                       subpage_id: Option<SubpageId>,
@@ -122,27 +57,22 @@ impl Pipeline {
                       time_profiler_chan: TimeProfilerChan,
                       window_size: WindowSizeData,
                       opts: Opts,
+                      script_pipeline: Option<Rc<Pipeline>>,
                       url: Url)
                       -> Pipeline {
-        let (script_port, script_chan) = ScriptChan::new();
         let (layout_port, layout_chan) = LayoutChan::new();
         let (render_port, render_chan) = RenderChan::new();
         let (render_shutdown_chan, render_shutdown_port) = channel();
         let (layout_shutdown_chan, layout_shutdown_port) = channel();
-        let pipeline = Pipeline::new(id,
-                                     subpage_id,
-                                     script_chan.clone(),
-                                     layout_chan.clone(),
-                                     render_chan.clone(),
-                                     layout_shutdown_port,
-                                     render_shutdown_port,
-                                     url);
 
         let failure = Failure {
             pipeline_id: id,
             subpage_id: subpage_id,
         };
 
+        let script_chan = match script_pipeline {
+            None => {
+                let (script_port, script_chan) = ScriptChan::new();
                 ScriptTask::create(id,
                                box compositor_chan.clone(),
                                layout_chan.clone(),
@@ -153,6 +83,21 @@ impl Pipeline {
                                resource_task,
                                image_cache_task.clone(),
                                window_size);
+                script_chan
+            }
+            Some(spipe) => {
+                let new_layout_info = NewLayoutInfo {
+                    old_pipeline_id: spipe.id.clone(),
+                    new_pipeline_id: id,
+                    subpage_id: subpage_id.expect("script_pipeline != None but subpage_id == None"),
+                    layout_chan: layout_chan.clone(),
+                };
+
+                let ScriptChan(ref chan) = spipe.script_chan;
+                chan.send(AttachLayoutMsg(new_layout_info));
+                spipe.script_chan.clone()
+            }
+        };
 
         RenderTask::create(id,
                            render_port,
@@ -178,7 +123,14 @@ impl Pipeline {
                                   time_profiler_chan,
                                   layout_shutdown_chan);
 
-        pipeline
+        Pipeline::new(id,
+                      subpage_id,
+                      script_chan,
+                      layout_chan,
+                      render_chan,
+                      layout_shutdown_port,
+                      render_shutdown_port,
+                      url)
     }
 
     pub fn new(id: PipelineId,