Bug 1243704 - Serialise errors sent over IPC; r=automatedtester

This fixes an instance of passing an Error prototype over the message
manager as a CPOW.  We solve this by marshaling the error, which is
now done automatically by the new AsyncMessageChannel.  It allows us to
create an (almost) transparent promise-based interface between chrome-
and content contexts.

The patch also makes AsyncMessageChannel reusable on both sides of the
message listener, but it's currently not used at its maximum potential
because of the way the listener is architected.

testing/marionette/driver.js

@@ -223,6 +223,9 @@ GeckoDriver.prototype.sendAsync = function(name, msg, cmdId) {
   let curRemoteFrame = this.curBrowser.frameManager.currentRemoteFrame;
   name = "Marionette:" + name;
 
+  // TODO(ato): When proxy.AsyncMessageChannel
+  // is used for all chrome <-> content communication
+  // this can be removed.
   if (cmdId) {
     msg.command_id = cmdId;
   }
@@ -242,8 +245,8 @@ GeckoDriver.prototype.sendAsync = function(name, msg, cmdId) {
       this.mm.sendAsyncMessage(name + remoteFrameId, msg);
     } catch (e) {
       switch(e.result) {
-        case Components.results.NS_ERROR_FAILURE:
-        case Components.results.NS_ERROR_NOT_INITIALIZED:
+        case Cr.NS_ERROR_FAILURE:
+        case Cr.NS_ERROR_NOT_INITIALIZED:
           throw new NoSuchWindowError();
         default:
           throw new WebDriverError(e.toString());

testing/marionette/error.js

@@ -4,7 +4,7 @@
 
 "use strict";
 
-var {interfaces: Ci, utils: Cu} = Components;
+const {interfaces: Ci, utils: Cu} = Components;
 
 const errors = [
   "ElementNotAccessibleError",
@@ -90,15 +90,21 @@ error.stringify = function(err) {
 };
 
 /**
- * Marshal an Error to a JSON structure.
+ * Marshal a WebDriverError prototype to a JSON dictionary.
  *
- * @param {Error} err
- *     The Error to serialise.
+ * @param {WebDriverError} err
+ *     Error to serialise.
  *
  * @return {Object.<string, Object>}
- *     JSON structure with the keys "error", "message", and "stacktrace".
+ *     JSON dictionary with the keys "error", "message", and "stacktrace".
+ * @throws {TypeError}
+ *     If error type is not serialisable.
  */
 error.toJson = function(err) {
+  if (!error.isWebDriverError(err)) {
+    throw new TypeError(`Unserialisable error type: ${err}`);
+  }
+
   let json = {
     error: err.status,
     message: err.message || null,
@@ -107,6 +113,28 @@ error.toJson = function(err) {
   return json;
 };
 
+/**
+ * Unmarshal a JSON dictionary to a WebDriverError prototype.
+ *
+ * @param {Object.<string, string>} json
+ *     JSON dictionary with the keys "error", "message", and "stacktrace".
+ *
+ * @return {WebDriverError}
+ *     Deserialised error prototype.
+ */
+error.fromJson = function(json) {
+  if (!statusLookup.has(json.error)) {
+    throw new TypeError(`Undeserialisable error type: ${json.error}`);
+  }
+
+  let errCls = statusLookup.get(json.error);
+  let err = new errCls(json.message);
+  if ("stacktrace" in json) {
+    err.stack = json.stacktrace;
+  }
+  return err;
+};
+
 /**
  * WebDriverError is the prototypal parent of all WebDriver errors.
  * It should not be used directly, as it does not correspond to a real
@@ -297,3 +325,12 @@ this.UnsupportedOperationError = function(msg) {
   this.status = "unsupported operation";
 };
 UnsupportedOperationError.prototype = Object.create(WebDriverError.prototype);
+
+const nameLookup = new Map();
+const statusLookup = new Map();
+for (let s of errors) {
+  let cls = this[s];
+  let inst = new cls();
+  nameLookup.set(inst.name, cls);
+  statusLookup.set(inst.status, cls);
+};

testing/marionette/listener.js

@@ -184,7 +184,7 @@ function dispatch(fn) {
       if (typeof rv == "undefined") {
         sendOk(id);
       } else {
-        sendResponse({value: rv}, id);
+        sendResponse(rv, id);
       }
     };
 
@@ -398,42 +398,56 @@ function deleteSession(msg) {
   actions.touchIds = {};
 }
 
-/*
- * Helper methods
- */
-
 /**
- * Generic method to send a message to the server
+ * Send asynchronous reply to chrome.
+ *
+ * @param {UUID} uuid
+ *     Unique identifier of the request.
+ * @param {AsyncContentSender.ResponseType} type
+ *     Type of response.
+ * @param {?=} data
+ *     JSON serialisable object to accompany the message.  Defaults to
+ *     an empty dictionary.
  */
-function sendToServer(path, data = {}, objs, id) {
-  if (id) {
-    data.command_id = id;
-  }
-  sendAsyncMessage(path, data, objs);
+function sendToServer(uuid, data = undefined) {
+  let channel = new proxy.AsyncMessageChannel(
+      () => this,
+      sendAsyncMessage.bind(this));
+  channel.reply(uuid, data);
 }
 
 /**
- * Send response back to server
+ * Send asynchronous reply with value to chrome.
+ *
+ * @param {?} obj
+ *     JSON serialisable object of arbitrary type and complexity.
+ * @param {UUID} uuid
+ *     Unique identifier of the request.
  */
-function sendResponse(value, id) {
-  let path = proxy.AsyncContentSender.makeReplyPath(id);
-  sendToServer(path, value, null, id);
+function sendResponse(obj, id) {
+  sendToServer(id, obj);
 }
 
 /**
- * Send ack back to server
+ * Send asynchronous reply to chrome.
+ *
+ * @param {UUID} uuid
+ *     Unique identifier of the request.
  */
-function sendOk(id) {
-  let path = proxy.AsyncContentSender.makeReplyPath(id);
-  sendToServer(path, {}, null, id);
+function sendOk(uuid) {
+  sendToServer(uuid);
 }
 
 /**
- * Send error message to server
+ * Send asynchronous error reply to chrome.
+ *
+ * @param {Error} err
+ *     Error to notify chrome of.
+ * @param {UUID} uuid
+ *     Unique identifier of the request.
  */
-function sendError(err, id) {
-  let path = proxy.AsyncContentSender.makeReplyPath(id);
-  sendToServer(path, {error: null}, {error: err}, id);
+function sendError(err, uuid) {
+  sendToServer(uuid, err);
 }
 
 /**
@@ -549,7 +563,7 @@ function createExecuteContentSandbox(win, timeout) {
           _emu_cbs = {};
           sendError(new WebDriverError("Emulator callback still pending when finish() called"), id);
         } else {
-          sendResponse({value: elementManager.wrapValue(obj)}, id);
+          sendResponse(elementManager.wrapValue(obj), id);
         }
       }
 
@@ -631,7 +645,7 @@ function executeScript(msg, directInject) {
         sendError(new JavaScriptError("Marionette.finish() not called"), asyncTestCommandId);
       }
       else {
-        sendResponse({value: elementManager.wrapValue(res)}, asyncTestCommandId);
+        sendResponse(elementManager.wrapValue(res), asyncTestCommandId);
       }
     }
     else {
@@ -657,7 +671,7 @@ function executeScript(msg, directInject) {
       sendSyncMessage("Marionette:shareData",
                       {log: elementManager.wrapValue(marionetteLogObj.getLogs())});
       marionetteLogObj.clearLogs();
-      sendResponse({value: elementManager.wrapValue(res)}, asyncTestCommandId);
+      sendResponse(elementManager.wrapValue(res), asyncTestCommandId);
     }
   } catch (e) {
     let err = new JavaScriptError(
@@ -1713,7 +1727,7 @@ function switchToFrame(msg) {
     checkTimer.initWithCallback(checkLoad, 100, Ci.nsITimer.TYPE_ONE_SHOT);
   }
 
-  sendResponse({value: rv}, command_id);
+  sendResponse(rv, command_id);
 }
 
 function addCookie(cookie) {
@@ -1766,8 +1780,8 @@ function deleteAllCookies() {
 }
 
 function getAppCacheStatus(msg) {
-  sendResponse({ value: curContainer.frame.applicationCache.status },
-               msg.json.command_id);
+  sendResponse(
+      curContainer.frame.applicationCache.status, msg.json.command_id);
 }
 
 // emulator callbacks

testing/marionette/proxy.js

@@ -6,6 +6,7 @@
 
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
+Cu.import("chrome://marionette/content/error.js");
 Cu.import("chrome://marionette/content/modal.js");
 
 this.EXPORTED_SYMBOLS = ["proxy"];
@@ -44,20 +45,19 @@ this.proxy = {};
  *     Callback for sending async messages.
  */
 proxy.toListener = function(mmFn, sendAsyncFn) {
-  let sender = new proxy.AsyncContentSender(mmFn, sendAsyncFn);
+  let sender = new proxy.AsyncMessageChannel(mmFn, sendAsyncFn);
   return new Proxy(sender, ownPriorityGetterTrap);
 };
 
 /**
- * With the AsyncContentSender it is possible to make asynchronous calls
- * to the message listener in a frame script.
+ * Provides a transparent interface between chrome- and content space.
  *
- * The responses from content are expected to be JSON Objects, where an
- * {@code error} key indicates that an error occured, and a {@code value}
- * entry that the operation was successful.  It is the value of the
- * {@code value} key that is returned to the consumer through a promise.
+ * The AsyncMessageChannel is an abstraction of the message manager
+ * IPC architecture allowing calls to be made to any registered message
+ * listener in Marionette.  The {@code #send(...)} method returns a promise
+ * that gets resolved when the message handler calls {@code .reply(...)}.
  */
-proxy.AsyncContentSender = class {
+proxy.AsyncMessageChannel = class {
   constructor(mmFn, sendAsyncFn) {
     this.sendAsync = sendAsyncFn;
     // TODO(ato): Bug 1242595
@@ -73,8 +73,14 @@ proxy.AsyncContentSender = class {
   }
 
   /**
-   * Call registered function in the frame script environment of the
-   * current browsing context's content frame.
+   * Send a message across the channel.  The name of the function to
+   * call must be registered as a message listener.
+   *
+   * Usage:
+   *
+   *     let channel = new AsyncMessageChannel(
+   *         messageManager, sendAsyncMessage.bind(this));
+   *     let rv = yield channel.send("remoteFunction", ["argument"]);
    *
    * @param {string} name
    *     Function to call in the listener, e.g. for the message listener
@@ -86,6 +92,10 @@ proxy.AsyncContentSender = class {
    *
    * @return {Promise}
    *     A promise that resolves to the result of the command.
+   * @throws {TypeError}
+   *     If an unsupported reply type is received.
+   * @throws {WebDriverError}
+   *     If an error is returned over the channel.
    */
   send(name, args = []) {
     let uuid = uuidgen.generateUUID().toString();
@@ -93,15 +103,27 @@ proxy.AsyncContentSender = class {
     this.activeMessageId = uuid;
 
     return new Promise((resolve, reject) => {
-      let path = proxy.AsyncContentSender.makeReplyPath(uuid);
+      let path = proxy.AsyncMessageChannel.makePath(uuid);
       let cb = msg => {
         this.activeMessageId = null;
-        if ("error" in msg.json) {
-          reject(msg.objects.error);
-        } else {
-          resolve(msg.json.value);
+
+        switch (msg.json.type) {
+          case proxy.AsyncMessageChannel.ReplyType.Ok:
+          case proxy.AsyncMessageChannel.ReplyType.Value:
+            resolve(msg.json.data);
+            break;
+
+          case proxy.AsyncMessageChannel.ReplyType.Error:
+            let err = error.fromJson(msg.json.data);
+            reject(err);
+            break;
+
+          default:
+            throw new TypeError(
+                `Unknown async response type: ${msg.json.type}`);
         }
       };
+
       this.dialogueObserver_ = (subject, topic) => {
         this.cancelAll();
         resolve();
@@ -112,13 +134,80 @@ proxy.AsyncContentSender = class {
       this.addListener_(path, cb);
       modal.addHandler(this.dialogueObserver_);
 
+      // sendAsync is GeckoDriver#sendAsync
       this.sendAsync(name, marshal(args), uuid);
     });
   }
 
+  /**
+   * Reply to an asynchronous request.
+   *
+   * Passing an WebDriverError prototype will cause the receiving channel
+   * to throw this error.
+   *
+   * Usage:
+   *
+   *     let channel = proxy.AsyncMessageChannel(
+   *         messageManager, sendAsyncMessage.bind(this));
+   *
+   *     // throws in requester:
+   *     channel.reply(uuid, new WebDriverError());
+   *
+   *     // returns with value:
+   *     channel.reply(uuid, "hello world!");
+   *
+   *     // returns with undefined:
+   *     channel.reply(uuid);
+   *
+   * @param {UUID} uuid
+   *     Unique identifier of the request.
+   * @param {?=} obj
+   *     Message data to reply with.
+   */
+  reply(uuid, obj = undefined) {
+    // TODO(ato): Eventually the uuid will be hidden in the dispatcher
+    // in listener, and passing it explicitly to this function will be
+    // unnecessary.
+    if (typeof obj == "undefined") {
+      this.sendReply_(uuid, proxy.AsyncMessageChannel.ReplyType.Ok);
+    } else if (error.isError(obj)) {
+      let serr = error.toJson(obj);
+      this.sendReply_(uuid, proxy.AsyncMessageChannel.ReplyType.Error, serr);
+    } else {
+      this.sendReply_(uuid, proxy.AsyncMessageChannel.ReplyType.Value, obj);
+    }
+  }
+
+  sendReply_(uuid, type, data = undefined) {
+    let path = proxy.AsyncMessageChannel.makePath(uuid);
+    let msg = {type: type, data: data};
+    // here sendAsync is actually the content frame's
+    // sendAsyncMessage(path, message) global
+    this.sendAsync(path, msg);
+  }
+
+  /**
+   * Produces a path, or a name, for the message listener handler that
+   * listens for a reply.
+   *
+   * @param {UUID} uuid
+   *     Unique identifier of the channel request.
+   *
+   * @return {string}
+   *     Path to be used for nsIMessageListener.addMessageListener.
+   */
+  static makePath(uuid) {
+    return "Marionette:asyncReply:" + uuid;
+  }
+
+  /**
+   * Abort listening for responses, remove all modal dialogue handlers,
+   * and cancel any ongoing requests in the listener.
+   */
   cancelAll() {
     this.removeAllListeners_();
     modal.removeHandler(this.dialogueObserver_);
+    // TODO(ato): It's not ideal to have listener specific behaviour here:
     this.sendAsync("cancelRequest");
   }
 
@@ -146,10 +235,11 @@ proxy.AsyncContentSender = class {
     }
     return ok;
   }
-
-  static makeReplyPath(uuid) {
-    return "Marionette:asyncReply:" + uuid;
-  }
+};
+proxy.AsyncMessageChannel.ReplyType = {
+  Ok: 0,
+  Value: 1,
+  Error: 2,
 };
 
 /**

testing/marionette/test_error.js

@@ -52,19 +52,40 @@ add_test(function test_stringify() {
 });
 
 add_test(function test_toJson() {
-  deepEqual({error: "a", message: null, stacktrace: null},
-      error.toJson({status: "a"}));
-  deepEqual({error: "a", message: "b", stacktrace: null},
-      error.toJson({status: "a", message: "b"}));
-  deepEqual({error: "a", message: "b", stacktrace: "c"},
-      error.toJson({status: "a", message: "b", stack: "c"}));
+  Assert.throws(() => error.toJson(new Error()),
+      /Unserialisable error type: [object Error]/);
 
-  let e1 = new Error("b");
-  deepEqual({error: undefined, message: "b", stacktrace: e1.stack},
+  let e1 = new WebDriverError("a");
+  deepEqual({error: e1.status, message: "a", stacktrace: null},
       error.toJson(e1));
-  let e2 = new WebDriverError("b");
-  deepEqual({error: e2.status, message: "b", stacktrace: null},
-      error.toJson(e2));
+
+  let e2 = new JavaScriptError("first", "second", "third", "fourth");
+  let e2s = error.toJson(e2);
+  equal(e2.status, e2s.error);
+  equal(e2.message, e2s.message);
+  ok(e2s.stacktrace.match(/second/));
+  ok(e2s.stacktrace.match(/third/));
+  ok(e2s.stacktrace.match(/fourth/));
+
+  run_next_test();
+});
+
+add_test(function test_fromJson() {
+  Assert.throws(() => error.fromJson({error: "foo"}),
+      /Undeserialisable error type: foo/);
+  Assert.throws(() => error.fromJson({error: "Error"}),
+      /Undeserialisable error type: Error/);
+  Assert.throws(() => error.fromJson({}),
+      /Undeserialisable error type: undefined/);
+
+  let e1 = new WebDriverError("1");
+  deepEqual(e1, error.fromJson({error: "webdriver error", message: "1"}));
+  let e2 = new InvalidArgumentError("2");
+  deepEqual(e2, error.fromJson({error: "invalid argument", message: "2"}));
+
+  let e3 = new JavaScriptError("first", "second", "third", "fourth");
+  let e3s = error.toJson(e3);
+  deepEqual(e3, error.fromJson(e3s));
 
   run_next_test();
 });