Closed
Bug 1243856
Opened 8 years ago
Closed 8 years ago
Don't use `AlarmService` for reconnect timers
Categories
(Core :: DOM: Notifications, defect, P1)
Core
DOM: Notifications
Tracking
()
RESOLVED
FIXED
mozilla47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: lina, Assigned: lina)
References
(Blocks 1 open bug)
Details
(Whiteboard: dom-triaged)
Attachments
(2 files)
It's not supported on Android, and complicates the reconnect timer and test code. Let's just replace it with timers for now; we can revisit when we need to support the new DOM API on B2G.
Updated•8 years ago
|
Comment 1•8 years ago
|
||
With this in place, we should be able to run the Push mochitests -- backed by websockets, not GCM and Autopush -- on Android. That'll go a long way to improving our confidence that the Web Push API is being exposed correctly across all platforms. We'll need other tests to ensure that swapping in the GCM and Autopush backend doesn't regress.
Updated•8 years ago
|
Whiteboard: dom-noted
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/33507/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33507/
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → kcambridge
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•8 years ago
|
||
https://reviewboard.mozilla.org/r/33507/#review30197 ::: dom/push/PushServiceWebSocket.jsm:492 (Diff revision 1) > + Object.keys(this._registerRequests).length > 0; Note to self: `_registerRequests` should really be a map.
Assignee | ||
Comment 4•8 years ago
|
||
Comment on attachment 8715550 [details] MozReview Request: Bug 1243856 - Remove alarms from the Push WebSocket backend. r?dragana Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33507/diff/1-2/
Attachment #8715550 -
Attachment description: MozReview Request: Bug 1243856 - Remove alarms from the Push WebSocket backend. → MozReview Request: Bug 1243856 - Remove alarms from the Push WebSocket backend. r?dragana
Attachment #8715550 -
Flags: review?(dd.mozilla)
Assignee | ||
Comment 5•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/33575/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33575/
Attachment #8715617 -
Flags: review?(dd.mozilla)
Comment 6•8 years ago
|
||
Comment on attachment 8715550 [details] MozReview Request: Bug 1243856 - Remove alarms from the Push WebSocket backend. r?dragana https://reviewboard.mozilla.org/r/33507/#review30303 looks good.
Attachment #8715550 -
Flags: review?(dd.mozilla) → review+
Comment 7•8 years ago
|
||
Comment on attachment 8715617 [details] MozReview Request: Bug 1243856 - Remove alarms from the Push H2 backend. r?dragana https://reviewboard.mozilla.org/r/33575/#review30305 ::: dom/push/PushServiceHttp2.jsm:626 (Diff revision 1) > + this._conns[subscriptionUri].channel.listener = null; this should be: this._conns[subscriptionUri].listener = null;
Attachment #8715617 -
Flags: review?(dd.mozilla) → review+
Updated•8 years ago
|
Whiteboard: dom-noted → dom-triaged
Assignee | ||
Comment 8•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=026b4742ac78
Assignee | ||
Comment 9•8 years ago
|
||
https://reviewboard.mozilla.org/r/33575/#review30305 > this should be: > this._conns[subscriptionUri].listener = null; Eek, that was an egregious copy-paste fail. Thanks! :-)
Comment 10•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8e2c9ff57a7 https://hg.mozilla.org/integration/mozilla-inbound/rev/a3a8b3286601
Comment 11•8 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/fcea28008700
Updated•8 years ago
|
Priority: -- → P1
Comment 12•8 years ago
|
||
The code that was clearing the timeout wasn't being run due to bug 1251113 and caused the intermittent crash. With the patch in bug 1251113 applied I no can no longer reproduce so hopefully it should be sufficient to re-land this work.
Assignee | ||
Comment 13•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=00c5c656a834
Comment 14•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e58bd4bbf964 https://hg.mozilla.org/integration/mozilla-inbound/rev/af9a9799032d
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e58bd4bbf964 https://hg.mozilla.org/mozilla-central/rev/af9a9799032d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in
before you can comment on or make changes to this bug.
Description
•