From 8dfbfc2c48f5c896e538534368af964f08ecb67a Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Fri, 9 Dec 2016 11:00:24 -1000 Subject: [PATCH 1/2] Remove `Constellation::child_processes`. Fixes https://github.com/servo/servo/issues/11459. --- components/constellation/constellation.rs | 13 ++-------- components/constellation/pipeline.rs | 30 ++++++++++------------- 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index c34f9b1597c..2e6b5071d41 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -35,7 +35,7 @@ use net_traits::image_cache_thread::ImageCacheThread; use net_traits::pub_domains::reg_suffix; use net_traits::storage_thread::{StorageThreadMsg, StorageType}; use offscreen_gl_context::{GLContextAttributes, GLLimits}; -use pipeline::{ChildProcess, InitialPipelineState, Pipeline}; +use pipeline::{InitialPipelineState, Pipeline}; use profile_traits::mem; use profile_traits::time; use rand::{Rng, SeedableRng, StdRng, random}; @@ -175,10 +175,6 @@ pub struct Constellation { scheduler_chan: IpcSender, - /// A list of child content processes. - #[cfg_attr(target_os = "windows", allow(dead_code))] - child_processes: Vec, - /// Document states for loaded pipelines (used only when writing screenshots). document_states: HashMap, @@ -558,7 +554,6 @@ impl Constellation phantom: PhantomData, webdriver: WebDriverData::new(), scheduler_chan: TimerScheduler::start(), - child_processes: Vec::new(), document_states: HashMap::new(), webrender_api_sender: state.webrender_api_sender, shutting_down: false, @@ -679,15 +674,11 @@ impl Constellation is_private: is_private, }); - let (pipeline, child_process) = match result { + let pipeline = match result { Ok(result) => result, Err(e) => return self.handle_send_error(pipeline_id, e), }; - if let Some(child_process) = child_process { - self.child_processes.push(child_process); - } - if let Some(host) = host { self.script_channels.entry(top_level_frame_id) .or_insert_with(HashMap::new) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index 890476e5010..81c7fead955 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -134,15 +134,14 @@ pub struct InitialPipelineState { impl Pipeline { /// 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> + pub fn spawn(state: InitialPipelineState) -> Result where LTF: LayoutThreadFactory, STF: ScriptThreadFactory { // Note: we allow channel creation to panic, since recovering from this // probably requires a general low-memory strategy. let (pipeline_chan, pipeline_port) = ipc::channel() - .expect("Pipeline main chan");; + .expect("Pipeline main chan"); let (layout_content_process_shutdown_chan, layout_content_process_shutdown_port) = ipc::channel().expect("Pipeline layout content shutdown chan"); @@ -180,7 +179,6 @@ impl Pipeline { } }; - let mut child_process = None; 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| { @@ -236,24 +234,22 @@ impl Pipeline { // // Yes, that's all there is to it! if opts::multiprocess() { - child_process = Some(try!(unprivileged_pipeline_content.spawn_multiprocess())); + let _ = try!(unprivileged_pipeline_content.spawn_multiprocess()); } else { unprivileged_pipeline_content.start_all::(false); } } - let pipeline = Pipeline::new(state.id, - state.frame_id, - state.parent_info, - script_chan, - pipeline_chan, - state.compositor_proxy, - state.is_private, - state.load_data.url, - state.window_size, - state.prev_visibility.unwrap_or(true)); - - Ok((pipeline, child_process)) + Ok(Pipeline::new(state.id, + state.frame_id, + state.parent_info, + script_chan, + pipeline_chan, + state.compositor_proxy, + state.is_private, + state.load_data.url, + state.window_size, + state.prev_visibility.unwrap_or(true))) } /// Creates a new `Pipeline`, after the script and layout threads have been From c11b333ae52df2bc341c10aa3b2a5811f8383759 Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Fri, 9 Dec 2016 11:36:57 -1000 Subject: [PATCH 2/2] Remove constellation `ChildProcess`. --- components/constellation/pipeline.rs | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index 81c7fead955..9784a68d57d 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -10,8 +10,6 @@ use constellation::ScriptChan; 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_traits::DevicePixel; use ipc_channel::ipc::{self, IpcReceiver, IpcSender}; @@ -39,13 +37,6 @@ use util::opts::{self, Opts}; use util::prefs::{PREFS, 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, @@ -460,7 +451,7 @@ impl UnprivilegedPipelineContent { } #[cfg(not(target_os = "windows"))] - pub fn spawn_multiprocess(self) -> Result { + pub fn spawn_multiprocess(self) -> Result<(), IOError> { use gaol::sandbox::{self, Sandbox, SandboxMethods}; use ipc_channel::ipc::IpcOneShotServer; use sandboxing::content_process_sandbox_profile; @@ -485,31 +476,30 @@ impl UnprivilegedPipelineContent { .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 { + if opts::get().sandbox { let mut command = sandbox::Command::me().expect("Failed to get current sandbox."); self.setup_common(&mut command, token); let profile = content_process_sandbox_profile(); - ChildProcess::Sandboxed(Sandbox::new(profile).start(&mut command) - .expect("Failed to start sandboxed child process!")) + let _ = 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); self.setup_common(&mut child_process, token); - - ChildProcess::Unsandboxed(child_process.spawn() - .expect("Failed to start unsandboxed child process!")) - }; + let _ = 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) + Ok(()) } #[cfg(target_os = "windows")] - pub fn spawn_multiprocess(self) -> Result { + pub fn spawn_multiprocess(self) -> Result<(), IOError> { error!("Multiprocess is not supported on Windows."); process::exit(1); }