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 341134 1401fcd673699f8d6e431ccb643ab46e166da2a9
parent 340935 a7b31be0a19ae5413db3e10ad4e5251cac84b08d
child 341135 81f4cc3f6f4c21758b63605d69b47fa6cb3c142a
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstromey
bugs1278413
milestone50.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 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()});`;
   }
-
 });