Bug 1164632 - use new worker helpers in debugger for pretty-printing r=jsantell

browser/devtools/scratchpad/scratchpad.js

@@ -49,6 +49,7 @@ const Telemetry = require("devtools/shared/telemetry");
 const Editor    = require("devtools/sourceeditor/editor");
 const TargetFactory = require("devtools/framework/target").TargetFactory;
 const EventEmitter = require("devtools/toolkit/event-emitter");
+const {DevToolsWorker} = require("devtools/toolkit/shared/worker");
 
 const { Promise: promise } = Cu.import("resource://gre/modules/Promise.jsm", {});
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
@@ -650,12 +651,11 @@ var Scratchpad = {
    */
   get prettyPrintWorker() {
     if (!this._prettyPrintWorker) {
-      this._prettyPrintWorker = new ChromeWorker(
+      this._prettyPrintWorker = new DevToolsWorker(
-        "resource://gre/modules/devtools/server/actors/pretty-print-worker.js");
+        "resource://gre/modules/devtools/server/actors/pretty-print-worker.js",
-
+        { name: 'pretty-print',
-      this._prettyPrintWorker.addEventListener("error", ({ message, filename, lineno }) => {
+          verbose: DevToolsUtils.dumpn.wantLogging }
-        DevToolsUtils.reportException(message + " @ " + filename + ":" + lineno);
+      );
-      }, false);
     }
     return this._prettyPrintWorker;
   },
@@ -670,34 +670,17 @@ var Scratchpad = {
   prettyPrint: function SP_prettyPrint() {
     const uglyText = this.getText();
     const tabsize = Services.prefs.getIntPref(TAB_SIZE);
-    const id = Math.random();
-    const deferred = promise.defer();
 
-    const onReply = ({ data }) => {
+    return this.prettyPrintWorker.performTask("pretty-print", {
-      if (data.id !== id) {
-        return;
-      }
-      this.prettyPrintWorker.removeEventListener("message", onReply, false);
-
-      if (data.error) {
-        let errorString = DevToolsUtils.safeErrorString(data.error);
-        this.writeAsErrorComment({ exception: errorString });
-        deferred.reject(errorString);
-      } else {
-        this.editor.setText(data.code);
-        deferred.resolve(data.code);
-      }
-    };
-
-    this.prettyPrintWorker.addEventListener("message", onReply, false);
-    this.prettyPrintWorker.postMessage({
-      id: id,
       url: "(scratchpad)",
       indent: tabsize,
       source: uglyText
+    }).then(data => {
+      this.editor.setText(data.code);
+    }).then(null, error => {
+      this.writeAsErrorComment({ exception: error });
+      throw error;
     });
-
-    return deferred.promise;
   },
 
   /**
@@ -1829,7 +1812,7 @@ var Scratchpad = {
     }
 
     if (this._prettyPrintWorker) {
-      this._prettyPrintWorker.terminate();
+      this._prettyPrintWorker.destroy();
       this._prettyPrintWorker = null;
     }
 

toolkit/devtools/server/actors/pretty-print-worker.js

@@ -27,27 +27,24 @@
  *     { id, error }
  */
 
+importScripts("resource://gre/modules/devtools/shared/worker-helper.js");
 importScripts("resource://gre/modules/devtools/acorn/acorn.js");
 importScripts("resource://gre/modules/devtools/source-map.js");
 importScripts("resource://gre/modules/devtools/pretty-fast.js");
 
-self.onmessage = (event) => {
+workerHelper.createTask(self, "pretty-print", ({ url, indent, source }) => {
-  const { data: { id, url, indent, source } } = event;
   try {
     const prettified = prettyFast(source, {
       url: url,
       indent: " ".repeat(indent)
     });
 
-    self.postMessage({
+    return {
-      id: id,
       code: prettified.code,
       mappings: prettified.map._mappings
-    });
+    };
-  } catch (e) {
-    self.postMessage({
-      id: id,
-      error: e.message + "\n" + e.stack
-    });
   }
-};
+  catch(e) {
+    return new Error(e.message + "\n" + e.stack);
+  }
+});

toolkit/devtools/server/actors/script.js

@@ -17,6 +17,7 @@ const promise = require("promise");
 const PromiseDebugging = require("PromiseDebugging");
 const xpcInspector = require("xpcInspector");
 const ScriptStore = require("./utils/ScriptStore");
+const {DevToolsWorker} = require("devtools/toolkit/shared/worker.js");
 
 const { defer, resolve, reject, all } = require("devtools/toolkit/deprecated-sync-thenables");
 
@@ -532,35 +533,15 @@ ThreadActor.prototype = {
   _prettyPrintWorker: null,
   get prettyPrintWorker() {
     if (!this._prettyPrintWorker) {
-      this._prettyPrintWorker = new ChromeWorker(
+      this._prettyPrintWorker = new DevToolsWorker(
-        "resource://gre/modules/devtools/server/actors/pretty-print-worker.js");
+        "resource://gre/modules/devtools/server/actors/pretty-print-worker.js",
-
+        { name: "pretty-print",
-      this._prettyPrintWorker.addEventListener(
+          verbose: dumpn.wantLogging }
-        "error", this._onPrettyPrintError, false);
+      );
-
-      if (dumpn.wantLogging) {
-        this._prettyPrintWorker.addEventListener("message", this._onPrettyPrintMsg, false);
-
-        const postMsg = this._prettyPrintWorker.postMessage;
-        this._prettyPrintWorker.postMessage = data => {
-          dumpn("Sending message to prettyPrintWorker: "
-                + JSON.stringify(data, null, 2) + "\n");
-          return postMsg.call(this._prettyPrintWorker, data);
-        };
-      }
     }
     return this._prettyPrintWorker;
   },
 
-  _onPrettyPrintError: function ({ message, filename, lineno }) {
-    reportError(new Error(message + " @ " + filename + ":" + lineno));
-  },
-
-  _onPrettyPrintMsg: function ({ data }) {
-    dumpn("Received message from prettyPrintWorker: "
-          + JSON.stringify(data, null, 2) + "\n");
-  },
-
   /**
    * Keep track of all of the nested event loops we use to pause the debuggee
    * when we hit a breakpoint/debugger statement/etc in one place so we can
@@ -623,11 +604,7 @@ ThreadActor.prototype = {
     this._threadLifetimePool = null;
 
     if (this._prettyPrintWorker) {
-      this._prettyPrintWorker.removeEventListener(
+      this._prettyPrintWorker.destroy();
-        "error", this._onPrettyPrintError, false);
-      this._prettyPrintWorker.removeEventListener(
-        "message", this._onPrettyPrintMsg, false);
-      this._prettyPrintWorker.terminate();
       this._prettyPrintWorker = null;
     }
 
@@ -2571,31 +2548,11 @@ SourceActor.prototype = {
    */
   _sendToPrettyPrintWorker: function (aIndent) {
     return ({ content }) => {
-      const deferred = promise.defer();
+      return this.prettyPrintWorker.performTask("pretty-print", {
-      const id = Math.random();
-
-      const onReply = ({ data }) => {
-        if (data.id !== id) {
-          return;
-        }
-        this.prettyPrintWorker.removeEventListener("message", onReply, false);
-
-        if (data.error) {
-          deferred.reject(new Error(data.error));
-        } else {
-          deferred.resolve(data);
-        }
-      };
-
-      this.prettyPrintWorker.addEventListener("message", onReply, false);
-      this.prettyPrintWorker.postMessage({
-        id: id,
         url: this.url,
         indent: aIndent,
         source: content
-      });
+      })
-
-      return deferred.promise;
     };
   },
 

toolkit/devtools/shared/worker-helper.js

@@ -1,100 +1,112 @@
 /* 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 (root, factory) {
+  "use strict";
 
-/**
+  if (typeof define === "function" && define.amd) {
- * This file is to only be included by ChromeWorkers. This exposes
+    define(factory);
- * a `createTask` function to workers to register tasks for communication
+  } else if (typeof exports === "object") {
- * back to `devtools/toolkit/shared/worker`.
+    module.exports = factory();
- *
+  } else {
- * Tasks can be send their responses via a return value, either a primitive
+    root.workerHelper = factory();
- * or a promise.
+  }
- *
+}(this, function () {
- * createTask(self, "average", function (data) {
+  "use strict";
- *   return data.reduce((sum, val) => sum + val, 0) / data.length;
- * });
- *
- * createTask(self, "average", function (data) {
- *   return new Promise((resolve, reject) => {
- *     resolve(data.reduce((sum, val) => sum + val, 0) / data.length);
- *   });
- * });
- *
- *
- * Errors:
- *
- * Returning an Error value, or if the returned promise is rejected, this
- * propagates to the DevToolsWorker as a rejected promise. If an error is
- * thrown in a synchronous function, that error is also propagated.
- */
 
-/**
+  /**
- * Takes a worker's `self` object, a task name, and a function to
+   * This file is to only be included by ChromeWorkers. This exposes
- * be called when that task is called. The task is called with the
+   * a `createTask` function to workers to register tasks for communication
- * passed in data as the first argument
+   * back to `devtools/toolkit/shared/worker`.
- *
+   *
- * @param {object} self
+   * Tasks can be send their responses via a return value, either a primitive
- * @param {string} name
+   * or a promise.
- * @param {function} fn
+   *
- */
+   * createTask(self, "average", function (data) {
-function createTask (self, name, fn) {
+   *   return data.reduce((sum, val) => sum + val, 0) / data.length;
-  // Store a hash of task name to function on the Worker
+   * });
-  if (!self._tasks) {
+   *
-    self._tasks = {};
+   * createTask(self, "average", function (data) {
+   *   return new Promise((resolve, reject) => {
+   *     resolve(data.reduce((sum, val) => sum + val, 0) / data.length);
+   *   });
+   * });
+   *
+   *
+   * Errors:
+   *
+   * Returning an Error value, or if the returned promise is rejected, this
+   * propagates to the DevToolsWorker as a rejected promise. If an error is
+   * thrown in a synchronous function, that error is also propagated.
+   */
+
+  /**
+   * Takes a worker's `self` object, a task name, and a function to
+   * be called when that task is called. The task is called with the
+   * passed in data as the first argument
+   *
+   * @param {object} self
+   * @param {string} name
+   * @param {function} fn
+   */
+  function createTask (self, name, fn) {
+    // Store a hash of task name to function on the Worker
+    if (!self._tasks) {
+      self._tasks = {};
+    }
+
+    // Create the onmessage handler if not yet created.
+    if (!self.onmessage) {
+      self.onmessage = createHandler(self);
+    }
+
+    // Store the task on the worker.
+    self._tasks[name] = fn;
   }
 
-  // Create the onmessage handler if not yet created.
+  /**
-  if (!self.onmessage) {
+   * Creates the `self.onmessage` handler for a Worker.
-    self.onmessage = createHandler(self);
+   *
+   * @param {object} self
+   * @return {function}
+   */
+  function createHandler (self) {
+    return function (e) {
+      let { id, task, data } = e.data;
+      let taskFn = self._tasks[task];
+
+      if (!taskFn) {
+        self.postMessage({ id, error: `Task "${task}" not found in worker.` });
+        return;
+      }
+
+      try {
+        let results;
+        handleResponse(taskFn(data));
+      } catch (e) {
+        handleError(e);
+      }
+
+      function handleResponse (response) {
+        // If a promise
+        if (response && typeof response.then === "function") {
+          response.then(val => self.postMessage({ id, response: val }), handleError);
+        }
+        // If an error object
+        else if (response instanceof Error) {
+          handleError(response);
+        }
+        // If anything else
+        else {
+          self.postMessage({ id, response });
+        }
+      }
+
+      function handleError (e="Error") {
+        self.postMessage({ id, error: e.message || e });
+      }
+    }
   }
 
-  // Store the task on the worker.
+  return { createTask: createTask };
-  self._tasks[name] = fn;
+}.bind(this)));
-}
-
-exports.createTask = createTask;
-
-/**
- * Creates the `self.onmessage` handler for a Worker.
- *
- * @param {object} self
- * @return {function}
- */
-function createHandler (self) {
-  return function (e) {
-    let { id, task, data } = e.data;
-    let taskFn = self._tasks[task];
-
-    if (!taskFn) {
-      self.postMessage({ id, error: `Task "${task}" not found in worker.` });
-      return;
-    }
-
-    try {
-      let results;
-      handleResponse(taskFn(data));
-    } catch (e) {
-      handleError(e);
-    }
-
-    function handleResponse (response) {
-      // If a promise
-      if (response && typeof response.then === "function") {
-        response.then(val => self.postMessage({ id, response: val }), handleError);
-      }
-      // If an error object
-      else if (response instanceof Error) {
-        handleError(response);
-      }
-      // If anything else
-      else {
-        self.postMessage({ id, response });
-      }
-    }
-
-    function handleError (e="Error") {
-      self.postMessage({ id, error: e.message || e });
-    }
-  }
-}

toolkit/devtools/shared/worker.js

@@ -5,18 +5,22 @@
 
 (function (factory) { // Module boilerplate
   if (this.module && module.id.indexOf("worker") >= 0) { // require
-    const { Cc, Ci, ChromeWorker } = require("chrome");
+    const { Cc, Ci, Cu, ChromeWorker } = require("chrome");
-    factory.call(this, require, exports, module, { Cc, Ci }, ChromeWorker);
+    const dumpn = require("devtools/toolkit/DevToolsUtils").dumpn;
+    factory.call(this, require, exports, module, { Cc, Ci, Cu }, ChromeWorker, dumpn);
   } else { // Cu.import
-      const { classes: Cc, interfaces: Ci, utils: Cu } = Components;
+    const { classes: Cc, interfaces: Ci, utils: Cu } = Components;
-      const { devtools } = Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
+    const { devtools } = Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
-      this.isWorker = false;
+    this.isWorker = false;
-      this.Promise = Cu.import("resource://gre/modules/Promise.jsm", {}).Promise;
+    this.Promise = Cu.import("resource://gre/modules/Promise.jsm", {}).Promise;
-      this.console = Cu.import("resource://gre/modules/devtools/Console.jsm", {}).console;
+    this.console = Cu.import("resource://gre/modules/devtools/Console.jsm", {}).console;
-      factory.call(this, devtools.require, this, { exports: this }, { Cc, Ci }, ChromeWorker);
+    factory.call(
-      this.EXPORTED_SYMBOLS = ["DevToolsWorker"];
+      this, devtools.require, this, { exports: this },
+      { Cc, Ci, Cu }, ChromeWorker, null
+    );
+    this.EXPORTED_SYMBOLS = ["DevToolsWorker"];
   }
-}).call(this, function (require, exports, module, { Ci, Cc }, ChromeWorker ) {
+}).call(this, function (require, exports, module, { Ci, Cc }, ChromeWorker, dumpn) {
 
 let MESSAGE_COUNTER = 0;
 
@@ -29,9 +33,18 @@ let MESSAGE_COUNTER = 0;
  *
  * @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) {
+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;
 
@@ -46,16 +59,31 @@ exports.DevToolsWorker = DevToolsWorker;
  *        Data to be passed into the task implemented by the worker.
  * @return {Promise}
  */
-DevToolsWorker.prototype.performTask = function DevToolsWorkerPerformTask (task, data) {
+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;
-  worker.postMessage({ task, id, data });
+  let payload = { task, id, data };
+
+  if(this._verbose && dumpn) {
+    dumpn("Sending message to worker" +
+          (this._name ? (" (" + this._name + ")") : "" ) +
+          ": " +
+          JSON.stringify(payload, null, 2));
+  }
+  worker.postMessage(payload);
+
+  return new Promise((resolve, reject) => {
+    let listener = ({ data }) => {
+      if(this._verbose && dumpn) {
+        dumpn("Received message from worker" +
+              (this._name ? (" (" + this._name + ")") : "" ) +
+              ": " +
+              JSON.stringify(data, null, 2));
+      }
 
-  return new Promise(function (resolve, reject) {
-    worker.addEventListener("message", function listener({ data }) {
       if (data.id !== id) {
         return;
       }
@@ -65,19 +93,25 @@ DevToolsWorker.prototype.performTask = function DevToolsWorkerPerformTask (task,
       } else {
         resolve(data.response);
       }
-    });
+    };
+
+    worker.addEventListener("message", listener);
   });
 }
 
 /**
  * Terminates the underlying worker. Use when no longer needing the worker.
  */
-DevToolsWorker.prototype.destroy = function DevToolsWorkerDestroy () {
+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.