servo: Merge #14536 - Remove `Constellation::child_processes` (from frewsxcv:constellation-child-processes); r=jdm
authorCorey Farwell <coreyf@rwell.org>
Wed, 14 Dec 2016 10:52:54 -0800
changeset 478506 c51f5a888776060e01179484d39239e2c0bf70cc
parent 478505 4e41c08478bc4f9a78652210436436dbdefae32f
child 478507 9f5a4f88bf77c4e89885341edf2d869ff74a5540
push id44079
push userbmo:gps@mozilla.com
push dateSat, 04 Feb 2017 00:14:49 +0000
reviewersjdm
servo: Merge #14536 - Remove `Constellation::child_processes` (from frewsxcv:constellation-child-processes); r=jdm Fixes https://github.com/servo/servo/issues/11459. Source-Repo: https://github.com/servo/servo Source-Revision: 01b6ad55bd435bc4f58e5eab2e8adb7e4febb50a
servo/components/constellation/constellation.rs
servo/components/constellation/pipeline.rs
--- a/servo/components/constellation/constellation.rs
+++ b/servo/components/constellation/constellation.rs
@@ -30,17 +30,17 @@ use log::{Log, LogLevel, LogLevelFilter,
 use msg::constellation_msg::{FrameId, FrameType, PipelineId};
 use msg::constellation_msg::{Key, KeyModifiers, KeyState};
 use msg::constellation_msg::{PipelineNamespace, PipelineNamespaceId, TraversalDirection};
 use net_traits::{self, IpcSend, ResourceThreads};
 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};
 use script_traits::{AnimationState, AnimationTickType, CompositorEvent};
 use script_traits::{ConstellationControlMsg, ConstellationMsg as FromCompositorMsg};
 use script_traits::{DocumentState, LayoutControlMsg, LoadData};
 use script_traits::{IFrameLoadInfo, IFrameLoadInfoWithData, IFrameSandboxState, TimerEventRequest};
 use script_traits::{LayoutMsg as FromLayoutMsg, ScriptMsg as FromScriptMsg, ScriptThreadFactory};
@@ -170,20 +170,16 @@ pub struct Constellation<Message, LTF, S
 
     window_size: WindowSizeData,
 
     /// Bits of state used to interact with the webdriver implementation
     webdriver: WebDriverData,
 
     scheduler_chan: IpcSender<TimerEventRequest>,
 
-    /// A list of child content processes.
-    #[cfg_attr(target_os = "windows", allow(dead_code))]
-    child_processes: Vec<ChildProcess>,
-
     /// Document states for loaded pipelines (used only when writing screenshots).
     document_states: HashMap<PipelineId, DocumentState>,
 
     // Webrender interface.
     webrender_api_sender: webrender_traits::RenderApiSender,
 
     /// Are we shutting down?
     shutting_down: bool,
@@ -553,17 +549,16 @@ impl<Message, LTF, STF> Constellation<Me
                     initial_viewport: opts::get().initial_window_size.to_f32() *
                         ScaleFactor::new(1.0),
                     device_pixel_ratio:
                         ScaleFactor::new(opts::get().device_pixels_per_px.unwrap_or(1.0)),
                 },
                 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,
                 handled_warnings: VecDeque::new(),
                 random_pipeline_closure: opts::get().random_pipeline_closure_probability.map(|prob| {
                     let seed = opts::get().random_pipeline_closure_seed.unwrap_or_else(random);
                     let rng = StdRng::from_seed(&[seed]);
                     warn!("Randomly closing pipelines.");
@@ -674,25 +669,21 @@ impl<Message, LTF, STF> Constellation<Me
             load_data: load_data,
             device_pixel_ratio: self.window_size.device_pixel_ratio,
             pipeline_namespace_id: self.next_pipeline_namespace_id(),
             prev_visibility: prev_visibility,
             webrender_api_sender: self.webrender_api_sender.clone(),
             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)
                 .insert(host, Rc::downgrade(&pipeline.script_chan));
         }
 
         assert!(!self.pipelines.contains_key(&pipeline_id));
         self.pipelines.insert(pipeline_id, pipeline);
--- a/servo/components/constellation/pipeline.rs
+++ b/servo/components/constellation/pipeline.rs
@@ -5,18 +5,16 @@
 use bluetooth_traits::BluetoothRequest;
 use compositing::CompositionPipeline;
 use compositing::CompositorProxy;
 use compositing::compositor_thread::Msg as CompositorMsg;
 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};
 use ipc_channel::router::ROUTER;
 use layout_traits::LayoutThreadFactory;
 use msg::constellation_msg::{FrameId, FrameType, PipelineId, PipelineNamespaceId};
 use net_traits::{IpcSend, ResourceThreads};
 use net_traits::image_cache_thread::ImageCacheThread;
@@ -34,23 +32,16 @@ use std::io::Error as IOError;
 use std::process;
 use std::rc::Rc;
 use std::sync::mpsc::Sender;
 use style_traits::{PagePx, ViewportPx};
 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,
     /// The ID of the frame that contains this Pipeline.
     pub frame_id: FrameId,
     pub parent_info: Option<(PipelineId, FrameType)>,
     pub script_chan: Rc<ScriptChan>,
     /// A channel to layout, for performing reflows and shutdown.
@@ -129,25 +120,24 @@ pub struct InitialPipelineState {
     pub webrender_api_sender: webrender_traits::RenderApiSender,
     /// Whether this pipeline is considered private.
     pub is_private: bool,
 }
 
 impl Pipeline {
     /// Starts a paint thread, layout thread, and possibly a script thread, in
     /// a new process if requested.
-    pub fn spawn<Message, LTF, STF>(state: InitialPipelineState)
-                                    -> Result<(Pipeline, Option<ChildProcess>), IOError>
+    pub fn spawn<Message, LTF, STF>(state: InitialPipelineState) -> Result<Pipeline, IOError>
         where LTF: LayoutThreadFactory<Message=Message>,
               STF: ScriptThreadFactory<Message=Message>
     {
         // 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");
 
         let device_pixel_ratio = state.device_pixel_ratio;
         let window_size = state.window_size.map(|size| {
             WindowSizeData {
                 visible_viewport: size,
@@ -175,17 +165,16 @@ impl Pipeline {
                 (script_chan, None)
             }
             None => {
                 let (script_chan, script_port) = ipc::channel().expect("Pipeline script chan");
                 (ScriptChan::new(script_chan), Some((script_port, pipeline_port)))
             }
         };
 
-        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| {
                 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::<ScriptToDevtoolsControlMsg>() {
@@ -231,34 +220,32 @@ impl Pipeline {
                 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!
             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::<Message, LTF, STF>(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
     /// spawned.
     pub fn new(id: PipelineId,
                frame_id: FrameId,
                parent_info: Option<(PipelineId, FrameType)>,
                script_chan: Rc<ScriptChan>,
@@ -459,17 +446,17 @@ impl UnprivilegedPipelineContent {
 
         if wait_for_completion {
             let _ = self.script_content_process_shutdown_port.recv();
             let _ = self.layout_content_process_shutdown_port.recv();
         }
     }
 
     #[cfg(not(target_os = "windows"))]
-    pub fn spawn_multiprocess(self) -> Result<ChildProcess, IOError> {
+    pub fn spawn_multiprocess(self) -> Result<(), IOError> {
         use gaol::sandbox::{self, Sandbox, SandboxMethods};
         use ipc_channel::ipc::IpcOneShotServer;
         use sandboxing::content_process_sandbox_profile;
 
         impl CommandMethods for sandbox::Command {
             fn arg<T>(&mut self, arg: T)
                 where T: AsRef<OsStr> {
                 self.arg(arg);
@@ -484,41 +471,40 @@ impl 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::<IpcSender<UnprivilegedPipelineContent>>::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 {
+        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<ChildProcess, IOError> {
+    pub fn spawn_multiprocess(self) -> Result<(), IOError> {
         error!("Multiprocess is not supported on Windows.");
         process::exit(1);
     }
 
     #[cfg(not(windows))]
     fn setup_common<C: CommandMethods>(&self, command: &mut C, token: String) {
         C::arg(command, "--content-process");
         C::arg(command, token);