Closed
Bug 1352772
Opened 8 years ago
Closed 8 years ago
Permissions not correctly transmitted to processes which load Service Workers
Categories
(Core :: DOM: Service Workers, defect)
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox52 | --- | unaffected |
firefox-esr52 | --- | unaffected |
firefox53 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | + | fixed |
People
(Reporter: 684sigma, Assigned: nika)
References
(Blocks 1 open bug, )
Details
(Keywords: regression)
Attachments
(1 file)
2.22 KB,
patch
|
catalinb
:
review+
|
Details | Diff | Splinter Review |
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
Has STR: --- → yes
Keywords: regression
Comment 1•8 years ago
|
||
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?
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
status-firefox52:
--- → unaffected
status-firefox53:
--- → unaffected
status-firefox54:
--- → unaffected
tracking-firefox55:
--- → ?
Ever confirmed: true
Flags: needinfo?(kit) → needinfo?(michael)
Assignee | ||
Comment 4•8 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•8 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 6•8 years ago
|
||
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+
Comment 7•8 years ago
|
||
Sorry I was on travel during this discussion. Looks like its been taken care of, though. Thanks!
Flags: needinfo?(bkelly)
Assignee | ||
Updated•8 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•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•8 years ago
|
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•