Bug 191986 – Updating a service worker during a navigation load sometimes makes the load fail

• Status: RESOLVED
• Resolution: FIXED
• Component: Service Workers

[youenn fablet]

According https://github.com/PWA-POLICE/pwa-bugs#problem-navigation-to-a-website-has-infinite-loading, updating a service worker during a navigation load sometimes trigger terminating a service worker that serves the content for a page, hence making the load fail.

[Radar WebKit Bug Importer]

<rdar://problem/46259790>

[Chris Dumez]

Seems to indicate that our TryActivate implementation is actually activating the newly installed worker even though there is already an active worker with clients.

https://w3c.github.io/ServiceWorker/#try-activate

[Chris Dumez]

Our tryActivate implementation does:
    // Invoke Activate with registration if either of the following is true:
    // - registration's active worker is null.
    // - The result of running Service Worker Has No Pending Events with registration's active worker is true,
    //   and no service worker client is using registration or registration's waiting worker's skip waiting flag is set.
    if (!activeWorker() || (!activeWorker()->hasPendingEvents() && (!hasClientsUsingRegistration() || waitingWorker()->isSkipWaitingFlagSet())))
        activate();

So somehow, even though there is an active worker and a pending load handled by that worker, we still call activate().

[youenn fablet]

We are probably registering the new document too late as a service worker client.
Currently we are creating/registering the Document when receiving the first data bytes which is too late.
Ideally, we should probably register the document as soon as we send the fetch event to the service worker.

Another approach would be to keep track of navigation loads a service worker is responsible for and update the activate logic according that new information.

[youenn fablet]

Created attachment 355889 [details]
Patch

[youenn fablet]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/14286

[EWS Watchlist]

Comment on attachment 355889 [details]
Patch

Attachment 355889 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10185320

New failing tests:
imported/w3c/web-platform-tests/webrtc/simplecall-no-ssrcs.https.html
media/no-fullscreen-when-hidden.html

[EWS Watchlist]

Created attachment 355907 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[youenn fablet]

Comment on attachment 355907 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

Error is unrelated

[WebKit Commit Bot]

Comment on attachment 355889 [details]
Patch

Clearing flags on attachment: 355889

Committed r238683: <https://trac.webkit.org/changeset/238683>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.