1477475 - Drop legacy capabilities propagation to Marionette in geckodriver

Status: RESOLVED FIXED

(Testing :: geckodriver, enhancement)

Description

[Andreas Tolfsen ❲:ato❳]

geckodriver currently writes a legacy capabilities structure to the
WebDriver:NewSession command it uses on creating a new WebDriver
session which duplicates the top-level object:

> {capabilities: {<caps>, desiredCapabilities: <caps>}}

Where caps can look something like:

> {acceptInsecureCerts: true, pageLoadStrategy: "normal", …}

Marionette has picked up the top-level "capabilities" key for a
very long time now, and there is no longer any need to duplicate
the body.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: Drop legacy Marionette capabilities duplication. r=automatedtester

geckodriver currently writes a legacy capabilities structure to the
WebDriver:NewSession command it uses on creating a new WebDriver
session which duplicates the top-level object:

	{capabilities: {<caps>, desiredCapabilities: <caps>}}

Where caps can look something like:

	{acceptInsecureCerts: true, pageLoadStrategy: "normal", ...}

Marionette has picked up the top-level "capabilities" key for a
very long time now, and there is no longer any need to duplicate
the body.

Comment 2

[Andreas Tolfsen ❲:ato❳]

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

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8993909 [details] [diff] [review]
Drop legacy Marionette capabilities duplication. r=automatedtester

Review of attachment 8993909 [details] [diff] [review]:
-----------------------------------------------------------------

So what about the `webdriver` crate where this is still present? I assume it has to also be removed there?

Please also update the TraceLog.md file to get rid of any `desiredCapabilities` reference.

Comment 4

[Andreas Tolfsen ❲:ato❳]

(In reply to Henrik Skupin (:whimboo) from comment #3)

> So what about the `webdriver` crate where this is still present? I
> assume it has to also be removed there?

What is still present in the webdriver crate that I remove here?

> Please also update the TraceLog.md file to get rid of any
> `desiredCapabilities` reference.

What do you mean? desiredCapabilities is still a thing that we
support.  This patch doesn’t drop support for it.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Oh, so we only want to drop the duplication for the Marionette protocol. Looks like I missed that, and now I even see that this is listed in the summary. 

Just to clarify, whatever is specified on the WebDriver part (desired vs alwaysMatch/firstMatch) we always send duplicated capabilities to Marionette?

Comment 6

[Andreas Tolfsen ❲:ato❳]

(In reply to Henrik Skupin (:whimboo) from comment #5)

> Oh, so we only want to drop the duplication for the Marionette
> protocol.  Looks like I missed that, and now I even see that this
> is listed in the summary.

Oh sorry.  I could’ve explained that more clearly.  I see the
original description is somewhat misleading.

> Just to clarify, whatever is specified on the WebDriver part
> (desired vs alwaysMatch/firstMatch) we always send duplicated
> capabilities to Marionette?

Marionette only gets the _negotiated/matched_ capabilities, we never
send it any of the _requested_ capabilities we receive from the
user.

However because Marionette used to look for the desiredCapabilities
property on the passed object and later moved to just look at the
object itself, we duplicated desiredCapabilities inside it, so that
we ended up with something like this:

	let capabilities = {pageLoadStrategy: "normal", {desiredCapabilities: {pageLoadStrategy: "normal"}}}

Does that make sense?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/100697883c7d
Drop legacy Marionette capabilities duplication. r=whimboo

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/100697883c7d