1719505 - Set "navigator.webdriver" property to true when Remote Agent is enabled

Status: RESOLVED FIXED

(Remote Protocol :: Agent, task, P2)

Description

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

With bug 1632821 the navigator.webdriver property is always true when Marionette is active. There is the follow-up bug 1696425 to even set it only when there is an active WebDriver session.

The same needs to be done for the Remote Agent when --remote-debugging-port has been specified on the command line. Whereby here we have to differentiate between WebDriver BiDi and CDP.

For WebDriver BiDi the property should behave similar to Marionette and rely on an active WebDriver session.

For CDP it would be up to us, but I would suggest to bind it to an active WebSocket connection.

For the implementation it might make sense to have a single property on the Remote Agent component that can be checked, and which internally will check all supported protocols. We could call the property active, or isInAutomation to match other modules in Firefox.

Lets discuss the priority in next weeks WebDriver meeting.

Comment 1

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

We actually already have a check in Navigator.cpp but that doesn't seem to work:
https://searchfox.org/mozilla-central/rev/9c91451cc2392d942a42493fc895f5aeeddde45d/dom/base/Navigator.cpp#2126-2133

Maybe we need a specific instance of Remote Agent for content processes like we have in Marionette. Lets wait for bug 1720676 being landed, which is including quite a big refactoring that we can base on here.

The work on this bug should block bug 1696425.

Comment 2

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

Not related to our milestone 2 work. Will have to check for dependencies.

Comment 3

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

We should discuss if this is a feature that we definitely need immediately before enabling BiDi on release channels. Alternatively I would suggest to have the same behavior as Marionette for now by setting navigator.webdriver=true when the Remote Agent is enabled via the command line argument. That would be more in-par with Marionette and easier to implement.

Comment 4

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

Given that bug 1696425 covers the truth state of the property for an active WebDriver session, lets use this bug to set the property whenever the Remote Agent is enabled as well.

Comment 5

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

As long as the Remote Agent can be started and stopped during the lifetime of Firefox this feature is not doable. As such we have to fix bug 1717899 first.

Comment 6

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

Attachment: Bug 1719505 - [wdspec] Fix browser fixture for already existing CDP or BiDi port files.

Comment 7

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

Attachment: Bug 1719505 - [wdspec] Improvements for WebDriver BiDi Python client.

The "session.new" command currently doesn't work because the
transport is started after sending the command. This needed
to be flipped.

Also it is helpful to have the event loop of the current
transport available as property on the BiDi session directly.

Depends on D144978

Comment 8

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

Attachment: Bug 1719505 - [remote] Remove no longer used methods from Remote Agent interface.

Both methods have been removed via bug 1717899 and can no
longer be used.

Depends on D144979

Comment 9

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

Attachment: Bug 1719505 - [remote] Unify Remote Agent's running state with Marionette.

Both components are related to WebDriver and should expose the
same flag for their running state.

Depends on D144980

Comment 10

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

Attachment: Bug 1719505 - [remote] Add Remote Agent class for content processes.

When accessing the nsIRemoteAgent interface from a content
process the running state cannot be determined without
communicating with the Remote Agent class from the parent
process.

Similar to Marionette this patch adds a dedicated class of
the Remote Agent for content processes, and exports an
instance of the appropriate class depending on in which
process the Remote Agent gets created in.

Depends on D144982

Comment 11

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

Attachment: Bug 1719505 - [remote] Add CDP and WebDriver BiDi tests for navigator.webdriver.

Depends on D144983

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/621024810236
[wdspec] Fix browser fixture for already existing CDP or BiDi port files. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/3e3df2a6b1e3
[wdspec] Improvements for WebDriver BiDi Python client. r=webdriver-reviewers,jgraham
https://hg.mozilla.org/integration/autoland/rev/f40c2fe6872d
[remote] Remove no longer used methods from Remote Agent interface. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/674fe8523cf6
[remote] Unify Remote Agent's running state with Marionette. r=webdriver-reviewers,mossop,smaug,keeler,jdescottes
https://hg.mozilla.org/integration/autoland/rev/8df69b3445e5
[remote] Add Remote Agent class for content processes. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/915b5331f6b2
[remote] Add CDP and WebDriver BiDi tests for navigator.webdriver. r=webdriver-reviewers,jdescottes

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/33869 for changes under testing/web-platform/tests

Comment 14

[Atila Butkovits]

Backed out for causing failures at browser_Troubleshoot.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/2c180f1d21acadafcde98e9101fa807f7aa9df69

Push where the failures started: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=a1961e821a9b985c32c1718c9c96ad07234b47c8&selectedTaskRun=aguenPH8QuuC-EMEYRe4wg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=376274703&repo=autoland&lineNumber=13928

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 16

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

I missed to rename the listening property of the Remote Agent to running in the following files:

• toolkit/content/aboutSupport.js
• toolkit/modules/Troubleshoot.jsm
• toolkit/modules/tests/browser/browser_Troubleshoot.js

I'm going to push once try reports the failing test as passing.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d59f342563c
[wdspec] Fix browser fixture for already existing CDP or BiDi port files. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/7ac82aff1852
[wdspec] Improvements for WebDriver BiDi Python client. r=webdriver-reviewers,jgraham
https://hg.mozilla.org/integration/autoland/rev/4feb57751cba
[remote] Remove no longer used methods from Remote Agent interface. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/553219c11b69
[remote] Unify Remote Agent's running state with Marionette. r=webdriver-reviewers,mossop,smaug,keeler,jdescottes
https://hg.mozilla.org/integration/autoland/rev/ff65f4a77f37
[remote] Add Remote Agent class for content processes. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/871093f41bb1
[remote] Add CDP and WebDriver BiDi tests for navigator.webdriver. r=webdriver-reviewers,jdescottes

Comment 18

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/7d59f342563c
https://hg.mozilla.org/mozilla-central/rev/7ac82aff1852
https://hg.mozilla.org/mozilla-central/rev/4feb57751cba
https://hg.mozilla.org/mozilla-central/rev/553219c11b69
https://hg.mozilla.org/mozilla-central/rev/ff65f4a77f37
https://hg.mozilla.org/mozilla-central/rev/871093f41bb1

Comment 19

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot