1691402 - Extract session specific code from driver.js into a dedicated session module

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, task, P2)

Description

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

The current implementation of Marionette embeds most of the session handling code within the GeckoDriver class of driver.js. To be able to share that specific code with the WebDriver BiDi implementation (/remote), all this code needs to be extracted into its own session module like session.js.

Julian, could you maybe have a look at driver.js and figure out how much work this would be? I assume we might have to split this bug into individual smaller pieces.

Comment 1

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

I'm going to take that now given that it's partly blocking me.

Comment 2

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

Attachment: Bug 1691402 - [marionette] Add session module for WebDriver session.

This patch creates a new module for the WebDriver session
specific implementation. It no longer requires session data
like capabilities to be directly bound to the GeckoDriver class.

Comment 3

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

The work here was actually simple, and only took a bit of time. It was mostly a refactoring of code. As such I would drop the points from 13 to just 2.

Comment 4

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

Attachment: Bug 1691402 - [marionette] Move current browsing context members to WebDriver session.

Depends on D107552

Comment 5

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

Attachment: Bug 1691402 - [marionette] Don't try to destroy a non-existent session.

If there is no active WebDriver session the code
related to deleting a session, and resetting other
state should not be run again.

This could actually happen when the
"Marionette:Quit" command is called, which itself
destroys the session, and the follwing socket
connection drop tries to run it again.

Depends on D108168

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c8e4a03fd2f
[marionette] Add session module for WebDriver session. r=marionette-reviewers,jdescottes,jgraham
https://hg.mozilla.org/integration/autoland/rev/5f9c65613b0d
[marionette] Move current browsing context members to WebDriver session. r=marionette-reviewers,jdescottes,jgraham
https://hg.mozilla.org/integration/autoland/rev/1f9f47742739
[marionette] Don't try to destroy a non-existent session. r=marionette-reviewers,jdescottes,jgraham

Comment 7

[Noemi Erli[:noemi_erli]]

Backed out 3 changesets (Bug 1691402) for causing failures in test_session.js CLOSED TREE

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

Failure log: https://treeherder.mozilla.org/logviewer?job_id=333446143&repo=autoland&lineNumber=1961
https://treeherder.mozilla.org/logviewer?job_id=333444413&repo=autoland&lineNumber=2455

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

Comment 8

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

(In reply to Noemi Erli[:noemi_erli] from comment #7)

Backed out 3 changesets (Bug 1691402) for causing failures in test_session.js CLOSED TREE

Failure log: https://treeherder.mozilla.org/logviewer?job_id=333446143&repo=autoland&lineNumber=1961
https://treeherder.mozilla.org/logviewer?job_id=333444413&repo=autoland&lineNumber=2455

No, this is for bug 1697159 but not this one. I'm going to land the set of patches on this bug again, but the appinfo one later once issues are fixed.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8bf7dc5dcf31
[marionette] Add session module for WebDriver session. r=marionette-reviewers,jdescottes,jgraham
https://hg.mozilla.org/integration/autoland/rev/b02976d0757b
[marionette] Move current browsing context members to WebDriver session. r=marionette-reviewers,jdescottes,jgraham
https://hg.mozilla.org/integration/autoland/rev/46da1dd655f9
[marionette] Don't try to destroy a non-existent session. r=marionette-reviewers,jdescottes,jgraham

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/8bf7dc5dcf31
https://hg.mozilla.org/mozilla-central/rev/b02976d0757b
https://hg.mozilla.org/mozilla-central/rev/46da1dd655f9