Closed
Bug 1364276
Opened 7 years ago
Closed 7 years ago
dom/test/mochitest/fetch/fetch_test_framework.js races service worker state change
Categories
(Core :: DOM: Service Workers, defect)
Core
DOM: Service Workers
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: bkelly, Assigned: bkelly)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
11.56 KB,
patch
|
asuth
:
review+
|
Details | Diff | Splinter Review |
The dom/test/mochitest/fetch/fetch_test_framework.js has this dubious code: navigator.serviceWorker.register("worker_wrapper.js", {scope: "."}) .then(function(registration) { if (registration.installing) { var done = false; registration.installing.onstatechange = function() { if (!done) { done = true; setupSW(registration); } }; } else { setupSW(registration); } }); This waits for the installing service worker to progress to waiting, but not to actually activate. The dom/cache version of this code does this instead: navigator.serviceWorker.ready.then(setupSW); navigator.serviceWorker.register("worker_wrapper.js", {scope: "."}); This avoids the race condition. I need to fix this because my changes in bug 1293277 trigger this failure much more frequently. It probably also explains a bunch of intermittent timeouts in these tests.
Assignee | ||
Updated•7 years ago
|
Summary: dom/test/mochitest/fetch/fetch_test_framework.js races service worker state change g → dom/test/mochitest/fetch/fetch_test_framework.js races service worker state change
Assignee | ||
Comment 1•7 years ago
|
||
Andrew, this just fixes the test framework to actually wait for the service worker to activate. Its a pretty clear race that was already fixed in the dom/cache version of this framework code. https://treeherder.mozilla.org/#/jobs?repo=try&revision=34a669b02d57fa679d06976348c78dea48566e36
Attachment #8867018 -
Flags: review?(bugmail)
Assignee | ||
Updated•7 years ago
|
Attachment #8867018 -
Attachment description: bug1364276_fetchframeowkr.patch → Wait for service worker to activate before proceeding in tests. r=asuth
Assignee | ||
Comment 2•7 years ago
|
||
Comment on attachment 8867018 [details] [diff] [review] Wait for service worker to activate before proceeding in tests. r=asuth Hmm. This causes other test failures in the same directory.
Attachment #8867018 -
Flags: review?(bugmail)
Assignee | ||
Comment 3•7 years ago
|
||
So, we can't use the ready promise universally because the _sw_reroute tests have multiple service worker registrations with overlapping scopes. Instead use a waitForState() helper throughout. https://treeherder.mozilla.org/#/jobs?repo=try&revision=df5a83b973e01c07e1bc4ea4afd2878e05b57454
Attachment #8867018 -
Attachment is obsolete: true
Attachment #8867426 -
Flags: review?(bugmail)
Assignee | ||
Comment 4•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac32aed900c18ef7d9e15b3420053bf22442bf70
Comment 5•7 years ago
|
||
Comment on attachment 8867426 [details] [diff] [review] Wait for service worker to activate before proceeding in tests. r=asuth Review of attachment 8867426 [details] [diff] [review]: ----------------------------------------------------------------- There's some irony in the ready promise being great for everyone but SW implementers, and that its essential foot-gun-ness is not truly revealed until multiple tests interact.
Attachment #8867426 -
Flags: review?(bugmail) → review+
Pushed by bkelly@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6237d34292cd Wait for service worker to activate before proceeding in tests. r=asuth
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6237d34292cd
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•