Closed Bug 1237158 Opened 10 years ago Closed 10 years ago

dom/canvas/test/test_offscreen_serviceworker.html should unregister service worker

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox44 --- wontfix
firefox45 --- fixed
firefox46 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1232558 +++ While investigating bug 1232558 I noticed that we are triggering a lot of navigation-based updates for the scope: http://mochi.test:8888/tests/dom/canvas/test/ This is because test_offscreen_serviceworker.html does not unregister its service worker when its done. All following tests in the directory then trigger an update for each navigation (modulo some coalescing due to the 1 second timer). We should unregister and avoid all this unnecessary work.
In 20 pushes I didn't get a single instance of bug 1232558 with this patch applied. It should help reduce the orange quite a bit.
Comment on attachment 8704466 [details] [diff] [review] Unregister service worker at end of test_offscreen_serviceworker.html. r=ehsan Review of attachment 8704466 [details] [diff] [review]: ----------------------------------------------------------------- Holy moly! Thanks for catching this. Do you think it's worth failing a mochitest if it forgets to unregister a SW when it finishes in order to avoid running into this in the future? ::: dom/canvas/test/test_offscreencanvas_serviceworker.html @@ +18,5 @@ > if (msg.type == "test") { > ok(msg.result, msg.name); > } > if (msg.type == "finish") { > + dump('### ### calling unregister\n'); Nit: please remove this.
Attachment #8704466 - Flags: review?(ehsan) → review+
(In reply to :Ehsan Akhgari from comment #3) > Do you think it's worth failing a mochitest if it forgets to unregister a SW > when it finishes in order to avoid running into this in the future? I think that would be useful if we could put it in the mochitest harness somehow. Also maybe create something like the pushPrefEnv() utility to execute a test with a service worker, but automatically unregister after the test.
I think we should uplift this to aurora to minimize orange on that branch.
Comment on attachment 8704685 [details] [diff] [review] Unregister service worker at end of test_offscreen_serviceworker.html. r=ehsan Approval Request Comment [Feature/regressing bug #]: Service workers. [User impact if declined]: Top 20 intermittent orange. [Describe test coverage new/current, TreeHerder]: existing leak tests [Risks and why]: None. Test only change. [String/UUID change made/needed]: None.
Attachment #8704685 - Flags: approval-mozilla-aurora?
See Also: → 1237363
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment on attachment 8704685 [details] [diff] [review] Unregister service worker at end of test_offscreen_serviceworker.html. r=ehsan Improve the test suite, taking it. FYI, you don't need approval for test-only changes.
Attachment #8704685 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: