Closed Bug 1230164 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

Attachments

(3 files, 4 obsolete files)

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+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: