1153832 - Introduce new dispatching technique to listener

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Andreas Tolfsen ❲:ato❳]

Bug 1107706 consciously did not make any (substantial) changes to the listener, but the ListenerProxy added facilitates the same style of dispatching technique there.

Currently when we make a call to a routine in listener space we need to do this.listener.get({url: "https://mozilla.org/"}) which the receiving function then is required to unpack from msg.json.

By wrapping command implementations in listener in a try…catch block that unpacks the arguments and apply them to the wrapped function, we could reduce this to this.listener.get("https://mozilla.org/").

By looking at the type of the arguments we can stay backwards compatible, i.e. that we don't have to upgrade all functions at once.

Comment 1

[Andreas Tolfsen ❲:ato❳]

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

Comment 2

[Andreas Tolfsen ❲:ato❳]

Attachment: MozReview Request: bz://1153832/ato

/r/7061 - Bug 1153832: New dispatch style framework in Marionette listener

Pull down this commit:

hg pull -r 43ef240735099221cbbddb753eaf9e2802a4de1d https://reviewboard-hg.mozilla.org/gecko/

Comment 3

[David Burns :automatedtester]

Comment on attachment 8592758 [details]
MozReview Request: bz://1153832/ato

https://reviewboard.mozilla.org/r/7059/#review5871

Ship It!

::: testing/marionette/listener.js
(Diff revision 1)
> -    sendError("Error setting cookie", 13, null, msg.json.command_id);
> +    sendError(new UnknownError("Error setting cookie"), msg.json.command_id);

This should be UnableToSetCookie when you do your follow up

Comment 4

[Andreas Tolfsen ❲:ato❳]

(In reply to David Burns :automatedtester from comment #3)
> ::: testing/marionette/listener.js
> (Diff revision 1)
> > -    sendError("Error setting cookie", 13, null, msg.json.command_id);
> > +    sendError(new UnknownError("Error setting cookie"), msg.json.command_id);
> 
> This should be UnableToSetCookie when you do your follow up

Filed bug 1154757 and made bug 1100545 track fixing up errors.

Comment 5

[Andreas Tolfsen ❲:ato❳]

Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/043a824dd7b7

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Backed out for mass-fail on Mn test suites.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e026141c2ce9

https://treeherder.mozilla.org/logviewer.html#?job_id=8886437&repo=mozilla-inbound

Comment 7

[Andreas Tolfsen ❲:ato❳]

There was a bug in sendKeysToElement where the arguments to sendError hadn't been updated.  Pushed a new try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b312cadcacdc

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5c83a36d9eba

Comment 9

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b6352f8ae6d

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Thanks Pulsebot. Now for some context, it was hitting Gip(a) failures like yesterday's landing.
https://treeherder.mozilla.org/logviewer.html#?job_id=8937623&repo=mozilla-inbound

Comment 11

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8592758 [details]
MozReview Request: bz://1153832/ato

/r/7061 - Bug 1153832: New dispatch style framework in Marionette listener

Pull down this commit:

hg pull -r 3d643839885fcf2f2eb335942b1fe00ecb29ab02 https://reviewboard-hg.mozilla.org/gecko/

Comment 12

[David Burns :automatedtester]

Comment on attachment 8592758 [details]
MozReview Request: bz://1153832/ato

https://reviewboard.mozilla.org/r/7059/#review5963

Ship It!

Comment 13

[Andreas Tolfsen ❲:ato❳]

Fixed the Gip(a) failures.  It was hilarious.  Ask me about it over a beer.

Pushed to inbound:  https://hg.mozilla.org/integration/mozilla-inbound/rev/0b28f5ff48a6

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0b28f5ff48a6

Comment 15

[Carsten Book [:Tomcat]]

sorry had to back this out for frequent Asan dt failures started with this push (its also in the try run from comment #7 like https://treeherder.mozilla.org/logviewer.html#?job_id=9057650&repo=mozilla-inbound

Comment 16

[Andreas Tolfsen ❲:ato❳]

I believe Asan dt tests were already flaky before I pushed this commit.  If you examine the log of bug 1073442, it looks like the timeout issue is older than this.

As far as I can tell, Asan dt tests don't use Marionette at all.  It seems to time out in running scripts/scripts/desktop_unittest.py.  Does that invoke Marionette at all?  How can I run that locally?

Comment 17

[Andreas Tolfsen ❲:ato❳]

Tomcat: This is test results from the push after your backout of this patch, which shows the problem persists: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=07e55dfe0411

Comment 18

[Pulsebot]

Backout:
https://hg.mozilla.org/mozilla-central/rev/a8e2d2de8d8b

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2df6451d16bb

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2df6451d16bb

Comment 22

[Andreas Tolfsen ❲:ato❳]

Attachment: MozReview Request: Bug 1153832: New dispatch style framework in Marionette listener