bug 1523104: remote: refactor Domains to not extend Map and wean us off custom EventEmitter; r=ochameau
authorAndreas Tolfsen <ato@sny.no>
Sat, 23 Feb 2019 18:59:31 +0000
changeset 521111 40a677ed5cc4
parent 521110 a1a5fdbf9bc8
child 521112 11a98da764d1
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1523104
milestone67.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 1523104: remote: refactor Domains to not extend Map and wean us off custom EventEmitter; r=ochameau
remote/ContentProcessSession.jsm
remote/Error.jsm
remote/Session.jsm
remote/domains/Domain.jsm
remote/domains/Domains.jsm
remote/test/unit/test_Domains.js
remote/test/unit/xpcshell.ini
--- a/remote/ContentProcessSession.jsm
+++ b/remote/ContentProcessSession.jsm
@@ -25,16 +25,30 @@ class ContentProcessSession {
   }
 
   destroy() {
     this.content.addEventListener("unload", this.destroy);
     this.messageManager.removeMessageListener("remote:request", this);
     this.messageManager.removeMessageListener("remote:destroy", this);
   }
 
+  // Domain event listener
+
+  onEvent(eventName, params) {
+    this.messageManager.sendAsyncMessage("remote:event", {
+      browsingContextId: this.browsingContext.id,
+      event: {
+        method: eventName,
+        params,
+      },
+    });
+  }
+
+  // nsIMessageListener
+
   async receiveMessage({name, data}) {
     const {browsingContextId} = data;
 
     // We may have more than one tab loaded in the same process,
     // and debug the two at the same time. We want to ensure not
     // mixing up the requests made against two such tabs.
     // Each tab is going to have its own frame script instance
     // and two communication channels are going to be set up via
@@ -74,23 +88,9 @@ class ContentProcessSession {
       }
       break;
 
     case "remote:destroy":
       this.destroy();
       break;
     }
   }
-
-  // EventEmitter
-
-  // This method is called when any Domain emit any event
-  // Domains register "*" listener on each instantiated Domain.
-  onevent(eventName, params) {
-    this.messageManager.sendAsyncMessage("remote:event", {
-      browsingContextId: this.browsingContext.id,
-      event: {
-        method: eventName,
-        params,
-      },
-    });
-  }
 }
--- a/remote/Error.jsm
+++ b/remote/Error.jsm
@@ -1,29 +1,31 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 var EXPORTED_SYMBOLS = [
   "FatalError",
+  "formatError",
+  "UnknownMethodError",
   "UnsupportedError",
-  "formatError",
 ];
 
 const {Log} = ChromeUtils.import("chrome://remote/content/Log.jsm");
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "log", Log.get);
 
 class RemoteAgentError extends Error {
   constructor(...args) {
     super(...args);
+    this.name = this.constructor.name;
     this.notify();
   }
 
   notify() {
     Cu.reportError(this);
     log.error(formatError(this), this);
   }
 }
@@ -46,16 +48,18 @@ class FatalError extends RemoteAgentErro
 
   quit(mode = Ci.nsIAppStartup.eForceQuit) {
     Services.startup.quit(mode);
   }
 }
 
 class UnsupportedError extends RemoteAgentError {}
 
+class UnknownMethodError extends RemoteAgentError {}
+
 function formatError(error, {stack = false} = {}) {
   const s = [];
   s.push(`${error.name}: ${error.message}`);
   s.push("");
   if (stack) {
     s.push("Stacktrace:");
   }
   s.push(error.stack);
--- a/remote/Session.jsm
+++ b/remote/Session.jsm
@@ -75,16 +75,25 @@ class Session {
   get mm() {
     return this.target.mm;
   }
 
   get browsingContext() {
     return this.target.browsingContext;
   }
 
+  // Domain event listener
+
+  onEvent(eventName, params) {
+    this.connection.send({
+      method: eventName,
+      params,
+    });
+  }
+
   // nsIMessageListener
 
   receiveMessage({name, data}) {
     const {id, result, event, error} = data;
 
     switch (name) {
     case "remote:result":
       this.connection.send({id, result});
@@ -95,17 +104,12 @@ class Session {
       break;
 
     case "remote:error":
       this.connection.send({id, error: formatError(error, {stack: true})});
       break;
     }
   }
 
-  // EventEmitter
-
-  onevent(eventName, params) {
-    this.connection.send({
-      method: eventName,
-      params,
-    });
+  toString() {
+    return `[object Session ${this.connection.id}]`;
   }
 }
--- a/remote/domains/Domain.jsm
+++ b/remote/domains/Domain.jsm
@@ -1,33 +1,66 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 var EXPORTED_SYMBOLS = ["Domain"];
 
-const {EventEmitter} = ChromeUtils.import("chrome://remote/content/EventEmitter.jsm");
-
 class Domain {
   constructor(session, target) {
     this.session = session;
     this.target = target;
     this.name = this.constructor.name;
 
-    EventEmitter.decorate(this);
+    this.eventListeners_ = new Set();
   }
 
   destructor() {}
 
+  emit(eventName, params = {}) {
+    for (const listener of this.eventListeners_) {
+      try {
+        if (isEventHandler(listener)) {
+          listener.onEvent(eventName, params);
+        } else {
+          listener.call(this, eventName, params);
+        }
+      } catch (e) {
+        Cu.reportError(e);
+      }
+    }
+  }
+
+  addEventListener(listener) {
+    if (typeof listener != "function" && !isEventHandler(listener)) {
+      throw new TypeError();
+    }
+    this.eventListeners_.add(listener);
+  }
+
+  // helpers
+
   get content() {
     return this.session.content;
   }
 
   get docShell() {
     return this.session.docShell;
   }
 
   get chromeEventHandler() {
     return this.docShell.chromeEventHandler;
   }
+
+  // static
+
+  static implements(methodName) {
+    return typeof this.prototype[methodName] == "function";
+  }
 }
+
+function isEventHandler(listener) {
+  return listener &&
+      "onEvent" in listener &&
+      typeof listener.onEvent == "function";
+}
--- a/remote/domains/Domains.jsm
+++ b/remote/domains/Domains.jsm
@@ -1,74 +1,103 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 var EXPORTED_SYMBOLS = ["Domains"];
 
-class Domains extends Map {
+const {UnknownMethodError} = ChromeUtils.import("chrome://remote/content/Error.jsm");
+const {Domain} = ChromeUtils.import("chrome://remote/content/domains/Domain.jsm");
+
+/**
+ * Lazy domain instance cache.
+ *
+ * Domains are loaded into each target's realm, and consequently
+ * there exists one domain cache per realm.  Domains are preregistered
+ * with this cache and then constructed lazily upon request.
+ *
+ * @param {Session} session
+ *     Session that domains should be associated with as they
+ *     are constructed.
+ * @param {Map.<string, string>} modules
+ *     Table defining JS modules available to this domain cache.
+ *     This should be a mapping between domain name
+ *     and JS module path passed to ChromeUtils.import.
+ */
+class Domains {
   constructor(session, modules) {
-    super();
     this.session = session;
     this.modules = modules;
-  }
-
-  domainSupportsMethod(name, method) {
-    const domain = this.modules[name];
-    return domain && typeof domain.prototype[method] == "function";
+    this.instances = new Map();
   }
 
-  get(name) {
-    let inst = super.get(name);
-    if (!inst) {
-      inst = this.new(name);
-      this.set(inst);
+  /** Test if domain supports method. */
+  domainSupportsMethod(name, method) {
+    const domain = this.modules[name];
+    if (domain) {
+      return domain.implements(method);
     }
-    return inst;
+    return false;
   }
 
-  set(domain) {
-    super.set(domain.name, domain);
-  }
+  /**
+   * Gets the current instance of the domain, or creates a new one,
+   * and associates it with the predefined session.
+   *
+   * @throws {UnknownMethodError}
+   *     If domain is not preregistered with this domain cache.
+   */
+  get(name) {
+    let inst = this.instances.get(name);
+    if (!inst) {
+      const Cls = this.modules[name];
+      if (!Cls) {
+        throw new UnknownMethodError(name);
+      }
+      if (!isConstructor(Cls)) {
+        throw new TypeError("Domain cannot be constructed");
+      }
 
-  new(name) {
-    const Cls = this.modules[name];
-    if (!Cls) {
-      throw new Error("No such domain: " + name);
+      inst = new Cls(this.session, this.session.target);
+      if (!(inst instanceof Domain)) {
+        throw new TypeError("Instance not a domain");
+      }
+
+      inst.addEventListener(this.session);
+
+      this.instances.set(name, inst);
     }
 
-    const inst = new Cls(this.session, this.session.target);
-    inst.on("*", this.session);
-
     return inst;
   }
 
-  delete(name) {
-    const inst = super.get(name);
-    if (inst) {
-      inst.off("*");
-      inst.destructor();
-      super.delete(inst.name);
-    }
+  get size() {
+    return this.instances.size;
   }
 
+  /** Calls destructor on each domain and clears the cache. */
   clear() {
-    for (const name of this.keys()) {
-      this.delete(name);
+    for (const inst of this.instances.values()) {
+      inst.destructor();
     }
+    this.instances.clear();
   }
 
-  // Splits a method name, e.g. "Browser.getVersion", into two components.
+  /** Splits a method name, e.g. "Browser.getVersion", into two components. */
   static splitMethod(method) {
     return split(method, ".", 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();
new file mode 100644
--- /dev/null
+++ b/remote/test/unit/test_Domains.js
@@ -0,0 +1,133 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const {Domain} = ChromeUtils.import("chrome://remote/content/domains/Domain.jsm");
+const {Domains} = ChromeUtils.import("chrome://remote/content/domains/Domains.jsm");
+
+class MockSession {
+  onEvent() {}
+}
+
+const noopSession = new MockSession();
+
+add_test(function test_Domains_constructor() {
+  new Domains(noopSession, {});
+
+  run_next_test();
+});
+
+add_test(function test_Domains_domainSupportsMethod() {
+  const modules = {
+    "Foo": class extends Domain {
+      bar() {}
+    },
+  };
+  const domains = new Domains(noopSession, modules);
+
+  ok(domains.domainSupportsMethod("Foo", "bar"));
+  ok(!domains.domainSupportsMethod("Foo", "baz"));
+  ok(!domains.domainSupportsMethod("foo", "bar"));
+
+  run_next_test();
+});
+
+add_test(function test_Domains_get_invalidModule() {
+  Assert.throws(() => {
+    const domains = new Domains(noopSession, {Foo: undefined});
+    domains.get("Foo");
+  }, /UnknownMethodError/);
+
+  run_next_test();
+});
+
+add_test(function test_Domains_get_missingConstructor() {
+  Assert.throws(() => {
+    const domains = new Domains(noopSession, {Foo: {}});
+    domains.get("Foo");
+  }, /TypeError/);
+
+  run_next_test();
+});
+
+add_test(function test_Domain_get_superClassNotDomain() {
+  Assert.throws(() => {
+    const domains = new Domain(noopSession, {Foo: class {}});
+    domains.get("Foo");
+  }, /TypeError/);
+
+  run_next_test();
+});
+
+add_test(function test_Domains_get_constructs() {
+  let eventFired;
+  class Session {
+    onEvent(event) {
+      eventFired = event;
+    }
+  }
+
+  let constructed = false;
+  class Foo extends Domain {
+    constructor() {
+      super();
+      constructed = true;
+    }
+  }
+
+  const session = new Session();
+  const domains = new Domains(session, {Foo});
+
+  const foo = domains.get("Foo");
+  ok(constructed);
+  ok(foo instanceof Foo);
+
+  const event = {};
+  foo.emit(event);
+  equal(event, eventFired);
+
+  run_next_test();
+});
+
+add_test(function test_Domains_size() {
+  class Foo extends Domain {}
+  const domains = new Domains(noopSession, {Foo});
+
+  equal(domains.size, 0);
+  domains.get("Foo");
+  equal(domains.size, 1);
+
+  run_next_test();
+});
+
+add_test(function test_Domains_clear() {
+  let dtorCalled = false;
+  class Foo extends Domain {
+    destructor() {
+      dtorCalled = true;
+    }
+  }
+
+  const domains = new Domains(noopSession, {Foo});
+
+  equal(domains.size, 0);
+  domains.get("Foo");
+  equal(domains.size, 1);
+
+  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"]);
+
+  run_next_test();
+});
--- a/remote/test/unit/xpcshell.ini
+++ b/remote/test/unit/xpcshell.ini
@@ -1,8 +1,9 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 [DEFAULT]
 skip-if = appname == "thunderbird"
 
+[test_Domains.js]
 [test_Session.js]