bug 1537775: remote: move method sanity check into Domains.splitMethod; r=ochameau
authorAndreas Tolfsen <ato@sny.no>
Thu, 04 Apr 2019 09:39:28 +0000
changeset 467984 2bccb12b0ee60033ee4555d7ef6fdf26970d0a89
parent 467983 28eb8a6b33f63322188eb1ca0e4bcb3b3fb43432
child 467985 0a10d3faab9eb28b4fecd19c05944871f4e6f466
push id112667
push useraiakab@mozilla.com
push dateThu, 04 Apr 2019 16:12:45 +0000
treeherdermozilla-inbound@230bb363f2f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1537775
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1537775: remote: move method sanity check into Domains.splitMethod; r=ochameau This moves the assertions related to the well-formedness of the method from the TabSession consumer to Domains.splitMethod. Following the introduction of TabSession, this was missing from the superclass Session. This also fixes the "Foo.bar.baz" test case in remote/test/unit/test_Domains.js by removing the split() function and instead relying on String#split() inside Domains.splitMethod. Thanks-to: Alexandre Poirot <ochameau@mozilla.com> Differential Revision: https://phabricator.services.mozilla.com/D25947
remote/domains/Domains.jsm
remote/sessions/Session.jsm
remote/sessions/TabSession.jsm
remote/test/unit/test_Domains.js
--- a/remote/domains/Domains.jsm
+++ b/remote/domains/Domains.jsm
@@ -85,47 +85,27 @@ class Domains {
   /** Calls destructor on each domain and clears the cache. */
   clear() {
     for (const inst of this.instances.values()) {
       inst.destructor();
     }
     this.instances.clear();
   }
 
-  /** Splits a method name, e.g. "Browser.getVersion", into two components. */
-  static splitMethod(method) {
-    return split(method, ".", 1);
+  /**
+   * Splits a method, e.g. "Browser.getVersion",
+   * into domain ("Browser") and command ("getVersion") components.
+   */
+  static splitMethod(s) {
+    const ss = s.split(".");
+    if (ss.length != 2 || ss[0].length == 0 || ss[1].length == 0) {
+      throw new TypeError(`Invalid method format: "${s}"`);
+    }
+    return {
+      domain: ss[0],
+      command: ss[1],
+    };
   }
 }
 
 function isConstructor(obj) {
   return !!obj.prototype && !!obj.prototype.constructor.name;
 }
-
-// Split s by sep, returning list of substrings.
-// If max is given, at most max splits are done.
-// If max is 0, there is no limit on the number of splits.
-function split(s, sep, max = 0) {
-  if (typeof s != "string" ||
-      typeof sep != "string" ||
-      typeof max != "number") {
-    throw new TypeError();
-  }
-  if (!Number.isInteger(max) || max < 0) {
-    throw new RangeError();
-  }
-
-  const rv = [];
-  let i = 0;
-
-  while (rv.length < max) {
-    const si = s.indexOf(sep, i);
-    if (!si) {
-      break;
-    }
-
-    rv.push(s.substring(i, si));
-    i = si + sep.length;
-  }
-
-  rv.push(s.substring(i));
-  return rv;
-}
--- a/remote/sessions/Session.jsm
+++ b/remote/sessions/Session.jsm
@@ -56,17 +56,17 @@ class Session {
     try {
       if (typeof id == "undefined") {
         throw new TypeError("Message missing 'id' field");
       }
       if (typeof method == "undefined") {
         throw new TypeError("Message missing 'method' field");
       }
 
-      const [domain, command] = Domains.splitMethod(method);
+      const {domain, command} = Domains.splitMethod(method);
       await this.execute(id, domain, command, params);
     } catch (e) {
       this.onError(id, e);
     }
   }
 
   async execute(id, domain, command, params) {
     const inst = this.domains.get(domain);
--- a/remote/sessions/TabSession.jsm
+++ b/remote/sessions/TabSession.jsm
@@ -55,21 +55,17 @@ class TabSession extends Session {
     try {
       if (typeof id == "undefined") {
         throw new TypeError("Message missing 'id' field");
       }
       if (typeof method == "undefined") {
         throw new TypeError("Message missing 'method' field");
       }
 
-      const [domain, command] = Domains.splitMethod(method);
-      if (typeof domain == "undefined" || typeof command == "undefined") {
-        throw new TypeError("'method' field is incorrect and doesn't define a domain " +
-                            "name and method separated by a dot.");
-      }
+      const {domain, command} = Domains.splitMethod(method);
       if (this.domains.has(domain)) {
         await this.execute(id, domain, command, params);
       } else {
         this.executeInChild(id, domain, command, params);
       }
     } catch (e) {
       this.onError(id, e);
     }
--- a/remote/test/unit/test_Domains.js
+++ b/remote/test/unit/test_Domains.js
@@ -118,16 +118,18 @@ add_test(function test_Domains_clear() {
   domains.clear();
   equal(domains.size, 0);
   ok(dtorCalled);
 
   run_next_test();
 });
 
 add_test(function test_Domains_splitMethod() {
-  deepEqual(Domains.splitMethod(""), []);
-  deepEqual(Domains.splitMethod("Foo"), ["Foo"]);
-  deepEqual(Domains.splitMethod("Foo."), ["Foo"]);
-  deepEqual(Domains.splitMethod("Foo.bar"), ["Foo", "bar"]);
-  deepEqual(Domains.splitMethod("Foo.bar.baz"), ["Foo", "bar"]);
+  for (const t of [42, null, true, {}, [], undefined]) {
+    Assert.throws(() => Domains.splitMethod(t), /TypeError/, `${typeof t} throws`);
+  }
+  for (const s of ["", ".", "foo.", ".bar", "foo.bar.baz"]) {
+    Assert.throws(() => Domains.splitMethod(s), /Invalid method format: ".*"/, `"${s}" throws`);
+  }
+  deepEqual(Domains.splitMethod("foo.bar"), {domain: "foo", command: "bar"});
 
   run_next_test();
 });