From 997f2df096a0726380ccbc3909ed0f2e885a8810 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 22 Jul 2014 13:43:54 -0400 Subject: [PATCH] servo: Merge #2887 - Clean up duplication in Pipeline (from arielb1:remove-compositing-layout) Source-Repo: https://github.com/servo/servo Source-Revision: 9f06b3ce174baff59b2d4d17056e0777c432b9b4 --- .../components/compositing/constellation.rs | 92 ++++++------- servo/src/components/compositing/pipeline.rs | 128 ++++++------------ 2 files changed, 81 insertions(+), 139 deletions(-) diff --git a/servo/src/components/compositing/constellation.rs b/servo/src/components/compositing/constellation.rs index 3ba7a67e63b6..3b13336cdfaa 100644 --- a/servo/src/components/compositing/constellation.rs +++ b/servo/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 { next_pipeline_id: PipelineId, pending_frames: Vec, pending_sizes: HashMap<(PipelineId, SubpageId), TypedRect>, - layout_task_factory: marker::CovariantType, pub time_profiler_chan: TimeProfilerChan, pub window_size: WindowSizeData, pub opts: Opts, @@ -266,7 +264,6 @@ impl Constellation { 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,19 +290,23 @@ impl Constellation { fn new_pipeline(&self, id: PipelineId, subpage_id: Option, + script_pipeline: Option>, url: Url) - -> Pipeline { - Pipeline::create::(id, - subpage_id, - self.chan.clone(), - self.compositor_chan.clone(), - self.image_cache_task.clone(), - self.font_cache_task.clone(), - self.resource_task.clone(), - self.time_profiler_chan.clone(), - self.window_size, - self.opts.clone(), - url) + -> Rc { + let pipe = Pipeline::create::(id, + subpage_id, + self.chan.clone(), + self.compositor_chan.clone(), + self.image_cache_task.clone(), + self.font_cache_task.clone(), + self.resource_task.clone(), + self.time_profiler_chan.clone(), + self.window_size, + self.opts.clone(), + script_pipeline, + url); + pipe.load(); + Rc::new(pipe) } @@ -445,39 +446,36 @@ impl Constellation { 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()); - pipeline.load(); + let pipeline = self.new_pipeline(new_id, subpage_id, None, + 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); - pipeline.load(); - let pipeline_wrapped = Rc::new(pipeline); + let pipeline = self.new_pipeline(next_pipeline_id, None, None, url); 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 Constellation { 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 { - debug!("Constellation: loading same-origin iframe at {}", url.serialize()); - // Reuse the script task if same-origin url's - Pipeline::with_script::(next_pipeline_id, - 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) + // Reuse the script task if the URL is same-origin + let new_pipeline = if same_script { + debug!("Constellation: loading same-origin iframe at {:?}", url); + Some(source_pipeline.clone()) } else { debug!("Constellation: loading cross-origin iframe at {:?}", url); - // Create a new script task if not same-origin url's - self.new_pipeline(next_pipeline_id, Some(subpage_id), url) + None }; - debug!("Constellation: sending load msg to pipeline {:?}", pipeline.id); - pipeline.load(); - let pipeline_wrapped = Rc::new(pipeline); + let pipeline = self.new_pipeline( + next_pipeline_id, + 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 Constellation { 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); - pipeline.load(); - let pipeline_wrapped = Rc::new(pipeline); + let pipeline = self.new_pipeline(next_pipeline_id, subpage_id, None, url); 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) { diff --git a/servo/src/components/compositing/pipeline.rs b/servo/src/components/compositing/pipeline.rs index 4351dc79b690..2c2a4e5e3947 100644 --- a/servo/src/components/compositing/pipeline.rs +++ b/servo/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 - /// struct. - pub fn with_script( - 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, - 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) - } - + /// Starts a render task, layout task, and possibly a script task. + /// Returns the channels wrapped in a struct. + /// If script_pipeline is not None, then subpage_id must also be not None. pub fn create( id: PipelineId, subpage_id: Option, @@ -122,37 +57,47 @@ impl Pipeline { time_profiler_chan: TimeProfilerChan, window_size: WindowSizeData, opts: Opts, + script_pipeline: Option>, 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, }; - ScriptTask::create(id, - box compositor_chan.clone(), - layout_chan.clone(), - script_port, - script_chan.clone(), - constellation_chan.clone(), - failure.clone(), - resource_task, - image_cache_task.clone(), - window_size); + let script_chan = match script_pipeline { + None => { + let (script_port, script_chan) = ScriptChan::new(); + ScriptTask::create(id, + box compositor_chan.clone(), + layout_chan.clone(), + script_port, + script_chan.clone(), + constellation_chan.clone(), + failure.clone(), + 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,