Closed Bug 1651522 Opened 4 years ago Closed 3 months ago

Create WorkerTargets for service workers

Categories

(DevTools :: Framework, enhancement)

enhancement

Tracking

(Fission Milestone:Future, firefox122 fixed)

RESOLVED FIXED
122 Branch
Fission Milestone Future
Tracking Status
firefox122 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Depends on 4 open bugs, Blocks 7 open bugs, Regressed 1 open bug)

Details

(Whiteboard: dt-fission-future)

Attachments

(5 files, 3 obsolete files)

Bug 1633712 will introduce the first framework code in order to create WorkerTargetActor's via the Watcher Actor. This will focus on the regular web page's workers, running in the web page's process.
Bug 1651518 will followup in order to also support parent process workers and shared workers.
Then, the last step would be to support service workers. Like bug 1651518, it may also be only about tweaking a few filtering methods. The filtering would be slightly more complex for service workers as we might have to filter based on the tab's current origin.
And who knows, it may be more complex than just tweaking a few filtering methods.

The filtering around tab's current origin is being explained over here:
https://phabricator.services.mozilla.com/D77425#2416452
3 options are mentioned.
I would personaly go for option C. I think that it would allow debugging SW in all cases from the debugger and stop having you to have to go through about:debugging in order to debug SW startup/shutdown, which may occur after the page navigated.

Harald told me:

Solution C does indeed seem useful, even though SWs don't react to navigation in any special way (apart from the usual tear down they have after inactivity). Other tools like Console and Network persist data; so showing a SW from a prior domain seems not that bad.

Finally, I think that this isn't strictly necessary for shipping fission.
For now, I'll flag this in the reserve list. But I may be wrong and we can revisit this triage.

Tracking dt-fission-m2-reserve bugs for Fission Beta milestone (M7).

Fission Milestone: --- → M7

We might want to first change the behavior of SW targets lifecycle via bug 1655439 before implementing this?

Depends on: 1655439

Bulk move of all dt-fission-m2-reserve bugs to Fission MVP milestone.

Fission Milestone: M7 → MVP
Blocks: 1672614
Whiteboard: dt-fission-m2-reserve → dt-fission-m2-mvp
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Whiteboard: dt-fission-m2-mvp → dt-fission-m3-mvp
Blocks: 1608848
Whiteboard: dt-fission-m3-mvp → dt-fission-m3-reserve
Assignee: nchevobbe → nobody
Status: ASSIGNED → NEW

Moving "dt-fission-m3-reserve" bugs to "dt-fission-future" because they don't block Fission MVP.

Fission Milestone: MVP → Future
Whiteboard: dt-fission-m3-reserve → dt-fission-future
Blocks: 1749341
Depends on: 1770130

I think we should NOT do that as it prevent debugging SW on navigation to another domain.
But we must probably have some helpful information in the frontend.

Attachment #9356928 - Attachment description: Bug 1651522 - [devtools] Filter the service workers by domain. → Bug 1651522 - [devtools] Filter the service workers by domain.
Attachment #9311327 - Attachment is obsolete: true

The SW wasn't properly unregistered between tests, probably causing failure in other test using this test page.

Depends on: 1861941

Comment on attachment 9359392 [details]
Bug 1651522 - [devtools] Avoid clearing documents when navigating.

Revision D191450 was moved to bug 1861941. Setting attachment 9359392 [details] to obsolete.

Attachment #9359392 - Attachment is obsolete: true
Depends on: 1861943
Depends on: 1862088
Depends on: 1862157
Assignee: nobody → poirot.alex
Status: NEW → ASSIGNED

Comment on attachment 9356184 [details]
Bug 1651522 - [devtools] Enable SW debugging in debugger by default

Revision D189825 was moved to bug 1862157. Setting attachment 9356184 [details] to obsolete.

Attachment #9356184 - Attachment is obsolete: true
Blocks: 1863776
Duplicate of this bug: 1726270
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9855542eb321
[devtools] Implement server side Service Worker targets. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/f80194bc4d6c
[devtools] Remove the now useless `destroyServiceWorkersOnNavigation` flag on TargetCommand. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/7871ba0badea
[devtools] Fix browser_dbg-windowless-service-workers-reload.js intermittent. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/81d3ee520ab6
[devtools] Filter the service workers by domain. r=devtools-reviewers,nchevobbe

I was having a race condition in the new shared helper to unregister a Service worker.
It should be fixed now.

Flags: needinfo?(poirot.alex)
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/17dc7ba17c9c
[devtools] Implement server side Service Worker targets. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/0ac0cfa3ed67
[devtools] Remove the now useless `destroyServiceWorkersOnNavigation` flag on TargetCommand. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/b44436c89af2
[devtools] Fix browser_dbg-windowless-service-workers-reload.js intermittent. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/ac5e5139fe3d
[devtools] Filter the service workers by domain. r=devtools-reviewers,nchevobbe
Regressions: 1865983
Blocks: 1866814

In bug 1866627 reported against Firefox 122 there are reports of ServiceWorker processes being created/hanging around; is it possible that these changes might lead to a change in the behavior around calls to nsIServiceWorkerInfo::attachDebugger?

Flags: needinfo?(poirot.alex)
See Also: → 1866627

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #21)

In bug 1866627 reported against Firefox 122 there are reports of ServiceWorker processes being created/hanging around; is it possible that these changes might lead to a change in the behavior around calls to nsIServiceWorkerInfo::attachDebugger?

Oh yes, that's a terrible mistake I did here.
These magic calls to {attach,detach}Debugger should only be called for Service Workers which actually relates to the currently debugged context.
At least when opening regular DevTools, debugging a Tab, only Service Workers related to that tab will be impacted.
But the issue would still remain for the Browser Toolbox, which is debugging all the service workers.
For that, we may have to further revisit this.

Flags: needinfo?(poirot.alex)
Blocks: 1867236
Regressions: 1868537
You need to log in before you can comment on or make changes to this bug.