Closed
Bug 1185544
Opened 9 years ago
Closed 9 years ago
Add data delivery to the WebSocket backend
Categories
(Core :: DOM: Push Subscriptions, defect)
Core
DOM: Push Subscriptions
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: lina, Assigned: lina)
References
Details
Attachments
(2 files)
Once we support data, we'll want to send a special flag in the Simple Push `hello` message. This will indicate that the Push server should use Web Push semantics for notifications. In particular: * Push endpoints will treat incoming bodies as opaque (Simple Push assumes a URL-encoded body with an optional `version` param). * Individual messages will be stored for delivery if the device is offline. * Incoming `notification` messages will include a `data` field instead of `version`. We'll want to modify `PushServiceWebSocket.jsm` to detect and decrypt this field. https://github.com/mozilla-services/autopush/issues/57 tracks the server work to support this.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → kcambridge
Assignee | ||
Updated•9 years ago
|
Summary: WebSocket backend: Send a "data delivery" flag in the opening handshake → Add data delivery to the WebSocket backend
Assignee | ||
Comment 1•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8654269 -
Flags: feedback?(dd.mozilla)
Comment 2•9 years ago
|
||
Comment on attachment 8654269 [details] [diff] [review] 0001-Bug-1185544-Add-data-delivery-to-the-WebSocket-backe.patch Review of attachment 8654269 [details] [diff] [review]: ----------------------------------------------------------------- I have one question: if registration happens during data.enable turned off, it will never be able to receive message even if pref flips, it needs to reregister... we do not have any notification that subscription needs to be updated" except checking pref. ::: dom/push/PushService.jsm @@ +27,5 @@ > Cu.import("resource://gre/modules/Promise.jsm"); > > const {PushServiceWebSocket} = Cu.import("resource://gre/modules/PushServiceWebSocket.jsm"); > const {PushServiceHttp2} = Cu.import("resource://gre/modules/PushServiceHttp2.jsm"); > +const {PushServiceHttp2Crypto} = Cu.import("resource://gre/modules/PushServiceHttp2Crypto.jsm"); we should rename this to PushServiceCrypto ::: modules/libpref/init/all.js @@ +4477,5 @@ > // This preference should be used in UX to enable/disable push. > pref("dom.push.connection.enabled", true); > > +// Enable data delivery over the WebSocket connection. > +pref("dom.push.data.enabled", false); maybe add ws, something like dom.push.ws.data.enable to be clear that it is only ws
Attachment #8654269 -
Flags: feedback?(dd.mozilla) → feedback+
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Dragana Damjanovic [:dragana] from comment #2) > I have one question: if registration happens during data.enable turned off, > it will never be able to receive message even if pref flips, it needs to > reregister... we do not have any notification that subscription needs to be > updated" except checking pref. Good catch! I should change this to match `PushServiceHttp2`, so we'll drop and recreate all subscriptions without keys if data is enabled.
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•9 years ago
|
||
Bug 1185544 - Add data delivery to the WebSocket backend. r?dragana
Attachment #8661441 -
Flags: review?(dd.mozilla)
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8661441 [details] MozReview Request: Bug 1185544 - Add data delivery to the WebSocket backend. r?dragana,nsm Bug 1185544 - Add data delivery to the WebSocket backend. r?dragana
Assignee | ||
Updated•9 years ago
|
Attachment #8661441 -
Attachment description: MozReview Request: Bug 1185544 - Add data delivery to the WebSocket backend. r?dragana → MozReview Request: Bug 1185544 - Add data delivery to the WebSocket backend. r?dragana,nsm
Attachment #8661441 -
Flags: review?(nsm.nikhil)
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8661441 [details] MozReview Request: Bug 1185544 - Add data delivery to the WebSocket backend. r?dragana,nsm Bug 1185544 - Add data delivery to the WebSocket backend. r?dragana,nsm
Assignee | ||
Comment 7•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f080e527ad8
Comment 8•9 years ago
|
||
Comment on attachment 8661441 [details] MozReview Request: Bug 1185544 - Add data delivery to the WebSocket backend. r?dragana,nsm https://reviewboard.mozilla.org/r/19351/#review17555 Looks good, just few comments ::: dom/push/PushService.jsm:728 (Diff revision 3) > + // is only going to happen on db upgrade from version 4 to higher. This is only true for http2. you should change db version for WS. ::: dom/push/PushService.jsm:797 (Diff revision 3) > + if (cryptoParams) { This is only false when enableData is false?
Attachment #8661441 -
Flags: review?(dd.mozilla) → review+
Attachment #8661441 -
Flags: review?(nsm.nikhil) → review+
Comment on attachment 8661441 [details] MozReview Request: Bug 1185544 - Add data delivery to the WebSocket backend. r?dragana,nsm https://reviewboard.mozilla.org/r/19351/#review17583 Great!
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Dragana Damjanovic [:dragana] from comment #8) > This is only false when enableData is false? Yup, that's one case (I removed `dom.push.ws.data.enabled`, but the server can still opt out of data by omitting `use_webpush` in the handshake). The second case is if you send a message without crypto headers or a body.
Assignee | ||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0a7044bb280b26897746b69829ea230e2e5ffec Bug 1185544 - Add data delivery to the WebSocket backend. r=dragana,nsm
Comment 12•9 years ago
|
||
(In reply to Kit Cambridge [:kitcambridge] from comment #10) > (In reply to Dragana Damjanovic [:dragana] from comment #8) > > This is only false when enableData is false? > > Yup, that's one case (I removed `dom.push.ws.data.enabled`, but the server > can still opt out of data by omitting `use_webpush` in the handshake). The > second case is if you send a message without crypto headers or a body. If someone sends message without a body we do not need crypto part as well, I agree with that. Just a thought: what about just dropping a message that has a body, data delivery is enabled, but there is no crypto headers? Just to discourage such a behavior?
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d0a7044bb280
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•