Bug 1243704 - Serialise errors sent over IPC; r?automatedtester draft
authorAndreas Tolfsen <ato@mozilla.com>
Fri, 29 Jan 2016 12:57:46 +0000
changeset 327295 8e0c36960759d25c73bbbc0f4f0bcb453c1028f4
parent 327247 54eea211e234217c0faa8c05bf4de9fd3005f5c2
child 513688 a1f693b4a435a78d9fde0454d3847ea18f6c3377
push id10227
push useratolfsen@mozilla.com
push dateSat, 30 Jan 2016 07:52:43 +0000
reviewersautomatedtester
bugs1243704
milestone47.0a1
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
testing/marionette/error.js
testing/marionette/listener.js
testing/marionette/proxy.js
testing/marionette/test_error.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -218,16 +218,19 @@ GeckoDriver.prototype.switchToGlobalMess
  *     JSON serialisable object to send to the listener.
  * @param {number=} cmdId
  *     Command ID to ensure synchronisity.
  */
 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;
   }
 
   if (curRemoteFrame === null) {
     this.curBrowser.executeWhenReady(() => {
       if (this.curBrowser.curFrameId) {
           this.mm.broadcastAsyncMessage(name + this.curBrowser.curFrameId, msg);
@@ -237,18 +240,18 @@ GeckoDriver.prototype.sendAsync = functi
       }
     });
   } else {
     let remoteFrameId = curRemoteFrame.targetFrameId;
     try {
       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());
       }
     }
   }
 };
 
--- a/testing/marionette/error.js
+++ b/testing/marionette/error.js
@@ -1,15 +1,15 @@
 /* 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 {interfaces: Ci, utils: Cu} = Components;
+const {interfaces: Ci, utils: Cu} = Components;
 
 const errors = [
   "ElementNotAccessibleError",
   "ElementNotVisibleError",
   "InvalidArgumentError",
   "InvalidElementStateError",
   "InvalidSelectorError",
   "InvalidSessionIdError",
@@ -85,34 +85,62 @@ error.stringify = function(err) {
     }
     return s;
   } catch (e) {
     return "<unprintable error>";
   }
 };
 
 /**
- * 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,
     stacktrace: err.stack || null,
   };
   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
  * error in the specification.
  */
 this.WebDriverError = function(msg) {
   Error.call(this, msg);
   this.name = "WebDriverError";
   this.message = msg;
@@ -292,8 +320,17 @@ this.UnknownError = function(msg) {
 UnknownError.prototype = Object.create(WebDriverError.prototype);
 
 this.UnsupportedOperationError = function(msg) {
   WebDriverError.call(this, msg);
   this.name = "UnsupportedOperationError";
   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);
+};
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -179,17 +179,17 @@ function dispatch(fn) {
         return yield fn(msg.json);
       }
     });
 
     let okOrValueResponse = rv => {
       if (typeof rv == "undefined") {
         sendOk(id);
       } else {
-        sendResponse({value: rv}, id);
+        sendResponse(rv, id);
       }
     };
 
     req.then(okOrValueResponse, err => sendError(err, id))
         .catch(error.report);
   };
 }
 
@@ -393,52 +393,66 @@ function deleteSession(msg) {
   }
   elementManager.reset();
   // reset container frame to the top-most frame
   curContainer = { frame: content, shadowRoot: null };
   curContainer.frame.focus();
   actions.touchIds = {};
 }
 
-/*
- * Helper methods
+/**
+ * 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.
  */
-
-/**
- * Generic method to send a message to the server
- */
-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);
 }
 
 /**
  * Send log message to server
  */
 function sendLog(msg) {
   sendToServer("Marionette:log", {message: msg});
 }
@@ -544,17 +558,17 @@ function createExecuteContentSandbox(win
 
       if (error.isError(obj)) {
         sendError(obj, id);
       } else {
         if (Object.keys(_emu_cbs).length) {
           _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);
         }
       }
 
       asyncTestRunning = false;
       asyncTestTimeoutId = undefined;
       asyncTestCommandId = undefined;
       inactivityTimeoutId = null;
     }
@@ -626,17 +640,17 @@ function executeScript(msg, directInject
       sendSyncMessage("Marionette:shareData",
                       {log: elementManager.wrapValue(marionetteLogObj.getLogs())});
       marionetteLogObj.clearLogs();
 
       if (res == undefined || res.passed == undefined) {
         sendError(new JavaScriptError("Marionette.finish() not called"), asyncTestCommandId);
       }
       else {
-        sendResponse({value: elementManager.wrapValue(res)}, asyncTestCommandId);
+        sendResponse(elementManager.wrapValue(res), asyncTestCommandId);
       }
     }
     else {
       try {
         sandbox.__marionetteParams = Cu.cloneInto(elementManager.convertWrappedArguments(
           msg.json.args, curContainer), sandbox, { wrapReflectors: true });
       } catch (e) {
         sendError(e, asyncTestCommandId);
@@ -652,17 +666,17 @@ function executeScript(msg, directInject
         let data = NetUtil.readInputStreamToString(stream, stream.available());
         stream.close();
         script = data + script;
       }
       let res = Cu.evalInSandbox(script, sandbox, "1.8", filename ? filename : "dummy file", 0);
       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(
         e,
         "execute_script",
         msg.json.filename,
         msg.json.line,
         script);
@@ -1708,17 +1722,17 @@ function switchToFrame(msg) {
     rv = {win: parWindow, frame: foundFrame};
   } else {
     curContainer.frame = curContainer.frame.contentWindow;
     if (msg.json.focus)
       curContainer.frame.focus();
     checkTimer.initWithCallback(checkLoad, 100, Ci.nsITimer.TYPE_ONE_SHOT);
   }
 
-  sendResponse({value: rv}, command_id);
+  sendResponse(rv, command_id);
 }
 
 function addCookie(cookie) {
   cookies.add(cookie.name, cookie.value, cookie);
 }
 
 /**
  * Get all cookies for the current domain.
@@ -1761,18 +1775,18 @@ function deleteCookie(name) {
  */
 function deleteAllCookies() {
   for (let cookie of cookies) {
     cookies.delete(cookie);
   }
 }
 
 function getAppCacheStatus(msg) {
-  sendResponse({ value: curContainer.frame.applicationCache.status },
-               msg.json.command_id);
+  sendResponse(
+      curContainer.frame.applicationCache.status, msg.json.command_id);
 }
 
 // emulator callbacks
 var _emu_cb_id = 0;
 var _emu_cbs = {};
 
 function runEmulatorCmd(cmd, callback) {
   if (callback) {
--- a/testing/marionette/proxy.js
+++ b/testing/marionette/proxy.js
@@ -1,16 +1,17 @@
 /* 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";
 
 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"];
 
 const uuidgen = Cc["@mozilla.org/uuid-generator;1"]
     .getService(Ci.nsIUUIDGenerator);
 
 // Proxy handler that traps requests to get a property.  Will prioritise
@@ -39,91 +40,179 @@ this.proxy = {};
  * backwards compatibility with listener.js.
  *
  * @param {function(): (nsIMessageSender|nsIMessageBroadcaster)} mmFn
  *     Closure function returning the current message manager.
  * @param {function(string, Object, number)} sendAsyncFn
  *     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
     this.activeMessageId = null;
 
     this.mmFn_ = mmFn;
     this.listeners_ = new Map();
     this.dialogueObserver_ = null;
   }
 
   get mm() {
     return this.mmFn_();
   }
 
   /**
-   * 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
    *     "Marionette:foo8", use "foo".
    * @param {Array.<?>=} args
    *     Argument list to pass the function.  If args has a single entry
    *     that is an object, we assume it's an old style dispatch, and
    *     the object will passed literally.
    *
    * @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();
     // TODO(ato): Bug 1242595
     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();
       };
 
       // start content message listener
       // and install observers for global- and tab modal dialogues
       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");
   }
 
   addListener_(path, callback) {
     let autoRemover = msg => {
       this.removeListener_(path);
       modal.removeHandler(this.dialogueObserver_);
       callback(msg);
@@ -141,20 +230,21 @@ proxy.AsyncContentSender = class {
 
   removeAllListeners_() {
     let ok = true;
     for (let [p, cb] of this.listeners_) {
       ok |= this.removeListener_(p);
     }
     return ok;
   }
-
-  static makeReplyPath(uuid) {
-    return "Marionette:asyncReply:" + uuid;
-  }
+};
+proxy.AsyncMessageChannel.ReplyType = {
+  Ok: 0,
+  Value: 1,
+  Error: 2,
 };
 
 /**
  * Creates a transparent interface from the content- to the chrome context.
  *
  * Calls to this object will be proxied via the frame's sendSyncMessage
  * (nsISyncMessageSender) function.  Since the message is synchronous,
  * the return value is presented as a return value.
--- a/testing/marionette/test_error.js
+++ b/testing/marionette/test_error.js
@@ -47,29 +47,50 @@ add_test(function test_stringify() {
       error.stringify(new WebDriverError("foo")).split("\n")[0]);
   equal("InvalidArgumentError: foo",
       error.stringify(new InvalidArgumentError("foo")).split("\n")[0]);
 
   run_next_test();
 });
 
 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 WebDriverError("a");
+  deepEqual({error: e1.status, message: "a", stacktrace: null},
+      error.toJson(e1));
+
+  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/));
 
-  let e1 = new Error("b");
-  deepEqual({error: undefined, message: "b", stacktrace: e1.stack},
-      error.toJson(e1));
-  let e2 = new WebDriverError("b");
-  deepEqual({error: e2.status, message: "b", stacktrace: null},
-      error.toJson(e2));
+  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();
 });
 
 add_test(function test_WebDriverError() {
   let err = new WebDriverError("foo");
   equal("WebDriverError", err.name);
   equal("foo", err.message);