1744251 - [meta] More things should be using nsCOMPtr<nsIGlobalObject> instead of WorkerPrivate*

Status: RESOLVED WORKSFORME

(Core :: DOM: Workers, task)

Description

[Jens Stutte [:jstutte]]

Meta: More things should be using nsCOMPtr<nsIGlobalObject>

nsIGlobalObject exists to allow code that wants to work with a global to not have to have a separate code path for windows and workers (plus there are more global types than just those!). There are a lot of cases where code is accessing WorkerPrivate when it really wants to be accessing nsIGlobalObject.

But it should be noted that in the nsIGlobalObject case, all the reasons that code already needs to use WorkerRefs still apply. Which is why in https://phabricator.services.mozilla.com/D125555#4135011 I proposed (and I think brought up elsewhere with :smaug? Maybe Matrix?) that nsIGlobalObject should likely have a generic concept that uses WorkerRefs on workers and something else on windows so that:

1. Code can indicate that it currently needs the global / its underpinnings to be kept alive, at least until...
2. It's notified that the global is terminating, at which point the code is responsible for cleaning up as quickly as possible and dropping its reference to the keepalive.

(Note that windows and nsIGlobalObject do already somewhat have a concept of this through DOMEventTargetHelper, but its semantics don't line up exactly.)

When performing cleanups where it might seem like we want more code to be holding a strong reference to WorkerPrivate, we really want them to be holding a reference to nsIGlobalObject and/or a keepalive.

Comment 1

[Jens Stutte [:jstutte]]

(from bug 1744025 comment 6)

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The meta keyword is there, the bug doesn't depend on other bugs and there is no activity for 12 months.
:aiunusov, maybe it's time to close this bug?

Comment 3

[Jens Stutte [:jstutte]]

I assume this meta bug is still meaningful, but we never became really concrete here. Can we? Should we? Or: If that intention is already captured better by other ongoing refactorings, we should just close this?

Comment 4

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

The bug still accurately captures our current sentiment and we've made meaningful progress both by ensuring new code uses nsIGlobalObject as well as converting some existing code, but we haven't used the bug for tracking purposes. It's probably fine to close unless we're planning to do a large triage pass to set up the bug relationships.

Comment 5

[Jens Stutte [:jstutte]]

I think WFM best captures the spirit of comment 4.