Closed Bug 1113522 Opened 10 years ago Closed 1 month ago

Expose ServiceWorker in Workers (aka ServiceWorkerRegistration returns null for all of installing, waiting, active in Workers/ServiceWorkers)

Categories

(Core :: DOM: Workers, defect, P3)

33 Branch
x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
133 Branch
Tracking Status
firefox133 --- fixed

People

(Reporter: nsm, Assigned: asuth)

References

(Blocks 2 open bugs)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 1 obsolete file)

Both the WebIDL Exposed= flag and relevant implementation.
registration-attribute.https.html examines this.
Blocks: 1189023
I think this might be a compat issue worth fixing soon. It seems somewhat reasonable that a service worker might try to do self.registration.active.postMessage() in an install event to communicate with the worker its replacing.
I'm going to take this because I need it for writing some other tests. Also its a major compat issue!
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Unfortunately I ran out of time to work on this.
Assignee: bkelly → nobody
Status: ASSIGNED → NEW
Does chrome already implement this?
Yes. They also failed to handle self-postMessage() properly allowing SW's to be kept alive infinitely. They are fixing now. We should be careful to avoid that as well. You may want to wait until I do my clients changes for this. I will be exposing Worker as a client and may be able to re-use infrastructure.
Summary: Expose ServiceWorker in Workers → Expose ServiceWorker in Workers (aka ServiceWorkerRegistration returns null for all of installing, waiting, active in Workers/ServiceWorkers)
Priority: -- → P3
Depends on: 1544232

Thanks for providing an update Andrew! Hoping to get a question about this answered, if this is an acceptable forum for it. This old bug is still blocking the ability to "silently" refresh a page on Firefox when handling a navigation request in a service worker. Use case: be able to update to latest service worker as a user manually refreshes the page, not as a separate step.

Implementing self.registration.waiting in the worker is necessary to use the technique described in https://redfin.engineering/how-to-fix-the-refresh-button-when-using-service-workers-a8e27af6df68. I'm not sure of another way to activate a waiting worker & refresh the page in one atomic action. Either you get a visible second refresh of the page, or you rely solely on user input. Are there good resources/ways around this that renders this bug less than blocking for this particular usecase?

Any insight into the roadmap here/near-term plans would be fantastic - thanks for everything you guys do!

Thank you for upbeat inquiry! We're currently stabilizing a multi-process architectural overhaul of our ServiceWorker implementation. Once that's done this should be on the shortlist of our compat fixes. The abuse scenarios are the primary concern and implementation complexity for this bug (that is, the subject of bug 1544232). If all goes well, we would like to get to this for Firefox 78 (release calendar at https://wiki.mozilla.org/Release_Management/Calendar with 78 becoming nightly as the calendar rolls over to May) but a) I may be overly optimistic and b) we may guard this behind a pref and keep the pref only enabled on nightly and/or beta for an extra release to keep things moving but mitigate risk until we're confident we have tests covering all the attack scenarios.

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

Thank you for upbeat inquiry! We're currently stabilizing a multi-process architectural overhaul of our ServiceWorker implementation. Once that's done this should be on the shortlist of our compat fixes. The abuse scenarios are the primary concern and implementation complexity for this bug (that is, the subject of bug 1544232). If all goes well, we would like to get to this for Firefox 78 (release calendar at https://wiki.mozilla.org/Release_Management/Calendar with 78 becoming nightly as the calendar rolls over to May) but a) I may be overly optimistic and b) we may guard this behind a pref and keep the pref only enabled on nightly and/or beta for an extra release to keep things moving but mitigate risk until we're confident we have tests covering all the attack scenarios.

Awesome! That's great to hear. Thanks for the detailed update. Will keep an eye out for FF 78. Let me know if there's any testing/help you guys need with an implementation that requires this compat fix.

Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Severity: normal → S3

We are able to remove ServiceWorkerVisible and instead use
ServiceWorkersEnabled in its place since we are no longer limiting
where ServiceWorker instances are exposed.

Attachment #9192613 - Attachment is obsolete: true
Attachment #9339003 - Attachment description: WIP: Bug 1113522 - Expose ServiceWorker in Workers. r=#dom-workers! → Bug 1113522 - Expose ServiceWorker in Workers. r=#dom-workers!

ServiceWorkers aren't exposed as clients, so when we are sending a postMessage
from them, we need to capture their ServiceWorkerDescriptor and propagate that
instead of the client state.

Pushed by bugmail@asutherland.org: https://hg.mozilla.org/integration/autoland/rev/098149d7f447 Expose ServiceWorker in Workers. r=dom-worker-reviewers,webidl,edenchuang,emilio https://hg.mozilla.org/integration/autoland/rev/4784166c0acf ExtendableMessageEvent.source should be ServiceWorker-aware. r=dom-worker-reviewers,edenchuang
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 133 Branch

FF133 MDN docs for this can be tracked in https://github.com/mdn/content/issues/36532

Can you confirm my understanding of this:

  • This exposes service workers inside a service worker. Specifically:
    • WorkerNavigator.serviceWorker is added which you gives you a handle to ServiceWorkerContainer. No other new APIs exposed.
    • That ServiceWorkerContainer represents the service worker container object of the window that owns this service worker. It gives you a handle to all the clients associated with this document. If this same document is in multiple tabs, it gives you access to all of the service workes for all of them right?
  • What this change allows then is to get a handle to all service workers associated with this document in any tab and send them messages.
    So you can sent the active "previous version" service workers a message to tell them to refresh themselves with the latest version.
    • The reason you need this is that an updated service worker does not activate until all other active tabs/window clients using the previous version have been closed. This allows a service worker to prompt the current active workers to shut themselves down. I think you could probably do this in other ways though.
    • Further, the refresh button won't work to load your new functionality, because the existing service worker remains alive on refresh. With this you can manage the refresh from your service worker.

This is heavily based on reading https://redfin.engineering/how-to-fix-the-refresh-button-when-using-service-workers-a8e27af6df68 - can you confirm that this is largely still what people should do (there is no better approach), and that the fix added here is primarily to allow the "Approach #4" step to work?

We are able to remove ServiceWorkerVisible and instead use ServiceWorkersEnabled in its place since we are no longer limiting where ServiceWorker instances are exposed.

This seems to be addressed by https://bugzilla.mozilla.org/show_bug.cgi?id=1131324) which changes exposure from Func="ServiceWorkersEnabled" rather than ServiceWorkerVisible for FetchEvent, PushSubscription, PushSubscriptionOptions, ServiceWorker.

My guess is that previously perhaps you'd get a usable value back for a service worker from those APIs unless the service worker was active (perhaps null otherwise), and now you get it back for any service worker even if it is waiting? Can you explain?

Flags: needinfo?(bugmail)

(In reply to Hamish Willee from comment #17)

FF133 MDN docs for this can be tracked in https://github.com/mdn/content/issues/36532

Can you confirm my understanding of this:

  • This exposes service workers inside a service worker. Specifically:

This bug is specifically about making it so that instead of returning null in workers (inclusive of dedicated workers, shared workers, and service workers) when there should be a ServiceWorker value, we now will properly return the ServiceWorker value.

This happened in https://phabricator.services.mozilla.com/D213725 tracked by bug 1131324 yes.

  • That ServiceWorkerContainer represents the service worker container object of the window that owns this service worker.

ServiceWorkerContainer I'd say has 3 purposes:

  1. Inspecting and manipulating the registrations for this origin (register, getRegistration, getRegistrations).
  2. Understanding if the current window/worker is controlled (controller) / becomes controlled (oncontrollerchange) and the foot-gun ready promise implementation (good issue on why what people really want is ServiceWorkerRegistration.ready) that is sorta related.
  3. The ability to receive messages from ServiceWorkers via Client.postMessage (startMessages, onmessage, onmessageerror)

ServiceWorkerContainer was previously only exposed on windows, but as per above bug 1131324 exposes it on workers now too. Note that it was already possible for workers to be controlled by service workers, they just could not access this interface.

It gives you a handle to all the clients associated with this document.

I think you're referring to the Clients API/Interface which is only exposed to ServiceWorkers and has been exposed for a long time.

If this same document is in multiple tabs, it gives you access to all of the service workes for all of them right?

  • What this change allows then is to get a handle to all service workers associated with this document in any tab and send them messages.

ServiceWorkerContainer was already exposed to documents/windows. The API to get the list of registrations would allow enumerating all the ServiceWorker instances and to postMessage them.

So the new capability here is that now Workers can do this too, inclusive of ServiceWorkers being able to see other ServiceWorkers and message them.

So you can sent the active "previous version" service workers a message to tell them to refresh themselves with the latest version.

I think I understand what you're getting at here. Basically, if update is called on a registration or an update check is automatically triggered by a fetch event of functional event being dispatched, then the registration can end up with a ServiceWorker in its waiting slot and a ServiceWorker in its active slot. Without the change on this bug, those 2 ServiceWorkers could not postMessage each other, but now they can.

In terms of "refresh themselves with the latest version", I think you might mean that the waiting ServiceWorker can call skipWaiting to replace the active ServiceWorker and start controlling the already-controlled pages. I could imagine that some kind of explicit hand-off between them could make sense in very rare cases.

  • The reason you need this is that an updated service worker does not activate until all other active tabs/window clients using the previous version have been closed. This allows a service worker to prompt the current active workers to shut themselves down. I think you could probably do this in other ways though.

Right (about the transition to active). As noted, the waiting worker doesn't need the permission of the active worker to promote itself, it just has to call skipWaiting. Since ServiceWorker globals are regularly shutdown (active is a slot, not a guarantee that the global is always alive), it would be weird/bad design for the ServiceWorker to have state that's only stored in the global's memory.

  • Further, the refresh button won't work to load your new functionality, because the existing service worker remains alive on refresh. With this you can manage the refresh from your service worker.

There is a bit of a foot-gun here with refresh, yeah, but again, skipWaiting() is usually enough on its own.

We are able to remove ServiceWorkerVisible and instead use ServiceWorkersEnabled in its place since we are no longer limiting where ServiceWorker instances are exposed.

This seems to be addressed by https://bugzilla.mozilla.org/show_bug.cgi?id=1131324) which changes exposure from Func="ServiceWorkersEnabled" rather than ServiceWorkerVisible for FetchEvent, PushSubscription, PushSubscriptionOptions, ServiceWorker.

My guess is that previously perhaps you'd get a usable value back for a service worker from those APIs unless the service worker was active (perhaps null otherwise), and now you get it back for any service worker even if it is waiting? Can you explain?

This is specifically about how previously would only expose the ServiceWorker interface on the main thread or ServiceWorkerGlobalScope where it was necessary to expose because we had to expose the ServiceWorkerRegistration on ServiceWorkerGlobalScope and ServiceWorkerRegistration explicitly exposes ServiceWorker. But this is more of a low-level implementation detail; as noted up top, the key thing is we now everywhere the spec calls for ServiceWorker to be defined and have non-null values, it is.

Flags: needinfo?(bugmail)
Regressions: 1928191
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: