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)
Core
DOM: Service Workers
Tracking
()
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: bkelly, Assigned: bkelly)
References
Details
Attachments
(3 files, 4 obsolete files)
1.03 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
1.33 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
39.65 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•8 years ago
|
||
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
Assignee | ||
Comment 2•8 years ago
|
||
Re-number now that I have some additional patches.
Attachment #8695321 -
Attachment is obsolete: true
Attachment #8695455 -
Flags: review?(ehsan)
Assignee | ||
Comment 3•8 years ago
|
||
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)
Assignee | ||
Comment 4•8 years ago
|
||
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)
Assignee | ||
Comment 5•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=366d5d6fcad4
Assignee | ||
Comment 6•8 years ago
|
||
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)
Assignee | ||
Comment 7•8 years ago
|
||
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)
Assignee | ||
Comment 8•8 years ago
|
||
Attachment #8695555 -
Flags: review?(ehsan)
Assignee | ||
Comment 9•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4de78e70cc38
Assignee | ||
Comment 10•8 years ago
|
||
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)
Assignee | ||
Comment 11•8 years ago
|
||
Try is green. I think this is ready to go once it passes review.
Updated•8 years ago
|
Attachment #8695468 -
Flags: review?(ehsan) → review+
Updated•8 years ago
|
Attachment #8695555 -
Flags: review?(ehsan) → review+
Updated•8 years ago
|
Attachment #8695909 -
Flags: review?(ehsan) → review+
Comment 12•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c79e993438bc https://hg.mozilla.org/integration/mozilla-inbound/rev/5806881dc7b6 https://hg.mozilla.org/integration/mozilla-inbound/rev/d885b325d6f4
Comment 13•8 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
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•