Bug 1410652 - Use invalid argument error for web element deserialisation. r=whimboo draft
authorAndreas Tolfsen <ato@sny.no>
Tue, 24 Oct 2017 17:42:32 +0100
changeset 689635 6475a04bb7709072849547ffc27bd604ec678b07
parent 689634 5fccbd53ab96849df2aece2679db2df84aced804
child 689636 e7a842bb69847b936fe449fc8985ceb60e533f17
push id87065
push userbmo:ato@sny.no
push dateTue, 31 Oct 2017 20:11:32 +0000
reviewerswhimboo
bugs1410652
milestone58.0a1
Bug 1410652 - Use invalid argument error for web element deserialisation. r=whimboo For user input we will want to return the appropriate invalid argument error. For internal input using TypeError is fine. MozReview-Commit-ID: AlOnZuhaczN
testing/marionette/element.js
testing/marionette/test_element.js
--- a/testing/marionette/element.js
+++ b/testing/marionette/element.js
@@ -5,16 +5,17 @@
 "use strict";
 /* global XPCNativeWrapper */
 
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("chrome://marionette/content/assert.js");
 Cu.import("chrome://marionette/content/atom.js");
 const {
+  InvalidArgumentError,
   InvalidSelectorError,
   NoSuchElementError,
   StaleElementReferenceError,
 } = Cu.import("chrome://marionette/content/error.js", {});
 const {pprint} = Cu.import("chrome://marionette/content/format.js", {});
 const {PollPromise} = Cu.import("chrome://marionette/content/sync.js", {});
 
 this.EXPORTED_SYMBOLS = [
@@ -1240,17 +1241,17 @@ class WebElement {
    * <code>WindowProxy</code>, or XUL element.
    *
    * @param {(Element|WindowProxy|XULElement)} node
    *     Node to construct a web element reference for.
    *
    * @return {(ContentWebElement|ChromeWebElement)}
    *     Web element reference for <var>el</var>.
    *
-   * @throws {TypeError}
+   * @throws {InvalidArgumentError}
    *     If <var>node</var> is neither a <code>WindowProxy</code>,
    *     DOM element, or a XUL element.
    */
   static from(node) {
     const uuid = WebElement.generateUUID();
 
     if (element.isDOMElement(node) || element.isSVGElement(node)) {
       return new ContentWebElement(uuid);
@@ -1258,43 +1259,39 @@ class WebElement {
       if (node.parent === node) {
         return new ContentWebWindow(uuid);
       }
       return new ContentWebFrame(uuid);
     } else if (element.isXULElement(node)) {
       return new ChromeWebElement(uuid);
     }
 
-    throw new TypeError("Expected DOM window/element " +
+    throw new InvalidArgumentError("Expected DOM window/element " +
         pprint`or XUL element, got: ${node}`);
   }
 
   /**
    * Unmarshals a JSON Object to one of {@link ContentWebElement},
    * {@link ContentWebWindow}, {@link ContentWebFrame}, or
    * {@link ChromeWebElement}.
    *
    * @param {Object.<string, string>} json
    *     Web element reference, which is supposed to be a JSON Object
    *     where the key is one of the {@link WebElement} concrete
    *     classes' UUID identifiers.
    *
    * @return {WebElement}
    *     Representation of the web element.
    *
-   * @throws {TypeError}
+   * @throws {InvalidArgumentError}
    *     If <var>json</var> is not a web element reference.
    */
   static fromJSON(json) {
-    let keys = [];
-    try {
-      keys = Object.keys(json);
-    } catch (e) {
-      throw new TypeError(`Expected JSON Object: ${e}`);
-    }
+    assert.object(json);
+    let keys = Object.keys(json);
 
     for (let key of keys) {
       switch (key) {
         case ContentWebElement.Identifier:
         case ContentWebElement.LegacyIdentifier:
           return ContentWebElement.fromJSON(json);
 
         case ContentWebWindow.Identifier:
@@ -1303,17 +1300,17 @@ class WebElement {
         case ContentWebFrame.Identifier:
           return ContentWebFrame.fromJSON(json);
 
         case ChromeWebElement.Identifier:
           return ChromeWebElement.fromJSON(json);
       }
     }
 
-    throw new TypeError(
+    throw new InvalidArgumentError(
         pprint`Expected web element reference, got: ${json}`);
   }
 
   /**
    * Constructs a {@link ContentWebElement} or {@link ChromeWebElement}
    * from a a string <var>uuid</var>.
    *
    * This whole function is a workaround for the fact that clients
@@ -1328,32 +1325,31 @@ class WebElement {
    *     Context, which is used to determine if the returned type
    *     should be a content web element or a chrome web element.
    *
    * @return {WebElement}
    *     One of {@link ContentWebElement} or {@link ChromeWebElement},
    *     based on <var>context</var>.
    *
    * @throws {InvalidArgumentError}
-   *     If <var>uuid</var> is not a string.
-   * @throws {TypeError}
-   *     If <var>context</var> is an invalid context.
+   *     If <var>uuid</var> is not a string or <var>context</var>
+   *     is an invalid context.
    */
   static fromUUID(uuid, context) {
     assert.string(uuid);
 
     switch (context) {
       case "chrome":
         return new ChromeWebElement(uuid);
 
       case "content":
         return new ContentWebElement(uuid);
 
       default:
-        throw new TypeError("Unknown context: " + context);
+        throw new InvalidArgumentError("Unknown context: " + context);
     }
   }
 
   /**
    * Checks if <var>ref<var> is a {@link WebElement} reference,
    * i.e. if it has {@link ContentWebElement.Identifier},
    * {@link ContentWebElement.LegacyIdentifier}, or
    * {@link ChromeWebElement.Identifier} as properties.
@@ -1402,17 +1398,17 @@ class ContentWebElement extends WebEleme
       [ContentWebElement.LegacyIdentifier]: this.uuid,
     };
   }
 
   static fromJSON(json) {
     const {Identifier, LegacyIdentifier} = ContentWebElement;
 
     if (!(Identifier in json) && !(LegacyIdentifier in json)) {
-      throw new TypeError(
+      throw new InvalidArgumentError(
           pprint`Expected web element reference, got: ${json}`);
     }
 
     let uuid = json[Identifier] || json[LegacyIdentifier];
     return new ContentWebElement(uuid);
   }
 }
 ContentWebElement.Identifier = "element-6066-11e4-a52e-4f735466cecf";
@@ -1429,17 +1425,17 @@ class ContentWebWindow extends WebElemen
     return {
       [ContentWebWindow.Identifier]: this.uuid,
       [ContentWebElement.LegacyIdentifier]: this.uuid,
     };
   }
 
   static fromJSON(json) {
     if (!(ContentWebWindow.Identifier in json)) {
-      throw new TypeError(
+      throw new InvalidArgumentError(
           pprint`Expected web window reference, got: ${json}`);
     }
     let uuid = json[ContentWebWindow.Identifier];
     return new ContentWebWindow(uuid);
   }
 }
 ContentWebWindow.Identifier = "window-fcc6-11e5-b4f8-330a88ab9d7f";
 this.ContentWebWindow = ContentWebWindow;
@@ -1454,17 +1450,18 @@ class ContentWebFrame extends WebElement
     return {
       [ContentWebFrame.Identifier]: this.uuid,
       [ContentWebElement.LegacyIdentifier]: this.uuid,
     };
   }
 
   static fromJSON(json) {
     if (!(ContentWebFrame.Identifier in json)) {
-      throw new TypeError(pprint`Expected web frame reference, got: ${json}`);
+      throw new InvalidArgumentError(
+          pprint`Expected web frame reference, got: ${json}`);
     }
     let uuid = json[ContentWebFrame.Identifier];
     return new ContentWebFrame(uuid);
   }
 }
 ContentWebFrame.Identifier = "frame-075b-4da1-b6ba-e579c2d3230a";
 this.ContentWebFrame = ContentWebFrame;
 
@@ -1477,17 +1474,17 @@ class ChromeWebElement extends WebElemen
     return {
       [ChromeWebElement.Identifier]: this.uuid,
       [ContentWebElement.LegacyIdentifier]: this.uuid,
     };
   }
 
   static fromJSON(json) {
     if (!(ChromeWebElement.Identifier in json)) {
-      throw new TypeError("Expected chrome element reference " +
+      throw new InvalidArgumentError("Expected chrome element reference " +
           pprint`for XUL/XBL element, got: ${json}`);
     }
     let uuid = json[ChromeWebElement.Identifier];
     return new ChromeWebElement(uuid);
   }
 }
 ChromeWebElement.Identifier = "chromeelement-9fc5-4b51-a3c8-01716eedeb04";
 this.ChromeWebElement = ChromeWebElement;
--- a/testing/marionette/test_element.js
+++ b/testing/marionette/test_element.js
@@ -7,16 +7,17 @@ const {utils: Cu} = Components;
 const {
   ChromeWebElement,
   ContentWebElement,
   ContentWebFrame,
   ContentWebWindow,
   element,
   WebElement,
 } = Cu.import("chrome://marionette/content/element.js", {});
+const {InvalidArgumentError} = Cu.import("chrome://marionette/content/error.js", {});
 
 const SVGNS = "http://www.w3.org/2000/svg";
 const XBLNS = "http://www.mozilla.org/xbl";
 const XHTMLNS = "http://www.w3.org/1999/xhtml";
 const XULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
 class Element {
   constructor(tagName, attrs = {}) {
@@ -240,17 +241,17 @@ add_test(function test_WebElemenet_is() 
 });
 
 add_test(function test_WebElement_from() {
   ok(WebElement.from(domEl) instanceof ContentWebElement);
   ok(WebElement.from(domWin) instanceof ContentWebWindow);
   ok(WebElement.from(domFrame) instanceof ContentWebFrame);
   ok(WebElement.from(xulEl) instanceof ChromeWebElement);
 
-  Assert.throws(() => WebElement.from({}), /Expected DOM window\/element/);
+  Assert.throws(() => WebElement.from({}), InvalidArgumentError);
 
   run_next_test();
 });
 
 add_test(function test_WebElement_fromJSON_ContentWebElement() {
   const {Identifier, LegacyIdentifier} = ContentWebElement;
 
   let refNew = {[Identifier]: "foo"};
@@ -313,31 +314,31 @@ add_test(function test_WebElement_fromJS
   let el = WebElement.fromJSON(ref);
   ok(el instanceof ChromeWebElement);
   equal(el.uuid, "foo");
 
   run_next_test();
 });
 
 add_test(function test_WebElement_fromJSON_malformed() {
-  Assert.throws(() => WebElement.fromJSON({}), /Expected web element reference/);
-  Assert.throws(() => WebElement.fromJSON(null), /Expected JSON Object/);
+  Assert.throws(() => WebElement.fromJSON({}), InvalidArgumentError);
+  Assert.throws(() => WebElement.fromJSON(null), InvalidArgumentError);
   run_next_test();
 });
 
 add_test(function test_WebElement_fromUUID() {
   let xulWebEl = WebElement.fromUUID("foo", "chrome");
   ok(xulWebEl instanceof ChromeWebElement);
   equal(xulWebEl.uuid, "foo");
 
   let domWebEl = WebElement.fromUUID("bar", "content");
   ok(domWebEl instanceof ContentWebElement);
   equal(domWebEl.uuid, "bar");
 
-  Assert.throws(() => WebElement.fromUUID("baz", "bah"), /Unknown context/);
+  Assert.throws(() => WebElement.fromUUID("baz", "bah"), InvalidArgumentError);
 
   run_next_test();
 });
 
 add_test(function test_WebElement_isReference() {
   for (let t of [42, true, "foo", [], {}]) {
     ok(!WebElement.isReference(t));
   }
@@ -384,17 +385,17 @@ add_test(function test_ContentWebElement
   let bothRef = {
     [Identifier]: "identifier-uuid",
     [LegacyIdentifier]: "legacyidentifier-foo",
   };
   let bothEl = ContentWebElement.fromJSON(bothRef);
   ok(bothEl instanceof ContentWebElement);
   equal(bothEl.uuid, "identifier-uuid");
 
-  Assert.throws(() => ContentWebElement.fromJSON({}), /Expected web element reference/);
+  Assert.throws(() => ContentWebElement.fromJSON({}), InvalidArgumentError);
 
   run_next_test();
 });
 
 add_test(function test_ContentWebWindow_toJSON() {
   let win = new ContentWebWindow("foo");
   let json = win.toJSON();
   ok(ContentWebWindow.Identifier in json);