Closed Bug 1633712 Opened 2 years ago Closed 1 year ago

Create WorkerTargets as soon as the worker thread start, by using watchTargets API on the actor side

Categories

(DevTools :: Framework, task, P1)

task

Tracking

(Fission Milestone:M7, firefox84 fixed)

RESOLVED FIXED
84 Branch
Fission Milestone M7
Tracking Status
firefox84 --- fixed

People

(Reporter: ochameau, Assigned: nchevobbe)

References

(Blocks 4 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
    The createTargets and destroyTargets should create and destroy the Worker Targets for the worker that already exists.
    watchResources and unwatchResources 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:

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!
  1. the worker just get created and we immediately call worker.setDebuggerReady(false), this is done by WorkerPauser
  2. Completely independently of that, we start creating the WorkerTargetActor, on demand, via listWorkers and WorkerTargetActorList
  3. We use WorkerConnector to create the WorkerTargetActor from the Worker thread
  4. The frontend receive the worker target
  5. The frontend attach the ThreadActor
  6. Some magic code from the worker thread notify the main thread that the Thread Actor is attached
  7. 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.

See Also: → 1593940
Blocks: 1642599
Duplicate of this bug: 1593940
Whiteboard: dt-fission-m2-mvp

Tracking dt-fission-m2 bugs for Fission Nightly (M6c)

Fission Milestone: --- → M6c
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Type: enhancement → task
Priority: -- → P1
Blocks: 1651518
Blocks: 1651522

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.

Depends on: 1656888
Depends on: 1657310

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

Depends on: 1662734
Blocks: 1595553
Attachment #9167025 - Attachment description: Bug 1633712 - Create WorkerTargets as soon as possible. r=ochameau. → Bug 1633712 - [devtools] Create WorkerTargets as soon as possible. r=ochameau.
Attachment #9172983 - Attachment description: Bug 1633712 - Add test cases to browser_target_list_tab_workers and remove the fission fail-if. r=jdescottes. → Bug 1633712 - [devtools] Add test cases to browser_target_list_tab_workers and remove the fission fail-if. r=jdescottes.
Attachment #9172984 - Attachment description: Bug 1633712 - Add a console message resource test for worker targets. r=jdescottes. → Bug 1633712 - [devtools] Add a console message resource test for worker targets. r=jdescottes.

Bulk move of all m2-mvp devtools bugs to Fission M7.

Fission Milestone: M6c → M7
Depends on: 1667839
Attachment #9180138 - Attachment description: Bug 1633712 - [devtools] Move shouldNotifyWindowGlobal to a util file. r=ochameau. → Bug 1633712 - [devtools] Move shouldNotifyWindowGlobal and getAllRemoteBrowsingContexts to a util file. r=ochameau.
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.

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

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=P_an57mbRAmBNnJvy0Ed_A.0&revision=03553b300b8629a56f5b5cf5620a8105e8376f22&group_state=expanded

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

Flags: needinfo?(nchevobbe)

I'm investigating those leaks

Flags: needinfo?(nchevobbe)
Blocks: 1671906
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.
Regressions: 1674115
You need to log in before you can comment on or make changes to this bug.