Allow providing a firefox binary through new session parameters. (#66)

Binary can be specified with firefox_binary capability. If none
is provided it falls back to the value provided with -b on the
command line, if any, and returns an error otherwise.

2 files changed, 39 insertions, 25 deletions

diff --git a/src/main.rs b/src/main.rs
index 3fbf1c5..ccfa774 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -16,13 +16,13 @@ extern crate zip;
 use std::borrow::ToOwned;
 use std::io::Write;
 use std::net::{SocketAddr, IpAddr};
-use std::path::Path;
+use std::path::PathBuf;
 use std::str::FromStr;
 
 use argparse::{ArgumentParser, IncrBy, StoreTrue, Store, StoreOption};
 use webdriver::server::start;
 
-use marionette::{MarionetteHandler, BrowserLauncher, LogLevel, MarionetteSettings, extension_routes};
+use marionette::{MarionetteHandler, LogLevel, MarionetteSettings, extension_routes};
 
 macro_rules! try_opt {
     ($expr:expr, $err_type:expr, $err_msg:expr) => ({
@@ -43,7 +43,7 @@ enum ExitCode {
 }
 
 struct Options {
-    binary: String,
+    binary: Option<String>,
     webdriver_host: String,
     webdriver_port: u16,
     marionette_port: Option<u16>,
@@ -56,7 +56,7 @@ struct Options {
 
 fn parse_args() -> Options {
     let mut opts = Options {
-        binary: "".to_owned(),
+        binary: None,
         webdriver_host: "127.0.0.1".to_owned(),
         webdriver_port: 4444u16,
         marionette_port: None,
@@ -72,7 +72,7 @@ fn parse_args() -> Options {
         parser.set_description("WebDriver to Marionette proxy.");
 
         parser.refer(&mut opts.binary)
-            .add_option(&["-b", "--binary"], Store,
+            .add_option(&["-b", "--binary"], StoreOption,
                         "Path to the Firefox binary");
         parser.refer(&mut opts.webdriver_host)
             .add_option(&["--webdriver-host"], Store,
@@ -132,9 +132,6 @@ fn run() -> ProgramResult {
         print_version();
         return Ok(())
     }
-    if opts.binary == "" && !opts.connect_existing {
-        return Err((ExitCode::Usage, "path to browser binary required unless --connect-existing".to_owned()))
-    }
 
     let host = &opts.webdriver_host[..];
     let port = opts.webdriver_port;
@@ -161,18 +158,12 @@ fn run() -> ProgramResult {
         }
     };
 
-    // TODO: what if binary isn't a valid path?
-    let launcher = if opts.connect_existing {
-        BrowserLauncher::None
-    } else {
-        BrowserLauncher::BinaryLauncher(Path::new(&opts.binary).to_path_buf())
-    };
-
     let settings = MarionetteSettings {
         port: opts.marionette_port,
-        launcher: launcher,
+        binary: opts.binary.map(|x| PathBuf::from(x)),
+        connect_existing: opts.connect_existing,
         e10s: opts.e10s,
-        log_level: log_level,
+        log_level: log_level
     };
     start(addr, MarionetteHandler::new(settings), extension_routes());
 
@@ -197,7 +188,7 @@ fn main() {
 #[cfg(test)]
 mod tests {
     use std::collections::BTreeMap;
-    use marionette::{MarionetteSettings, MarionetteHandler, BrowserLauncher};
+    use marionette::{MarionetteSettings, MarionetteHandler};
     use webdriver::command::NewSessionParameters;
     use rustc_serialize::json::Json;
     use std::fs::File;
@@ -230,7 +221,8 @@ mod tests {
 
         let settings = MarionetteSettings {
             port: None,
-            launcher: BrowserLauncher::None,
+            binary: None,
+            connect_existing: false,
             e10s: false,
             log_level: None,
         };
diff --git a/src/marionette.rs b/src/marionette.rs
index 91d53f8..ee1fe3e 100644
--- a/src/marionette.rs
+++ b/src/marionette.rs
@@ -278,7 +278,8 @@ impl FromStr for LogLevel {
 
 pub struct MarionetteSettings {
     pub port: Option<u16>,
-    pub launcher: BrowserLauncher,
+    pub binary: Option<PathBuf>,
+    pub connect_existing: bool,
 
     /// Enable or disable Electrolysis, or multi-processing, in Gecko.
     pub e10s: bool,
@@ -291,7 +292,8 @@ pub struct MarionetteSettings {
 
 pub struct MarionetteHandler {
     connection: Mutex<Option<MarionetteConnection>>,
-    launcher: BrowserLauncher,
+    binary: Option<PathBuf>,
+    connect_existing: bool,
     browser: Option<FirefoxRunner>,
     port: Option<u16>,
     e10s: bool,
@@ -302,7 +304,8 @@ impl MarionetteHandler {
     pub fn new(settings: MarionetteSettings) -> MarionetteHandler {
         MarionetteHandler {
             connection: Mutex::new(None),
-            launcher: settings.launcher,
+            binary: settings.binary,
+            connect_existing: settings.connect_existing,
             browser: None,
             port: settings.port,
             e10s: settings.e10s,
@@ -319,7 +322,26 @@ impl MarionetteHandler {
             Some(x) => x,
             None => try!(get_free_port())
         };
-        match self.start_browser(port, profile, args) {
+        let launcher = if self.connect_existing {
+            BrowserLauncher::None
+        } else {
+            let binary = if let Some(binary_capability) = capabilities.get("firefox_binary") {
+                Some(PathBuf::from(try!(binary_capability
+                                        .as_string()
+                                        .ok_or(WebDriverError::new(
+                                            ErrorStatus::InvalidArgument,
+                                            "'firefox_binary' capability was not a string")))))
+            } else {
+                self.binary.as_ref().map(|x| x.clone())
+            };
+            if let Some(path) = binary {
+                BrowserLauncher::BinaryLauncher(path)
+            } else {
+                return Err(WebDriverError::new(ErrorStatus::UnknownError,
+                                               "'firefox_binary' capability not provided and no binary set on the command line"));
+            }
+        };
+        match self.start_browser(launcher, port, profile, args) {
             Err(e) => {
                 return Err(WebDriverError::new(ErrorStatus::UnknownError,
                                                e.description().to_owned()));
@@ -335,10 +357,10 @@ impl MarionetteHandler {
         Ok(())
     }
 
-    fn start_browser(&mut self, port: u16, profile: Option<Profile>, args: Option<Vec<String>>) -> Result<(), RunnerError> {
+    fn start_browser(&mut self, launcher: BrowserLauncher, port: u16, profile: Option<Profile>, args: Option<Vec<String>>) -> Result<(), RunnerError> {
         let custom_profile = profile.is_some();
 
-        match self.launcher {
+        match launcher {
             BrowserLauncher::BinaryLauncher(ref binary) => {
                 let mut runner = try!(FirefoxRunner::new(&binary, profile));
                 if let Some(cmd_args) = args {