service worker wpt can leak workers across tests depending on iframe GC

RESOLVED FIXED in Firefox 45

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

Trunk
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(3 attachments, 4 obsolete attachments)

Many of the SW tests follow this pattern:

1) register a SW
2) create a controlled iframe
3) test things
4) remove frame
5) unregister SW

Some tests, however, fail to remove their frames in (4).  This is a problem because the registration is not actually removed until the client using it goes away.  So another test can start before the frame is GC'd and resurrect the previous test SW.  (The spec changed such that register() with same scope removed the uninstalling flag.)

This is particularly problematic for the tests that use the same scope.  Typically this is resources/blank.html.

We should make sure all tests remove their frames before completing.
This causes one of the update tests to hit an assert consistently.  I have a work around in the try, but I need to investigate more.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8cfc36233ddd
Re-number now that I have some additional patches.
Attachment #8695321 - Attachment is obsolete: true
Attachment #8695455 - Flags: review?(ehsan)
The failure I mentioned above was due to an update marking an activated worker activated again.  We simply have to catch this and not change the state in this case.
Attachment #8695457 - Flags: review?(ehsan)
There is a race in the tests that perform navigation triggered updates.  Since we do not currently delay until after navigation load completes these tests may miss the update since they only start looking for it after the frame is loaded.

These changes could possibly be removed after bug 1226443 lands.
Attachment #8695466 - Flags: review?(ehsan)
Remove some debugging that slipped into the last patch.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6c7152ceee6
Attachment #8695457 - Attachment is obsolete: true
Attachment #8695457 - Flags: review?(ehsan)
Attachment #8695468 - Flags: review?(ehsan)
Comment on attachment 8695466 [details] [diff] [review]
P3 Start waiting for updates prior to navigation load completion to avoid race. r=ehsan

These tests are still flakey.  They are going to race until I fix navigation updates to fire at a later time in bug 1226443.  I'm going to disable them for now.
Attachment #8695466 - Attachment is obsolete: true
Attachment #8695466 - Flags: review?(ehsan)
Rebase and fix a bug in clients-matchall-including-uncontrolled.https.html.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=234de64b829e
Attachment #8695455 - Attachment is obsolete: true
Attachment #8695455 - Flags: review?(ehsan)
Attachment #8695909 - Flags: review?(ehsan)
Try is green.  I think this is ready to go once it passes review.
Attachment #8695468 - Flags: review?(ehsan) → review+
Attachment #8695555 - Flags: review?(ehsan) → review+
Attachment #8695909 - Flags: review?(ehsan) → review+

Comment 13

4 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c79e993438bc
https://hg.mozilla.org/mozilla-central/rev/5806881dc7b6
https://hg.mozilla.org/mozilla-central/rev/d885b325d6f4
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.