Closed Bug 1191931 Opened 5 years ago Closed 4 years ago

Add tests for using `pushManager` from a worker

Categories

(Core :: DOM: Push Notifications, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: Lina, Assigned: Lina)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

No description provided.
Attached patch 1191931.patch (obsolete) — Splinter Review
Assignee: nobody → kcambridge
Attachment #8644491 - Attachment is obsolete: true
Blocks: 1243781
Blocks: 1243789
Status: NEW → ASSIGNED
Comment on attachment 8713220 [details]
MozReview Request: Bug 1191931, Part 1 - Use tasks in the Push subscription tests. r?mt

https://reviewboard.mozilla.org/r/32695/#review29605

::: dom/push/test/test_register.html:41
(Diff revision 1)
> -  function checkPermissionState(swr) {
> -    return swr.pushManager.permissionState().then(function(state) {
> -      ok(state === "granted", "permissionState() should resolve to granted.");
> +  var controlledFrame;
> +  add_task(function* createControlledIFrame() {
> +    controlledFrame = yield injectControlledIFrame();

I wonder if you might not go a little further with this change and make the "controlledFrame" variable a wrapper.  The call to controlledFrame.contentWindow.waitOnPushMessage() might be wrapped more cleanly, or even combined with the push itself.  And the controlledFrame.parentNode.removeChild() call might be turned into a .remove() function.

It's not much, so your call.
Attachment #8713220 - Flags: review?(martin.thomson) → review+
Comment on attachment 8713221 [details]
MozReview Request: Bug 1191931, Part 2 - Test resubscribing from a worker. r?mt

https://reviewboard.mozilla.org/r/32773/#review29607

::: dom/push/test/worker.js:93
(Diff revision 1)
> +    }).then(subscription => {
> +      return {
> +        endpoint: subscription.endpoint,
> +      };
>      }));

Hmm, this reminds me, are we able to structured clone subscriptions.
Attachment #8713221 - Flags: review?(martin.thomson) → review+
https://hg.mozilla.org/mozilla-central/rev/02bcedf966fd
https://hg.mozilla.org/mozilla-central/rev/6df33c3ef63b
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
backed out since we still have perma failures like https://treeherder.mozilla.org/logviewer.html#?job_id=3216622&repo=mozilla-central
Flags: needinfo?(kcambridge)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Re-landing for now. We might still see intermittent oranges, since we're talking to the production server. Bug 1244816 tracks the work to switch us to mocks. I'm sorry for the trouble, Tomcat.
Flags: needinfo?(kcambridge)
https://hg.mozilla.org/mozilla-central/rev/9e8c9326500b
https://hg.mozilla.org/mozilla-central/rev/d95ddccaece5
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed-aurora]
Whiteboard: [checkin-needed-aurora]
You need to log in before you can comment on or make changes to this bug.