From 15f966d632dba814e8adf596752c29d43a0798bf Mon Sep 17 00:00:00 2001
From: Alex Rodionov <p0deje@gmail.com>
Date: Tue, 21 May 2019 08:37:35 +0600
Subject: [PATCH] Lookup and pass Firefox binary location to GeckoDriver

Closes #7219
---
 rb/lib/selenium/webdriver/common/service.rb   | 33 +++++++++++++------
 .../webdriver/firefox/service_spec.rb         | 14 ++++----
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/rb/lib/selenium/webdriver/common/service.rb b/rb/lib/selenium/webdriver/common/service.rb
index 7fec813ac60bd..b5ef8ac12c826 100644
--- a/rb/lib/selenium/webdriver/common/service.rb
+++ b/rb/lib/selenium/webdriver/common/service.rb
@@ -32,25 +32,38 @@ class Service
       class << self
         attr_reader :driver_path
 
-        def chrome(*args)
-          Chrome::Service.new(*args)
+        def chrome(**opts)
+          Chrome::Service.new(**opts)
         end
 
-        def firefox(*args)
-          Firefox::Service.new(*args)
+        def firefox(**opts)
+          binary_path = Firefox::Binary.path
+          args = opts.delete(:args)
+          case args
+          when Hash
+            args[:binary] ||= binary_path
+            opts[:args] = args
+          when Array
+            opts[:args] = ["--binary=#{binary_path}"]
+            opts[:args] += args
+          else
+            opts[:args] = ["--binary=#{binary_path}"]
+          end
+
+          Firefox::Service.new(**opts)
         end
 
-        def ie(*args)
-          IE::Service.new(*args)
+        def ie(**opts)
+          IE::Service.new(**opts)
         end
         alias_method :internet_explorer, :ie
 
-        def edge(*args)
-          Edge::Service.new(*args)
+        def edge(**opts)
+          Edge::Service.new(**opts)
         end
 
-        def safari(*args)
-          Safari::Service.new(*args)
+        def safari(**opts)
+          Safari::Service.new(**opts)
         end
 
         def driver_path=(path)
diff --git a/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb b/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb
index 4d107956678a7..775d4e4a4971b 100644
--- a/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb
+++ b/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb
@@ -26,6 +26,7 @@ module WebDriver
 
       before do
         allow(Platform).to receive(:assert_executable).and_return(true)
+        allow(Firefox::Binary).to receive(:path).and_return('/foo/bar')
       end
 
       describe '#new' do
@@ -84,12 +85,12 @@ module WebDriver
           expect(service.executable_path).to eq path
         end
 
-        it 'does not create args by default' do
+        it 'provides binary path by default' do
           allow(Platform).to receive(:find_binary).and_return(service_path)
 
           service = Service.firefox
 
-          expect(service.instance_variable_get('@extra_args')).to be_empty
+          expect(service.instance_variable_get('@extra_args')).to eq(['--binary=/foo/bar'])
         end
 
         it 'uses provided args' do
@@ -97,7 +98,7 @@ module WebDriver
 
           service = Service.firefox(args: ['--foo', '--bar'])
 
-          expect(service.instance_variable_get('@extra_args')).to eq ['--foo', '--bar']
+          expect(service.instance_variable_get('@extra_args')).to include('--foo', '--bar')
         end
 
         # This is deprecated behavior
@@ -107,7 +108,7 @@ module WebDriver
           service = Service.firefox(args: {log: '/path/to/log',
                                            marionette_port: 4})
 
-          expect(service.instance_variable_get('@extra_args')).to eq ['--log=/path/to/log', '--marionette-port=4']
+          expect(service.instance_variable_get('@extra_args')).to include('--log=/path/to/log', '--marionette-port=4')
         end
       end
     end
@@ -119,6 +120,7 @@ module Firefox
 
         before do
           allow(Remote::Bridge).to receive(:new).and_return(bridge)
+          allow(Firefox::Binary).to receive(:path).and_return('/foo/bar')
         end
 
         it 'is not created when :url is provided' do
@@ -138,7 +140,7 @@ module Firefox
 
           expect(Service).to receive(:new).with(path: driver_path,
                                                 port: nil,
-                                                args: nil).and_return(service)
+                                                args: ['--binary=/foo/bar']).and_return(service)
 
           expect {
             described_class.new(driver_path: driver_path)
@@ -150,7 +152,7 @@ module Firefox
 
           expect(Service).to receive(:new).with(path: nil,
                                                 port: driver_port,
-                                                args: nil).and_return(service)
+                                                args: ['--binary=/foo/bar']).and_return(service)
 
           expect {
             described_class.new(port: driver_port)