1936774 - Investigate potential issues with nsIWorkerDebugger.initialize

Status: RESOLVED FIXED

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

Description

[Julian Descottes [:jdescottes]]

While prototyping worker support in BiDi, I noticed we rely on nsIWorkerDebugger initialize to load the "startup" script in devtools and this in turn makes the nsIWorkerDebugger instance initialized.

https://searchfox.org/mozilla-central/rev/39e6d5b609b74a19c0824d98ae654b60d6310152/dom/workers/nsIWorkerDebugger.idl#52

void initialize(in AString url);

https://searchfox.org/mozilla-central/rev/39e6d5b609b74a19c0824d98ae654b60d6310152/dom/workers/nsIWorkerDebugger.idl#32

readonly attribute boolean isInitialized;

This sounds fine as long as only DevTools is only using the API. But if we consider scenarios where BiDi is monitoring events on workers and we also want to open the DevTools toolbox, both protocols might try to call initialize on the same nsIWorkerDebugger instance. If BiDi already "initialized" the WorkerDebugger, DevTools will not try to load its script and won't be able to interact with the worker.

I can see two options here:

• we extend initialize to make it more flexible
• or both DevTools and BiDi use the same startup script?

Alex: any thoughts here?

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1936774 - [remote] Rename NavigationListener actors to WebProgressListener

The goal is to reduce the confusion between the high level navigation manager/listener which
provide events about the navigation to WebDriver BiDi with some logic specific to BiDi and the underlying
listeners directly monitoring the platform notifications.

In the next patch in the stack we need to introduce an alternate way to listen to those platform
notifications so having more distinct names could be useful.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1936774 - [remote] Prototype parent process NavigationListener

Depends on D245945

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/842565c0705e
[remote] Rename NavigationListener actors to WebProgressListener r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/35f43832e05c
[remote] Prototype parent process NavigationListener r=webdriver-reviewers,Sasha

Comment 4

[Julian Descottes [:jdescottes]]

Those patches should have been attached to Bug 1930616, requesting backout.

Comment 5

[Atila Butkovits]

Backed out as requested: https://hg-edge.mozilla.org/integration/autoland/rev/e4f091d4d51f346c1f8ed2d3d58220461c06cced

Comment 6

[Phabricator Automation]

Comment on attachment 9479719 [details]
Bug 1936774 - [remote] Prototype parent process NavigationListener

Revision D245946 was moved to bug 1930616. Setting attachment 9479719 [details] to obsolete.

Comment 7

[Phabricator Automation]

Comment on attachment 9479718 [details]
Bug 1936774 - [remote] Rename NavigationListener actors to WebProgressListener

Revision D245945 was moved to bug 1930616. Setting attachment 9479718 [details] to obsolete.

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: Bug 1936774 - [devtools] Refactor worker initializer to be easier to reuse outside of devtools

Comment 9

[Julian Descottes [:jdescottes]]

(discussed on phab)

Comment 10

[Julian Descottes [:jdescottes]]

We should be able to reuse devtools/server/startup/shared-worker-initializer.js easily for BiDi now. The change was rather small so downgrading to 2P.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/733370ffdd0d
https://hg.mozilla.org/integration/autoland/rev/b41b6ad00ee0
[devtools] Refactor worker initializer to be easier to reuse outside of devtools r=ochameau

Comment 12

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/b41b6ad00ee0