Permissions not correctly transmitted to processes which load Service Workers

RESOLVED FIXED in Firefox 55

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: 684sigma, Assigned: Nika)

Tracking

(Blocks 1 bug, {regression})

55 Branch
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54 unaffected, firefox55+ fixed)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
During the testing of Bug 1352764, Bug 1352765 and Bug 1352769, I detected a problem in Nightly 55 (2017-04-01). It didn't happen in Nightly (2017-03-20).
One specific push notification demo now doesn't work. Here's how to reproduce the bug:

1. Open https://gauntface.github.io/simple-push-demo/
2. Enable Push Notifications on the page
3. Add some payload text and click "Send a push via XHR" button

Result: Notification doesn't appear (tested on clear profile)
Expected: Notification should appear. Sometimes it appears, very rarely
(Reporter)

Updated

2 years ago
Has STR: --- → yes
Keywords: regression
Yea, stopped working for me as well when I upgraded from March 30 to April 1 nightly FF55.

I'm on travel at the moment and can't look at it for a few days.  Kit, any ideas?
Flags: needinfo?(kit)

Comment 2

2 years ago
Reg range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d2cfefb8ff3966368090c222e2c26195772ba9b1&tochange=f86ef08fdae0a012c28e1264e83238cfb31ccc2e

regressed by bug 1337056.
Blocks: 1337056
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Ever confirmed: true
Flags: needinfo?(kit) → needinfo?(michael)
Tracking 55+ for this regression.
(Assignee)

Updated

2 years ago
Depends on: 1353179
(Assignee)

Comment 4

2 years ago
The reason why this is occurring is because the ServiceWorker is being loaded in a process which has not loaded gauntface.github.io yet. This means that the permissions for that domain haven't been sent down yet. This should have been caught by my assertions, but I messed up when implementing the assertion and the assertions don't fire in this specific case. I've filed bug 1353179 to fix this assertion.

To fix this specific problem, I need to send down permissions to the content process loading the ServiceWorker whenever the ServiceWorker is loaded. I don't currently understand the events which occur during the creation of a ServiceWorker. If there is a convenient place where we are in the parent process, and know the principal of the ServiceWorker which we are just about to ask to load a ServiceWorker, that would be a great place to send the permissions down. bkelly, is there a convenient place like that?
Assignee: nobody → michael
Component: DOM: Push Notifications → DOM: Service Workers
Flags: needinfo?(michael) → needinfo?(bkelly)
Summary: Push notifications in one specific demo stopped working (Nightly 2017-04-01) → Permissions not correctly transmitted to processes which load Service Workers
(Assignee)

Comment 5

2 years ago
This patch should send down the permissions to the content process before
starting the Service Worker in it. Like we talked about on IRC, this is mostly a
stopgap solution which will need to be changed in the future when we get a
single ServiceWorker per origin per firefox instance.

MozReview-Commit-ID: DVLjumi9iBJ
Attachment #8854529 - Flags: review?(catalin.badea392)
Comment on attachment 8854529 [details] [diff] [review]
Send permissions to the content process before dispatching push events

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

lgtm.
Attachment #8854529 - Flags: review?(catalin.badea392) → review+
Sorry I was on travel during this discussion.  Looks like its been taken care of, though.  Thanks!
Flags: needinfo?(bkelly)
(Assignee)

Updated

2 years ago
Blocks: 1353179
No longer depends on: 1353179

Comment 8

2 years ago
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f19bf1a2ce6
Send permissions to the content process before dispatching push events, r=catalinb

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0f19bf1a2ce6
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.