From cc83cb8b09820b751cadcd005eea74c70dfa97a4 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 26 May 2016 10:11:51 +0200 Subject: [PATCH 01/11] Privatize Pipeline::new(). --- components/constellation/pipeline.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index a5f9d952a1b..762a34c25e0 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -245,17 +245,17 @@ impl Pipeline { (pipeline, unprivileged_pipeline_content, privileged_pipeline_content) } - pub fn new(id: PipelineId, - parent_info: Option<(PipelineId, SubpageId, FrameType)>, - script_chan: IpcSender, - layout_chan: LayoutControlChan, - compositor_proxy: Box, - chrome_to_paint_chan: Sender, - layout_shutdown_port: IpcReceiver<()>, - paint_shutdown_port: IpcReceiver<()>, - url: Url, - size: Option>) - -> Pipeline { + fn new(id: PipelineId, + parent_info: Option<(PipelineId, SubpageId, FrameType)>, + script_chan: IpcSender, + layout_chan: LayoutControlChan, + compositor_proxy: Box, + chrome_to_paint_chan: Sender, + layout_shutdown_port: IpcReceiver<()>, + paint_shutdown_port: IpcReceiver<()>, + url: Url, + size: Option>) + -> Pipeline { Pipeline { id: id, parent_info: parent_info, From 82832f134be329071667c3f825cec34ee281e4df Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 26 May 2016 09:41:55 +0200 Subject: [PATCH 02/11] Move spawn_multiprocess into UnprivilegedPipelineContent. --- components/constellation/constellation.rs | 76 ++--------------------- components/constellation/pipeline.rs | 64 +++++++++++++++++++ 2 files changed, 69 insertions(+), 71 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index d183b95b7ff..aafe234896d 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -19,14 +19,8 @@ use compositing::compositor_thread::Msg as ToCompositorMsg; use devtools_traits::{ChromeToDevtoolsControlMsg, DevtoolsControlMsg}; use euclid::scale_factor::ScaleFactor; use euclid::size::{Size2D, TypedSize2D}; -#[cfg(not(target_os = "windows"))] -use gaol; -#[cfg(not(target_os = "windows"))] -use gaol::sandbox::{self, Sandbox, SandboxMethods}; use gfx::font_cache_thread::FontCacheThread; use gfx_traits::Epoch; -#[cfg(not(target_os = "windows"))] -use ipc_channel::ipc::IpcOneShotServer; use ipc_channel::ipc::{self, IpcSender}; use ipc_channel::router::ROUTER; use layout_traits::{LayoutControlChan, LayoutThreadFactory}; @@ -43,12 +37,10 @@ use net_traits::image_cache_thread::ImageCacheThread; use net_traits::storage_thread::StorageThreadMsg; use net_traits::{self, ResourceThreads, IpcSend}; use offscreen_gl_context::{GLContextAttributes, GLLimits}; -use pipeline::{InitialPipelineState, Pipeline, UnprivilegedPipelineContent}; +use pipeline::{ChildProcess, InitialPipelineState, Pipeline}; use profile_traits::mem; use profile_traits::time; use rand::{random, Rng, SeedableRng, StdRng}; -#[cfg(not(target_os = "windows"))] -use sandboxing::content_process_sandbox_profile; use script_traits::{AnimationState, AnimationTickType, CompositorEvent}; use script_traits::{ConstellationControlMsg, ConstellationMsg as FromCompositorMsg}; use script_traits::{DocumentState, LayoutControlMsg}; @@ -57,8 +49,6 @@ use script_traits::{LayoutMsg as FromLayoutMsg, ScriptMsg as FromScriptMsg, Scri use script_traits::{MozBrowserEvent, MozBrowserErrorType}; use std::borrow::ToOwned; use std::collections::HashMap; -#[cfg(not(target_os = "windows"))] -use std::env; use std::io::Error as IOError; use std::marker::PhantomData; use std::mem::replace; @@ -312,13 +302,6 @@ enum ExitPipelineMode { Force, } -enum ChildProcess { - #[cfg(not(target_os = "windows"))] - Sandboxed(gaol::platform::process::Process), - #[cfg(not(target_os = "windows"))] - Unsandboxed(process::Child), -} - impl Constellation where LTF: LayoutThreadFactory, STF: ScriptThreadFactory @@ -455,7 +438,10 @@ impl Constellation // // Yes, that's all there is to it! if opts::multiprocess() { - self.spawn_multiprocess(pipeline_id, unprivileged_pipeline_content); + match unprivileged_pipeline_content.spawn_multiprocess() { + Ok(child_process) => self.child_processes.push(child_process), + Err(e) => self.handle_send_error(pipeline_id, e), + } } else { unprivileged_pipeline_content.start_all::(false); } @@ -465,58 +451,6 @@ impl Constellation self.pipelines.insert(pipeline_id, pipeline); } - #[cfg(not(target_os = "windows"))] - fn spawn_multiprocess(&mut self, - pipeline_id: PipelineId, - unprivileged_pipeline_content: UnprivilegedPipelineContent) - { - // Note that this function can panic, due to process creation, - // avoiding this panic would require a mechanism for dealing - // with low-resource scenarios. - let (server, token) = - IpcOneShotServer::>::new() - .expect("Failed to create IPC one-shot server."); - - // If there is a sandbox, use the `gaol` API to create the child process. - let child_process = if opts::get().sandbox { - let mut command = sandbox::Command::me().expect("Failed to get current sandbox."); - command.arg("--content-process").arg(token); - - if let Ok(value) = env::var("RUST_BACKTRACE") { - command.env("RUST_BACKTRACE", value); - } - - let profile = content_process_sandbox_profile(); - ChildProcess::Sandboxed(Sandbox::new(profile).start(&mut command) - .expect("Failed to start sandboxed child process!")) - } else { - let path_to_self = env::current_exe() - .expect("Failed to get current executor."); - let mut child_process = process::Command::new(path_to_self); - child_process.arg("--content-process"); - child_process.arg(token); - - if let Ok(value) = env::var("RUST_BACKTRACE") { - child_process.env("RUST_BACKTRACE", value); - } - - ChildProcess::Unsandboxed(child_process.spawn() - .expect("Failed to start unsandboxed child process!")) - }; - - self.child_processes.push(child_process); - let (_receiver, sender) = server.accept().expect("Server failed to accept."); - if let Err(e) = sender.send(unprivileged_pipeline_content) { - self.handle_send_error(pipeline_id, e); - } - } - - #[cfg(target_os = "windows")] - fn spawn_multiprocess(&mut self, _: PipelineId, _: UnprivilegedPipelineContent) { - error!("Multiprocess is not supported on Windows."); - process::exit(1); - } - // Push a new (loading) pipeline to the list of pending frame changes fn push_pending_frame(&mut self, new_pipeline_id: PipelineId, old_pipeline_id: Option) { diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index 762a34c25e0..0e6acf571d4 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -8,6 +8,8 @@ use compositing::compositor_thread::Msg as CompositorMsg; use devtools_traits::{DevtoolsControlMsg, ScriptToDevtoolsControlMsg}; use euclid::scale_factor::ScaleFactor; use euclid::size::TypedSize2D; +#[cfg(not(target_os = "windows"))] +use gaol; use gfx::font_cache_thread::FontCacheThread; use gfx::paint_thread::{ChromeToPaintMsg, LayoutToPaintMsg, PaintThread}; use ipc_channel::ipc::{self, IpcReceiver, IpcSender}; @@ -25,7 +27,9 @@ use script_traits::{ConstellationControlMsg, InitialScriptState, MozBrowserEvent use script_traits::{LayoutControlMsg, LayoutMsg, NewLayoutInfo, ScriptMsg}; use script_traits::{ScriptThreadFactory, TimerEventRequest}; use std::collections::HashMap; +use std::io::Error as IOError; use std::mem; +use std::process; use std::sync::mpsc::{Receiver, Sender, channel}; use url::Url; use util; @@ -35,6 +39,13 @@ use util::opts::{self, Opts}; use util::prefs::{self, Pref}; use webrender_traits; +pub enum ChildProcess { + #[cfg(not(target_os = "windows"))] + Sandboxed(gaol::platform::process::Process), + #[cfg(not(target_os = "windows"))] + Unsandboxed(process::Child), +} + /// A uniquely-identifiable pipeline of script thread, layout thread, and paint thread. pub struct Pipeline { pub id: PipelineId, @@ -447,6 +458,59 @@ impl UnprivilegedPipelineContent { } } + #[cfg(not(target_os = "windows"))] + pub fn spawn_multiprocess(self) -> Result { + use gaol::sandbox::{self, Sandbox, SandboxMethods}; + use ipc_channel::ipc::IpcOneShotServer; + use sandboxing::content_process_sandbox_profile; + use std::env; + + // Note that this function can panic, due to process creation, + // avoiding this panic would require a mechanism for dealing + // with low-resource scenarios. + let (server, token) = + IpcOneShotServer::>::new() + .expect("Failed to create IPC one-shot server."); + + // If there is a sandbox, use the `gaol` API to create the child process. + let child_process = if opts::get().sandbox { + let mut command = sandbox::Command::me().expect("Failed to get current sandbox."); + command.arg("--content-process").arg(token); + + if let Ok(value) = env::var("RUST_BACKTRACE") { + command.env("RUST_BACKTRACE", value); + } + + let profile = content_process_sandbox_profile(); + ChildProcess::Sandboxed(Sandbox::new(profile).start(&mut command) + .expect("Failed to start sandboxed child process!")) + } else { + let path_to_self = env::current_exe() + .expect("Failed to get current executor."); + let mut child_process = process::Command::new(path_to_self); + child_process.arg("--content-process"); + child_process.arg(token); + + if let Ok(value) = env::var("RUST_BACKTRACE") { + child_process.env("RUST_BACKTRACE", value); + } + + ChildProcess::Unsandboxed(child_process.spawn() + .expect("Failed to start unsandboxed child process!")) + }; + + let (_receiver, sender) = server.accept().expect("Server failed to accept."); + try!(sender.send(self)); + + Ok(child_process) + } + + #[cfg(target_os = "windows")] + pub fn spawn_multiprocess(self) -> Result { + error!("Multiprocess is not supported on Windows."); + process::exit(1); + } + pub fn opts(&self) -> Opts { self.opts.clone() } From 739e091e8bb037c7dec0b21ae99a1e431be3223d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 26 May 2016 10:10:22 +0200 Subject: [PATCH 03/11] Introduce Pipeline::spawn(). --- components/constellation/constellation.rs | 65 ++++++++++------------- components/constellation/pipeline.rs | 36 +++++++++++-- 2 files changed, 59 insertions(+), 42 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index aafe234896d..8266fe0fe40 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -406,45 +406,36 @@ impl Constellation initial_window_size: Option>, script_channel: Option>, load_data: LoadData) { - let spawning_content = script_channel.is_none(); - let (pipeline, unprivileged_pipeline_content, privileged_pipeline_content) = - Pipeline::create::(InitialPipelineState { - id: pipeline_id, - parent_info: parent_info, - constellation_chan: self.script_sender.clone(), - layout_to_constellation_chan: self.layout_sender.clone(), - panic_chan: self.panic_sender.clone(), - scheduler_chan: self.scheduler_chan.clone(), - compositor_proxy: self.compositor_proxy.clone_compositor_proxy(), - devtools_chan: self.devtools_chan.clone(), - bluetooth_thread: self.bluetooth_thread.clone(), - image_cache_thread: self.image_cache_thread.clone(), - font_cache_thread: self.font_cache_thread.clone(), - resource_threads: self.resource_threads.clone(), - time_profiler_chan: self.time_profiler_chan.clone(), - mem_profiler_chan: self.mem_profiler_chan.clone(), - window_size: initial_window_size, - script_chan: script_channel, - load_data: load_data, - device_pixel_ratio: self.window_size.device_pixel_ratio, - pipeline_namespace_id: self.next_pipeline_namespace_id(), - webrender_api_sender: self.webrender_api_sender.clone(), - }); + let result = Pipeline::spawn::(InitialPipelineState { + id: pipeline_id, + parent_info: parent_info, + constellation_chan: self.script_sender.clone(), + layout_to_constellation_chan: self.layout_sender.clone(), + panic_chan: self.panic_sender.clone(), + scheduler_chan: self.scheduler_chan.clone(), + compositor_proxy: self.compositor_proxy.clone_compositor_proxy(), + devtools_chan: self.devtools_chan.clone(), + bluetooth_thread: self.bluetooth_thread.clone(), + image_cache_thread: self.image_cache_thread.clone(), + font_cache_thread: self.font_cache_thread.clone(), + resource_threads: self.resource_threads.clone(), + time_profiler_chan: self.time_profiler_chan.clone(), + mem_profiler_chan: self.mem_profiler_chan.clone(), + window_size: initial_window_size, + script_chan: script_channel, + load_data: load_data, + device_pixel_ratio: self.window_size.device_pixel_ratio, + pipeline_namespace_id: self.next_pipeline_namespace_id(), + webrender_api_sender: self.webrender_api_sender.clone(), + }); - privileged_pipeline_content.start(); + let (pipeline, child_process) = match result { + Ok(result) => result, + Err(e) => return self.handle_send_error(pipeline_id, e), + }; - if spawning_content { - // Spawn the child process. - // - // Yes, that's all there is to it! - if opts::multiprocess() { - match unprivileged_pipeline_content.spawn_multiprocess() { - Ok(child_process) => self.child_processes.push(child_process), - Err(e) => self.handle_send_error(pipeline_id, e), - } - } else { - unprivileged_pipeline_content.start_all::(false); - } + if let Some(child_process) = child_process { + self.child_processes.push(child_process); } assert!(!self.pipelines.contains_key(&pipeline_id)); diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index 0e6acf571d4..d11129794d1 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -120,10 +120,8 @@ pub struct InitialPipelineState { } impl Pipeline { - /// Starts a paint thread, layout thread, and possibly a script thread. - /// Returns the channels wrapped in a struct. - pub fn create(state: InitialPipelineState) - -> (Pipeline, UnprivilegedPipelineContent, PrivilegedPipelineContent) + fn create(state: InitialPipelineState) + -> (Pipeline, UnprivilegedPipelineContent, PrivilegedPipelineContent) where LTF: LayoutThreadFactory, STF: ScriptThreadFactory { @@ -256,6 +254,34 @@ impl Pipeline { (pipeline, unprivileged_pipeline_content, privileged_pipeline_content) } + /// Starts a paint thread, layout thread, and possibly a script thread, in + /// a new process if requested. + pub fn spawn(state: InitialPipelineState) + -> Result<(Pipeline, Option), IOError> + where LTF: LayoutThreadFactory, + STF: ScriptThreadFactory + { + let spawning_content = state.script_chan.is_none(); + let (pipeline, unprivileged_pipeline_content, privileged_pipeline_content) = + Pipeline::create::(state); + + privileged_pipeline_content.start(); + + let mut child_process = None; + if spawning_content { + // Spawn the child process. + // + // Yes, that's all there is to it! + if opts::multiprocess() { + child_process = Some(try!(unprivileged_pipeline_content.spawn_multiprocess())); + } else { + unprivileged_pipeline_content.start_all::(false); + } + } + + Ok((pipeline, child_process)) + } + fn new(id: PipelineId, parent_info: Option<(PipelineId, SubpageId, FrameType)>, script_chan: IpcSender, @@ -520,7 +546,7 @@ impl UnprivilegedPipelineContent { } } -pub struct PrivilegedPipelineContent { +struct PrivilegedPipelineContent { id: PipelineId, compositor_proxy: Box, font_cache_thread: FontCacheThread, From 88a36e6947294600be18b1310163799aff1598e8 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 26 May 2016 10:31:18 +0200 Subject: [PATCH 04/11] Inline Pipeline::create() into Pipeline::spawn(). --- components/constellation/pipeline.rs | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index d11129794d1..e45904dc77e 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -120,11 +120,15 @@ pub struct InitialPipelineState { } impl Pipeline { - fn create(state: InitialPipelineState) - -> (Pipeline, UnprivilegedPipelineContent, PrivilegedPipelineContent) - where LTF: LayoutThreadFactory, - STF: ScriptThreadFactory + /// Starts a paint thread, layout thread, and possibly a script thread, in + /// a new process if requested. + pub fn spawn(state: InitialPipelineState) + -> Result<(Pipeline, Option), IOError> + where LTF: LayoutThreadFactory, + STF: ScriptThreadFactory { + let spawning_content = state.script_chan.is_none(); + // Note: we allow channel creation to panic, since recovering from this // probably requires a general low-memory strategy. let (layout_to_paint_chan, layout_to_paint_port) = util::ipc::optional_ipc_channel(); @@ -251,20 +255,6 @@ impl Pipeline { paint_shutdown_chan: paint_shutdown_chan, }; - (pipeline, unprivileged_pipeline_content, privileged_pipeline_content) - } - - /// Starts a paint thread, layout thread, and possibly a script thread, in - /// a new process if requested. - pub fn spawn(state: InitialPipelineState) - -> Result<(Pipeline, Option), IOError> - where LTF: LayoutThreadFactory, - STF: ScriptThreadFactory - { - let spawning_content = state.script_chan.is_none(); - let (pipeline, unprivileged_pipeline_content, privileged_pipeline_content) = - Pipeline::create::(state); - privileged_pipeline_content.start(); let mut child_process = None; From 9e0aa099842923f28a81b6ef1e166d1101d6d652 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 26 May 2016 10:36:06 +0200 Subject: [PATCH 05/11] Remove PrivilegedPipelineContent. --- components/constellation/pipeline.rs | 59 ++++++---------------------- 1 file changed, 12 insertions(+), 47 deletions(-) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index e45904dc77e..a280e084ce1 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -30,7 +30,7 @@ use std::collections::HashMap; use std::io::Error as IOError; use std::mem; use std::process; -use std::sync::mpsc::{Receiver, Sender, channel}; +use std::sync::mpsc::{Sender, channel}; use url::Url; use util; use util::geometry::{PagePx, ViewportPx}; @@ -241,21 +241,17 @@ impl Pipeline { webrender_api_sender: state.webrender_api_sender, }; - let privileged_pipeline_content = PrivilegedPipelineContent { - id: state.id, - compositor_proxy: state.compositor_proxy, - font_cache_thread: state.font_cache_thread, - time_profiler_chan: state.time_profiler_chan, - mem_profiler_chan: state.mem_profiler_chan, - load_data: state.load_data, - panic_chan: state.panic_chan, - layout_to_paint_port: layout_to_paint_port, - chrome_to_paint_chan: chrome_to_paint_chan, - chrome_to_paint_port: chrome_to_paint_port, - paint_shutdown_chan: paint_shutdown_chan, - }; - - privileged_pipeline_content.start(); + PaintThread::create(state.id, + state.load_data.url, + chrome_to_paint_chan, + layout_to_paint_port, + chrome_to_paint_port, + state.compositor_proxy, + state.panic_chan, + state.font_cache_thread, + state.time_profiler_chan, + state.mem_profiler_chan, + paint_shutdown_chan); let mut child_process = None; if spawning_content { @@ -535,34 +531,3 @@ impl UnprivilegedPipelineContent { self.prefs.clone() } } - -struct PrivilegedPipelineContent { - id: PipelineId, - compositor_proxy: Box, - font_cache_thread: FontCacheThread, - time_profiler_chan: time::ProfilerChan, - mem_profiler_chan: profile_mem::ProfilerChan, - load_data: LoadData, - panic_chan: IpcSender, - layout_to_paint_port: Receiver, - chrome_to_paint_chan: Sender, - chrome_to_paint_port: Receiver, - paint_shutdown_chan: IpcSender<()>, -} - -impl PrivilegedPipelineContent { - pub fn start(self) { - PaintThread::create(self.id, - self.load_data.url, - self.chrome_to_paint_chan, - self.layout_to_paint_port, - self.chrome_to_paint_port, - self.compositor_proxy, - self.panic_chan, - self.font_cache_thread, - self.time_profiler_chan, - self.mem_profiler_chan, - self.paint_shutdown_chan); - - } -} From e6546a54e806e7819991d0413817626f548a30ae Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 26 May 2016 10:39:21 +0200 Subject: [PATCH 06/11] Avoid some unnecessary work if we're not spawning content. --- components/constellation/pipeline.rs | 139 +++++++++++++-------------- 1 file changed, 69 insertions(+), 70 deletions(-) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index a280e084ce1..179f4b8a3ff 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -127,8 +127,6 @@ impl Pipeline { where LTF: LayoutThreadFactory, STF: ScriptThreadFactory { - let spawning_content = state.script_chan.is_none(); - // Note: we allow channel creation to panic, since recovering from this // probably requires a general low-memory strategy. let (layout_to_paint_chan, layout_to_paint_port) = util::ipc::optional_ipc_channel(); @@ -140,34 +138,10 @@ impl Pipeline { let (pipeline_chan, pipeline_port) = ipc::channel() .expect("Pipeline main chan");; - let window_size = state.window_size.map(|size| { - WindowSizeData { - visible_viewport: size, - initial_viewport: size * ScaleFactor::new(1.0), - device_pixel_ratio: state.device_pixel_ratio, - } - }); - - // Route messages coming from content to devtools as appropriate. - let script_to_devtools_chan = state.devtools_chan.as_ref().map(|devtools_chan| { - let (script_to_devtools_chan, script_to_devtools_port) = ipc::channel() - .expect("Pipeline script to devtools chan"); - let devtools_chan = (*devtools_chan).clone(); - ROUTER.add_route(script_to_devtools_port.to_opaque(), box move |message| { - match message.to::() { - Err(e) => error!("Cast to ScriptToDevtoolsControlMsg failed ({}).", e), - Ok(message) => if let Err(e) = devtools_chan.send(DevtoolsControlMsg::FromScript(message)) { - warn!("Sending to devtools failed ({})", e) - }, - } - }); - script_to_devtools_chan - }); - let (layout_content_process_shutdown_chan, layout_content_process_shutdown_port) = ipc::channel().expect("Pipeline layout content shutdown chan"); - let (script_chan, script_port, pipeline_port) = match state.script_chan { + let (script_chan, script_port, pipeline_port, spawning_content) = match state.script_chan { Some(script_chan) => { let (containing_pipeline_id, subpage_id, _) = state.parent_info.expect("script_pipeline != None but subpage_id == None"); @@ -187,17 +161,14 @@ impl Pipeline { if let Err(e) = script_chan.send(ConstellationControlMsg::AttachLayout(new_layout_info)) { warn!("Sending to script during pipeline creation failed ({})", e); } - (script_chan, None, None) + (script_chan, None, None, false) } None => { let (script_chan, script_port) = ipc::channel().expect("Pipeline script chan"); - (script_chan, Some(script_port), Some(pipeline_port)) + (script_chan, Some(script_port), Some(pipeline_port), true) } }; - let (script_content_process_shutdown_chan, script_content_process_shutdown_port) = - ipc::channel().expect("Pipeline script content process shutdown chan"); - let pipeline = Pipeline::new(state.id, state.parent_info, script_chan.clone(), @@ -209,52 +180,80 @@ impl Pipeline { state.load_data.url.clone(), state.window_size); - let unprivileged_pipeline_content = UnprivilegedPipelineContent { - id: state.id, - parent_info: state.parent_info.map(|(parent_id, subpage_id, _)| (parent_id, subpage_id)), - constellation_chan: state.constellation_chan, - scheduler_chan: state.scheduler_chan, - devtools_chan: script_to_devtools_chan, - bluetooth_thread: state.bluetooth_thread, - image_cache_thread: state.image_cache_thread, - font_cache_thread: state.font_cache_thread.clone(), - resource_threads: state.resource_threads, - time_profiler_chan: state.time_profiler_chan.clone(), - mem_profiler_chan: state.mem_profiler_chan.clone(), - window_size: window_size, - layout_to_constellation_chan: state.layout_to_constellation_chan, - script_chan: script_chan, - load_data: state.load_data.clone(), - panic_chan: state.panic_chan.clone(), - script_port: script_port, - opts: (*opts::get()).clone(), - prefs: prefs::get_cloned(), - layout_to_paint_chan: layout_to_paint_chan, - pipeline_port: pipeline_port, - layout_shutdown_chan: layout_shutdown_chan, - paint_shutdown_chan: paint_shutdown_chan.clone(), - pipeline_namespace_id: state.pipeline_namespace_id, - layout_content_process_shutdown_chan: layout_content_process_shutdown_chan, - layout_content_process_shutdown_port: layout_content_process_shutdown_port, - script_content_process_shutdown_chan: script_content_process_shutdown_chan, - script_content_process_shutdown_port: script_content_process_shutdown_port, - webrender_api_sender: state.webrender_api_sender, - }; - PaintThread::create(state.id, - state.load_data.url, + state.load_data.url.clone(), chrome_to_paint_chan, layout_to_paint_port, chrome_to_paint_port, state.compositor_proxy, - state.panic_chan, - state.font_cache_thread, - state.time_profiler_chan, - state.mem_profiler_chan, - paint_shutdown_chan); + state.panic_chan.clone(), + state.font_cache_thread.clone(), + state.time_profiler_chan.clone(), + state.mem_profiler_chan.clone(), + paint_shutdown_chan.clone()); let mut child_process = None; if spawning_content { + // Route messages coming from content to devtools as appropriate. + let script_to_devtools_chan = state.devtools_chan.as_ref().map(|devtools_chan| { + let (script_to_devtools_chan, script_to_devtools_port) = ipc::channel() + .expect("Pipeline script to devtools chan"); + let devtools_chan = (*devtools_chan).clone(); + ROUTER.add_route(script_to_devtools_port.to_opaque(), box move |message| { + match message.to::() { + Err(e) => error!("Cast to ScriptToDevtoolsControlMsg failed ({}).", e), + Ok(message) => if let Err(e) = devtools_chan.send(DevtoolsControlMsg::FromScript(message)) { + warn!("Sending to devtools failed ({})", e) + }, + } + }); + script_to_devtools_chan + }); + + let device_pixel_ratio = state.device_pixel_ratio; + let window_size = state.window_size.map(|size| { + WindowSizeData { + visible_viewport: size, + initial_viewport: size * ScaleFactor::new(1.0), + device_pixel_ratio: device_pixel_ratio, + } + }); + + let (script_content_process_shutdown_chan, script_content_process_shutdown_port) = + ipc::channel().expect("Pipeline script content process shutdown chan"); + + let unprivileged_pipeline_content = UnprivilegedPipelineContent { + id: state.id, + parent_info: state.parent_info.map(|(parent_id, subpage_id, _)| (parent_id, subpage_id)), + constellation_chan: state.constellation_chan, + scheduler_chan: state.scheduler_chan, + devtools_chan: script_to_devtools_chan, + bluetooth_thread: state.bluetooth_thread, + image_cache_thread: state.image_cache_thread, + font_cache_thread: state.font_cache_thread, + resource_threads: state.resource_threads, + time_profiler_chan: state.time_profiler_chan, + mem_profiler_chan: state.mem_profiler_chan, + window_size: window_size, + layout_to_constellation_chan: state.layout_to_constellation_chan, + script_chan: script_chan, + load_data: state.load_data, + panic_chan: state.panic_chan, + script_port: script_port, + opts: (*opts::get()).clone(), + prefs: prefs::get_cloned(), + layout_to_paint_chan: layout_to_paint_chan, + pipeline_port: pipeline_port, + layout_shutdown_chan: layout_shutdown_chan, + paint_shutdown_chan: paint_shutdown_chan, + pipeline_namespace_id: state.pipeline_namespace_id, + layout_content_process_shutdown_chan: layout_content_process_shutdown_chan, + layout_content_process_shutdown_port: layout_content_process_shutdown_port, + script_content_process_shutdown_chan: script_content_process_shutdown_chan, + script_content_process_shutdown_port: script_content_process_shutdown_port, + webrender_api_sender: state.webrender_api_sender, + }; + // Spawn the child process. // // Yes, that's all there is to it! From c943c745a97967e8a50a46e301bdbd537ecffcce Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 26 May 2016 11:19:25 +0200 Subject: [PATCH 07/11] Improve code flow in Pipeline::spawn(). --- components/constellation/pipeline.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index 179f4b8a3ff..c2851dc3aab 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -141,7 +141,7 @@ impl Pipeline { let (layout_content_process_shutdown_chan, layout_content_process_shutdown_port) = ipc::channel().expect("Pipeline layout content shutdown chan"); - let (script_chan, script_port, pipeline_port, spawning_content) = match state.script_chan { + let (script_chan, content_ports) = match state.script_chan { Some(script_chan) => { let (containing_pipeline_id, subpage_id, _) = state.parent_info.expect("script_pipeline != None but subpage_id == None"); @@ -161,11 +161,11 @@ impl Pipeline { if let Err(e) = script_chan.send(ConstellationControlMsg::AttachLayout(new_layout_info)) { warn!("Sending to script during pipeline creation failed ({})", e); } - (script_chan, None, None, false) + (script_chan, None) } None => { let (script_chan, script_port) = ipc::channel().expect("Pipeline script chan"); - (script_chan, Some(script_port), Some(pipeline_port), true) + (script_chan, Some((script_port, pipeline_port))) } }; @@ -193,7 +193,7 @@ impl Pipeline { paint_shutdown_chan.clone()); let mut child_process = None; - if spawning_content { + if let Some((script_port, pipeline_port)) = content_ports { // Route messages coming from content to devtools as appropriate. let script_to_devtools_chan = state.devtools_chan.as_ref().map(|devtools_chan| { let (script_to_devtools_chan, script_to_devtools_port) = ipc::channel() @@ -239,11 +239,11 @@ impl Pipeline { script_chan: script_chan, load_data: state.load_data, panic_chan: state.panic_chan, - script_port: script_port, + script_port: Some(script_port), opts: (*opts::get()).clone(), prefs: prefs::get_cloned(), layout_to_paint_chan: layout_to_paint_chan, - pipeline_port: pipeline_port, + pipeline_port: Some(pipeline_port), layout_shutdown_chan: layout_shutdown_chan, paint_shutdown_chan: paint_shutdown_chan, pipeline_namespace_id: state.pipeline_namespace_id, From f6ae542c154d59d9057d29510814aa9e8fe4b5ad Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 26 May 2016 11:21:49 +0200 Subject: [PATCH 08/11] Avoid some unnecessary runtime checks. --- components/constellation/pipeline.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index c2851dc3aab..72fb06be2c8 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -28,7 +28,6 @@ use script_traits::{LayoutControlMsg, LayoutMsg, NewLayoutInfo, ScriptMsg}; use script_traits::{ScriptThreadFactory, TimerEventRequest}; use std::collections::HashMap; use std::io::Error as IOError; -use std::mem; use std::process; use std::sync::mpsc::{Sender, channel}; use url::Url; @@ -239,11 +238,11 @@ impl Pipeline { script_chan: script_chan, load_data: state.load_data, panic_chan: state.panic_chan, - script_port: Some(script_port), + script_port: script_port, opts: (*opts::get()).clone(), prefs: prefs::get_cloned(), layout_to_paint_chan: layout_to_paint_chan, - pipeline_port: Some(pipeline_port), + pipeline_port: pipeline_port, layout_shutdown_chan: layout_shutdown_chan, paint_shutdown_chan: paint_shutdown_chan, pipeline_namespace_id: state.pipeline_namespace_id, @@ -407,12 +406,12 @@ pub struct UnprivilegedPipelineContent { script_chan: IpcSender, load_data: LoadData, panic_chan: IpcSender, - script_port: Option>, + script_port: IpcReceiver, layout_to_paint_chan: OptionalIpcSender, opts: Opts, prefs: HashMap, paint_shutdown_chan: IpcSender<()>, - pipeline_port: Option>, + pipeline_port: IpcReceiver, pipeline_namespace_id: PipelineNamespaceId, layout_shutdown_chan: IpcSender<()>, layout_content_process_shutdown_chan: IpcSender<()>, @@ -423,7 +422,7 @@ pub struct UnprivilegedPipelineContent { } impl UnprivilegedPipelineContent { - pub fn start_all(mut self, wait_for_completion: bool) + pub fn start_all(self, wait_for_completion: bool) where LTF: LayoutThreadFactory, STF: ScriptThreadFactory { @@ -431,7 +430,7 @@ impl UnprivilegedPipelineContent { id: self.id, parent_info: self.parent_info, control_chan: self.script_chan.clone(), - control_port: mem::replace(&mut self.script_port, None).expect("No script port."), + control_port: self.script_port, constellation_chan: self.constellation_chan.clone(), scheduler_chan: self.scheduler_chan.clone(), panic_chan: self.panic_chan.clone(), @@ -450,7 +449,7 @@ impl UnprivilegedPipelineContent { self.load_data.url.clone(), self.parent_info.is_some(), layout_pair, - self.pipeline_port.expect("No pipeline port."), + self.pipeline_port, self.layout_to_constellation_chan, self.panic_chan, self.script_chan.clone(), From 948b69f503dee7f47e07cc5177c86cd7dd8442db Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 26 May 2016 11:26:09 +0200 Subject: [PATCH 09/11] Move the Pipeline creation to the end of Pipeline::spawn(). --- components/constellation/pipeline.rs | 30 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index 72fb06be2c8..ef2d8dd8a91 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -168,23 +168,12 @@ impl Pipeline { } }; - let pipeline = Pipeline::new(state.id, - state.parent_info, - script_chan.clone(), - LayoutControlChan(pipeline_chan), - state.compositor_proxy.clone_compositor_proxy(), - chrome_to_paint_chan.clone(), - layout_shutdown_port, - paint_shutdown_port, - state.load_data.url.clone(), - state.window_size); - PaintThread::create(state.id, state.load_data.url.clone(), - chrome_to_paint_chan, + chrome_to_paint_chan.clone(), layout_to_paint_port, chrome_to_paint_port, - state.compositor_proxy, + state.compositor_proxy.clone_compositor_proxy(), state.panic_chan.clone(), state.font_cache_thread.clone(), state.time_profiler_chan.clone(), @@ -235,8 +224,8 @@ impl Pipeline { mem_profiler_chan: state.mem_profiler_chan, window_size: window_size, layout_to_constellation_chan: state.layout_to_constellation_chan, - script_chan: script_chan, - load_data: state.load_data, + script_chan: script_chan.clone(), + load_data: state.load_data.clone(), panic_chan: state.panic_chan, script_port: script_port, opts: (*opts::get()).clone(), @@ -263,6 +252,17 @@ impl Pipeline { } } + let pipeline = Pipeline::new(state.id, + state.parent_info, + script_chan, + LayoutControlChan(pipeline_chan), + state.compositor_proxy, + chrome_to_paint_chan, + layout_shutdown_port, + paint_shutdown_port, + state.load_data.url, + state.window_size); + Ok((pipeline, child_process)) } From 43bf035f6ccf360af7c50537baf5f315c69def79 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 27 May 2016 13:38:56 +0200 Subject: [PATCH 10/11] Avoid some clones in UnprivilegedPipelineContent::start_all. --- components/constellation/pipeline.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index ef2d8dd8a91..dbcf55403d3 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -431,10 +431,10 @@ impl UnprivilegedPipelineContent { parent_info: self.parent_info, control_chan: self.script_chan.clone(), control_port: self.script_port, - constellation_chan: self.constellation_chan.clone(), - scheduler_chan: self.scheduler_chan.clone(), + constellation_chan: self.constellation_chan, + scheduler_chan: self.scheduler_chan, panic_chan: self.panic_chan.clone(), - bluetooth_thread: self.bluetooth_thread.clone(), + bluetooth_thread: self.bluetooth_thread, resource_threads: self.resource_threads, image_cache_thread: self.image_cache_thread.clone(), time_profiler_chan: self.time_profiler_chan.clone(), @@ -442,24 +442,24 @@ impl UnprivilegedPipelineContent { devtools_chan: self.devtools_chan, window_size: self.window_size, pipeline_namespace_id: self.pipeline_namespace_id, - content_process_shutdown_chan: self.script_content_process_shutdown_chan.clone(), + content_process_shutdown_chan: self.script_content_process_shutdown_chan, }, self.load_data.clone()); LTF::create(self.id, - self.load_data.url.clone(), + self.load_data.url, self.parent_info.is_some(), layout_pair, self.pipeline_port, self.layout_to_constellation_chan, self.panic_chan, - self.script_chan.clone(), - self.layout_to_paint_chan.clone(), + self.script_chan, + self.layout_to_paint_chan, self.image_cache_thread, self.font_cache_thread, self.time_profiler_chan, self.mem_profiler_chan, self.layout_shutdown_chan, - self.layout_content_process_shutdown_chan.clone(), + self.layout_content_process_shutdown_chan, self.webrender_api_sender); if wait_for_completion { From 5d0bbe87a7a9f6c6106ced250608d21b6607d215 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 27 May 2016 13:41:02 +0200 Subject: [PATCH 11/11] Remove unused UnprivilegedPipelineContent::paint_shutdown_chan. --- components/constellation/pipeline.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index dbcf55403d3..0490f573c12 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -178,7 +178,7 @@ impl Pipeline { state.font_cache_thread.clone(), state.time_profiler_chan.clone(), state.mem_profiler_chan.clone(), - paint_shutdown_chan.clone()); + paint_shutdown_chan); let mut child_process = None; if let Some((script_port, pipeline_port)) = content_ports { @@ -233,7 +233,6 @@ impl Pipeline { layout_to_paint_chan: layout_to_paint_chan, pipeline_port: pipeline_port, layout_shutdown_chan: layout_shutdown_chan, - paint_shutdown_chan: paint_shutdown_chan, pipeline_namespace_id: state.pipeline_namespace_id, layout_content_process_shutdown_chan: layout_content_process_shutdown_chan, layout_content_process_shutdown_port: layout_content_process_shutdown_port, @@ -410,7 +409,6 @@ pub struct UnprivilegedPipelineContent { layout_to_paint_chan: OptionalIpcSender, opts: Opts, prefs: HashMap, - paint_shutdown_chan: IpcSender<()>, pipeline_port: IpcReceiver, pipeline_namespace_id: PipelineNamespaceId, layout_shutdown_chan: IpcSender<()>,