Bug 1278413 - Clean up code style in worker.js, event-emitter.js. r=tromey
authorJ. Ryan Stinnett <jryans@gmail.com>
Mon, 06 Jun 2016 17:58:08 -0500
changeset 300828 1401fcd673699f8d6e431ccb643ab46e166da2a9
parent 300827 a7b31be0a19ae5413db3e10ad4e5251cac84b08d
child 301039 81f4cc3f6f4c21758b63605d69b47fa6cb3c142a
push id19598
push userjryans@gmail.com
push dateWed, 08 Jun 2016 03:32:48 +0000
treeherderfx-team@1401fcd67369 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstromey
bugs1278413
milestone50.0a1
Bug 1278413 - Clean up code style in worker.js, event-emitter.js. r=tromey MozReview-Commit-ID: LVbo9i0rPqI
.eslintignore
devtools/client/framework/toolbox.js
devtools/shared/event-emitter.js
devtools/shared/worker/worker.js
--- a/.eslintignore
+++ b/.eslintignore
@@ -109,16 +109,17 @@ devtools/client/webconsole/**
 devtools/client/webide/**
 devtools/server/**
 !devtools/server/actors/webbrowser.js
 !devtools/server/actors/styles.js
 !devtools/server/actors/string.js
 devtools/shared/*.js
 !devtools/shared/css-lexer.js
 !devtools/shared/defer.js
+!devtools/shared/event-emitter.js
 !devtools/shared/task.js
 devtools/shared/*.jsm
 devtools/shared/apps/**
 devtools/shared/client/**
 devtools/shared/discovery/**
 devtools/shared/gcli/**
 !devtools/shared/gcli/templater.js
 devtools/shared/heapsnapshot/**
@@ -131,16 +132,17 @@ devtools/shared/security/**
 devtools/shared/shims/**
 devtools/shared/tests/**
 !devtools/shared/tests/unit/test_csslexer.js
 devtools/shared/touch/**
 devtools/shared/transport/**
 !devtools/shared/transport/transport.js
 devtools/shared/webconsole/test/**
 devtools/shared/worker/**
+!devtools/shared/worker/worker.js
 
 # Ignore devtools pre-processed files
 devtools/client/framework/toolbox-process-window.js
 devtools/client/performance/system.js
 devtools/client/webide/webide-prefs.js
 devtools/client/preferences/**
 
 # Ignore devtools third-party libs
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -1235,17 +1235,17 @@ Toolbox.prototype = {
       // Prevent flicker while loading by waiting to make visible until now.
       iframe.style.visibility = "visible";
 
       // The build method should return a panel instance, so events can
       // be fired with the panel as an argument. However, in order to keep
       // backward compatibility with existing extensions do a check
       // for a promise return value.
       let built = definition.build(iframe.contentWindow, this);
-      if (!(built instanceof Promise)) {
+      if (!(typeof built.then == "function")) {
         let panel = built;
         iframe.panel = panel;
 
         // The panel instance is expected to fire (and listen to) various
         // framework events, so make sure it's properly decorated with
         // appropriate API (on, off, once, emit).
         // In this case we decorate panel instances directly returned by
         // the tool definition 'build' method.
--- a/devtools/shared/event-emitter.js
+++ b/devtools/shared/event-emitter.js
@@ -1,24 +1,24 @@
 /* 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/. */
 
-/**
- * EventEmitter.
- */
+"use strict";
 
-(function (factory) { // Module boilerplate
-  if (this.module && module.id.indexOf("event-emitter") >= 0) { // require
+(function (factory) {
+  if (this.module && module.id.indexOf("event-emitter") >= 0) {
+    // require
     factory.call(this, require, exports, module);
-  } else { // Cu.import
+  } else {
+    // Cu.import
     this.isWorker = false;
-      // Bug 1259045: This module is loaded early in firefox startup as a JSM,
-      // but it doesn't depends on any real module. We can save a few cycles
-      // and bytes by not loading Loader.jsm.
+    // Bug 1259045: This module is loaded early in firefox startup as a JSM,
+    // but it doesn't depends on any real module. We can save a few cycles
+    // and bytes by not loading Loader.jsm.
     let require = function (module) {
       const Cu = Components.utils;
       switch (module) {
         case "promise":
           return Cu.import("resource://gre/modules/Promise.jsm", {}).Promise;
         case "Services":
           return Cu.import("resource://gre/modules/Services.jsm", {}).Services;
         case "chrome":
@@ -28,156 +28,156 @@
           };
       }
       return null;
     };
     factory.call(this, require, this, { exports: this });
     this.EXPORTED_SYMBOLS = ["EventEmitter"];
   }
 }).call(this, function (require, exports, module) {
-
-  this.EventEmitter = function EventEmitter() {};
+  let EventEmitter = this.EventEmitter = function () {};
   module.exports = EventEmitter;
 
-// See comment in JSM module boilerplate when adding a new dependency.
-  const { Cu, components } = require("chrome");
+  // See comment in JSM module boilerplate when adding a new dependency.
+  const { components } = require("chrome");
   const Services = require("Services");
   const promise = require("promise");
-  var loggingEnabled = true;
+  let loggingEnabled = true;
 
   if (!isWorker) {
     loggingEnabled = Services.prefs.getBoolPref("devtools.dump.emit");
     Services.prefs.addObserver("devtools.dump.emit", {
       observe: () => {
         loggingEnabled = Services.prefs.getBoolPref("devtools.dump.emit");
       }
     }, false);
   }
 
-/**
- * Decorate an object with event emitter functionality.
- *
- * @param Object aObjectToDecorate
- *        Bind all public methods of EventEmitter to
- *        the aObjectToDecorate object.
- */
-  EventEmitter.decorate = function EventEmitter_decorate(aObjectToDecorate) {
+  /**
+   * Decorate an object with event emitter functionality.
+   *
+   * @param Object objectToDecorate
+   *        Bind all public methods of EventEmitter to
+   *        the objectToDecorate object.
+   */
+  EventEmitter.decorate = function (objectToDecorate) {
     let emitter = new EventEmitter();
-    aObjectToDecorate.on = emitter.on.bind(emitter);
-    aObjectToDecorate.off = emitter.off.bind(emitter);
-    aObjectToDecorate.once = emitter.once.bind(emitter);
-    aObjectToDecorate.emit = emitter.emit.bind(emitter);
+    objectToDecorate.on = emitter.on.bind(emitter);
+    objectToDecorate.off = emitter.off.bind(emitter);
+    objectToDecorate.once = emitter.once.bind(emitter);
+    objectToDecorate.emit = emitter.emit.bind(emitter);
   };
 
   EventEmitter.prototype = {
-  /**
-   * Connect a listener.
-   *
-   * @param string aEvent
-   *        The event name to which we're connecting.
-   * @param function aListener
-   *        Called when the event is fired.
-   */
-    on: function EventEmitter_on(aEvent, aListener) {
-      if (!this._eventEmitterListeners)
+    /**
+     * Connect a listener.
+     *
+     * @param string event
+     *        The event name to which we're connecting.
+     * @param function listener
+     *        Called when the event is fired.
+     */
+    on(event, listener) {
+      if (!this._eventEmitterListeners) {
         this._eventEmitterListeners = new Map();
-      if (!this._eventEmitterListeners.has(aEvent)) {
-        this._eventEmitterListeners.set(aEvent, []);
       }
-      this._eventEmitterListeners.get(aEvent).push(aListener);
+      if (!this._eventEmitterListeners.has(event)) {
+        this._eventEmitterListeners.set(event, []);
+      }
+      this._eventEmitterListeners.get(event).push(listener);
     },
 
-  /**
-   * Listen for the next time an event is fired.
-   *
-   * @param string aEvent
-   *        The event name to which we're connecting.
-   * @param function aListener
-   *        (Optional) Called when the event is fired. Will be called at most
-   *        one time.
-   * @return promise
-   *        A promise which is resolved when the event next happens. The
-   *        resolution value of the promise is the first event argument. If
-   *        you need access to second or subsequent event arguments (it's rare
-   *        that this is needed) then use aListener
-   */
-    once: function EventEmitter_once(aEvent, aListener) {
+    /**
+     * Listen for the next time an event is fired.
+     *
+     * @param string event
+     *        The event name to which we're connecting.
+     * @param function listener
+     *        (Optional) Called when the event is fired. Will be called at most
+     *        one time.
+     * @return promise
+     *        A promise which is resolved when the event next happens. The
+     *        resolution value of the promise is the first event argument. If
+     *        you need access to second or subsequent event arguments (it's rare
+     *        that this is needed) then use listener
+     */
+    once(event, listener) {
       let deferred = promise.defer();
 
-      let handler = (aEvent, aFirstArg, ...aRest) => {
-        this.off(aEvent, handler);
-        if (aListener) {
-          aListener.apply(null, [aEvent, aFirstArg, ...aRest]);
+      let handler = (_, first, ...rest) => {
+        this.off(event, handler);
+        if (listener) {
+          listener.apply(null, [event, first, ...rest]);
         }
-        deferred.resolve(aFirstArg);
+        deferred.resolve(first);
       };
 
-      handler._originalListener = aListener;
-      this.on(aEvent, handler);
+      handler._originalListener = listener;
+      this.on(event, handler);
 
       return deferred.promise;
     },
 
-  /**
-   * Remove a previously-registered event listener.  Works for events
-   * registered with either on or once.
-   *
-   * @param string aEvent
-   *        The event name whose listener we're disconnecting.
-   * @param function aListener
-   *        The listener to remove.
-   */
-    off: function EventEmitter_off(aEvent, aListener) {
-      if (!this._eventEmitterListeners)
+    /**
+     * Remove a previously-registered event listener.  Works for events
+     * registered with either on or once.
+     *
+     * @param string event
+     *        The event name whose listener we're disconnecting.
+     * @param function listener
+     *        The listener to remove.
+     */
+    off(event, listener) {
+      if (!this._eventEmitterListeners) {
         return;
-      let listeners = this._eventEmitterListeners.get(aEvent);
+      }
+      let listeners = this._eventEmitterListeners.get(event);
       if (listeners) {
-        this._eventEmitterListeners.set(aEvent, listeners.filter(l => {
-          return l !== aListener && l._originalListener !== aListener;
+        this._eventEmitterListeners.set(event, listeners.filter(l => {
+          return l !== listener && l._originalListener !== listener;
         }));
       }
     },
 
-  /**
-   * Emit an event.  All arguments to this method will
-   * be sent to listener functions.
-   */
-    emit: function EventEmitter_emit(aEvent) {
-      this.logEvent(aEvent, arguments);
+    /**
+     * Emit an event.  All arguments to this method will
+     * be sent to listener functions.
+     */
+    emit(event) {
+      this.logEvent(event, arguments);
 
-      if (!this._eventEmitterListeners || !this._eventEmitterListeners.has(aEvent)) {
+      if (!this._eventEmitterListeners || !this._eventEmitterListeners.has(event)) {
         return;
       }
 
-      let originalListeners = this._eventEmitterListeners.get(aEvent);
-      for (let listener of this._eventEmitterListeners.get(aEvent)) {
-      // If the object was destroyed during event emission, stop
-      // emitting.
+      let originalListeners = this._eventEmitterListeners.get(event);
+      for (let listener of this._eventEmitterListeners.get(event)) {
+        // If the object was destroyed during event emission, stop
+        // emitting.
         if (!this._eventEmitterListeners) {
           break;
         }
 
-      // If listeners were removed during emission, make sure the
-      // event handler we're going to fire wasn't removed.
-        if (originalListeners === this._eventEmitterListeners.get(aEvent) ||
-          this._eventEmitterListeners.get(aEvent).some(l => l === listener)) {
+        // If listeners were removed during emission, make sure the
+        // event handler we're going to fire wasn't removed.
+        if (originalListeners === this._eventEmitterListeners.get(event) ||
+          this._eventEmitterListeners.get(event).some(l => l === listener)) {
           try {
             listener.apply(null, arguments);
+          } catch (ex) {
+            // Prevent a bad listener from interfering with the others.
+            let msg = ex + ": " + ex.stack;
+            console.error(msg);
+            dump(msg + "\n");
           }
-        catch (ex) {
-          // Prevent a bad listener from interfering with the others.
-          let msg = ex + ": " + ex.stack;
-          console.error(msg);
-          dump(msg + "\n");
-        }
         }
       }
     },
 
-    logEvent: function (aEvent, args) {
+    logEvent(event, args) {
       if (!loggingEnabled) {
         return;
       }
 
       let caller, func, path;
       if (!isWorker) {
         caller = components.stack.caller.caller;
         func = caller.name;
@@ -185,26 +185,26 @@
         if (file.includes(" -> ")) {
           file = caller.filename.split(/ -> /)[1];
         }
         path = file + ":" + caller.lineNumber;
       }
 
       let argOut = "(";
       if (args.length === 1) {
-        argOut += aEvent;
+        argOut += event;
       }
 
       let out = "EMITTING: ";
 
-    // We need this try / catch to prevent any dead object errors.
+      // We need this try / catch to prevent any dead object errors.
       try {
         for (let i = 1; i < args.length; i++) {
           if (i === 1) {
-            argOut = "(" + aEvent + ", ";
+            argOut = "(" + event + ", ";
           } else {
             argOut += ", ";
           }
 
           let arg = args[i];
           argOut += arg;
 
           if (arg && arg.nodeName) {
@@ -214,20 +214,19 @@
             }
             if (arg.className) {
               argOut += "." + arg.className;
             }
             argOut += ")";
           }
         }
       } catch (e) {
-      // Object is dead so the toolbox is most likely shutting down,
-      // do nothing.
+        // Object is dead so the toolbox is most likely shutting down,
+        // do nothing.
       }
 
       argOut += ")";
       out += "emit" + argOut + " from " + func + "() -> " + path + "\n";
 
       dump(out);
     },
   };
-
 });
--- a/devtools/shared/worker/worker.js
+++ b/devtools/shared/worker/worker.js
@@ -1,146 +1,151 @@
 /* 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";
 
-(function (factory) { // Module boilerplate
-  if (this.module && module.id.indexOf("worker") >= 0) { // require
+/* global ChromeWorker */
+
+(function (factory) {
+  if (this.module && module.id.indexOf("worker") >= 0) {
+    // require
     const { Cc, Ci, Cu, ChromeWorker } = require("chrome");
     const dumpn = require("devtools/shared/DevToolsUtils").dumpn;
     factory.call(this, require, exports, module, { Cc, Ci, Cu }, ChromeWorker, dumpn);
-  } else { // Cu.import
+  } else {
+    // Cu.import
     const { classes: Cc, interfaces: Ci, utils: Cu } = Components;
     const { require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
     this.isWorker = false;
     this.Promise = Cu.import("resource://gre/modules/Promise.jsm", {}).Promise;
     this.console = Cu.import("resource://gre/modules/Console.jsm", {}).console;
     factory.call(
       this, require, this, { exports: this },
       { Cc, Ci, Cu }, ChromeWorker, null
     );
     this.EXPORTED_SYMBOLS = ["DevToolsWorker"];
   }
 }).call(this, function (require, exports, module, { Ci, Cc }, ChromeWorker, dumpn) {
-
-  var MESSAGE_COUNTER = 0;
+  let MESSAGE_COUNTER = 0;
 
-/**
- * Creates a wrapper around a ChromeWorker, providing easy
- * communication to offload demanding tasks. The corresponding URL
- * must implement the interface provided by `devtools/shared/worker/helper`.
- *
- * @see `./devtools/client/shared/widgets/GraphsWorker.js`
- *
- * @param {string} url
- *        The URL of the worker.
- * @param Object opts
- *        An option with the following optional fields:
- *        - name: a name that will be printed with logs
- *        - verbose: log incoming and outgoing messages
- */
+  /**
+   * Creates a wrapper around a ChromeWorker, providing easy
+   * communication to offload demanding tasks. The corresponding URL
+   * must implement the interface provided by `devtools/shared/worker/helper`.
+   *
+   * @see `./devtools/client/shared/widgets/GraphsWorker.js`
+   *
+   * @param {string} url
+   *        The URL of the worker.
+   * @param Object opts
+   *        An option with the following optional fields:
+   *        - name: a name that will be printed with logs
+   *        - verbose: log incoming and outgoing messages
+   */
   function DevToolsWorker(url, opts) {
     opts = opts || {};
     this._worker = new ChromeWorker(url);
     this._verbose = opts.verbose;
     this._name = opts.name;
 
     this._worker.addEventListener("error", this.onError, false);
   }
   exports.DevToolsWorker = DevToolsWorker;
 
-/**
- * Performs the given task in a chrome worker, passing in data.
- * Returns a promise that resolves when the task is completed, resulting in
- * the return value of the task.
- *
- * @param {string} task
- *        The name of the task to execute in the worker.
- * @param {any} data
- *        Data to be passed into the task implemented by the worker.
- * @return {Promise}
- */
+  /**
+   * Performs the given task in a chrome worker, passing in data.
+   * Returns a promise that resolves when the task is completed, resulting in
+   * the return value of the task.
+   *
+   * @param {string} task
+   *        The name of the task to execute in the worker.
+   * @param {any} data
+   *        Data to be passed into the task implemented by the worker.
+   * @return {Promise}
+   */
   DevToolsWorker.prototype.performTask = function (task, data) {
     if (this._destroyed) {
       return Promise.reject("Cannot call performTask on a destroyed DevToolsWorker");
     }
     let worker = this._worker;
     let id = ++MESSAGE_COUNTER;
     let payload = { task, id, data };
 
     if (this._verbose && dumpn) {
       dumpn("Sending message to worker" +
-          (this._name ? (" (" + this._name + ")") : "") +
-          ": " +
-          JSON.stringify(payload, null, 2));
+            (this._name ? (" (" + this._name + ")") : "") +
+            ": " +
+            JSON.stringify(payload, null, 2));
     }
     worker.postMessage(payload);
 
     return new Promise((resolve, reject) => {
-      let listener = ({ data }) => {
+      let listener = ({ data: result }) => {
         if (this._verbose && dumpn) {
           dumpn("Received message from worker" +
-              (this._name ? (" (" + this._name + ")") : "") +
-              ": " +
-              JSON.stringify(data, null, 2));
+                (this._name ? (" (" + this._name + ")") : "") +
+                ": " +
+                JSON.stringify(result, null, 2));
         }
 
-        if (data.id !== id) {
+        if (result.id !== id) {
           return;
         }
         worker.removeEventListener("message", listener);
-        if (data.error) {
-          reject(data.error);
+        if (result.error) {
+          reject(result.error);
         } else {
-          resolve(data.response);
+          resolve(result.response);
         }
       };
 
       worker.addEventListener("message", listener);
     });
   };
 
-/**
- * Terminates the underlying worker. Use when no longer needing the worker.
- */
+  /**
+   * Terminates the underlying worker. Use when no longer needing the worker.
+   */
   DevToolsWorker.prototype.destroy = function () {
     this._worker.terminate();
     this._worker = null;
     this._destroyed = true;
   };
 
   DevToolsWorker.prototype.onError = function ({ message, filename, lineno }) {
     dump(new Error(message + " @ " + filename + ":" + lineno) + "\n");
   };
 
-/**
- * Takes a function and returns a Worker-wrapped version of the same function.
- * Returns a promise upon resolution.
- * @see `./devtools/shared/shared/tests/browser/browser_devtools-worker-03.js
- *
- * * * * ! ! ! This should only be used for tests or A/B testing performance ! ! ! * * * * * *
- *
- * The original function must:
- *
- * Be a pure function, that is, not use any variables not declared within the
- * function, or its arguments.
- *
- * Return a value or a promise.
- *
- * Note any state change in the worker will not affect the callee's context.
- *
- * @param {function} fn
- * @return {function}
- */
+  /**
+   * Takes a function and returns a Worker-wrapped version of the same function.
+   * Returns a promise upon resolution.
+   * @see `./devtools/shared/shared/tests/browser/browser_devtools-worker-03.js
+   *
+   * ⚠ This should only be used for tests or A/B testing performance ⚠
+   *
+   * The original function must:
+   *
+   * Be a pure function, that is, not use any variables not declared within the
+   * function, or its arguments.
+   *
+   * Return a value or a promise.
+   *
+   * Note any state change in the worker will not affect the callee's context.
+   *
+   * @param {function} fn
+   * @return {function}
+   */
   function workerify(fn) {
-    console.warn(`\`workerify\` should only be used in tests or measuring performance.
-  This creates an object URL on the browser window, and should not be used in production.`);
-  // Fetch via window/utils here as we don't want to include
-  // this module normally.
+    console.warn("`workerify` should only be used in tests or measuring performance. " +
+                 "This creates an object URL on the browser window, and should not be " +
+                 "used in production.");
+    // Fetch via window/utils here as we don't want to include
+    // this module normally.
     let { getMostRecentBrowserWindow } = require("sdk/window/utils");
     let { URL, Blob } = getMostRecentBrowserWindow();
     let stringifiedFn = createWorkerString(fn);
     let blob = new Blob([stringifiedFn]);
     let url = URL.createObjectURL(blob);
     let worker = new DevToolsWorker(url);
 
     let wrapperFn = data => worker.performTask("workerifiedTask", data);
@@ -149,20 +154,18 @@
       URL.revokeObjectURL(url);
       worker.destroy();
     };
 
     return wrapperFn;
   }
   exports.workerify = workerify;
 
-/**
- * Takes a function, and stringifies it, attaching the worker-helper.js
- * boilerplate hooks.
- */
+  /**
+   * Takes a function, and stringifies it, attaching the worker-helper.js
+   * boilerplate hooks.
+   */
   function createWorkerString(fn) {
     return `importScripts("resource://gre/modules/workers/require.js");
-    const { createTask } = require("resource://devtools/shared/worker/helper.js");
-    createTask(self, "workerifiedTask", ${fn.toString()});
-  `;
+            const { createTask } = require("resource://devtools/shared/worker/helper.js");
+            createTask(self, "workerifiedTask", ${fn.toString()});`;
   }
-
 });