[java] Checking for changes in frozen preferences in XPI-based Firefo…

…xDriver only
SeleniumHQ · Sep 26, 2019 · 90dd53c · 90dd53c
1 parent d9aa681
commit 90dd53c
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 16 deletions.
diff --git a/java/client/src/org/openqa/selenium/firefox/FirefoxProfile.java b/java/client/src/org/openqa/selenium/firefox/FirefoxProfile.java
@@ -380,6 +380,10 @@ public void cleanTemporaryModel() {
     clean(model);
   }
 
+  public void checkForChangesInFrozenPreferences() {
+    additionalPrefs.checkForChangesInFrozenPreferences();
+  }
+
   /**
    * Call this to cause the current profile to be written to disk. The profile directory is
    * returned. Note that this profile directory is a temporary one and will be deleted when the JVM

diff --git a/java/client/src/org/openqa/selenium/firefox/Preferences.java b/java/client/src/org/openqa/selenium/firefox/Preferences.java
@@ -17,8 +17,8 @@
 
 package org.openqa.selenium.firefox;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static org.openqa.selenium.json.Json.MAP_TYPE;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -142,7 +142,6 @@ private void readPreferences(Reader reader) throws IOException {
   }
 
   public void setPreference(String key, String value) {
-    checkPreference(key, value);
     if (isStringified(value)) {
       throw new IllegalArgumentException(
           String.format("Preference values must be plain strings: %s: %s",
@@ -152,12 +151,10 @@ public void setPreference(String key, String value) {
   }
 
   public void setPreference(String key, boolean value) {
-    checkPreference(key, value);
     allPrefs.put(key, value);
   }
 
   public void setPreference(String key, int value) {
-    checkPreference(key, value);
     allPrefs.put(key, value);
   }
 
@@ -220,9 +217,13 @@ public void putAll(Map<String, Object> frozenPreferences) {
     allPrefs.putAll(frozenPreferences);
   }
 
+  void checkForChangesInFrozenPreferences() {
+    allPrefs.forEach((this::checkPreference));
+  }
+
   private void checkPreference(String key, Object value) {
     checkNotNull(value);
-    checkArgument(!immutablePrefs.containsKey(key) ||
+    checkState(!immutablePrefs.containsKey(key) ||
                   (immutablePrefs.containsKey(key) && value.equals(immutablePrefs.get(key))),
                   "Preference %s may not be overridden: frozen value=%s, requested value=%s",
                   key, immutablePrefs.get(key), value);
@@ -233,10 +234,10 @@ private void checkPreference(String key, Object value) {
       } else if (value instanceof Integer) {
         n = (Integer) value;
       } else {
-        throw new IllegalArgumentException(String.format(
+        throw new IllegalStateException(String.format(
             "%s value must be a number: %s", MAX_SCRIPT_RUN_TIME_KEY, value.getClass().getName()));
       }
-      checkArgument(n == 0 || n >= DEFAULT_MAX_SCRIPT_RUN_TIME,
+      checkState(n == 0 || n >= DEFAULT_MAX_SCRIPT_RUN_TIME,
                     "%s must be == 0 || >= %s",
                     MAX_SCRIPT_RUN_TIME_KEY,
                     DEFAULT_MAX_SCRIPT_RUN_TIME);

diff --git a/java/client/src/org/openqa/selenium/firefox/xpi/XpiDriverService.java b/java/client/src/org/openqa/selenium/firefox/xpi/XpiDriverService.java
@@ -123,6 +123,7 @@ public void start() throws IOException {
     try {
       profile.setPreference(PORT_PREFERENCE, port);
       addWebDriverExtension(profile);
+      profile.checkForChangesInFrozenPreferences();
       profileDir = profile.layoutOnDisk();
 
       ImmutableMap.Builder<String, String> envBuilder = new ImmutableMap.Builder<String, String>()

diff --git a/java/client/test/org/openqa/selenium/firefox/FirefoxProfileTest.java b/java/client/test/org/openqa/selenium/firefox/FirefoxProfileTest.java
@@ -18,6 +18,7 @@
 package org.openqa.selenium.firefox;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.junit.Assert.fail;
 
 import org.junit.Before;
@@ -134,16 +135,17 @@ public void shouldSetDefaultPreferences() throws Exception {
   }
 
   @Test
+  public void shouldAllowSettingFrozenPreferences() throws Exception {
+    profile.setPreference("network.http.phishy-userpass-length", 1024);
+    assertPreferenceValueEquals("network.http.phishy-userpass-length", 1024);
+  }
 
-  public void shouldNotResetFrozenPreferences() throws Exception {
-    try {
-      profile.setPreference("network.http.phishy-userpass-length", 1024);
-      fail("Should not be able to reset a frozen preference");
-    } catch (IllegalArgumentException ex) {
-      // expected
-    }
-
-    assertPreferenceValueEquals("network.http.phishy-userpass-length", 255);
+  @Test
+  public void shouldAllowCheckingForChangesInFrozenPreferences() throws Exception {
+    profile.setPreference("network.http.phishy-userpass-length", 1024);
+    assertThatExceptionOfType(IllegalStateException.class).isThrownBy(
+        () -> profile.checkForChangesInFrozenPreferences()
+    ).withMessageContaining("network.http.phishy-userpass-length");
   }
 
   @Test