The Marionette server handles requests separately with a global sense of state which makes it hard to introduce generalised behaviour to many commands. This effectively slows down protocol implementation because each command request individually needs to do heavy lifting. A possible solution to this is to make abstractions that call down to the protocol implementation to hide the details that are unimportant. This would relieve – and to some extent separate – the WebDriver implementation from the infrastructural parts. It would be advantageous to build a more expressive architecture through a series of refactorings to address these issues, as well as making Marionette more readable and approachable. Reducing complexity is a long term goal, but we are currently lacking the correct design to make that an achievable goal. Specifically we should aim to do the following: - Request handling and command dispatching - Error handling and ensuring uncaught errors are handled - Introduce changes that make it easier to implement the wire protocol as defined in the specification - Investigate and possibly suggest a less laborious chrome/content communication model

pushlog: https://hg.mozilla.org/try/pushloghtml?changeset=1d217e25e6e0 try desktop: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d217e25e6e0 try mobile: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e4209addece2

(In reply to Andreas Tolfsen (:ato) from comment #1) > pushlog: https://hg.mozilla.org/try/pushloghtml?changeset=1d217e25e6e0 > try desktop: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d217e25e6e0 > try mobile: > https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e4209addece2 https://hg.mozilla.org/try/rev/71912d44d6cc#l1.204 s/finalUIStartup/finalUiStartup/

The problem I currently see in the latest try push: JavaScript error: chrome://marionette/content/cmdproc.js, line 1: Error: chrome://marionette/content/error.js - EXPORTED_SYMBOLS is not an array. is almost certainly caused by needing to express EXPORTED_SYMBOLS as this.EXPORTED_SYMBOLS for B2G. B2G reuses the global scope, so 'this.' is needed to force EXPORTED_SYMBOLS into the file scope being used. See bug 798491 for more context.

Brief update on the progress: Fixed many issues. Tests are now running on B2G, but I expect ~80 failures or so. pushlog: https://hg.mozilla.org/try/pushloghtml?changeset=7c5049dbb1b4 try desktop: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c5049dbb1b4

try desktop, with fix for marionette-frame-manager.js, shows expected results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=511665f0b699

try B2G emulators, including hidden jobs: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8e108a41440&exclusion_state=all

try desktop: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b45d08941772&exclusion_state=all try mobile: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba0e2a7aa926&exclusion_state=all Some issues with ScriptTimeoutError not being thrown in executeScript in chrome context.

try desktop: https://treeherder.mozilla.org/#/jobs?repo=try&revision=da65548a4f39 try mobile: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b22e90e493fd

try desktop: https://treeherder.mozilla.org/#/jobs?repo=try&revision=914275b9c36c try mobile: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7509d0739f77 pushlog: https://hg.mozilla.org/try/pushloghtml?changeset=d1cc07eaf969

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb30b176a5a4

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e94277928022&exclusion_profile=false

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=98b2eaf940af&exclusion_profile=false

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca6f8d4c23b8&exclusion_profile=false Note that Mn and Mnw on B2G ICS are disabled on Treeherder. Mn is failing spectacularly on m-c and Mnw are showing approximately the same number of failures (202/8/2 vs. 202/6/2).

Rebased patch series, try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e55f6a99edf5&exclusion_profile=false

This is a bit ugly (and rough), but ports bug 1096488 to apply on top of the patch series in a way that passes the tests of interests there with e10s on.

Thanks bzexport, but no thanks!

try with :chmanchester's rebased remoteness change diff applied: https://treeherder.mozilla.org/#/jobs?repo=try&revision=64a7dadcfca3&exclusion_profile=false

try with modal dialogues fixed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e45022498ddb&exclusion_profile=false

try with emulator callbacks fixed yet again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6b4ba4173b3&exclusion_profile=false

/r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5461 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 68d42c9da326dd70453488a34501bc0b6d94334a

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5461 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 68d42c9da326dd70453488a34501bc0b6d94334a

https://reviewboard.mozilla.org/r/5443/#review4393 ::: testing/marionette/marionette-server.js (Diff revision 1) > - command_id, this.context); > + command_id, true /* ignore visibility check */); Do we usually put comments next to arguments like this? I haven't seen it around.

https://reviewboard.mozilla.org/r/5445/#review4395 ::: testing/marionette/error.js (Diff revision 1) > + if (obj === null || typeof obj != "object") I don't think it's a rule written down anywhere, but I am strongly inclined to use braces in cases like these. ::: testing/marionette/error.js (Diff revision 1) > + Cu.reportError(msg); This is the only place in this file I see a dependency on chrome, is this required? ::: testing/marionette/error.js (Diff revision 1) > + * Checks if obj is an instance of the Error prototype in a safely manner. nit: s/safely/safe/

https://reviewboard.mozilla.org/r/5443/#review4397 > Do we usually put comments next to arguments like this? I haven't seen it around. I find that for functions with very long argument sequences it helps to say what an argument is for if it's not obvious from its name or type. I'm not suggesting we make a rule of that. Ideally the number of arguments to a function will be short and easy to remember. When they aren't it's usually a warning sign (like this case) that there are too many arguments.

https://reviewboard.mozilla.org/r/5447/#review4403 ::: testing/marionette/command.js (Diff revision 1) > + }.bind(this)).then(resolve, reject); spawn returns a promise, wouldn't it be equivalent to remove the additional Promise layer and have .then(resp.send.bind(resp.send.bing(resp) ... right here?

https://reviewboard.mozilla.org/r/5449/#review4419 ::: testing/marionette/dispatcher.js (Diff revision 1) > +loader.loadSubScript("resource://gre/modules/devtools/transport/transport.js"); Is there anything in this file from transport.js? ::: testing/marionette/dispatcher.js (Diff revision 1) > +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); I don't see anything in this file using XPCOMUtils

https://reviewboard.mozilla.org/r/5443/#review4439 ::: testing/marionette/marionette-server.js (Diff revision 1) > - if (this.context == "chrome") { > + switch (this.context) { Hopefully this is just a mozreview thing but can you indent you `case` statements ::: testing/marionette/marionette-server.js (Diff revision 1) > + case Context.CHROME: Hopefully this is just a mozreview thing but can you indent you `case` statements ::: testing/marionette/marionette-server.js (Diff revision 1) > - if (this.context == "chrome") { > + switch (this.context) { Hopefully this is just a mozreview thing but can you indent you `case` statements ::: testing/marionette/marionette-server.js (Diff revision 1) > + case Context.CHROME: as above ::: testing/marionette/marionette-server.js (Diff revision 1) > + switch (this.context) { and here :) ::: testing/marionette/marionette-server.js (Diff revision 1) > + switch (this.context) { Check the rest of the file too, not going to flag all of them :)

https://reviewboard.mozilla.org/r/5451/#review4423 ::: testing/marionette/components/marionettecomponent.js (Diff revision 1) > - this._loaded = false; > + this.loaded_ = false; I haven't seen a trailing underscore around our codebase anywhere. ::: testing/marionette/components/marionettecomponent.js (Diff revision 1) > - aWindow.removeEventListener("load", onLoad); > + win.removeEventListener("load", this); I don't think "this" is what you want here.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato https://reviewboard.mozilla.org/r/5441/#review4445 ::: testing/marionette/server.js (Diff revision 1) > +Cu.import("resource://gre/modules/FileUtils.jsm"); I don't see FileUtils used in this file. ::: testing/marionette/server.js (Diff revision 1) > + Services.prefs.setBoolPref(SPECIAL_POWERS_PREF, true); It looks like the circumstances where this is set changes with this patch. ::: testing/marionette/dispatcher.js (Diff revision 1) > + if (Services.appinfo.name != "Firefox") { I get the impression appName isn't the same as Services.appinfo.name for mulet, so this has changed slightly.

https://reviewboard.mozilla.org/r/5449/#review4443 ::: testing/marionette/dispatcher.js (Diff revision 1) > + * A factory function that takes an Emulator as arguments and produces s/arguments/argument/ ::: testing/marionette/dispatcher.js (Diff revision 1) > + * Requests handlers defined in this.requests take presedence s/Requests/Request/ ::: testing/marionette/dispatcher.js (Diff revision 1) > + * Responses from commands as well as messages from listener. This documentation is just appalling. ::: testing/marionette/dispatcher.js (Diff revision 1) > + * cmdId is a unique identifier assigned to the client's request Again, documentation isn't great. Should mention why emulator callbacks are handled specially (i.e. they are “transparent”) and why `this.commandId` is only reset for client responses. ::: testing/marionette/dispatcher.js (Diff revision 1) > + * Sends given payload as-is to the connected client s/Sends/Send/ Documentation should also include that it does a series of checks, which `sendRaw` skips. ::: testing/marionette/dispatcher.js (Diff revision 1) > +Dispatcher.prototype.beginNewCommand = function() { Documentation.

https://reviewboard.mozilla.org/r/5457/#review4447 ::: testing/marionette/marionette-simpletest.js (Diff revision 1) > +Cu.import("chrome://marionette/content/emulator.js"); Unused imports

https://reviewboard.mozilla.org/r/5445/#review4441 ::: testing/marionette/error.js (Diff revision 1) > + this.status = ""; // not defined in spec defined as `no such alert`. See w3c.github.io/webdriver/webdriver-spec.html#handling-errors

https://reviewboard.mozilla.org/r/5447/#review4449 ::: testing/marionette/command.js (Diff revision 1) > + * @param {(Error|Object)} err The error to send, either an instance of Line break ::: testing/marionette/command.js (Diff revision 1) > + */ Argument docs ::: testing/marionette/command.js (Diff revision 1) > +CommandProcessor.prototype.execute = function( Argument docs ::: testing/marionette/command.js (Diff revision 1) > + {sessionId: this.driver.sessionId}); Assign {sessionId: …} to a legible variable first. ::: testing/marionette/command.js (Diff revision 1) > + get name() { return this.data.get("name") }, Missing lots of semicolons

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5461 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 68d42c9da326dd70453488a34501bc0b6d94334a

https://reviewboard.mozilla.org/r/5451/#review4451 ::: testing/marionette/driver.js (Diff revision 1) > + for (let [t,o] of this) { > + Services.obs.addObserver(o, t, false); > + } Doing this here means we don't handle modals that arise as a result of anything happening in chrome scope. ::: testing/marionette/driver.js (Diff revision 1) > + let obs = new Map(); This doesn't look like it should be a Map.

https://reviewboard.mozilla.org/r/5447/#review4453 ::: testing/marionette/command.js (Diff revision 1) > +CommandProcessor.prototype.execute = function( can you put the function signature on one line to make it easier to read. It puts the line to 87 chars and I am ok with that :)

https://reviewboard.mozilla.org/r/5461/#review4455 Ship It!

https://reviewboard.mozilla.org/r/5459/#review4457 Ship It!

https://reviewboard.mozilla.org/r/5451/#review4461 ::: testing/marionette/components/marionettecomponent.js (Diff revision 1) > - onSocketAccepted: function mc_onSocketAccepted(aSocket, aTransport) { > +MarionetteComponent.prototype.onSocketAccepted = function(so, transport) { s/so/socket the original argument name was more legible ::: testing/marionette/components/marionettecomponent.js (Diff revision 1) > - onStopListening: function mc_onStopListening(aSocket, status) { > +MarionetteComponent.prototype.onStopListening = function(so, status) { s/so/socket the original argument name was more legible ::: testing/marionette/server.js (Diff revision 1) > +MarionetteServer.prototype.onSocketAccepted = function(serverSo, clientSo) { can we put socket instead of so to make this more readable and then people can understand what we are expecting ::: testing/marionette/server.js (Diff revision 1) > +Cu.import("resource://gre/modules/NetUtil.jsm"); Is this import required? Can't see it being used? ::: testing/marionette/server.js (Diff revision 1) > + Does this need documentation? Well, do all the methods in this function require docs?

https://reviewboard.mozilla.org/r/5455/#review4465 Ship It!

https://reviewboard.mozilla.org/r/5453/#review4489 Ship It!

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5461 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 17b496358653c4326159c681762347c9faaa5cd0

https://reviewboard.mozilla.org/r/5441/#review4491 > It looks like the circumstances where this is set changes with this patch. It used to be set on `MarionetteServerConnection.prototype.newSession`. Now it's set only once for every instance of `MarionetteServer`. Do you think it's a problem?

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5461 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r aaff6848958a580e2f65fe04ca742085cfe333c6

https://reviewboard.mozilla.org/r/5445/#review4493 > I don't think it's a rule written down anywhere, but I am strongly inclined to use braces in cases like these. I don't care either way. Changed all instances I could find to be wrapped in curly braces.

https://reviewboard.mozilla.org/r/5445/#review4495 > This is the only place in this file I see a dependency on chrome, is this required? This is the same as what devtools do for their error reporting. As I understand it, it means Marionette errors will appear in the Browser Console which can be useful if you have the debugger attached.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G Pull down these commits: hg pull review -r f15a90f8c61b19ed490b3824ceef371bf2fa9c95

https://reviewboard.mozilla.org/r/5447/#review4497 > can you put the function signature on one line to make it easier to read. It puts the line to 87 chars and I am ok with that :) OK, I was just following the Google Javascript Style Guide here.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 12957f71543ea7c7fe43e45a8b2a42a065ea3446

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 61a501e238e7e1ffc3fa5e74483da332723f15af

https://reviewboard.mozilla.org/r/5449/#review4501 > Is there anything in this file from transport.js? Nope, removing.

https://reviewboard.mozilla.org/r/5451/#review4505 > I haven't seen a trailing underscore around our codebase anywhere. I was following the Google JavaScript Style Guide (for the lack of any such equivalent at Mozilla) which says private properties should have a trailing underscore. I found some support for this in the Mozilla codebase. (https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml#JavaScript_Style_Rules) I'm happy to change it to be the other way around, but there doesn't seem to be concensus in our codebase in any direction on this. > I don't think "this" is what you want here. Isn't `this` the function scope? The function doesn't rebind `this`.

https://reviewboard.mozilla.org/r/5451/#review4507 > This doesn't look like it should be a Map. It's a map so that we can use the `for (let [k,v] of obs)` iterator. What data structure do you think would be better? > Doing this here means we don't handle modals that arise as a result of anything happening in chrome scope. This isn't currently a problem as the modals arise from content events. At least test_modal_dialogs.py is passing.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 2c5b9285bf09b3cedc076de35589440777b3cbfb

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 914cd77db333580d1a245b3c4d348553e0a462ed

https://reviewboard.mozilla.org/r/5441/#review4523 > It used to be set on `MarionetteServerConnection.prototype.newSession`. Now it's set only once for every instance of `MarionetteServer`. Do you think it's a problem? I'm not sure, but I would double check.

https://reviewboard.mozilla.org/r/5451/#review4529 > This isn't currently a problem as the modals arise from content events. At least test_modal_dialogs.py is passing. I can imagine this being significant for firefox-ui-tests, maybe not immediately.

https://reviewboard.mozilla.org/r/5451/#review4525 > Isn't `this` the function scope? The function doesn't rebind `this`. I'd wager "this" is undefined at this point.

https://reviewboard.mozilla.org/r/5451/#review4527 ::: testing/marionette/driver.js (Diff revision 1) > + let obs = new Map(); Oh, I misread. The map works :)

https://reviewboard.mozilla.org/r/5451/#review4533 ::: testing/marionette/driver.js (Diff revision 7) > + let listenerWindow = Services.wm.getOuterWindowWithId(id); I ran into problems with this and e10s, did you establish this isn't a problem? ::: testing/marionette/driver.js (Diff revision 7) > + let {inactivityTimeout: inactivityTimeout, > + scriptTimeout: timeout, > + script: script, > + newSandbox: newSandbox, > + args: args, > + specialPowers: specialPowers, > + filename: filename, > + line: line} = cmd.parameters; You don't have to restate the property names unless you want to rename them locally, this can be let {inactivityTimeout, scriptTimeout, ... } = cmd.parameters; ::: testing/marionette/driver.js (Diff revision 7) > +GeckoDriver.prototype.registerPromise = Task.async(function*() { > + const li = "Marionette:register"; > + > + yield new Promise((resolve) => { This Task isn't doing a lot that the promise isn't -- this would probably work as a regular function that returns the promise immediately without the Task.async part. It looks like all that happens with the Promse is it gets constructed and then a little later the caller blocks on it. I noticed this pattern a few other places, I think the comment applies there too.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato https://reviewboard.mozilla.org/r/5441/#review4531 ::: testing/marionette/dispatcher.js (Diff revision 8) > +const loader = Cc["@mozilla.org/moz/jssubscript-loader;1"] loader is unused in this file. ::: testing/marionette/marionette-simpletest.js (Diff revision 8) > 'runEmulatorCmd', 'runEmulatorShell', 'TEST_PASS', 'TEST_KNOWN_FAIL', the emulator methods can be removed from exports. ::: testing/marionette/driver.js (Diff revision 8) > + nit: extra whitespace So, this is pretty complicated patch, and it wouldn't surprise me if I missed something. That said, the proof is in the pudding, and if it passes try and sticks on landing, then any other issues can be dealt with in follow-ups!

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 55b502aadaa0964fd2e4e6a1167e54685df60849

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato Carrying on r+ from jmaher and jgriffin because there's a bug with mozreview when rewriting history and pushing that resets to r?.

https://reviewboard.mozilla.org/r/5451/#review4597 > You don't have to restate the property names unless you want to rename them locally, this can be let {inactivityTimeout, scriptTimeout, ... } = cmd.parameters; Thanks for pointing this out! I've fixed up all of the instances where it makes sense to drop the renames. There are a few cases where we still have to, but those can more than likely be fixed when we upgrade the listener to also use the new dispatching strategy.

https://reviewboard.mozilla.org/r/5451/#review4599 > I ran into problems with this and e10s, did you establish this isn't a problem? It doesn't appear to be when I run `./mach marionette-test` with `--e10s`. But I'll be sure to include marionette-e10s in my next try run to be sure.

https://reviewboard.mozilla.org/r/5451/#review4603 > This Task isn't doing a lot that the promise isn't -- this would probably work as a regular function that returns the promise immediately without the Task.async part. It looks like all that happens with the Promse is it gets constructed and then a little later the caller blocks on it. > > I noticed this pattern a few other places, I think the comment applies there too. I agree. It would've been different if `registerPromise` had to wait for the promise and then complete another action before returning. Just returning the promise makes sense.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 03ee5852be3eff9cab47d492042a31b686388eac

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G Pull down these commits: hg pull review -r 03ee5852be3eff9cab47d492042a31b686388eac

try run with e10s: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1cb5750712d&exclusion_profile=false

https://reviewboard.mozilla.org/r/5533/#review4627 Ship It!

https://reviewboard.mozilla.org/r/5459/#review4629 Ship It!

(In reply to Andreas Tolfsen (:ato) from comment #69) > try run with e10s: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=f1cb5750712d&exclusion_profile=false mn-e10s is only linux32 :/

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support Pull down these commits: hg pull review -r c26d9f95ea3f71494c0d7720ca0cf109a6a0b578

https://reviewboard.mozilla.org/r/5441/#review4703 > I'm not sure, but I would double check. All the tests pass, so I'm assuming it's fine.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support Pull down these commits: hg pull review -r c26d9f95ea3f71494c0d7720ca0cf109a6a0b578

https://reviewboard.mozilla.org/r/5451/#review4705 > I can imagine this being significant for firefox-ui-tests, maybe not immediately. Thanks for this feedback. I've fixed this now in part 11.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support Pull down these commits: hg pull review -r 09d5857aea8fc164668db0ca9d3363689e4ab764

https://reviewboard.mozilla.org/r/5441/#review4707 > All the tests pass, so I'm assuming it's fine. There was a bug recently about this, but I can't seem to find it now. The pref has implications outside of tests failing if it's set more often than it should be.

https://reviewboard.mozilla.org/r/5441/#review4711 > There was a bug recently about this, but I can't seem to find it now. The pref has implications outside of tests failing if it's set more often than it should be. Reopening but I'm not sure what to check.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support /r/5737 - Bug 1107706: Part 12: Drop marionette-* prefix on files Pull down these commits: hg pull review -r 1977dc2838f18b9da7eeea7b2bb8c37295fb3d90

https://reviewboard.mozilla.org/r/5441/#review4713 > Reopening but I'm not sure what to check. I'd just double check with jgriffin. Although I guess he already looked at this, so I'm probably just being paranoid.

try run with 32-bit Linux (for marionette-e10s): https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c5f2e266a7e&exclusion_profile=false

https://reviewboard.mozilla.org/r/5451/#review4715 > I'd wager "this" is undefined at this point. Good you noticed, fixed this.

https://reviewboard.mozilla.org/r/5533/#review4717 Ship It!

https://reviewboard.mozilla.org/r/5459/#review4719 Ship It!

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato Carrying forward r+ from jgriffin.

https://reviewboard.mozilla.org/r/5445/#review4727 Ship It!

https://reviewboard.mozilla.org/r/5445/#review4733 Ship It!

https://reviewboard.mozilla.org/r/5443/#review4729 ::: testing/marionette/marionette-server.js (Diff revision 3) > + case Context.CHROME: indent case statements so they are not directly under the switch ::: testing/marionette/marionette-server.js (Diff revision 3) > + case Context.CONTENT: and here ::: testing/marionette/marionette-server.js (Diff revision 3) > + case Context.CHROME: indent case so that it is not directly under the switch ::: testing/marionette/marionette-server.js (Diff revision 3) > + case Context.CONTENT: indent ::: testing/marionette/marionette-server.js (Diff revision 3) > + case Context.CHROME: Please check switch statements throughout this file.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato https://reviewboard.mozilla.org/r/5441/#review4731 ::: testing/marionette/modal.js (Diff revision 13) > + handlers.splice(i, 1); Modifying the array while you're iterating over it in this fashion could cause a problem if you had lot more handlers, right? Maybe you can use a Set for this instead.

https://reviewboard.mozilla.org/r/5447/#review4735 Ship It!

https://reviewboard.mozilla.org/r/5449/#review4737 Ship It!

https://reviewboard.mozilla.org/r/5451/#review4739 ::: testing/marionette/driver.js (Diff revision 10) > + if (this.curFrame) missing braces ::: testing/marionette/driver.js (Diff revision 10) > + if (cmdId) missing braces ::: testing/marionette/driver.js (Diff revision 10) > + if (file.exists()) missing braces ::: testing/marionette/driver.js (Diff revision 10) > + if (mainContent) missing braces ::: testing/marionette/driver.js (Diff revision 10) > + if (this.curBrowser.isNewSession) missing braces ::: testing/marionette/driver.js (Diff revision 10) > + if (ctx === null) missing braces ::: testing/marionette/driver.js (Diff revision 10) > + if (!scriptTimeout) missing braces ::: testing/marionette/driver.js (Diff revision 10) > + if (error.isError(val)) missing braces, there are quite a few through this file so do a check ::: testing/marionette/driver.js (Diff revision 10) > + case Context.CHROME: indent case to match other methods e.g L1919 ::: testing/marionette/components/marionettecomponent.js (Diff revision 10) > - this.logger.info("marionette enabled via build flag and pref"); > + a few trailing whitespaces found through the document

https://reviewboard.mozilla.org/r/5455/#review4741 Ship It!

https://reviewboard.mozilla.org/r/5735/#review4745 Ship It!

https://reviewboard.mozilla.org/r/5737/#review4747 ::: testing/marionette/common.js (Diff revision 1) > + Nit: what space

https://reviewboard.mozilla.org/r/5441/#review4751 > Modifying the array while you're iterating over it in this fashion could cause a problem if you had lot more handlers, right? Maybe you can use a Set for this instead. Excellent point. I've updated this to use sets.

https://reviewboard.mozilla.org/r/5443/#review4753 > I find that for functions with very long argument sequences it helps to say what an argument is for if it's not obvious from its name or type. > > I'm not suggesting we make a rule of that. Ideally the number of arguments to a function will be short and easy to remember. When they aren't it's usually a warning sign (like this case) that there are too many arguments. It's consistent across Marionette at least.

https://reviewboard.mozilla.org/r/5443/#review4755 > indent case statements so they are not directly under the switch This is fixed in part 5.

https://reviewboard.mozilla.org/r/5451/#review4757 > I was following the Google JavaScript Style Guide (for the lack of any such equivalent at Mozilla) which says private properties should have a trailing underscore. I found some support for this in the Mozilla codebase. (https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml#JavaScript_Style_Rules) > > I'm happy to change it to be the other way around, but there doesn't seem to be concensus in our codebase in any direction on this. At least it's now consistent.

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support /r/5737 - Bug 1107706: Part 12: Drop marionette-* prefix on files /r/5787 - Bug 1107706: Part 13: Style fixes Pull down these commits: hg pull review -r fcdf56246d2cb3496bac7ec1e24e9b4e3c9a863b

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=17a6331516ee&exclusion_profile=false

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support /r/5737 - Bug 1107706: Part 12: Drop marionette-* prefix on files /r/5787 - Bug 1107706: Part 13: Style fixes Pull down these commits: hg pull review -r a06a9e20403939480f44696050530dbe9e7b9f55

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato rebased try, which should fix Mac OS X 10.6: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f0e53865204&exclusion_profile=false

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support /r/5737 - Bug 1107706: Part 12: Drop marionette-* prefix on files /r/5787 - Bug 1107706: Part 13: Style fixes /r/5805 - Bug 1107706: Part 14: Fix quitApplication Pull down these commits: hg pull review -r 7179c03849a9e9d89c45edcdb0351be6335ff809

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d4849bc67b7&exclusion_profile=false

https://reviewboard.mozilla.org/r/5805/#review4781 ::: testing/marionette/dispatcher.js (Diff revision 1) > + this.stopSignal_(); > + this.sendOk(id); > + > this.driver.sessionTearDown(); > Services.startup.quit(flags); This ends up a bit different from where we ended up in 1142404, but as long as the server's listener is closed before the client starts trying to reconnect we're ok, which will be ok as long as closing the listener is synchronous, which I think it probably is.

https://reviewboard.mozilla.org/r/5735/#review4783 ::: testing/marionette/modal.js (Diff revision 4) > + if (win && win.parent) Braces! ::: testing/marionette/modal.js (Diff revision 4) > + if (win) > + return win.Dialog.ui; Braces :) ::: testing/marionette/driver.js (Diff revision 4) > + if (topic == modal.COMMON_DIALOG_LOADED) > + winr = Cu.getWeakReference(subject); Braces.

https://reviewboard.mozilla.org/r/5805/#review4785 Ship It!

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support /r/5737 - Bug 1107706: Part 12: Drop marionette-* prefix on files /r/5787 - Bug 1107706: Part 13: Style fixes /r/5805 - Bug 1107706: Part 14: Fix quitApplication /r/5865 - Bug 1107706: Part 15: Fix emulator callbacks for content process Pull down these commits: hg pull review -r ab6bb2cf63f9434d8bd614f71a9685781a3d2737

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=51967d7567c4&exclusion_profile=false

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato https://reviewboard.mozilla.org/r/5441/#review4813 Meant to r+ this before. Happy to check anything else before landing.

https://reviewboard.mozilla.org/r/5443/#review4857 Ship It!

https://reviewboard.mozilla.org/r/5445/#review4859 Ship It!

https://reviewboard.mozilla.org/r/5447/#review4861 Ship It!

https://reviewboard.mozilla.org/r/5449/#review4863 Ship It!

https://reviewboard.mozilla.org/r/5451/#review4865 Ship It!

https://reviewboard.mozilla.org/r/5455/#review4867 Ship It!

https://reviewboard.mozilla.org/r/5457/#review4743 Ship It! ::: testing/marionette/emulator.js (Diff revision 11) > + if (!this.onresult) missing braces

https://reviewboard.mozilla.org/r/5459/#review4869 Ship It!

https://reviewboard.mozilla.org/r/5533/#review4871 Ship It!

https://reviewboard.mozilla.org/r/5735/#review4873 Ship It!

https://reviewboard.mozilla.org/r/5737/#review4875 Ship It!

https://reviewboard.mozilla.org/r/5787/#review4879 Ship It!

https://reviewboard.mozilla.org/r/5805/#review4881 Ship It!

https://reviewboard.mozilla.org/r/5865/#review4883 Ship It!

https://reviewboard.mozilla.org/r/5453/#review4885 Ship It!

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support /r/5737 - Bug 1107706: Part 12: Drop marionette-* prefix on files /r/5787 - Bug 1107706: Part 13: Style fixes /r/5805 - Bug 1107706: Part 14: Fix quitApplication /r/5865 - Bug 1107706: Part 15: Fix emulator callbacks for content process Pull down these commits: hg pull review -r 7a7d1bddb0cff5fecd1524cddc27edabe98345ae

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support /r/5737 - Bug 1107706: Part 12: Drop marionette-* prefix on files /r/5787 - Bug 1107706: Part 13: Style fixes /r/5805 - Bug 1107706: Part 14: Fix quitApplication /r/5865 - Bug 1107706: Part 15: Fix emulator callbacks for content process Pull down these commits: hg pull review -r 8101a841bd878d7c7377662beca08ad9287c14c9

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato https://reviewboard.mozilla.org/r/5441/#review4889 Ship It!

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato /r/5443 - Bug 1107706: Part 1: Change context from strings to enum /r/5445 - Bug 1107706: Part 2: Add error module and WebDriver error objects /r/5447 - Bug 1107706: Part 3: Add a command processor to Marionette /r/5449 - Bug 1107706: Part 4: Add dispatching mechanism to encapsulate connection /r/5451 - Bug 1107706: Part 5: Refactor Marionette chrome/content communication /r/5453 - Bug 1107706: Part 6: Make SpecialPowersError a prototypal Error /r/5455 - Bug 1107706: Part 7: Add timeout test for async scripts /r/5457 - Bug 1107706: Part 8: Adapt emulator callbacks /r/5459 - Bug 1107706: Part 9: Disable XUL component tests on B2G /r/5533 - Bug 1107706: Part 10: Disable test_anonymous_content.py on B2G /r/5735 - Bug 1107706: Part 11: Global modal dialogue support /r/5737 - Bug 1107706: Part 12: Drop marionette-* prefix on files /r/5787 - Bug 1107706: Part 13: Style fixes /r/5805 - Bug 1107706: Part 14: Fix quitApplication /r/5865 - Bug 1107706: Part 15: Fix emulator callbacks for content process Pull down these commits: hg pull review -r 9a6ec3e07ddd8cb79c84839af7dfb1cd862e578d

Comment on attachment 8578113 [details] MozReview Request: bz://1107706/ato Carrying on r+ from chmanchester, dburns, and jgriffin.

Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=3c25064e24da https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=3c25064e24da

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/c44d46087f59 for Mn and Mn-e10s bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=7963798&repo=mozilla-inbound

Fixed the rebase of action chains in chrome space and pushed to inbound again: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=d026794b4c0b https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=d026794b4c0b

Backed out for Gaia JS integration test permafail. https://hg.mozilla.org/integration/mozilla-inbound/rev/4c2c50594967 https://treeherder.mozilla.org/logviewer.html#?job_id=7995011&repo=mozilla-inbound

Also B2G Desktop mochitest failures. https://treeherder.mozilla.org/logviewer.html#?job_id=7998009&repo=mozilla-inbound

Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=821bb2b2dca6 https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=821bb2b2dca6

https://hg.mozilla.org/mozilla-central/rev/31df2186195a https://hg.mozilla.org/mozilla-central/rev/42f14f453f1f https://hg.mozilla.org/mozilla-central/rev/a76f6b26484d https://hg.mozilla.org/mozilla-central/rev/dbd619c23f57 https://hg.mozilla.org/mozilla-central/rev/6613699885fa https://hg.mozilla.org/mozilla-central/rev/8b695334df94 https://hg.mozilla.org/mozilla-central/rev/853e7da58109 https://hg.mozilla.org/mozilla-central/rev/4ef84ad178b5 https://hg.mozilla.org/mozilla-central/rev/6af978a7ee32 https://hg.mozilla.org/mozilla-central/rev/6dcbd23298de https://hg.mozilla.org/mozilla-central/rev/cef97fe74c4a https://hg.mozilla.org/mozilla-central/rev/59ad68ff1972 https://hg.mozilla.org/mozilla-central/rev/800504a8ac36 https://hg.mozilla.org/mozilla-central/rev/b46060440357 https://hg.mozilla.org/mozilla-central/rev/0736c222ecb9 https://hg.mozilla.org/mozilla-central/rev/821bb2b2dca6