From 0cedb9ad97f54dc2f23f42645fa54f3465de6889 Mon Sep 17 00:00:00 2001
From: Andreas Tolfsen <ato@sny.no>
Date: Fri, 27 Jul 2018 14:17:59 +0100
Subject: [PATCH] Bug 1478979 - Disallow pageLoadStrategy to be null. r=whimboo

Capabilities matching is now done in geckodriver and Marionette
receives the negotiated set of capabilities, so there is no need
to derive default values in case the value is null.
---
 testing/marionette/capabilities.js            | 31 ++++++-------------
 .../tests/unit/test_capabilities.py           |  7 +----
 .../marionette/test/unit/test_capabilities.js |  1 +
 3 files changed, 12 insertions(+), 27 deletions(-)

diff --git a/testing/marionette/capabilities.js b/testing/marionette/capabilities.js
index 4edbda4d8ac3..c00699c06db0 100644
--- a/testing/marionette/capabilities.js
+++ b/testing/marionette/capabilities.js
@@ -464,24 +464,17 @@ class Capabilities extends Map {
     for (let [k, v] of Object.entries(json)) {
       switch (k) {
         case "acceptInsecureCerts":
-          assert.boolean(v,
-              pprint`Expected ${k} to be a boolean, got ${v}`);
+          assert.boolean(v, pprint`Expected ${k} to be a boolean, got ${v}`);
           matched.set("acceptInsecureCerts", v);
           break;
 
         case "pageLoadStrategy":
-          if (v === null) {
-            matched.set("pageLoadStrategy", PageLoadStrategy.Normal);
-          } else {
-            assert.string(v,
-                pprint`Expected ${k} to be a string, got ${v}`);
+          assert.string(v, pprint`Expected ${k} to be a string, got ${v}`);
 
-            if (Object.values(PageLoadStrategy).includes(v)) {
-              matched.set("pageLoadStrategy", v);
-            } else {
-              throw new InvalidArgumentError(
-                  "Unknown page load strategy: " + v);
-            }
+          if (Object.values(PageLoadStrategy).includes(v)) {
+            matched.set("pageLoadStrategy", v);
+          } else {
+            throw new InvalidArgumentError("Unknown page load strategy: " + v);
           }
 
           break;
@@ -497,8 +490,7 @@ class Capabilities extends Map {
           break;
 
         case "unhandledPromptBehavior":
-          assert.string(v,
-              pprint`Expected ${k} to be a string, got ${v}`);
+          assert.string(v, pprint`Expected ${k} to be a string, got ${v}`);
 
           if (Object.values(UnhandledPromptBehavior).includes(v)) {
             matched.set("unhandledPromptBehavior", v);
@@ -510,20 +502,17 @@ class Capabilities extends Map {
           break;
 
         case "moz:accessibilityChecks":
-          assert.boolean(v,
-              pprint`Expected ${k} to be a boolean, got ${v}`);
+          assert.boolean(v, pprint`Expected ${k} to be a boolean, got ${v}`);
           matched.set("moz:accessibilityChecks", v);
           break;
 
         case "moz:useNonSpecCompliantPointerOrigin":
-          assert.boolean(v,
-              pprint`Expected ${k} to be a boolean, got ${v}`);
+          assert.boolean(v, pprint`Expected ${k} to be a boolean, got ${v}`);
           matched.set("moz:useNonSpecCompliantPointerOrigin", v);
           break;
 
         case "moz:webdriverClick":
-          assert.boolean(v,
-              pprint`Expected ${k} to be a boolean, got ${v}`);
+          assert.boolean(v, pprint`Expected ${k} to be a boolean, got ${v}`);
           matched.set("moz:webdriverClick", v);
           break;
       }
diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py b/testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py
index dc06bc05bac6..d003c2f52bcf 100644
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py
@@ -143,12 +143,7 @@ class TestCapabilityMatching(MarionetteTestCase):
             self.marionette.start_session({"pageLoadStrategy": strategy})
             self.assertEqual(self.marionette.session_capabilities["pageLoadStrategy"], strategy)
 
-        # A null value should be treatend as "normal"
-        self.delete_session()
-        self.marionette.start_session({"pageLoadStrategy": None})
-        self.assertEqual(self.marionette.session_capabilities["pageLoadStrategy"], "normal")
-
-        for value in ["", "EAGER", True, 42, {}, []]:
+        for value in ["", "EAGER", True, 42, {}, [], None]:
             print("invalid strategy {}".format(value))
             with self.assertRaisesRegexp(SessionNotCreatedException, "InvalidArgumentError"):
                 self.marionette.start_session({"pageLoadStrategy": value})
diff --git a/testing/marionette/test/unit/test_capabilities.js b/testing/marionette/test/unit/test_capabilities.js
index 763a31591f0f..39b6b06955e7 100644
--- a/testing/marionette/test/unit/test_capabilities.js
+++ b/testing/marionette/test/unit/test_capabilities.js
@@ -456,6 +456,7 @@ add_test(function test_Capabilities_fromJSON() {
     equal(strategy, caps.get("pageLoadStrategy"));
   }
   Assert.throws(() => fromJSON({pageLoadStrategy: "foo"}), InvalidArgumentError);
+  Assert.throws(() => fromJSON({pageLoadStrategy: null}), InvalidArgumentError);
 
   let proxyConfig = {proxyType: "manual"};
   caps = fromJSON({proxy: proxyConfig});