1361987 - Make sure generator functions return the correct type

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, enhancement)

Description

[David Burns :automatedtester]

We have a number of places where we use `return` in generator functions which is not correct syntax.

Comment 1

[David Burns :automatedtester]

Attachment: Bug 1361987 - Move maximizeWindow call to not use a return in a generator.

The maximizeWindow call in Marionette uses return which is not valid javascript.
This change update the response that is then picked up by the server before
returning to the remote end.

Review commit: https://reviewboard.mozilla.org/r/136180/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136180/

Comment 2

[David Burns :automatedtester]

Attachment: Bug 1361987 - Move setWindowRect to not return from a generator.

setWindowRect is using `return` from a generator which is not valid
javascript. This now populates the response before it is returned to
the client.

Review commit: https://reviewboard.mozilla.org/r/136182/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136182/

Comment 3

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8864502 [details]
Bug 1361987 - Move maximizeWindow call to not use a return in a generator.

https://reviewboard.mozilla.org/r/136180/#review139624

::: testing/marionette/driver.js:2909
(Diff revision 1)
> -  return {
> +  resp.body.value = {
>      x: win.screenX,
>      y: win.screenY,
>      width: win.outerWidth,
>      height: win.outerHeight,
>    };

This change is backwards incompatible because you change the way the returned object is wrapped.  This will likely cause issues with Firefox UI tests and geckodriver.

It’s also not really necessary to wrap JSON Objects in a value field on the Marionette level since the payloads returned to the clients are always an array (see http://searchfox.org/mozilla-central/rev/6580dd9a4eb0224773897388dba2ddf5ed7f4216/testing/marionette/README#27).

I will give this an r+ with the expectation that this is changed from resp.body.value = {…} to resp.body = {…].

Comment 4

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8864503 [details]
Bug 1361987 - Move setWindowRect to not return from a generator.

https://reviewboard.mozilla.org/r/136182/#review139626

::: testing/marionette/driver.js:1423
(Diff revision 1)
> -  return {
> +  resp.body.value = {
>      "x": win.screenX,
>      "y": win.screenY,
>      "width": win.outerWidth,
>      "height": win.outerHeight,
>    };

Same situation as in previous patch.  s/resp.body.value/resp.body/

Comment 5

[David Burns :automatedtester]

Comment on attachment 8864502 [details]
Bug 1361987 - Move maximizeWindow call to not use a return in a generator.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136180/diff/1-2/

Comment 6

[David Burns :automatedtester]

Comment on attachment 8864503 [details]
Bug 1361987 - Move setWindowRect to not return from a generator.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136182/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d330e31dcb23
Move maximizeWindow call to not use a return in a generator. r=ato
https://hg.mozilla.org/integration/autoland/rev/ed74f44b40c1
Move setWindowRect to not return from a generator. r=ato

Comment 8

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/d330e31dcb23
https://hg.mozilla.org/mozilla-central/rev/ed74f44b40c1