servo: Merge #12635 - Extracted common parts of the starting content process (from nc4rrillo:refactor-spawn-multiprocess); r=Manishearth
authornc4rrillo <ncarrillo1@outlook.com>
Fri, 29 Jul 2016 06:43:19 -0500
changeset 339398 e34e1364c1bfce9fa8ab4398fdec136314c82709
parent 339397 ae91243180b1811bd139182b58765ead31b9db3e
child 339399 a6c164e7a941953424f7470b5a565bf6e4e11201
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersManishearth
servo: Merge #12635 - Extracted common parts of the starting content process (from nc4rrillo:refactor-spawn-multiprocess); r=Manishearth <!-- Please describe your changes on the following line: --> --- I implemented a trait for both `process::Command` and `sandbox::Command` in order to constrain my generic `setup_common` method. <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #12622 (github issue number if applicable). <!-- Either: --> - [X] These changes do not require tests because we are extracting common functionality across two code branches into a method <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 94e6f59174f7102d37eae7107e6bd502a9b0ebe1
servo/components/constellation/pipeline.rs
--- a/servo/components/constellation/pipeline.rs
+++ b/servo/components/constellation/pipeline.rs
@@ -23,16 +23,18 @@ use net_traits::bluetooth_thread::Blueto
 use net_traits::image_cache_thread::ImageCacheThread;
 use net_traits::{ResourceThreads, IpcSend};
 use profile_traits::mem as profile_mem;
 use profile_traits::time;
 use script_traits::{ConstellationControlMsg, InitialScriptState, MozBrowserEvent};
 use script_traits::{LayoutControlMsg, LayoutMsg, NewLayoutInfo, ScriptMsg, SWManagerMsg, SWManagerSenders};
 use script_traits::{ScriptThreadFactory, TimerEventRequest, WindowSizeData};
 use std::collections::HashMap;
+use std::env;
+use std::ffi::OsStr;
 use std::io::Error as IOError;
 use std::process;
 use std::sync::mpsc::{Sender, channel};
 use style_traits::{PagePx, ViewportPx};
 use url::Url;
 use util;
 use util::ipc::OptionalIpcSender;
 use util::opts::{self, Opts};
@@ -480,55 +482,49 @@ impl UnprivilegedPipelineContent {
         }
     }
 
     #[cfg(not(target_os = "windows"))]
     pub fn spawn_multiprocess(self) -> Result<ChildProcess, IOError> {
         use gaol::sandbox::{self, Sandbox, SandboxMethods};
         use ipc_channel::ipc::IpcOneShotServer;
         use sandboxing::content_process_sandbox_profile;
-        use std::env;
+
+        impl CommandMethods for sandbox::Command {
+            fn arg<T>(&mut self, arg: T)
+                where T: AsRef<OsStr> {
+                self.arg(arg);
+            }
+
+            fn env<T, U>(&mut self, key: T, val: U)
+                where T: AsRef<OsStr>, U: AsRef<OsStr> {
+                self.env(key, val);
+            }
+        }
 
         // 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 {
             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);
-            }
-
-            if let Ok(value) = env::var("RUST_LOG") {
-                command.env("RUST_LOG", value);
-            }
+            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!"))
         } 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);
-            }
-
-            if let Ok(value) = env::var("RUST_LOG") {
-                child_process.env("RUST_LOG", value);
-            }
+            self.setup_common(&mut child_process, token);
 
             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));
 
@@ -536,16 +532,29 @@ impl UnprivilegedPipelineContent {
     }
 
     #[cfg(target_os = "windows")]
     pub fn spawn_multiprocess(self) -> Result<ChildProcess, IOError> {
         error!("Multiprocess is not supported on Windows.");
         process::exit(1);
     }
 
+    fn setup_common<C: CommandMethods>(&self, command: &mut C, token: String) {
+        C::arg(command, "--content-process");
+        C::arg(command, token);
+
+        if let Ok(value) = env::var("RUST_BACKTRACE") {
+            C::env(command, "RUST_BACKTRACE", value);
+        }
+
+        if let Ok(value) = env::var("RUST_LOG") {
+            C::env(command, "RUST_LOG", value);
+        }
+    }
+
     pub fn constellation_chan(&self) -> IpcSender<ScriptMsg> {
         self.constellation_chan.clone()
     }
 
     pub fn opts(&self) -> Opts {
         self.opts.clone()
     }
 
@@ -555,8 +564,28 @@ impl UnprivilegedPipelineContent {
 
     pub fn swmanager_senders(&self) -> SWManagerSenders {
         SWManagerSenders {
             swmanager_sender: self.swmanager_thread.clone(),
             resource_sender: self.resource_threads.sender()
         }
     }
 }
+
+trait CommandMethods {
+    fn arg<T>(&mut self, arg: T)
+        where T: AsRef<OsStr>;
+
+    fn env<T, U>(&mut self, key: T, val: U)
+        where T: AsRef<OsStr>, U: AsRef<OsStr>;
+}
+
+impl CommandMethods for process::Command {
+    fn arg<T>(&mut self, arg: T)
+        where T: AsRef<OsStr> {
+        self.arg(arg);
+    }
+
+    fn env<T, U>(&mut self, key: T, val: U)
+        where T: AsRef<OsStr>, U: AsRef<OsStr> {
+        self.env(key, val);
+    }
+}