1510809 - ServiceWorkerContainer.register fires updatefound before its promise resolves

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, enhancement, P2)

Description

[Blake Kaplan (:mrbkap) (inactive)]

See the failure at [1] (this is with parent-intercept enabled and including the currently-failing service worker tests in the test set).

The basic problem is that the test does:

let p = navigator.serviceWorker.register(...);
p.then((swr) => {
  swr.addEventListener("updatefound", () => { ok(true, "this should be caught"); });
};

but we fire updatefound at the ServiceWorkerRegistration before register's promise resolves. According to [2], we are required to resolve the promise before enqueuing a task to fire updatefound.

[1] https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=213233858&repo=try&lineNumber=10843
[2] https://w3c.github.io/ServiceWorker/#installation-algorithm

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=50f77969c74ce82a98c0088e734319564a513bc

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fire updatefound correctly for SWC.register

Currently, we are required to fire updatefound in three cases:

  * When a "soft" update finds an update.
  * When a call to ServiceWorkerRegistration.update finds an update.
  * When ServiceWorkerContainer.register registers a ServiceWorker.

In the first case, there are no requirements on the timing of the event. For
the second two cases, however, the promise returned by update or register
needs to resolve before updatefound is dispatched. We hack around the first
case by explicitly counting the calls to update and only dispatching
updatefound when the final promise resolves. In the case of SWC.register, the
ServiceWorkerRegistration object might not even exist when it notices that we
need to fire updatefound, which suggests that we need some code to tell it
when to fire the event; except that in the soft update case, there is no
obvious place to do so. So, the easiest way to resolve this is to have the
parent process tell the ServiceWorkerRegistration when to fire updatefound
itself. This way, we don't rely on any tricky timing and everything is
consistent with itself.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b447ab87004
Fire updatefound correctly for SWC.register r=asuth

Comment 4

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/6b447ab87004