From 023ec5201df0fbd62acf7f8891c0c412c203031f Mon Sep 17 00:00:00 2001 From: Andreas Tolfsen Date: Fri, 29 Jan 2016 12:57:46 +0000 Subject: [PATCH] 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 | 7 +- testing/marionette/error.js | 47 ++++++++++-- testing/marionette/listener.js | 72 ++++++++++------- testing/marionette/proxy.js | 128 ++++++++++++++++++++++++++----- testing/marionette/test_error.js | 43 ++++++++--- 5 files changed, 231 insertions(+), 66 deletions(-) diff --git a/testing/marionette/driver.js b/testing/marionette/driver.js index 16b8a75c886..4e920324971 100644 --- a/testing/marionette/driver.js +++ b/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()); diff --git a/testing/marionette/error.js b/testing/marionette/error.js index d06529169d7..fb00c40b2b1 100644 --- a/testing/marionette/error.js +++ b/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.} - * 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.} 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); +}; diff --git a/testing/marionette/listener.js b/testing/marionette/listener.js index f3404071d67..0cec32be334 100644 --- a/testing/marionette/listener.js +++ b/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 diff --git a/testing/marionette/proxy.js b/testing/marionette/proxy.js index d11ec5e136e..1a294394f1f 100644 --- a/testing/marionette/proxy.js +++ b/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, }; /** diff --git a/testing/marionette/test_error.js b/testing/marionette/test_error.js index 5d9dbca532e..2ee7c18996b 100644 --- a/testing/marionette/test_error.js +++ b/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(); });