FirefoxOptions now picks up System property set values

Specifically for the profile, binary, and whether to use
marionette.

Closes #3649

Author: shs96c

java/client/src/org/openqa/selenium/firefox/FirefoxOptions.java

@@ -82,7 +82,7 @@ public class FirefoxOptions {
   private Map<String, Integer> intPrefs = new HashMap<>();
   private Map<String, String> stringPrefs = new HashMap<>();
   private Level logLevel = null;
-  private Boolean legacy;
+  private boolean legacy;
   private DesiredCapabilities desiredCapabilities = new DesiredCapabilities();
   private DesiredCapabilities requiredCapabilities = new DesiredCapabilities();
 
@@ -143,6 +143,31 @@ private static <T> T getOption(Map<String, Object> map, String key, Class<T> typ
             "In FirefoxOptions, expected key '%s' to be a %s: %s", key, type.getSimpleName(), map));
   }
 
+  public FirefoxOptions() {
+    // Read system properties and use those if they are set, allowing users to override them later
+    // should they want to.
+
+    String binary = System.getProperty(FirefoxDriver.SystemProperty.BROWSER_BINARY);
+    if (binary != null) {
+      setBinary(binary);
+    }
+
+    String forceMarionette = System.getProperty(FirefoxDriver.SystemProperty.DRIVER_USE_MARIONETTE);
+    if (forceMarionette != null) {
+      setLegacy(!Boolean.getBoolean(FirefoxDriver.SystemProperty.DRIVER_USE_MARIONETTE));
+    }
+
+    String profileName = System.getProperty(FirefoxDriver.SystemProperty.BROWSER_PROFILE);
+    if (profileName != null) {
+      this.profile = new ProfilesIni().getProfile(profileName);
+      if (this.profile == null) {
+        throw new WebDriverException(String.format(
+            "Firefox profile '%s' named in system property '%s' not found",
+            profileName, FirefoxDriver.SystemProperty.BROWSER_PROFILE));
+      }
+    }
+  }
+
   public FirefoxOptions setLegacy(boolean legacy) {
     this.legacy = legacy;
     desiredCapabilities.setCapability(MARIONETTE, !legacy);
@@ -151,15 +176,7 @@ public FirefoxOptions setLegacy(boolean legacy) {
   }
 
   public boolean isLegacy() {
-    String forceMarionette = System.getProperty(FirefoxDriver.SystemProperty.DRIVER_USE_MARIONETTE);
-    if (forceMarionette != null) {
-      return !Boolean.valueOf(forceMarionette);
-    }
-    if (legacy != null) {
-      return legacy;
-    }
-
-    return false;
+    return legacy;
   }
 
   public FirefoxOptions setBinary(FirefoxBinary binary) {
@@ -256,6 +273,10 @@ public FirefoxOptions setProfile(FirefoxProfile profile) {
   }
 
   public FirefoxProfile getProfile() {
+    return getProfileOrNull().orElseGet(() -> fullyPopulateProfile(new FirefoxProfile()));
+  }
+
+  public Optional<FirefoxProfile> getProfileOrNull() {
     FirefoxProfile profileToUse = profile;
     if (profileToUse == null) {
       profileToUse = extractProfile(requiredCapabilities);
@@ -264,29 +285,21 @@ public FirefoxProfile getProfile() {
       profileToUse = extractProfile(desiredCapabilities);
     }
     if (profileToUse == null) {
-      String suggestedProfile = System.getProperty(FirefoxDriver.SystemProperty.BROWSER_PROFILE);
-      if (suggestedProfile != null) {
-        profileToUse = new ProfilesIni().getProfile(suggestedProfile);
-        if (profileToUse == null) {
-          throw new WebDriverException(String.format(
-              "Firefox profile '%s' named in system property '%s' not found",
-              suggestedProfile, FirefoxDriver.SystemProperty.BROWSER_PROFILE));
-        }
-      }
-    }
-    if (profileToUse == null) {
-      profileToUse = new FirefoxProfile();
+      return Optional.empty();
     }
 
-    populateProfile(profileToUse, desiredCapabilities);
-    populateProfile(profileToUse, requiredCapabilities);
+    return Optional.of(fullyPopulateProfile(profileToUse));
+  }
+
+  private FirefoxProfile fullyPopulateProfile(FirefoxProfile profile) {
+    populateProfile(profile, desiredCapabilities);
+    populateProfile(profile, requiredCapabilities);
 
-    FirefoxProfile prefHolder = profileToUse;
-    booleanPrefs.entrySet().forEach(pref -> prefHolder.setPreference(pref.getKey(), pref.getValue()));
-    intPrefs.entrySet().forEach(pref -> prefHolder.setPreference(pref.getKey(), pref.getValue()));
-    stringPrefs.entrySet().forEach(pref -> prefHolder.setPreference(pref.getKey(), pref.getValue()));
+    booleanPrefs.entrySet().forEach(pref -> profile.setPreference(pref.getKey(), pref.getValue()));
+    intPrefs.entrySet().forEach(pref -> profile.setPreference(pref.getKey(), pref.getValue()));
+    stringPrefs.entrySet().forEach(pref -> profile.setPreference(pref.getKey(), pref.getValue()));
 
-    return profileToUse;
+    return profile;
   }
 
   private static void populateProfile(FirefoxProfile profile, Capabilities capabilities) {

java/client/src/org/openqa/selenium/firefox/internal/ProfilesIni.java

@@ -109,8 +109,9 @@ protected File newProfile(String name, File appData, String path, boolean isRela
 
   public FirefoxProfile getProfile(String profileName) {
     File profileDir = profiles.get(profileName);
-    if (profileDir == null)
+    if (profileDir == null) {
       return null;
+    }
 
     // Make a copy of the profile to use
     File tempDir = TemporaryFilesystem.getDefaultTmpFS().createTempDir("userprofile", "copy");

java/client/test/org/openqa/selenium/firefox/FirefoxOptionsTest.java

@@ -17,17 +17,36 @@
 
 package org.openqa.selenium.firefox;
 
+import static java.nio.file.StandardOpenOption.DELETE_ON_CLOSE;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeNotNull;
+import static org.junit.Assume.assumeThat;
+import static org.openqa.selenium.firefox.FirefoxDriver.SystemProperty.BROWSER_BINARY;
+import static org.openqa.selenium.firefox.FirefoxDriver.SystemProperty.BROWSER_PROFILE;
+import static org.openqa.selenium.firefox.FirefoxDriver.SystemProperty.DRIVER_USE_MARIONETTE;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.gson.JsonObject;
 
 import org.junit.Test;
 import org.openqa.selenium.Capabilities;
+import org.openqa.selenium.WebDriverException;
+import org.openqa.selenium.firefox.internal.ProfilesIni;
 import org.openqa.selenium.remote.DesiredCapabilities;
+import org.openqa.selenium.testing.JreSystemProperty;
 
 import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.attribute.PosixFilePermission;
+import java.util.Optional;
 
 public class FirefoxOptionsTest {
 
@@ -72,4 +91,82 @@ public void pathBasedBinaryRemainsAbsoluteIfSetAsAbsolute() throws IOException {
 
     assertEquals("/i/like/cheese", json.getAsJsonPrimitive("binary").getAsString());
   }
+
+  @Test
+  public void shouldPickUpBinaryFromSystemPropertyIfSet() throws IOException {
+    JreSystemProperty property = new JreSystemProperty(BROWSER_BINARY);
+    String resetValue = property.get();
+
+    Path binary = Files.createTempFile("firefox", ".exe");
+    try (OutputStream ignored = Files.newOutputStream(binary, DELETE_ON_CLOSE)) {
+      Files.write(binary, "".getBytes());
+      Files.setPosixFilePermissions(binary, ImmutableSet.of(PosixFilePermission.OWNER_EXECUTE));
+      property.set(binary.toString());
+      FirefoxOptions options = new FirefoxOptions();
+
+      FirefoxBinary firefoxBinary =
+          options.getBinaryOrNull().orElseThrow(() -> new AssertionError("No binary"));
+
+      assertEquals(binary.toString(), firefoxBinary.getPath());
+    } finally {
+      property.set(resetValue);
+    }
+  }
+
+  @Test
+  public void shouldPickUpLegacyValueFromSystemProperty() throws IOException {
+    JreSystemProperty property = new JreSystemProperty(DRIVER_USE_MARIONETTE);
+    String resetValue = property.get();
+
+    try {
+      // No value should default to using Marionette
+      property.set(null);
+      FirefoxOptions options = new FirefoxOptions();
+      assertFalse(options.isLegacy());
+
+      property.set("false");
+      options = new FirefoxOptions();
+      assertTrue(options.isLegacy());
+
+      property.set("true");
+      options = new FirefoxOptions();
+      assertFalse(options.isLegacy());
+    } finally {
+      property.set(resetValue);
+    }
+  }
+
+  @Test
+  public void shouldPickUpProfileFromSystemProperty() throws IOException {
+    FirefoxProfile defaultProfile = new ProfilesIni().getProfile("default");
+    assumeNotNull(defaultProfile);
+
+    JreSystemProperty property = new JreSystemProperty(BROWSER_PROFILE);
+    String resetValue = property.get();
+    try {
+      property.set("default");
+      FirefoxOptions options = new FirefoxOptions();
+      Optional<FirefoxProfile> profile = options.getProfileOrNull();
+
+      assertTrue(profile.isPresent());
+    } finally {
+      property.set(resetValue);
+    }
+  }
+
+  @Test(expected = WebDriverException.class)
+  public void shouldThrowAnExceptionIfSystemPropertyProfileDoesNotExist() {
+    String unlikelyProfileName = "this-profile-does-not-exist-also-cheese";
+    FirefoxProfile foundProfile = new ProfilesIni().getProfile(unlikelyProfileName);
+    assumeThat(foundProfile, is(nullValue()));
+
+    JreSystemProperty property = new JreSystemProperty(BROWSER_PROFILE);
+    String resetValue = property.get();
+    try {
+      property.set(unlikelyProfileName);
+      new FirefoxOptions();
+    } finally {
+      property.set(resetValue);
+    }
+  }
 }

java/client/test/org/openqa/selenium/testing/BUCK

@@ -22,6 +22,7 @@ java_library(name = 'helpers',
   srcs = [
     'DevMode.java',
     'InProject.java',
+    'JreSystemProperty.java',
   ],
   deps = [
     '//java/client/src/org/openqa/selenium:exceptions',

java/client/test/org/openqa/selenium/testing/JreSystemProperty.java

@@ -0,0 +1,41 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.selenium.testing;
+
+import com.google.common.base.Preconditions;
+
+public class JreSystemProperty {
+
+  private String name;
+
+  public JreSystemProperty(String name) {
+    this.name = Preconditions.checkNotNull(name);
+  }
+
+  public String get() {
+    return System.getProperty(name);
+  }
+
+  public void set(String value) {
+    if (value == null) {
+      System.clearProperty(name);
+    } else {
+      System.setProperty(name, value);
+    }
+  }
+}