Create WorkerTargets as soon as the worker thread start, by using watchTargets API on the actor side
Categories
(DevTools :: Framework, task, P1)
Tracking
(Fission Milestone:M7, firefox84 fixed)
Tracking | Status | |
---|---|---|
firefox84 | --- | fixed |
People
(Reporter: ochameau, Assigned: nchevobbe)
References
(Blocks 2 open bugs)
Details
(Whiteboard: dt-fission-m2-mvp)
Attachments
(4 files)
Similarly to bug 1593937 and bug 1620248, which will help creating the BrowsingContextTargetActor before the process or document starts loading, we should do the same with WorkerTargetActor.
We should introduce a "worker target helper" over here:
https://searchfox.org/mozilla-central/source/devtools/server/actors/descriptors/watcher/target-helpers/
Which exposes 4 methods:
- createTargets
- destroyTargets
- watchResources
- unwatchResources
ThecreateTargets
anddestroyTargets
should create and destroy the Worker Targets for the worker that already exists.
watchResources
andunwatchResources
should communicate the new watched/unwatched resources for all existing worker targets.
In parallel to that, we should have some code, which would watch all:
- new worker being created and call WatcherActor.notifyTargetCreated
- workers that have been destroyed and call WatcherActor.notifyTargetDestroyed
We might have to hook up into WatcherRegistry in order to register/unregister this "worker watching" code. Similarly to how we register/unregister the DevToolsFrame JSWindow Actor. (Or see my other approach for content process target,maybeRegisterMessageListeners
in this patch)
The goal here would be to start by only supporting Tab Target. So only when WatcherActor._browser
/WatcherActor.browsingContextID
are set. And so only when we focus on just one BrowsingContext. Supporting the browser toolbox means having to inspect all the processes, while for one tab, we can focus on just one process at a time. (We may support navigation to another process in a followup)
Compared to the current implementation, using legacy listeners and TabTarget.listWorkers
, here, we should avoid involving TabTarget/BrowsingContextTargetActor. We might have to introduce a new JS Window Actor, or reuse DevToolsFrame JS Window Actor (if relevant).
You might want to contribute to the existing TargetList test specific to Worker targets:
https://searchfox.org/mozilla-central/rev/21f2b48e01f2e14a94e8d39a665b56fcc08ecbdb/devtools/shared/resources/tests/browser_target_list_workers.js#190-234
In order to support more usecases:
- destruction
- workers created after the call to watchTargets
- may be have two distinct test files? One for BrowserToolbox codepath and another one for Tabs?
Last but not list, a quick note about the existing codebase.
The code watching for new workers and workers being destroyed should be inspired from WorkerTargetActorList.
matchWorkerDebugger
should be tweaked to check WatcherActor's BrowsingContextID instead of window.- WorkerPauser should be kept until we address bug 1573327
WorkerPauser is actually including a very important bit that is a bit convoluted and not very well known.
Right here, on every Worker instantiation, we pause the worker immediately, from the main thread, preventing it to run anything. And much later on, there, we resume it, still from the main thread. But the path between these two callsites is convoluted!
- the worker just get created and we immediately call worker.setDebuggerReady(false), this is done by WorkerPauser
- Completely independently of that, we start creating the WorkerTargetActor, on demand, via listWorkers and WorkerTargetActorList
- We use WorkerConnector to create the WorkerTargetActor from the Worker thread
- The frontend receive the worker target
- The frontend attach the ThreadActor
- Some magic code from the worker thread notify the main thread that the Thread Actor is attached
- Some other magic code in the main thread receive this message and resume the worker
All this "magic" (complexity) relates to bug 1573327 as it will allow to pass the breakpoint to the Watcher Actor, and stop having to freeze the worker untils ThreadActor.attach is called. Today, we pass the breakpoint via this thread actor method. We will pass the breakpoints as we pass the "watched resource types" to all processes and threads.
Reporter | ||
Updated•4 years ago
|
Comment 2•4 years ago
|
||
Tracking dt-fission-m2 bugs for Fission Nightly (M6c)
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Reporter | ||
Comment 3•4 years ago
|
||
I opened some followups upfront.
Bug 1651518 in order to expose workers from the parent process.
Bug 1651522 in order to expose service workers.
I'm not 100% confident about the execution plan, so feel free to reorganise the bugs as you make progress!
At least, bug 1651522 should help track when we will address the performance issues of service worker debugging from the debugger.
Assignee | ||
Comment 5•4 years ago
|
||
Check that when removing iframes, we're notified about the worker unregistration,
and check that the target list works as expected when we have multiple iframes
on same origin (both remote and same-origin as main document).
Depends on D85399
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 7•4 years ago
|
||
Bulk move of all m2-mvp devtools bugs to Fission M7.
Updated•4 years ago
|
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/10f97f47c883 [devtools] Move shouldNotifyWindowGlobal and getAllRemoteBrowsingContexts to a util file. r=ochameau. https://hg.mozilla.org/integration/autoland/rev/6eaa4d12b58c [devtools] Create WorkerTargets as soon as possible. r=ochameau,devtools-backward-compat-reviewers. https://hg.mozilla.org/integration/autoland/rev/19c8ea0b43df [devtools] Add test cases to browser_target_list_tab_workers and remove the fission fail-if. r=jdescottes. https://hg.mozilla.org/integration/autoland/rev/03553b300b86 [devtools] Add a console message resource test for worker targets. r=jdescottes.
Comment 10•4 years ago
|
||
Backed out for leaks on leak at mozilla::net::WebSocketEventService::GetOrCreate, mozilla::dom::WebSocketImpl::Init, mozilla::dom::WebSocket::ConstructorCommon, mozilla::dom::WebSocket::Constructor
backout: https://hg.mozilla.org/integration/autoland/rev/a6aaf0c8e183f7eff7a668f9b4c6c52c39958c9b
failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318695738&repo=autoland&lineNumber=13556
[task 2020-10-15T06:26:32.428Z] 06:26:32 INFO - TEST-START | devtools/shared/resources/tests/browser_target_list_preffedoff.js
[task 2020-10-15T06:26:32.923Z] 06:26:32 INFO - GECKO(3389) | =================================================================
[task 2020-10-15T06:26:32.924Z] 06:26:32 ERROR - GECKO(3389) | ==3548==ERROR: LeakSanitizer: detected memory leaks
[task 2020-10-15T06:26:32.924Z] 06:26:32 INFO - GECKO(3389) | Direct leak of 192 byte(s) in 2 object(s) allocated from:
[task 2020-10-15T06:26:32.924Z] 06:26:32 INFO - GECKO(3389) | #0 0x55b68589bb3d in malloc /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
[task 2020-10-15T06:26:32.924Z] 06:26:32 INFO - GECKO(3389) | #1 0x55b6858e076d in moz_xmalloc /builds/worker/checkouts/gecko/memory/mozalloc/mozalloc.cpp:52:15
[task 2020-10-15T06:26:32.924Z] 06:26:32 INFO - GECKO(3389) | #2 0x7f9a9aaae6ad in operator new /builds/worker/workspace/obj-build/dist/include/mozilla/cxxalloc.h:33:10
[task 2020-10-15T06:26:32.925Z] 06:26:32 INFO - GECKO(3389) | #3 0x7f9a9aaae6ad in XPCWrappedNative::GetNewOrUsed(JSContext*, xpcObjectHelper&, XPCWrappedNativeScope*, XPCNativeInterface*, XPCWrappedNative**) /builds/worker/checkouts/gecko/js/xpconnect/src/XPCWrappedNative.cpp:425:15
[task 2020-10-15T06:26:32.925Z] 06:26:32 INFO - GECKO(3389) | #4 0x7f9a9aa242d0 in XPCConvert::NativeInterface2JSObject(JSContext*, JS::MutableHandle<JS::Value>, xpcObjectHelper&, nsID const*, bool, nsresult*) /builds/worker/checkouts/gecko/js/xpconnect/src/XPCConvert.cpp:937:17
[task 2020-10-15T06:26:32.925Z] 06:26:32 INFO - GECKO(3389) | #5 0x7f9a9aa219c3 in XPCConvert::NativeData2JS(JSContext*, JS::MutableHandle<JS::Value>, void const*, nsXPTType const&, nsID const*, unsigned int, nsresult*) /builds/worker/checkouts/gecko/js/xpconnect/src/XPCConvert.cpp:355:14
[task 2020-10-15T06:26:32.925Z] 06:26:32 INFO - GECKO(3389) | #6 0x7f9a9aaa6bd1 in nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) /builds/worker/checkouts/gecko/js/xpconnect/src/XPCWrappedJSClass.cpp:922:12
[task 2020-10-15T06:26:32.926Z] 06:26:32 INFO - GECKO(3389) | #7 0x7f9a98fe59b0 in PrepareAndDispatch /builds/worker/checkouts/gecko/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:115:37
[task 2020-10-15T06:26:32.927Z] 06:26:32 INFO - GECKO(3389) | #8 0x7f9a98fe474a in SharedStub (/builds/worker/workspace/build/application/firefox/libxul.so+0x2d3c74a)
[task 2020-10-15T06:26:32.928Z] 06:26:32 INFO - GECKO(3389) | #9 0x7f9a9fdba357 in mozilla::dom::WorkerDebuggerManager::RegisterDebuggerMainThread(mozilla::dom::WorkerPrivate*, bool) /builds/worker/checkouts/gecko/dom/workers/WorkerDebuggerManager.cpp:282:17
[task 2020-10-15T06:26:32.929Z] 06:26:32 INFO - GECKO(3389) | #10 0x7f9a9fdfd4fe in RegisterWorkerDebugger /builds/worker/checkouts/gecko/dom/workers/WorkerDebuggerManager.h:93:12
[task 2020-10-15T06:26:32.930Z] 06:26:32 INFO - GECKO(3389) | #11 0x7f9a9fdfd4fe in EnableDebugger /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:1479:7
[...][task 2020-10-15T06:27:16.367Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at mozilla::net::WebSocketEventService::GetOrCreate, mozilla::dom::WebSocketImpl::Init, mozilla::dom::WebSocket::ConstructorCommon, mozilla::dom::WebSocket::Constructor
[task 2020-10-15T06:27:16.368Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at mozilla::dom::WorkerDebuggerManager::RegisterDebuggerMainThread, RegisterWorkerDebugger, EnableDebugger, mozilla::dom::WorkerPrivate::Constructor
[task 2020-10-15T06:27:16.369Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCNativeSet::NewInstance, XPCNativeSet::GetNewOrUsed, XPCWrappedNative::GetNewOrUsed, XPCConvert::NativeInterface2JSObject
[task 2020-10-15T06:27:16.370Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCWrappedNative::GetNewOrUsed, XPCConvert::NativeInterface2JSObject, GetService, xpc::Services_Resolve
[task 2020-10-15T06:27:16.370Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCNativeSet::NewInstanceMutate, XPCNativeSet::GetNewOrUsed, XPCWrappedNative::ExtendSet, XPCWrappedNative::InitTearOff
[task 2020-10-15T06:27:16.371Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCWrappedNativeProto::GetNewOrUsed, XPCWrappedNative::GetNewOrUsed, XPCConvert::NativeInterface2JSObject, GetService
[task 2020-10-15T06:27:16.372Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCWrappedNative::GetNewOrUsed, XPCConvert::NativeInterface2JSObject, XPCConvert::NativeData2JS, nsXPCWrappedJS::CallMethod
[task 2020-10-15T06:27:16.373Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCNativeInterface::NewInstance, XPCNativeInterface::GetNewOrUsed, XPCWrappedNative::FindTearOff, GetService
[task 2020-10-15T06:27:16.374Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCNativeInterface::NewInstance, XPCNativeInterface::GetNewOrUsed, XPCConvert::NativeInterface2JSObject, XPCConvert::NativeData2JS
[task 2020-10-15T06:27:16.374Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at MakeUnique, XPCWrappedNativeTearOff::AddTearOff, XPCWrappedNative::FindTearOff, XPCWrappedNative::GetNewOrUsed
[task 2020-10-15T06:27:16.375Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCNativeInterface::NewInstance, XPCNativeInterface::GetNewOrUsed, XPCNativeSet::GetNewOrUsed, XPCWrappedNativeProto::GetNewOrUsed
[task 2020-10-15T06:27:16.376Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCWrappedNativeProto::GetNewOrUsed, XPCWrappedNative::GetNewOrUsed, XPCConvert::NativeInterface2JSObject, XPCConvert::NativeData2JS
[task 2020-10-15T06:27:16.376Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCNativeInterface::NewInstance, XPCNativeInterface::GetNewOrUsed, XPCConvert::NativeInterface2JSObject, GetService
[task 2020-10-15T06:27:16.377Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at nsSupportsWeakReference::GetWeakReference, NS_GetWeakReference, do_GetWeakReference, nsMaybeWeakPtrArray
[task 2020-10-15T06:27:16.377Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at MakeUnique, XPCWrappedNativeTearOff::AddTearOff, XPCWrappedNative::FindTearOff, XPCWrappedNative::FindTearOff
[task 2020-10-15T06:27:16.377Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at PLDHashTable::Add, PutEntry, nsBaseHashtable, nsBaseHashtable
[task 2020-10-15T06:27:16.377Z] 06:27:16 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at XPCNativeSet::NewInstance, XPCNativeSet::GetNewOrUsed, XPCWrappedNativeProto::GetNewOrUsed, XPCWrappedNative::GetNewOrUsed
[task 2020-10-15T06:27:16.378Z] 06:27:16 INFO - runtests.py | Application ran for: 0:01:45.878651
Comment 12•4 years ago
|
||
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2ce79a9ed2d5 [devtools] Move shouldNotifyWindowGlobal and getAllRemoteBrowsingContexts to a util file. r=ochameau. https://hg.mozilla.org/integration/autoland/rev/895cf08432c4 [devtools] Create WorkerTargets as soon as possible. r=ochameau,devtools-backward-compat-reviewers. https://hg.mozilla.org/integration/autoland/rev/12e7595a7a94 [devtools] Add test cases to browser_target_list_tab_workers and remove the fission fail-if. r=jdescottes. https://hg.mozilla.org/integration/autoland/rev/7b94fd81e68b [devtools] Add a console message resource test for worker targets. r=jdescottes.
Comment 13•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2ce79a9ed2d5
https://hg.mozilla.org/mozilla-central/rev/895cf08432c4
https://hg.mozilla.org/mozilla-central/rev/12e7595a7a94
https://hg.mozilla.org/mozilla-central/rev/7b94fd81e68b
Description
•