test_bug1408734.html needs to wait for service worker to activate

RESOLVED FIXED in Firefox 57

Status

()

defect
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: bkelly, Assigned: edenchuang)

Tracking

(Blocks 1 bug)

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed, firefox58 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

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!
Flags: needinfo?(echuang)
Assignee

Comment 1

2 years ago
Of course. I will update the test case today.
Assignee: nobody → echuang
Status: NEW → ASSIGNED
Flags: needinfo?(echuang)
Assignee

Comment 2

2 years ago
Posted patch Bug 1413056 - (obsolete) — Splinter Review
Assignee

Comment 3

2 years ago
Tom, could you help review this patch?
Attachment #8923657 - Attachment is obsolete: true
Attachment #8923659 - Flags: review?(ttung)
Assignee

Comment 4

2 years ago
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.
Attachment #8923659 - Flags: review?(ttung) → review?(bkelly)
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.
Attachment #8923659 - Flags: feedback+
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!
Attachment #8923659 - Flags: review?(bkelly) → review+
Assignee

Updated

2 years ago
Keywords: checkin-needed
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

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a137a0d96fb6
Use utils.js in test_bug1408734.html for waitForState and waitForControlled. r=bkelly
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a137a0d96fb6
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Whiteboard: [checkin-needed-beta]
You need to log in before you can comment on or make changes to this bug.