1694144 - Implement basic support for session.subscribe (for log.entryAdded)

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Description

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

For subscribing to events we need the following command implemented:
https://w3c.github.io/webdriver-bidi/#command-session-subscribe

Comment 1

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

This bug should only implement the necessary bits of session.subscribe that are needed to register for log events. It can even miss setting certain browsing contexts, so that we register the event for all tabs. As such we can reduce the scope.

Comment 2

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

As we actually agreed on in one of the last triage sessions none of the actual dependencies are needed for the basic implementation that has to support log.entryAdded. We are going to subscribe to all browsing contexts, and for extracting the event name we do not need any special deserialization logic.

Comment 3

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

Julian pointed out that there are WiP patches available that my work could be based on:

https://phabricator.services.mozilla.com/D120655
https://phabricator.services.mozilla.com/D121118

Comment 4

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

Attachment: Bug 1694144 - [WebDriver BiDi] Add basic support for session.subscribe command.

Comment 5

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

Attachment: Bug 1694144 - [wdspec] Add basic tests for "session.subscribe".

Depends on D122018

Comment 7

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

Attachment: Bug 1694144 - [wdspec] Split fixture module for WebDriver HTTP and BiDi.

Depends on D122018

Comment 8

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

Attachment: Bug 1694144 - [wdspec] Add BiDi error classes to BiDi client.

Depends on D122336

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9fb6c43c144d
[WebDriver BiDi] Add basic support for session.subscribe command. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/118ad1445b60
[wdspec] Split fixture module for WebDriver HTTP and BiDi. r=webdriver-reviewers,jgraham
https://hg.mozilla.org/integration/autoland/rev/c3cf28582710
[wdspec] Add BiDi error classes to BiDi client. r=webdriver-reviewers,jgraham,jdescottes
https://hg.mozilla.org/integration/autoland/rev/34b56695cd5c
[wdspec] Add basic tests for "session.subscribe". r=jgraham,webdriver-reviewers,jdescottes

Comment 10

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

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

Comment 11

[Cristian Brindusan [:cbrindusan]]

Backed out 5 changesets (bug 1694144, bug 1724227) for causing bc failures in browser_all_files_referenced.js.
https://hg.mozilla.org/integration/autoland/rev/17857f4eb7cee954fd2a56876596ed96ff076c87

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=34b56695cd5cae44500a1375d93eab057e3e91e0&selectedTaskRun=K41TA9pIQAev-uHUXXxCWQ.0

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

Comment 12

[Julian Descottes [:jdescottes]]

Attachment: Bug 1694144 - [remote] Add a simple ModuleIndex to import BiDi modules

Depends on D122019
Probably misses comments. Actually inspired by the CDP domain caches.

Comment 13

[Julian Descottes [:jdescottes]]

Attachment: Bug 1694144 - [remote] Add a simple ModuleIndex to keep track of all BiDi modules

Depends on D122019

This is a minimal change only to satisfy the browser_all_files_referenced.js test
The module imports are not actually used here, but at least there is very little code change to review

Comment 14

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

Going to re-land with the module registry in place, which references all the known modules.

Comment 15

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

Attachment: Bug 1694144 - [WebDriver BiDi] Add basic support for session.subscribe command.

This patch also includes the addition of the Module Registry,
which has been done by Julian Descottes <[e-mail]>.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6184d54ab163
[WebDriver BiDi] Add basic support for session.subscribe command. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/b4ff28974976
[wdspec] Split fixture module for WebDriver HTTP and BiDi. r=webdriver-reviewers,jgraham
https://hg.mozilla.org/integration/autoland/rev/c6b0d34d017e
[wdspec] Add BiDi error classes to BiDi client. r=webdriver-reviewers,jgraham,jdescottes
https://hg.mozilla.org/integration/autoland/rev/78a950fc4caf
[wdspec] Add basic tests for "session.subscribe". r=jgraham,webdriver-reviewers,jdescottes

Comment 17

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

Attachment: Bug 1694144 - [remote] Add missing EXPORTED_SYMBOLS export for new module registry.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/452e5b72978f
[remote] Add missing EXPORTED_SYMBOLS export for new module registry.

Comment 19

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6184d54ab163
https://hg.mozilla.org/mozilla-central/rev/b4ff28974976
https://hg.mozilla.org/mozilla-central/rev/c6b0d34d017e
https://hg.mozilla.org/mozilla-central/rev/78a950fc4caf
https://hg.mozilla.org/mozilla-central/rev/452e5b72978f

Comment 20

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

Upstream PR merged by jgraham

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/34d48ab3f222
[wpt PR 29996] - [Gecko Bug 1694144] [wdspec] Split fixture module for WebDriver HTTP and BiDi., a=testonly

Comment 22

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/34d48ab3f222