Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WorkerGlobalScope worker set does not appear to be used #6592

Closed
domfarolino opened this issue Apr 17, 2021 · 2 comments · Fixed by #6596
Closed

WorkerGlobalScope worker set does not appear to be used #6592

domfarolino opened this issue Apr 17, 2021 · 2 comments · Fixed by #6596

Comments

@domfarolino
Copy link
Member

The title says it all: https://html.spec.whatwg.org/multipage/workers.html#the-worker's-workers is written to upon sub-worker creation, but never seems to be referenced elsewhere. Is it subtly needed in ways I'm not seeing?

@annevk
Copy link
Member

annevk commented Apr 18, 2021

I suspect I modeled it after "the worker's workers", see 59a4750. It looks like that was only there to populate the owner set (formerly the worker's Documents) though so I suspect it can be removed.

(I have vague concerns that we don't deal with lifetime and ownership well. E.g., what holds a Worker or SharedWorker object alive if all it has are event listeners? But this doesn't really address that nor would it be the right solution I think.)

@domenic
Copy link
Member

domenic commented Apr 19, 2021

Agreed, it'd be nice to remove this. "owner set" is the thing that specs use.

annevk added a commit that referenced this issue Apr 20, 2021
It used to be a helper for managing the owner set, but at the time owner set got refactored we did not realize that was all worker set did.

Fixes #6592.
domenic pushed a commit that referenced this issue Apr 20, 2021
It used to be a helper for managing the owner set, but at the time owner set got refactored we did not realize that was all worker set did.

Fixes #6592.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants