1899503 - Making WorkerDebugger communication through IPC instead of runnable dispatching between threads.

Status: RESOLVED FIXED

(Core :: DOM: Workers, enhancement, P2)

Description

[Eden Chuang[:edenchuang]]

This is a part of bug 1672491. This bug is specific for improving the communication between Worker and WorkerDebugger by the IPC mechanism.

Comment 1

[Eden Chuang[:edenchuang]]

Attachment: Bug 1899503 - P1 New top-level IPC PRemoteWorkerDebuggerManager and PRemoteWorkerDebugger to support nsIWorkerDebugger interface for Workers in the parent process. r=asuth

Comment 2

[Eden Chuang[:edenchuang]]

Attachment: Bug 1899503 - P3 PRemoteWorkerDebugger binding implementation. r=asuth

Depends on D230259

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

:ochameau, I'm setting a needinfo on you for https://phabricator.services.mozilla.com/D230260#7992817 because it's very easy to not see phabricator name-checks.

Comment 4

[Eden Chuang[:edenchuang]]

Attachment: Bug 1899503 - P2 PRemoteWorkerDebuggerMangaer binding implementation. r=asuth

Depends on D230259

Comment 5

[Eden Chuang[:edenchuang]]

Attachment: Bug 1899503 - P4 Update SharedWorker windowID for remote worker debugger. r=asuth

Depends on D230260

Comment 6

[Julian Descottes [:jdescottes]]

Hi!

I'm currently working on adding worker support for WebDriver BiDi (which shares a lot of similarities with what we try to do in DevTools), and it's going to be one of our focus for 2025, so I'd like to get a picture of the potential impacts of this change.

From looking at the comments on phabricator, it's not clear to me if the nsIWorkerDebugger will now be created in the parent process main thread for all workers, or if that only applies to shared/service workers.

My initial goal is to be able to interact with regular web workers, and for now my setup is similar to devtools: I spawn a WorkerDebugger in content process main thread, and then load a script via initialize to load the WebDriver BiDi logic in the worker thread.

Can you confirm if this will need to change with this Bug (and to be clear, that would probably only simplify things for us, so that's a nice win for the BiDi implementation, just trying to assess the impacts).
Thanks

Comment 7

[Eden Chuang[:edenchuang]]

After the bug, nsIWorkerDebugger could be registered in parent process main thread for all types of Worker. Basically, it has the same behavior with the nsIWorkerDebugger in the content process main thread. You can still call nsIWorkerDebugger::Initilaize(url) in the parent process, and the debugger script will be loaded in the content process. And any notification, such as onmessage, onerror, are propagated to the parent process nsIWorkerDebuggerListener.

After this bug, we will not get rid of the content process nsIWorkerDebugger immediately. There will a bug for the debugger movement from content to parent.

Comment 8

[Julian Descottes [:jdescottes]]

Thanks for the feedback, this sounds like it will really simplify the implementation for us in WebDriver BiDi. So I'm hesitant to start working on this topic before this bug lands. I see you already have patches attached and reviews in progress, do you think this has a good chance to land early 2025?

Comment 9

[Eden Chuang[:edenchuang]]

The patch is still being reviewed. Yes, we expect this will be landed in the early of 2025.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2596059bf325
P1 New top-level IPC PRemoteWorkerDebuggerManager and PRemoteWorkerDebugger to support nsIWorkerDebugger interface for Workers in the parent process. r=asuth
https://hg.mozilla.org/integration/autoland/rev/8973242af52a
P2 PRemoteWorkerDebuggerMangaer binding implementation. r=asuth
https://hg.mozilla.org/integration/autoland/rev/b0cfee4dccb7
P3 PRemoteWorkerDebugger binding implementation. r=asuth
https://hg.mozilla.org/integration/autoland/rev/ffad31a6d765
P4 Update SharedWorker windowID for remote worker debugger. r=asuth

Comment 11

[Sandor Molnar[:smolnar]]

Backed out for causing multiple bustages/failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/1c03fe24b2f9b783980ae836da55fbcbcb5f2b12

Push with failures

Failure log bustages

Failure log xpc

Failure log devtools

Failure log browsertime

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e80b18ca9165
P1 New top-level IPC PRemoteWorkerDebuggerManager and PRemoteWorkerDebugger to support nsIWorkerDebugger interface for Workers in the parent process. r=asuth
https://hg.mozilla.org/integration/autoland/rev/9bd2a82481f4
P2 PRemoteWorkerDebuggerMangaer binding implementation. r=asuth
https://hg.mozilla.org/integration/autoland/rev/01fe98c08d10
P3 PRemoteWorkerDebugger binding implementation. r=asuth
https://hg.mozilla.org/integration/autoland/rev/d926f7fefc9d
P4 Update SharedWorker windowID for remote worker debugger. r=asuth

Comment 13

[amarc]

Backed out for causing multiple failures

• Backout link
• Push with failures
• Failure Log -> build bustage
• Failure Log -> btime failure
• Failure Log -> xpcshell failure
• Failure Log 2 -> xpcshell failure
• Failure log -> dt failure

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d39af040a61
P1 New top-level IPC PRemoteWorkerDebuggerManager and PRemoteWorkerDebugger to support nsIWorkerDebugger interface for Workers in the parent process. r=asuth
https://hg.mozilla.org/integration/autoland/rev/df07abc82381
P2 PRemoteWorkerDebuggerMangaer binding implementation. r=asuth
https://hg.mozilla.org/integration/autoland/rev/07833c96f60c
P3 PRemoteWorkerDebugger binding implementation. r=asuth
https://hg.mozilla.org/integration/autoland/rev/18bb85ce72bd
P4 Update SharedWorker windowID for remote worker debugger. r=asuth

Comment 15

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5d39af040a61
https://hg.mozilla.org/mozilla-central/rev/df07abc82381
https://hg.mozilla.org/mozilla-central/rev/07833c96f60c
https://hg.mozilla.org/mozilla-central/rev/18bb85ce72bd

Comment 16

[Atila Butkovits]

Backed out for causing high frequency failures in RtlRetrieveNtUserPfn.

Backout link: https://hg.mozilla.org/integration/autoland/rev/265c68e6df9ecbb37e8afd49a9342220b26543fb

Push where failures started: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=windows%2C11%2Cx64%2C24h2%2Casan%2Cmochitests%2Ctest-windows11-64-24h2-asan%2Fopt-mochitest-devtools-chrome%2Cdt1&revision=8e7e2ea9dd4a4596b2ba9973ad9aa9812c2f832d

Failure log: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=windows%2C11%2Cx64%2C24h2%2Casan%2Cmochitests%2Ctest-windows11-64-24h2-asan%2Fopt-mochitest-devtools-chrome%2Cdt1&revision=8e7e2ea9dd4a4596b2ba9973ad9aa9812c2f832d&selectedTaskRun=GkJNtZZXSMqcwJ7g0LRZuw.0

Comment 17

[Cosmin Sabou [:CosminS]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/265c68e6df9e

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e22fbf8ab8f
P1 New top-level IPC PRemoteWorkerDebuggerManager and PRemoteWorkerDebugger to support nsIWorkerDebugger interface for Workers in the parent process. r=asuth
https://hg.mozilla.org/integration/autoland/rev/b003e26209cf
P2 PRemoteWorkerDebuggerMangaer binding implementation. r=asuth
https://hg.mozilla.org/integration/autoland/rev/1e774bd7f365
P3 PRemoteWorkerDebugger binding implementation. r=asuth
https://hg.mozilla.org/integration/autoland/rev/d40050ad198b
P4 Update SharedWorker windowID for remote worker debugger. r=asuth

Comment 19

[agoloman]

Backed out for causing wpt failures @nsTArray.h.

• Backout link
• Push with failures
• Failure Log

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77eda17d0795
P1 New top-level IPC PRemoteWorkerDebuggerManager and PRemoteWorkerDebugger to support nsIWorkerDebugger interface for Workers in the parent process. r=asuth
https://hg.mozilla.org/integration/autoland/rev/604c4c52574a
P2 PRemoteWorkerDebuggerMangaer binding implementation. r=asuth
https://hg.mozilla.org/integration/autoland/rev/f2debe329bca
P3 PRemoteWorkerDebugger binding implementation. r=asuth
https://hg.mozilla.org/integration/autoland/rev/53705a19f919
P4 Update SharedWorker windowID for remote worker debugger. r=asuth

Comment 21

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/77eda17d0795
https://hg.mozilla.org/mozilla-central/rev/604c4c52574a
https://hg.mozilla.org/mozilla-central/rev/f2debe329bca
https://hg.mozilla.org/mozilla-central/rev/53705a19f919