Fix service worker updates when importScripts() redirects

RESOLVED FIXED in Firefox 57

Status

()

enhancement
P2
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

(Blocks 1 bug)

57 Branch
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(URL)

Attachments

(2 attachments)

+++ This bug was initially created as a clone of Bug #1398484 +++

There is another bogus assertion like in bug 1398484 that assumes importScripts() cannot redirect.  This is in SW update code:

http://searchfox.org/mozilla-central/source/dom/workers/ServiceWorkerScriptCache.cpp#853

We should fix this and also make sure we have test coverage.
Actually, it seems we don't allow redirects in importScripts() during the update process.  I think this just fell through the cracks when we added importScripts() update checking.
Summary: Fix assertion in service worker update code preventing importScripts() redirect → Fix service worker updates when importScripts() redirects
Ho-Pang, there are two fixes here:

1. We loosen the assertion like in bug 1398484.
2. We also need to remove the redirect limitation for non-main scripts.
Attachment #8908782 - Flags: review?(bhsu)
This adds another WPT test to verify we can update a service worker when a redirected importScripts() changes.  Without the P1 patch this test times out.  With the P1 patch this test passes.
Attachment #8908791 - Flags: review?(bhsu)
Priority: -- → P2

Comment 5

2 years ago
Comment on attachment 8908782 [details] [diff] [review]
P1 Support redirected importScripts() when updating a service worker script. r=hopang

Review of attachment 8908782 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM
Attachment #8908782 - Flags: review?(bhsu) → review+

Comment 6

2 years ago
Comment on attachment 8908791 [details] [diff] [review]
P2 Add a WPT test that verifies a redirecting importScripts() can trigger an update. r=hopang

Review of attachment 8908791 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, thanks!
Attachment #8908791 - Flags: review?(bhsu) → review+

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5fc997003df2
https://hg.mozilla.org/mozilla-central/rev/bb67d7dc1ff2
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.