1720676 - Don't run Remote Agent specific code when --remote-debugging-port hasn't been specified

Status: RESOLVED FIXED

(Remote Protocol :: Agent, defect, P2)

Description

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

Ideally the component shouldn't be available at all when --remote-debugging-port hasn't been specified. But right now the command line handling code sits in the Rust Remote Agent component, and automatically creates the JSM equivalent.

Given that this isn't an easy change we should make sure to not run any code that is related to Remote Agent if it's not explicitly enabled via the command line argument.

This implies:

• Do not log debug lines like which protocols are active
• Stop enabled protocols only when the Remote Agent is listening

Comment 1

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

I would like to propose this for M1 which makes it much easier to handle the state of the Remote Agent.

Comment 2

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

Discussed with Julian on Element and we are going to add this to the M1 list given that it is a regression from two M1 releated fixes.

Comment 3

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

Attachment: Bug 1720676 - [marionette] Move component registration into components.conf.

Depends on D121094

Comment 4

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

Attachment: Bug 1720676 - [remote] Remove Remote Agent's Rust implementation.

By implementing the command line handling and the
printing to stderr in the JS module of the Remote
Agent, the Rust implementation is no longer necessary.

This change allows the JS component to hold each detail
about the Remote Agent's status. Also it doesn't
require a full build anymore when changes are needed.

Depends on D121095

Comment 5

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

CC'ing Geoff to let him know about upcoming build config changes for Marionette.

Comment 6

[Geoff Lankow (:darktrojan)]

Thanks whimboo, I'll make another bug or this could get messy.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f915c04b9c35
[marionette] Move component registration into components.conf. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/93a9c870d3d8
[remote] Remove Remote Agent's Rust implementation. r=webdriver-reviewers,jdescottes

Comment 8

[Natalia Csoregi [:nataliaCs]]

Backed out for remote fails on browser_agent.js

• backout: https://hg.mozilla.org/integration/autoland/rev/7a6ba4880649d1a54c4c41796607353bf73210c5

• push: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=UkRyTZr9SCKhieCKiVf7BA.0&revision=93a9c870d3d81e39aebae3b50f4e8f16b68dc1ba&group_state=expanded

• failure log: https://treeherder.mozilla.org/logviewer?job_id=346767376&repo=autoland&lineNumber=2115

[task 2021-07-29T18:04:56.592Z] 18:04:56     INFO - TEST-PASS | remote/cdp/test/browser/component/browser_agent.js | remote-listening observer notified enabled state - 
[task 2021-07-29T18:04:56.593Z] 18:04:56     INFO - Buffered messages finished
[task 2021-07-29T18:04:56.593Z] 18:04:56     INFO - TEST-UNEXPECTED-FAIL | remote/cdp/test/browser/component/browser_agent.js | remote-listening observer notified disabled state - Got null, expected "false"
[task 2021-07-29T18:04:56.593Z] 18:04:56     INFO - Stack trace:
[task 2021-07-29T18:04:56.593Z] 18:04:56     INFO - chrome://mochikit/content/browser-test.js:test_is:1371
[task 2021-07-29T18:04:56.593Z] 18:04:56     INFO - chrome://mochitests/content/browser/remote/cdp/test/browser/component/browser_agent.js:remoteListeningNotification:68
[task 2021-07-29T18:04:56.594Z] 18:04:56     INFO - Leaving test bound remoteListeningNotification
[task 2021-07-29T18:04:56.594Z] 18:04:56     INFO - Entering test bound listenTakesString
[task 2021-07-29T18:04:56.595Z] 18:04:56     INFO - GECKO(3728) | DevTools listening on ws://localhost:50118/devtools/browser/066e2f3c-6204-4f5b-a0d8-44846dca8193
[task 2021-07-29T18:04:56.595Z] 18:04:56     INFO - GECKO(3728) | WebDriver BiDi listening on ws://localhost:50118
[task 2021-07-29T18:04:56.595Z] 18:04:56     INFO - GECKO(3728) | 1627581896470	RemoteAgent	DEBUG	Resetting recommended pref browser.contentblocking.features.standard
[task 2021-07-29T18:04:56.596Z] 18:04:56     INFO - GECKO(3728) | 1627581896470	RemoteAgent	DEBUG	Resetting recommended pref network.cookie.cookieBehavior
[task 2021-07-29T18:04:56.596Z] 18:04:56     INFO - Leaving test bound listenTakesString
[task 2021-07-29T18:04:56.597Z] 18:04:56     INFO - Entering test bound listenNonURL
[task 2021-07-29T18:04:56.597Z] 18:04:56     INFO - TEST-PASS | remote/cdp/test/browser/component/browser_agent.js | undefined assertion name -

Comment 9

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

The linter suggested to remove false as third argument for Services.obs.notifyObservers(). That means callbacks of the remote-listening observer listener will receive null now.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b58937c59f63
[marionette] Move component registration into components.conf. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/3c2599c00332
[remote] Remove Remote Agent's Rust implementation. r=webdriver-reviewers,jdescottes

Comment 11

[Noemi Erli[:noemi_erli]]

Backed out 2 changesets (Bug 1720676) for causing remote failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=3c2599c0033255a84eb62c9e92694477fce80e0e&selectedTaskRun=VgNZG_T2RvCzq_Y9VJtmtA.0

Log: https://treeherder.mozilla.org/logviewer?job_id=346787715&repo=autoland&lineNumber=2901

Backout: https://hg.mozilla.org/integration/autoland/rev/eddb1c07e7243663e7c8065eed45b2f8a08e32fe

Comment 12

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

Problem here is that we register the quit-application only when the --remote-debugging-port argument has been specified, but Mochitests don't do that and as such we leak the running httpd.js server, and several other parts of the Remote Agents code base. Fix for that will follow soon.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/36fe2def8460
[marionette] Move component registration into components.conf. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/5c01aa328586
[remote] Remove Remote Agent's Rust implementation. r=webdriver-reviewers,jdescottes

Comment 14

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/36fe2def8460
https://hg.mozilla.org/mozilla-central/rev/5c01aa328586