Message sequencing allows Marionette to provide an asynchronous, parallel pipelining user-facing interface, limit chances of payload race conditions, and remove stylistic inconsistencies in how commands and responses are dispatched internally. Clients that deliver a blocking WebDriver interface are still be expected to not send further command requests before the response from the last command has come back, but if they still happen to do so because of programming error or otherwise, no harm will be done. This will guard against bugs such as bug 1207125. I also intend to formalise the command and response concepts, and in particular apply these concepts to emulator callbacks which for long have been a wart in Marionette. Through the new message format, Marionette will be able to provide two-way parallel communication. In other words, the server will be able to instruct the client to perform a command in a non ad-hoc way. runEmulatorCmd and runEmulatorShell are both turned into command instructions originating from the server. This should resolve a lot of technical debt in the Marionette server code because they are no longer special-cased to circumvent the dispatching technique used for all other commands; commands may originate from either the client or the server providing parallel pipelining enforced through message sequencing: client server | | msgid=1 |----------->| | command | | | msgid=2 |<-----------| | command | | | msgid=2 |----------->| | response | | | msgid=1 |<-----------| | response | | | The protocol will consist of a "Command" message and the corresponding "Response" message. A "Response" message must always be sent in reply to a "Command" message. The command message is a four element array: [type, msgid, name, params] type: Must be zero (number) indicating that it is a "Command" message. msgid: Sequence number. The opposite remote replies with a "Response" message with the same number. name: Method/command name to perform. params: Arbitrary object with arguments to the command. The response message is also a four element array: [type, msgid, error, result] type: Must be one (number) indicating that it is a "Response" message. msgid: Sequence number corresponding to the command message. error: If the command was executed correctly, this field is null. Otherwise it is an arbitrary object representing an error. result: Arbitrary object if successfully executed. If an error occurred, this field is null. These changes will require bumping the Marionette protocol level and making some changes to existing clients. The changes should however be minimal, since the data structure of the response from the commands isn’t changing.

try with level 3 client: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf40d3d9e776

rebased try with level 3 client: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0468bd4a929

Bug 1211489: Provide message sequencing in Marionette Message sequencing allows Marionette to provide an asynchronous, parallel pipelining user-facing interface, limit chances of payload race conditions, and remove stylistic inconsistencies in how commands and responses are dispatched internally. Clients that deliver a blocking WebDriver interface are still be expected to not send further command requests before the response from the last command has come back, but if they still happen to do so because of programming error or otherwise, no harm will be done. This will guard against bugs such as bug 1207125. This patch formalises the command and response concepts, and applies these concepts to emulator callbacks. Through the new message format, Marionette is able to provide two-way parallel communication. In other words, the server will be able to instruct the client to perform a command in a non ad-hoc way. runEmulatorCmd and runEmulatorShell are both turned into command instructions originating from the server. This resolves a lot of technical debt in the server code because they are no longer special-cased to circumvent the dispatching technique used for all other commands; commands may originate from either the client or the server providing parallel pipelining enforced through message sequencing: client server | | msgid=1 |----------->| | command | | | msgid=2 |<-----------| | command | | | msgid=2 |----------->| | response | | | msgid=1 |<-----------| | response | | | The protocol now consists of a "Command" message and the corresponding "Response" message. A "Response" message must always be sent in reply to a "Command" message. This bumps the Marionette protocol level to 3. r=dburns r=jgriffin

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Bug 1211489: Provide message sequencing in Marionette Message sequencing allows Marionette to provide an asynchronous, parallel pipelining user-facing interface, limit chances of payload race conditions, and remove stylistic inconsistencies in how commands and responses are dispatched internally. Clients that deliver a blocking WebDriver interface are still be expected to not send further command requests before the response from the last command has come back, but if they still happen to do so because of programming error or otherwise, no harm will be done. This will guard against bugs such as bug 1207125. This patch formalises the command and response concepts, and applies these concepts to emulator callbacks. Through the new message format, Marionette is able to provide two-way parallel communication. In other words, the server will be able to instruct the client to perform a command in a non ad-hoc way. runEmulatorCmd and runEmulatorShell are both turned into command instructions originating from the server. This resolves a lot of technical debt in the server code because they are no longer special-cased to circumvent the dispatching technique used for all other commands; commands may originate from either the client or the server providing parallel pipelining enforced through message sequencing: client server | | msgid=1 |----------->| | command | | | msgid=2 |<-----------| | command | | | msgid=2 |----------->| | response | | | msgid=1 |<-----------| | response | | | The protocol now consists of a "Command" message and the corresponding "Response" message. A "Response" message must always be sent in reply to a "Command" message. This bumps the Marionette protocol level to 3. r=dburns r=jgriffin

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Bug 1211489: Provide message sequencing in Marionette Message sequencing allows Marionette to provide an asynchronous, parallel pipelining user-facing interface, limit chances of payload race conditions, and remove stylistic inconsistencies in how commands and responses are dispatched internally. Clients that deliver a blocking WebDriver interface are still be expected to not send further command requests before the response from the last command has come back, but if they still happen to do so because of programming error or otherwise, no harm will be done. This will guard against bugs such as bug 1207125. This patch formalises the command and response concepts, and applies these concepts to emulator callbacks. Through the new message format, Marionette is able to provide two-way parallel communication. In other words, the server will be able to instruct the client to perform a command in a non ad-hoc way. runEmulatorCmd and runEmulatorShell are both turned into command instructions originating from the server. This resolves a lot of technical debt in the server code because they are no longer special-cased to circumvent the dispatching technique used for all other commands; commands may originate from either the client or the server providing parallel pipelining enforced through message sequencing: client server | | msgid=1 |----------->| | command | | | msgid=2 |<-----------| | command | | | msgid=2 |----------->| | response | | | msgid=1 |<-----------| | response | | | The protocol now consists of a "Command" message and the corresponding "Response" message. A "Response" message must always be sent in reply to a "Command" message. This bumps the Marionette protocol level to 3. r=dburns r=jgriffin

https://reviewboard.mozilla.org/r/22203/#review19813 ::: testing/marionette/driver.js:1136 (Diff revision 3) > - this.sandboxes[sandboxName].runEmulatorCmd = (cmd, cb) => { > - let ecb = new EmulatorCallback(); > - ecb.onresult = cb; > - ecb.onerror = chromeAsyncError; > + this.sandboxes[sandboxName].runEmulatorCmd = > + (cmd, cb) => this.emulator.command(cmd, cb, chromeAsyncError); > + this.sandboxes[sandboxName].runEmulatorShell = > + (args, cb) => this.emulator.shell(args, cb, chromeAsyncError); Maybe we should make these return closures?

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Bug 1211489: Provide message sequencing in Marionette Message sequencing allows Marionette to provide an asynchronous, parallel pipelining user-facing interface, limit chances of payload race conditions, and remove stylistic inconsistencies in how commands and responses are dispatched internally. Clients that deliver a blocking WebDriver interface are still be expected to not send further command requests before the response from the last command has come back, but if they still happen to do so because of programming error or otherwise, no harm will be done. This will guard against bugs such as bug 1207125. This patch formalises the command and response concepts, and applies these concepts to emulator callbacks. Through the new message format, Marionette is able to provide two-way parallel communication. In other words, the server will be able to instruct the client to perform a command in a non ad-hoc way. runEmulatorCmd and runEmulatorShell are both turned into command instructions originating from the server. This resolves a lot of technical debt in the server code because they are no longer special-cased to circumvent the dispatching technique used for all other commands; commands may originate from either the client or the server providing parallel pipelining enforced through message sequencing: client server | | msgid=1 |----------->| | command | | | msgid=2 |<-----------| | command | | | msgid=2 |----------->| | response | | | msgid=1 |<-----------| | response | | | The protocol now consists of a "Command" message and the corresponding "Response" message. A "Response" message must always be sent in reply to a "Command" message. This bumps the Marionette protocol level to 3. r=dburns r=jgriffin

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Bug 1211489: Provide message sequencing in Marionette Message sequencing allows Marionette to provide an asynchronous, parallel pipelining user-facing interface, limit chances of payload race conditions, and remove stylistic inconsistencies in how commands and responses are dispatched internally. Clients that deliver a blocking WebDriver interface are still be expected to not send further command requests before the response from the last command has come back, but if they still happen to do so because of programming error or otherwise, no harm will be done. This will guard against bugs such as bug 1207125. This patch formalises the command and response concepts, and applies these concepts to emulator callbacks. Through the new message format, Marionette is able to provide two-way parallel communication. In other words, the server will be able to instruct the client to perform a command in a non ad-hoc way. runEmulatorCmd and runEmulatorShell are both turned into command instructions originating from the server. This resolves a lot of technical debt in the server code because they are no longer special-cased to circumvent the dispatching technique used for all other commands; commands may originate from either the client or the server providing parallel pipelining enforced through message sequencing: client server | | msgid=1 |----------->| | command | | | msgid=2 |<-----------| | command | | | msgid=2 |----------->| | response | | | msgid=1 |<-----------| | response | | | The protocol now consists of a "Command" message and the corresponding "Response" message. A "Response" message must always be sent in reply to a "Command" message. This bumps the Marionette protocol level to 3. r=dburns r=jgriffin

Hm, question beside... is that Mozreview which adds all those extra non-helpful comments to the bug while you update a patch? I feel we should stop it from doing so for updates.

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Bug 1211489: Provide message sequencing in Marionette Message sequencing allows Marionette to provide an asynchronous, parallel pipelining user-facing interface, limit chances of payload race conditions, and remove stylistic inconsistencies in how commands and responses are dispatched internally. Clients that deliver a blocking WebDriver interface are still be expected to not send further command requests before the response from the last command has come back, but if they still happen to do so because of programming error or otherwise, no harm will be done. This will guard against bugs such as bug 1207125. This patch formalises the command and response concepts, and applies these concepts to emulator callbacks. Through the new message format, Marionette is able to provide two-way parallel communication. In other words, the server will be able to instruct the client to perform a command in a non ad-hoc way. runEmulatorCmd and runEmulatorShell are both turned into command instructions originating from the server. This resolves a lot of technical debt in the server code because they are no longer special-cased to circumvent the dispatching technique used for all other commands; commands may originate from either the client or the server providing parallel pipelining enforced through message sequencing: client server | | msgid=1 |----------->| | command | | | msgid=2 |<-----------| | command | | | msgid=2 |----------->| | response | | | msgid=1 |<-----------| | response | | | The protocol now consists of a "Command" message and the corresponding "Response" message. A "Response" message must always be sent in reply to a "Command" message. This bumps the Marionette protocol level to 3. r=dburns r=jgriffin

(In reply to Henrik Skupin (:whimboo) from comment #9) > Hm, question beside... is that Mozreview which adds all those extra > non-helpful comments to the bug while you update a patch? I feel we should > stop it from doing so for updates. It is actually quite useful for reviewers to know when the review has been updated, but I agree it’s not so useful to post them to Bugzilla or even what they contain. Would you mind filing a bug with MozReview?

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette https://reviewboard.mozilla.org/r/22203/#review19907 ::: testing/marionette/message.js:51 (Diff revision 5) > + Nit: extra spaces ::: testing/marionette/message.js:51 (Diff revision 6) > + nit: spaces

(In reply to Andreas Tolfsen (:ato) from comment #11) > Would you mind filing a bug with MozReview? It's now bug 1216078.

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette https://reviewboard.mozilla.org/r/22203/#review20041 Whew, lots to absorb! Looks great, though, thanks for tackling this.

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Bug 1211489: Provide message sequencing in Marionette Message sequencing allows Marionette to provide an asynchronous, parallel pipelining user-facing interface, limit chances of payload race conditions, and remove stylistic inconsistencies in how commands and responses are dispatched internally. Clients that deliver a blocking WebDriver interface are still be expected to not send further command requests before the response from the last command has come back, but if they still happen to do so because of programming error or otherwise, no harm will be done. This will guard against bugs such as bug 1207125. This patch formalises the command and response concepts, and applies these concepts to emulator callbacks. Through the new message format, Marionette is able to provide two-way parallel communication. In other words, the server will be able to instruct the client to perform a command in a non ad-hoc way. runEmulatorCmd and runEmulatorShell are both turned into command instructions originating from the server. This resolves a lot of technical debt in the server code because they are no longer special-cased to circumvent the dispatching technique used for all other commands; commands may originate from either the client or the server providing parallel pipelining enforced through message sequencing: client server | | msgid=1 |----------->| | command | | | msgid=2 |<-----------| | command | | | msgid=2 |----------->| | response | | | msgid=1 |<-----------| | response | | | The protocol now consists of a "Command" message and the corresponding "Response" message. A "Response" message must always be sent in reply to a "Command" message. This bumps the Marionette protocol level to 3. r=dburns r=jgriffin

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Bug 1211489: Provide message sequencing in Marionette Message sequencing allows Marionette to provide an asynchronous, parallel pipelining user-facing interface, limit chances of payload race conditions, and remove stylistic inconsistencies in how commands and responses are dispatched internally. Clients that deliver a blocking WebDriver interface are still be expected to not send further command requests before the response from the last command has come back, but if they still happen to do so because of programming error or otherwise, no harm will be done. This will guard against bugs such as bug 1207125. This patch formalises the command and response concepts, and applies these concepts to emulator callbacks. Through the new message format, Marionette is able to provide two-way parallel communication. In other words, the server will be able to instruct the client to perform a command in a non ad-hoc way. runEmulatorCmd and runEmulatorShell are both turned into command instructions originating from the server. This resolves a lot of technical debt in the server code because they are no longer special-cased to circumvent the dispatching technique used for all other commands; commands may originate from either the client or the server providing parallel pipelining enforced through message sequencing: client server | | msgid=1 |----------->| | command | | | msgid=2 |<-----------| | command | | | msgid=2 |----------->| | response | | | msgid=1 |<-----------| | response | | | The protocol now consists of a "Command" message and the corresponding "Response" message. A "Response" message must always be sent in reply to a "Command" message. This bumps the Marionette protocol level to 3. r=dburns r=jgriffin

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Bug 1211489: Provide message sequencing in Marionette Message sequencing allows Marionette to provide an asynchronous, parallel pipelining user-facing interface, limit chances of payload race conditions, and remove stylistic inconsistencies in how commands and responses are dispatched internally. Clients that deliver a blocking WebDriver interface are still be expected to not send further command requests before the response from the last command has come back, but if they still happen to do so because of programming error or otherwise, no harm will be done. This will guard against bugs such as bug 1207125. This patch formalises the command and response concepts, and applies these concepts to emulator callbacks. Through the new message format, Marionette is able to provide two-way parallel communication. In other words, the server will be able to instruct the client to perform a command in a non ad-hoc way. runEmulatorCmd and runEmulatorShell are both turned into command instructions originating from the server. This resolves a lot of technical debt in the server code because they are no longer special-cased to circumvent the dispatching technique used for all other commands; commands may originate from either the client or the server providing parallel pipelining enforced through message sequencing: client server | | msgid=1 |----------->| | command | | | msgid=2 |<-----------| | command | | | msgid=2 |----------->| | response | | | msgid=1 |<-----------| | response | | | The protocol now consists of a "Command" message and the corresponding "Response" message. A "Response" message must always be sent in reply to a "Command" message. This bumps the Marionette protocol level to 3. r=dburns r=jgriffin

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22203/diff/9-10/

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22203/diff/10-11/

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22203/diff/11-12/

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22203/diff/12-13/

The reason for the Gij failures is that the Marionette JS client in Gaia is sending the unserialised variable `body` in client.js’ `send()` function, rather than the serialised `data`. This only happens to work right now because the server is not using protocol level 3 yet.

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22203/diff/13-14/

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=78dda8881bbf&group_state=expanded

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22203/diff/14-15/

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

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/0f006147383c

17:58:17 INFO - Running tests in /home/worker/gaia/tests/jsmarionette/runner/marionette-js-runner/test/integration/clientasync.js 18:08:23 INFO - .......[marionette-mocha] 2 failing 18:08:23 INFO - [marionette-mocha] 18:08:23 INFO - [marionette-mocha] 1) client async "before each" hook: 18:08:23 INFO - Error: timeout of 300000ms exceeded 18:08:23 INFO - at null.<anonymous> (/home/worker/gaia/node_modules/mocha/lib/runnable.js:158:19) 18:08:23 INFO - at Timer.listOnTimeout (timers.js:92:15) 18:08:23 INFO - [marionette-mocha] 2) client async "after each" hook: 18:08:23 INFO - Error: timeout of 300000ms exceeded 18:08:23 INFO - at null.<anonymous> (/home/worker/gaia/node_modules/mocha/lib/runnable.js:158:19) 18:08:23 INFO - at Timer.listOnTimeout (timers.js:92:15) 18:08:23 INFO - [marionette-mocha] 18:08:23 INFO - . 18:08:23 INFO - /home/worker/gaia/tests/jsmarionette/runner/marionette-js-runner/test/integration/clientasync.js failed. Will retry. 18:08:28 INFO - Running tests in /home/worker/gaia/tests/jsmarionette/runner/marionette-js-runner/test/integration/clientasync.js 18:18:34 INFO - .........[marionette-mocha] 2 failing 18:18:34 INFO - [marionette-mocha] 18:18:34 INFO - [marionette-mocha] 1) client async "before each" hook: 18:18:34 INFO - Error: timeout of 300000ms exceeded 18:18:34 INFO - at null.<anonymous> (/home/worker/gaia/node_modules/mocha/lib/runnable.js:158:19) 18:18:34 INFO - at Timer.listOnTimeout (timers.js:92:15) 18:18:34 INFO - [marionette-mocha] 2) client async "after each" hook: 18:18:34 INFO - Error: timeout of 300000ms exceeded 18:18:34 INFO - at null.<anonymous> (/home/worker/gaia/node_modules/mocha/lib/runnable.js:158:19) 18:18:34 INFO - at Timer.listOnTimeout (timers.js:92:15) 18:18:34 INFO - . 18:18:34 INFO - /home/worker/gaia/tests/jsmarionette/runner/marionette-js-runner/test/integration/clientasync.js failed. Will retry. 18:18:39 INFO - Running tests in /home/worker/gaia/tests/jsmarionette/runner/marionette-js-runner/test/integration/clientasync.js 18:28:45 INFO - ....[marionette-mocha] 2 failing 18:28:45 INFO - [marionette-mocha] 1) client async "before each" hook: 18:28:45 INFO - Error: timeout of 300000ms exceeded 18:28:45 INFO - at null.<anonymous> (/home/worker/gaia/node_modules/mocha/lib/runnable.js:158:19) 18:28:45 INFO - at Timer.listOnTimeout (timers.js:92:15) 18:28:45 INFO - [marionette-mocha] 2) client async "after each" hook: 18:28:45 INFO - Error: timeout of 300000ms exceeded 18:28:45 INFO - at null.<anonymous> (/home/worker/gaia/node_modules/mocha/lib/runnable.js:158:19) 18:28:45 INFO - at Timer.listOnTimeout (timers.js:92:15) 18:28:45 INFO - [marionette-mocha] 18:28:45 INFO - . 18:28:45 INFO - /home/worker/gaia/tests/jsmarionette/runner/marionette-js-runner/test/integration/clientasync.js failed. Will retry. 18:28:50 INFO - Running tests in /home/worker/gaia/tests/jsmarionette/runner/marionette-js-runner/test/integration/clientasync.js 18:38:56 INFO - .....[marionette-mocha] 2 failing 18:38:56 INFO - [marionette-mocha] 1) client async "before each" hook: 18:38:56 INFO - Error: timeout of 300000ms exceeded 18:38:56 INFO - at null.<anonymous> (/home/worker/gaia/node_modules/mocha/lib/runnable.js:158:19) 18:38:56 INFO - at Timer.listOnTimeout (timers.js:92:15) 18:38:56 INFO - [marionette-mocha] 2) client async "after each" hook: 18:38:56 INFO - Error: timeout of 300000ms exceeded 18:38:56 INFO - at null.<anonymous> (/home/worker/gaia/node_modules/mocha/lib/runnable.js:158:19) 18:38:56 INFO - at Timer.listOnTimeout (timers.js:92:15) 18:38:56 INFO - [marionette-mocha] 18:38:56 INFO - . 18:38:56 INFO - /home/worker/gaia/tests/jsmarionette/runner/marionette-js-runner/test/integration/clientasync.js failed. Will retry. 18:39:01 INFO - Running tests in /home/worker/gaia/tests/jsmarionette/runner/marionette-js-runner/test/integration/clientasync.js

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22203/diff/15-16/

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

Comment on attachment 8674292 [details] MozReview Request: Bug 1211489: Provide message sequencing in Marionette Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22203/diff/16-17/

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=82ac5aac6261&group_state=expanded