|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
58 bytes, text/x-review-board-request
|Details | Review|
I see multiple instances of the following PushServiceWebSocket error stack trace in the browser console (Nightly 48): PushServiceWebSocket:getNetworkInformation: Error recovering mobile network information. Error: UDP disabled Stack trace: PushNetworkInfo.getNetworkInformation@resource://gre/modules/PushServiceWebSocket.jsm:1393:15 PushNetworkInfo.getNetworkState@resource://gre/modules/PushServiceWebSocket.jsm:1449:23 this.PushServiceWebSocket._wsOnStart@resource://gre/modules/PushServiceWebSocket.jsm:1187:5 PushWebSocketListener.prototype.onStart@resource://gre/modules/PushServiceWebSocket.jsm:98:5
Ugh, that's annoying for sure, especially since the error is expected behavior. I think we can remove UDP wake-up entirely. It was used in the older B2G-only Simple Push implementation for a third-party server, but Web Push isn't supported on B2G. Ben, JR: I know we use the UDP close code as a way to tell misbehaving clients to go away. Once the adaptive pings are removed (bug 1265915), do you see a reason to keep the close code around, or can we remove it from the client, too?
Created attachment 8743076 [details] MozReview Request: Bug 1265914 - Remove Push UDP wake-up. r?dragana Review commit: https://reviewboard.mozilla.org/r/47543/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47543/
I kept the special close code because it's the only way we can tell a client, "go away and don't come back for a while." Experience with Sync and Simple Push suggests this is a useful failsafe. But I removed the listener and everything else that mentions "UDP."
Assignee: nobody → kcambridge
Comment on attachment 8743076 [details] MozReview Request: Bug 1265914 - Remove Push UDP wake-up. r?dragana https://reviewboard.mozilla.org/r/47543/#review45459 looks good
Comment on attachment 8743076 [details] MozReview Request: Bug 1265914 - Remove Push UDP wake-up. r?dragana https://reviewboard.mozilla.org/r/47543/#review45561
Attachment #8743076 - Flags: review+
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Will this fix be uplifted to 48?
I think it's OK to let this and bug 1265915 ride the trains. The only side effect is benign logspam, though it's definitely annoying because the "error" is expected. (The removed code only worked on Firefox OS, but, even though it's reported as an error, Push runs fine without it). Setting the "dom.push.loglevel" pref to "off" should silence the logging.
status-firefox46: --- → wontfix
status-firefox47: --- → wontfix
status-firefox48: affected → wontfix
This is going to affect the release? I have this warning from time to time in beta and it is confusing.
We'd want to uplift this and bug 1265915 to Aurora and Beta. Most of it just removes a lot of (unused) code, but it's a moderately-sized patch. Sylvestre, would you approve uplifts for both patches if I requested it? If you'd rather let both ride the trains, we can instead flip "dom.push.loglevel" to "off" and uplift that instead.
This is a lot of changes to silent a warning :) Flipping the pref is probably better if you don't foresee potential side effects.
You need to log in before you can comment on or make changes to this bug.