Open Bug 1836707 Opened 1 year ago Updated 1 year ago

Add helper in nsIServiceWorkerManager to resolve BrowsingContextID to the controlling nsIServiceWorkerRegistrationInfo

Categories

(Core :: DOM: Workers, task)

task

Tracking

()

People

(Reporter: jdescottes, Unassigned)

References

(Blocks 1 open bug)

Details

(From to Andrew Sutherland [:asuth] (he/him) from Bug 1267119 comment #16)

We could certainly add a helper to nsIServiceWorkerManager that could resolve a BrowsingContextID to the nsIServiceWorkerRegistrationInfo that is controlling a window.

Such an API would be really useful in devtools in order to match scripts or requests related to service workers when the toolbox is only debugging a specific tab.

We would probably also want to handle the situation where a (window belonging to a) BrowsingContextID has called navigator.serviceWorker.register() but the window is not controlled. Since this can happen dynamically on the fly, this is a little more complicated. It's not immediately obvious to me how to best structure this.

Moving to the appropriate component.

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

We would probably also want to handle the situation where a (window belonging to a) BrowsingContextID has called navigator.serviceWorker.register() but the window is not controlled. Since this can happen dynamically on the fly, this is a little more complicated. It's not immediately obvious to me how to best structure this.

Right, it sounds useful especially for the requests performed to download the script. For those scenarios it might make more sense to have something on the request itself in order to link it to the browsing context which called register? For regular workers using PFetch, I think Eden made it so that it could work, but maybe this is different? A nsIServiceWorkerManager API that would also handle this use case sounds harder to design.

Component: Netmonitor → DOM: Workers
Product: DevTools → Core

(In reply to Julian Descottes [:jdescottes] from comment #2)

Right, it sounds useful especially for the requests performed to download the script. For those scenarios it might make more sense to have something on the request itself in order to link it to the browsing context which called register? For regular workers using PFetch, I think Eden made it so that it could work, but maybe this is different? A nsIServiceWorkerManager API that would also handle this use case sounds harder to design.

An additional complication is that because service workers has a job queue with coalescing semantics in step 6 of schedule job we end up with a situation where potentially multiple browsing contexts could be associated with a single ServiceWorker register() job and the resulting new registration. Like if you open 5 tabs in parallel and they all call register, they would all end up with promises waiting on the same single register job.

And for completeness, some related edge cases:

  • A ServiceWorker registered for a scope that doesn't actually exist for interception purposes (ex: "/this-url-does-not-exist"), but does exist for push notification purposes or some other rationale where a page would call getRegistrations() to locate the registration whose scope does not match the page, and then postMessage() the SW.
  • A page "/foo/bar" is controlled by the SW with scope "/". Another tab registers a new SW with scope "/foo" that claims the pre-existing "/foo/bar" page (which did not call register). This is extreme (and probably a bad idea for the site), but possible.
  • The update check in step 7 of the update algorithm is not performed by the script loader (which would re-do any loads if the update check finds changes, although those might then be cached).

Maybe a sketch for an API that could handle the various permutations would be:

  • Add nsIServiceWorkerManager::CreateBrowsingContextFilter method that takes a browsing context id and some flags to scope level of interest and the method returns a new class/interface nsIServiceWorkerFilter.
  • nsIServiceWorkerFilter::FilterLoadInfo(nsILoadInfo) is the key method that can be used to efficiently check if the fetch is coming from an involved ServiceWorker, and it returns an enum or bitmask describing the relationship between the browsing context and the SW. I propose integer enum values below in order of decreasing precedence where 0 would be that the fetch isn't interesting or related:
    • Controlling: The SW is controlling the page. This could perhaps be further broken down into:
      • ControllingLoaded: The page has been controlled by the SW since its load. (Note that a SW that does not handle "fetch" events is still notionally controlling a page, so this does not distinguish if the page load actually went to the network or not.)
      • ControllingClaimed: The page was claimed by the SW (and may or may not have been previously claimed by a different SW).
    • PageRegistered: The page issued a register call to this SW.
    • RegistrationUpdating: The page didn't call register() but an update check for the registration resulting from a functional event or handle fetch algorithm ran and dediced we need to update the SW and the SW is somewhere in the evaluating/installing/waiting lifecycle, with "active" corresponding to the ControllingClaimed value above.
    • RegistrationUpdateCheck: There isn't actually a SW yet, we're just in the update-check as part of the update algorithm, so there's no script loader active yet. This is the predecessor state of RegistrationUpdating above.
    • ScopeMatches: The page's URL will match the scope of the SW but the page didn't call register, but this inherently would be the result of another page issuing a fresh call to register().
    • PageMessaged: The page used ServiceWorker.postMessage to talk to the SW. (A page can find all SWs via getRegistrations().)
    • ServiceWorkerMessaged: The ServiceWorker messaged the page by using the Clients API. (Both this and the above could be true, which might be an argument for a bit-mask or separate flags.)
  • We could have a bunch of (less performant) helpers that could be used after the (faster) filter check above to return the ServiceWorkerID and/or the nsIServiceWorkerInfo for the ServiceWorker (although there would be neither of these for an update check fetch).
  • Medium-term future: The filter could let devtools indicate that it wants all the ServiceWorkers to effectively have setDebuggerReady(false) called on them as they're created and allow for a listener callback that gets invoked with the corresponding (new) nsIWorkerDebugger. I guess this would also implicitly call attachDebugger on the nsIServiceWorkerInfo.
    • The context is that, as part of our attempt to improve ServiceWorker performance by not having them be owned by the main thread, we will likely move to having their exposed nsIWorkerDebuggers be on the parent process main thread rather than on the main thread of the content process where they live. This would also end up happening for SharedWorkers too. But it means we can have the association between nsIServiceWorkerInfo and nsIWorkerDebugger be much more direct because they will be exposed in the same process.

This would represent a shift of more decision-making into ServiceWorker C++ logic, but this is desirable because a number of the notable changes for a SW becoming relevant to a browsing context already happen on the PBackground thread, and in the future it's quite possible we could move even more of them to a background thread and we could update the state more atomically with this API so that fetches don't get lost, whereas if we had to message devtools for it to update its own predicates first, it might miss a bunch of fetches, especially if/as we shard PBackground into multiple threads/task queues and start doing more toplevel IPC endpoint stuff.

You need to log in before you can comment on or make changes to this bug.