1413056 - test_bug1408734.html needs to wait for service worker to activate

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

Looks like test_bug1408734.html added another test case using the pattern that was fixed in bug 1411746.  Basically, before we trigger the claim() call we need to wait for the service worker to activate.

Ironically, I both reviewed this test and also implemented the fix it was missing.  I should have caught this.  Sorry!

Eden, do you have any time to look at this today?  If not, I can take it tomorrow.  No worries either way.  Thanks!

Comment 1

[Eden Chuang[:edenchuang]]

Of course. I will update the test case today.

Comment 2

[Eden Chuang[:edenchuang]]

Attachment: Bug 1413056 -

Comment 3

[Eden Chuang[:edenchuang]]

Attachment: Bug 1413056 - Using utils.js in test_bug1408734.html for waitForState and waitForControlled. r?tt

Tom, could you help review this patch?

Comment 4

[Eden Chuang[:edenchuang]]

Comment on attachment 8923659 [details] [diff] [review]
Bug 1413056 - Using utils.js in test_bug1408734.html for waitForState and waitForControlled. r?tt

Change reviewer to bkelly.

Comment 5

[Tom Tung [:tt, :ttung]]

Comment on attachment 8923659 [details] [diff] [review]
Bug 1413056 - Using utils.js in test_bug1408734.html for waitForState and waitForControlled. r?tt

Review of attachment 8923659 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, just remove the extra empty line. Thanks!

::: dom/workers/test/serviceworkers/test_bug1408734.html
@@ +31,5 @@
>    // register a service worker
>    let registration = await navigator.serviceWorker.register("fetch.js", {scope: "./"});
> +  let sw = registration.installing || registration.active;
> +
> +

nit: remove the extra empty line here.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8923659 [details] [diff] [review]
Bug 1413056 - Using utils.js in test_bug1408734.html for waitForState and waitForControlled. r?tt

Review of attachment 8923659 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 7

[Eden Chuang[:edenchuang]]

Attachment: Bug 1413056 - Using utils.js in test_bug1408734.html for waitForState and waitForControlled. r=tt

Comment 8

[Eden Chuang[:edenchuang]]

Attachment: Bug 1413056 - Using utils.js in test_bug1408734.html for waitForState and waitForControlled. r=bkelly

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

I've run this through try a number of times in bug 1293277.  For example, this push includes it:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3038dc4655896649965c0a6ed5d9c5b2ffb95cc

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a137a0d96fb6
Use utils.js in test_bug1408734.html for waitForState and waitForControlled. r=bkelly

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/a137a0d96fb6

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/de5b0fe8f4e5 (FIREFOX_57b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/bcb1d6d8c13d