Refactor Firefox binary start up to match the other binaries

lib/Selenium/Binary.pm

@@ -4,6 +4,8 @@ package Selenium::Binary;
 use File::Which qw/which/;
 use IO::Socket::INET;
 use Selenium::Waiter qw/wait_until/;
+use Selenium::Firefox::Binary qw/firefox_path setup_firefox_binary_env/;
+use Selenium::Firefox::Profile;
 
 require Exporter;
 our @ISA = qw/Exporter/;
@@ -15,6 +17,9 @@ sub start_binary_on_port {
 
     my $executable = _find_executable($process);
     $port = _find_open_port_above($port);
+    if ($process eq 'firefox') {
+        setup_firefox_binary_env($port);
+    }
     my $command = _construct_command($executable, $port);
 
     system($command);
@@ -30,13 +35,18 @@ sub start_binary_on_port {
 sub _find_executable {
     my ($binary) = @_;
 
-    my $executable = which($binary);
-
-    if (not defined $executable) {
-        die qq(Unable to find the $binary binary in your \$PATH. We'll try falling back to standard Remote Driver);
+    if ($binary eq 'firefox') {
+        return firefox_path();
     }
     else {
-        return $executable;
+        my $executable = which($binary);
+
+        if (not defined $executable) {
+            die qq(Unable to find the $binary binary in your \$PATH. We'll try falling back to standard Remote Driver);
+        }
+        else {
+            return $executable;
+        }
     }
 }
 
@@ -55,6 +65,9 @@ sub _construct_command {
             webdriver => '127.0.0.1:' . $port
         );
     }
+    elsif ($executable =~ /firefox/i) {
+        $executable .= ' -no-remote ';
+    }
 
     my @args = map { '--' . $_ . '=' . $args{$_} } keys %args;
 

lib/Selenium/Firefox.pm

@@ -1,10 +1,8 @@
 package Selenium::Firefox;
 
 # ABSTRACT: A convenience package for creating a Firefox instance
-use Selenium::Binary qw/_find_open_port_above _probe_port/;
-use Selenium::Firefox::Binary qw/firefox_path/;
-use Selenium::Firefox::Profile;
-use Selenium::Waiter qw/wait_until/;
+use Selenium::Binary qw/start_binary_on_port/;
+use Try::Tiny;
 use Moo;
 use namespace::clean;
 extends 'Selenium::Remote::Driver';
@@ -45,31 +43,27 @@ has '+firefox_profile' => (
 has 'binary_mode' => (
     is => 'ro',
     init_arg => undef,
-    builder => sub {
-        my ($self) = @_;
-
-        my $profile = Selenium::Firefox::Profile->new;
-
-        my $port = _find_open_port_above(FIREFOX_PORT);
-        $profile->add_webdriver($port);
-
-        $ENV{'XRE_PROFILE_PATH'} = $profile->_layout_on_disk;
-        $ENV{'MOZ_NO_REMOTE'} = '1'; # able to launch multiple instances
-        $ENV{'MOZ_CRASHREPORTER_DISABLE'} = '1'; # disable breakpad
-        $ENV{'NO_EM_RESTART'} = '1'; # prevent the binary from detaching from the console.log
+    builder => 1
+);
 
-        my $binary = firefox_path();
-        system( $binary . ' -no-remote > /dev/null 2>&1 & ');
+sub _build_binary_mode {
+    my ($self) = @_;
 
-        my $success = wait_until { _probe_port($port) } timeout => 10;
-        if ($success) {
+    if (! $self->has_remote_server_addr && ! $self->has_port) {
+        try {
+            my $port = start_binary_on_port('firefox', FIREFOX_PORT);
             $self->port($port);
-            return 1
+            return 1;
         }
-        else {
+        catch {
+            warn $_;
             return 0;
         }
     }
-);
+    else {
+        return 0;
+    }
+}
+
 
 1;

lib/Selenium/Firefox/Binary.pm

@@ -1,18 +1,19 @@
 package Selenium::Firefox::Binary;
 
-# ABSTRACT: Portable handler to start the firefox binary
+# ABSTRACT: Subroutines for locating and properly initializing the Firefox Binary
 use File::Which qw/which/;
+use Selenium::Firefox::Profile;
 
 require Exporter;
 our @ISA = qw/Exporter/;
-our @EXPORT_OK = qw/firefox_path/;
+our @EXPORT_OK = qw/firefox_path setup_firefox_binary_env/;
 
-sub _windows_path {
+sub _firefox_windows_path {
     # TODO: make this slightly less dumb
     return which('firefox');
 }
 
-sub _darwin_path {
+sub _firefox_darwin_path {
     my $default_firefox = '/Applications/Firefox.app/Contents/MacOS/firefox-bin';
 
     if (-e $default_firefox) {
@@ -23,7 +24,7 @@ sub _darwin_path {
     }
 }
 
-sub _unix_path {
+sub _firefox_unix_path {
     # TODO: maybe which('firefox3'), which('firefox2') ?
     return which('firefox') || '/usr/bin/firefox';
 }
@@ -31,13 +32,13 @@ sub _unix_path {
 sub firefox_path {
     my $path;
     if ($^O eq 'MSWin32') {
-        $path =_windows_path();
+        $path =_firefox_windows_path();
     }
     elsif ($^O eq 'darwin') {
-        $path =_darwin_path();
+        $path = _firefox_darwin_path();
     }
     else {
-        $path = _unix_path;
+        $path = _firefox_unix_path;
     }
 
     if (not -x $path) {
@@ -47,4 +48,18 @@ sub firefox_path {
     return $path;
 }
 
+sub setup_firefox_binary_env {
+    my ($port) = @_;
+
+    # TODO: respect the user's profile instead of overwriting it
+    my $profile = Selenium::Firefox::Profile->new;
+    $profile->add_webdriver($port);
+
+    $ENV{'XRE_PROFILE_PATH'} = $profile->_layout_on_disk;
+    $ENV{'MOZ_NO_REMOTE'} = '1';             # able to launch multiple instances
+    $ENV{'MOZ_CRASHREPORTER_DISABLE'} = '1'; # disable breakpad
+    $ENV{'NO_EM_RESTART'} = '1';             # prevent the binary from detaching from the console.log
+}
+
+
 1;

t/binary.t

@@ -39,6 +39,10 @@ CHROME: {
 FIREFOX: {
     my $binary = Selenium::Firefox::Binary::firefox_path();
     ok(-x $binary, 'we can find some sort of firefox');
+
+    my $command = Selenium::Binary::_construct_command('firefox', 1234);
+    ok($command =~ /firefox -no-remote/, 'firefox command has proper args');
+
     my $firefox = Selenium::Firefox->new;
     isnt( $firefox->port, 4444, 'firefox can start up its own binary');