Making Marionette the default for .NET FirefoxDriver in 3.0

Author: jimevans

dotnet/src/webdriver/Firefox/FirefoxDriver.cs

@@ -98,15 +98,11 @@ public class FirefoxDriver : RemoteWebDriver
         /// </summary>
         public static readonly bool AssumeUntrustedCertificateIssuer = true;
 
-        private FirefoxBinary binary;
-
-        private FirefoxProfile profile;
-
         /// <summary>
         /// Initializes a new instance of the <see cref="FirefoxDriver"/> class.
         /// </summary>
         public FirefoxDriver()
-            : this(new FirefoxBinary(), null)
+            : this(new FirefoxOptions(null, null))
         {
         }
 
@@ -116,7 +112,7 @@ public FirefoxDriver()
         /// <param name="profile">A <see cref="FirefoxProfile"/> object representing the profile settings
         /// to be used in starting Firefox.</param>
         public FirefoxDriver(FirefoxProfile profile)
-            : this(new FirefoxBinary(), profile)
+            : this(new FirefoxOptions(profile, null))
         {
         }
 
@@ -125,8 +121,9 @@ public FirefoxDriver(FirefoxProfile profile)
         /// </summary>
         /// <param name="capabilities">The <see cref="ICapabilities"/> object containing the desired
         /// capabilities of this FirefoxDriver.</param>
+        [Obsolete("FirefoxDriver should not be constructed with a raw ICapabilities or DesiredCapabilities object. Use FirefoxOptions instead. This constructor will be removed before the 3.0 release.")]
         public FirefoxDriver(ICapabilities capabilities)
-            : this(ExtractBinary(capabilities), ExtractProfile(capabilities), capabilities, RemoteWebDriver.DefaultCommandTimeout)
+            : this(CreateOptionsFromCapabilities(capabilities))
         {
         }
 
@@ -137,8 +134,9 @@ public FirefoxDriver(ICapabilities capabilities)
         /// environmental settings used when running Firefox.</param>
         /// <param name="profile">A <see cref="FirefoxProfile"/> object representing the profile settings
         /// to be used in starting Firefox.</param>
+        [Obsolete("FirefoxDriver should not be constructed with a FirefoxBinary object. Use FirefoxOptions instead. This constructor will be removed before the 3.0 release.")]
         public FirefoxDriver(FirefoxBinary binary, FirefoxProfile profile)
-            : this(binary, profile, RemoteWebDriver.DefaultCommandTimeout)
+            : this(new FirefoxOptions(profile, binary))
         {
         }
 
@@ -150,8 +148,9 @@ public FirefoxDriver(FirefoxBinary binary, FirefoxProfile profile)
         /// <param name="profile">A <see cref="FirefoxProfile"/> object representing the profile settings
         /// to be used in starting Firefox.</param>
         /// <param name="commandTimeout">The maximum amount of time to wait for each command.</param>
+        [Obsolete("FirefoxDriver should not be constructed  with a FirefoxBinary object. Use FirefoxOptions instead. This constructor will be removed before the 3.0 release.")]
         public FirefoxDriver(FirefoxBinary binary, FirefoxProfile profile, TimeSpan commandTimeout)
-            : this(binary, profile, DesiredCapabilities.Firefox(), commandTimeout)
+            : this(null, new FirefoxOptions(profile, binary), commandTimeout)
         {
         }
 
@@ -180,15 +179,8 @@ public FirefoxDriver(FirefoxDriverService service)
         /// <param name="options">The <see cref="FirefoxOptions"/> to be used with the Firefox driver.</param>
         /// <param name="commandTimeout">The maximum amount of time to wait for each command.</param>
         public FirefoxDriver(FirefoxDriverService service, FirefoxOptions options, TimeSpan commandTimeout)
-            : base(new DriverServiceCommandExecutor(service, commandTimeout), ConvertOptionsToCapabilities(options))
-        {
-        }
-
-        private FirefoxDriver(FirefoxBinary binary, FirefoxProfile profile, ICapabilities capabilities, TimeSpan commandTimeout)
-            : base(CreateExtensionConnection(binary, profile, commandTimeout), RemoveUnneededCapabilities(capabilities))
+            : base(CreateExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
         {
-            this.binary = binary;
-            this.profile = profile;
         }
 
         /// <summary>
@@ -218,22 +210,6 @@ public bool IsMarionette
             get { return this.IsSpecificationCompliant; }
         }
 
-        /// <summary>
-        /// Gets the FirefoxBinary and its details for subclasses
-        /// </summary>
-        protected FirefoxBinary Binary
-        {
-            get { return this.binary; }
-        }
-
-        /// <summary>
-        /// Gets the FirefoxProfile that is currently in use by subclasses
-        /// </summary>
-        protected FirefoxProfile Profile
-        {
-            get { return this.profile; }
-        }
-
         /// <summary>
         /// In derived classes, the <see cref="PrepareEnvironment"/> method prepares the environment for test execution.
         /// </summary>
@@ -252,6 +228,98 @@ protected override RemoteWebElement CreateElement(string elementId)
             return new FirefoxWebElement(this, elementId);
         }
 
+        private static ICommandExecutor CreateExecutor(FirefoxDriverService service, FirefoxOptions options, TimeSpan commandTimeout)
+        {
+            ICommandExecutor executor = null;
+            if (options.UseLegacyImplementation)
+            {
+                // Note: If BrowserExecutableLocation is null or empty, the legacy driver
+                // will still do the right thing, and find Firefox in the default location.
+                FirefoxBinary binary = new FirefoxBinary(options.BrowserExecutableLocation);
+
+                FirefoxProfile profile = options.Profile;
+                if (profile == null)
+                {
+                    profile = new FirefoxProfile();
+                }
+
+                executor = CreateExtensionConnection(binary, profile, commandTimeout);
+            }
+            else
+            {
+                if (service == null)
+                {
+                    throw new ArgumentNullException("service", "You requested a service-based implementation, but passed in a null service object.");
+                }
+
+                return new DriverServiceCommandExecutor(service, commandTimeout);
+            }
+
+            return executor;
+        }
+
+        private static ICommandExecutor CreateExtensionConnection(FirefoxBinary binary, FirefoxProfile profile, TimeSpan commandTimeout)
+        {
+            FirefoxProfile profileToUse = profile;
+
+            string suggestedProfile = Environment.GetEnvironmentVariable("webdriver.firefox.profile");
+            if (profileToUse == null && suggestedProfile != null)
+            {
+                profileToUse = new FirefoxProfileManager().GetProfile(suggestedProfile);
+            }
+            else if (profileToUse == null)
+            {
+                profileToUse = new FirefoxProfile();
+            }
+
+            FirefoxDriverCommandExecutor executor = new FirefoxDriverCommandExecutor(binary, profileToUse, "localhost", commandTimeout);
+            return executor;
+        }
+
+        private static ICapabilities ConvertOptionsToCapabilities(FirefoxOptions options)
+        {
+            if (options == null)
+            {
+                throw new ArgumentNullException("options", "options must not be null");
+            }
+
+            ICapabilities capabilities = options.ToCapabilities();
+            if (options.UseLegacyImplementation)
+            {
+                capabilities = RemoveUnneededCapabilities(capabilities);
+            }
+
+            return capabilities;
+        }
+
+        private static ICapabilities RemoveUnneededCapabilities(ICapabilities capabilities)
+        {
+            DesiredCapabilities caps = capabilities as DesiredCapabilities;
+            caps.CapabilitiesDictionary.Remove(FirefoxDriver.ProfileCapabilityName);
+            caps.CapabilitiesDictionary.Remove(FirefoxDriver.BinaryCapabilityName);
+            return caps;
+        }
+
+        private static FirefoxOptions CreateOptionsFromCapabilities(ICapabilities capabilities)
+        {
+            // This is awkward and hacky. To be removed when the legacy driver is retired.
+            FirefoxBinary binary = ExtractBinary(capabilities);
+            FirefoxProfile profile = ExtractProfile(capabilities);
+            DesiredCapabilities desiredCaps = RemoveUnneededCapabilities(capabilities) as DesiredCapabilities;
+
+            FirefoxOptions options = new FirefoxOptions(profile, binary);
+            if (desiredCaps != null)
+            {
+                Dictionary<string, object> capsDictionary = desiredCaps.ToDictionary();
+                foreach (KeyValuePair<string, object> capability in capsDictionary)
+                {
+                    options.AddAdditionalCapability(capability.Key, capability.Value);
+                }
+            }
+
+            return options;
+        }
+
         private static FirefoxBinary ExtractBinary(ICapabilities capabilities)
         {
             if (capabilities.GetCapability(BinaryCapabilityName) != null)
@@ -314,41 +382,5 @@ private static FirefoxProfile ExtractProfile(ICapabilities capabilities)
 
             return profile;
         }
-
-        private static ICommandExecutor CreateExtensionConnection(FirefoxBinary binary, FirefoxProfile profile, TimeSpan commandTimeout)
-        {
-            FirefoxProfile profileToUse = profile;
-
-            string suggestedProfile = Environment.GetEnvironmentVariable("webdriver.firefox.profile");
-            if (profileToUse == null && suggestedProfile != null)
-            {
-                profileToUse = new FirefoxProfileManager().GetProfile(suggestedProfile);
-            }
-            else if (profileToUse == null)
-            {
-                profileToUse = new FirefoxProfile();
-            }
-
-            FirefoxDriverCommandExecutor executor = new FirefoxDriverCommandExecutor(binary, profileToUse, "localhost", commandTimeout);
-            return executor;
-        }
-
-        private static ICapabilities RemoveUnneededCapabilities(ICapabilities capabilities)
-        {
-            DesiredCapabilities caps = capabilities as DesiredCapabilities;
-            caps.CapabilitiesDictionary.Remove(FirefoxDriver.ProfileCapabilityName);
-            caps.CapabilitiesDictionary.Remove(FirefoxDriver.BinaryCapabilityName);
-            return caps;
-        }
-
-        private static ICapabilities ConvertOptionsToCapabilities(FirefoxOptions options)
-        {
-            if (options == null)
-            {
-                throw new ArgumentNullException("options", "options must not be null");
-            }
-
-            return options.ToCapabilities();
-        }
     }
 }

dotnet/src/webdriver/Firefox/FirefoxDriverService.cs

@@ -31,7 +31,7 @@ public sealed class FirefoxDriverService : DriverService
     {
         private const string FirefoxDriverServiceFileName = "geckodriver.exe";
         private static readonly Uri FirefoxDriverDownloadUrl = new Uri("https://github.com/mozilla/geckodriver/releases");
-        private string browserBinaryPath = @"C:\Program Files (x86)\Nightly\firefox.exe";
+        private string browserBinaryPath = string.Empty;
         private int browserCommunicationPort = -1;
 
         /// <summary>

dotnet/src/webdriver/Firefox/FirefoxOptions.cs

@@ -17,6 +17,8 @@
 // </copyright>
 
 using System;
+using System.Collections.Generic;
+using System.Globalization;
 using OpenQA.Selenium.Remote;
 
 namespace OpenQA.Selenium.Firefox
@@ -46,17 +48,79 @@ namespace OpenQA.Selenium.Firefox
     /// </example>
     public class FirefoxOptions : DriverOptions
     {
+        private const string IsMarionetteCapability = "marionette";
+        private const string FirefoxProfileCapability = "firefox_profile";
+        private const string FirefoxBinaryCapability = "firefox_binary";
+
         private bool isMarionette = true;
+        private string browserBinaryLocation;
+        private FirefoxProfile profile;
+        private Dictionary<string, object> additionalCapabilities = new Dictionary<string, object>();
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="FirefoxOptions"/> class.
+        /// </summary>
+        public FirefoxOptions()
+            : base()
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="FirefoxOptions"/> class for the given profile and binary.
+        /// </summary>
+        /// <param name="profile">The <see cref="FirefoxProfile"/> to use in the options.</param>
+        /// <param name="binary">The <see cref="FirefoxBinary"/> to use in the options.</param>
+        internal FirefoxOptions(FirefoxProfile profile, FirefoxBinary binary)
+        {
+            if (profile != null)
+            {
+                this.profile = profile;
+            }
+
+            if (binary != null)
+            {
+                this.browserBinaryLocation = binary.BinaryExecutable.ExecutablePath;
+            }
+        }
 
         /// <summary>
         /// Gets or sets a value indicating whether or not to use the Mozilla-provided Marionette implementation.
+        /// Defaults to <see langword="true"/>.
         /// </summary>
+        [Obsolete("Use the UseLegacyImplementation property instead. This property will be removed before the 3.0 release.")]
         public bool IsMarionette
         {
             get { return this.isMarionette; }
             set { this.isMarionette = value; }
         }
 
+        /// <summary>
+        /// Gets or sets a value indicating whether to use the legacy driver implementation.
+        /// </summary>
+        public bool UseLegacyImplementation
+        {
+            get { return !this.isMarionette; }
+            set { this.isMarionette = !value; }
+        }
+
+        /// <summary>
+        /// Gets or sets the <see cref="FirefoxProfile"/> object to be used with this instance.
+        /// </summary>
+        public FirefoxProfile Profile
+        {
+            get { return this.profile; }
+            set { this.profile = value; }
+        }
+
+        /// <summary>
+        /// Gets or sets the path and file name of the Firefox browser executable.
+        /// </summary>
+        public string BrowserExecutableLocation
+        {
+            get { return this.browserBinaryLocation; }
+            set { this.browserBinaryLocation = value; }
+        }
+
         /// <summary>
         /// Provides a means to add additional capabilities not yet added as type safe options
         /// for the Firefox driver.
@@ -72,6 +136,20 @@ public bool IsMarionette
         /// change in the future.</remarks>
         public override void AddAdditionalCapability(string capabilityName, object capabilityValue)
         {
+            if (capabilityName == IsMarionetteCapability ||
+                capabilityName == FirefoxProfileCapability ||
+                capabilityName == FirefoxBinaryCapability)
+            {
+                string message = string.Format(CultureInfo.InvariantCulture, "There is already an option for the {0} capability. Please use that instead.", capabilityName);
+                throw new ArgumentException(message, "capabilityName");
+            }
+
+            if (string.IsNullOrEmpty(capabilityName))
+            {
+                throw new ArgumentException("Capability name may not be null an empty string.", "capabilityName");
+            }
+
+            this.additionalCapabilities[capabilityName] = capabilityValue;
         }
 
         /// <summary>
@@ -83,7 +161,27 @@ public override void AddAdditionalCapability(string capabilityName, object capab
         public override ICapabilities ToCapabilities()
         {
             DesiredCapabilities capabilities = DesiredCapabilities.Firefox();
-            capabilities.SetCapability("marionette", this.isMarionette);
+            capabilities.SetCapability(IsMarionetteCapability, this.isMarionette);
+
+            if (this.profile != null)
+            {
+                capabilities.SetCapability(FirefoxProfileCapability, this.profile.ToBase64String());
+            }
+
+            if (!string.IsNullOrEmpty(this.browserBinaryLocation))
+            {
+                capabilities.SetCapability(FirefoxBinaryCapability, this.browserBinaryLocation);
+            }
+            else
+            {
+                capabilities.SetCapability(FirefoxBinaryCapability, new FirefoxBinary().BinaryExecutable.ExecutablePath);
+            }
+
+            foreach (KeyValuePair<string, object> pair in this.additionalCapabilities)
+            {
+                capabilities.SetCapability(pair.Key, pair.Value);
+            }
+
             return capabilities;
         }
     }