The Android-related PushService doesn't work right now with GeckoView. Initializing it hangs forever because it's waiting for responses that will never come. We should avoid this path until we actually support Web Push on GeckoView. The most straightforward way for this seems to be to set dom.push.enabled to false, but we appear to initialize PushService.jsm in PushComponents.js anyway.

Comment on attachment 8984607 [details] Bug 1467919 - Don't initialize PushService unless dom.push.enabled is true https://reviewboard.mozilla.org/r/250488/#review256756 Thanks, Snorp! I think this is a good enough workaround for now, but I have a suggestion about where to check for `this.service` instead. r=lina with that fixed. On the one hand, this makes it a bit awkward to toggle the pref at runtime. On the other, that's probably complexity that we don't need, anyway, and lifting all the `.enabled` pref handling out of `PushService.jsm`, and into `PushComponents.js` is a big chunk of work. ::: dom/push/PushComponents.js:24 (Diff revision 1) > + if (Services.prefs.getBoolPref("dom.push.enabled")) { > - const {PushService} = ChromeUtils.import("resource://gre/modules/PushService.jsm", > + const {PushService} = ChromeUtils.import("resource://gre/modules/PushService.jsm", > - {}); > + {}); > - PushService.init(); > + PushService.init(); > - return PushService; > + return PushService; > + } else { Nit: No need for `else` after return. ::: dom/push/PushComponents.js:258 (Diff revision 1) > ChromeUtils.originAttributesToSuffix(principal.originAttributes); > > return data; > }, > > _handleRequest(name, principal, data) { How about making this function `async`, so that exceptions get transformed into rejections, then checking if `PushService` exists, and throwing something like `NS_ERROR_NOT_AVAILABLE` in https://searchfox.org/mozilla-central/rev/edbf2c009992315d85eeb885e1b8edbbd43c84b7/dom/push/PushComponents.js#299 instead of here? That should give us slightly better errors for the synchronous push service methods, too.

Pushed by jwillcox@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e04f93535a93 Don't initialize PushService unless dom.push.enabled is true r=lina

Backed out for xpcshell failures Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/d5f39774d8f1f37814824016ee716846edae4df5 Push link: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=f55e78d6593656aff9f61a684bfbc7ba1f3ce264 Log link1: https://treeherder.mozilla.org/logviewer.html#?job_id=185433695&repo=mozilla-inbound&lineNumber=1885 Log link2: https://treeherder.mozilla.org/logviewer.html#?job_id=185430765&repo=mozilla-inbound&lineNumber=2065

Comment on attachment 8988839 [details] Bug 1467919 - Don't try to clear PushService data if Push disabled https://reviewboard.mozilla.org/r/253994/#review261052

Pushed by jwillcox@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/22a25d017920 Don't initialize PushService unless dom.push.enabled is true r=lina https://hg.mozilla.org/integration/mozilla-inbound/rev/3e55c9eebbbc Don't try to clear PushService data if Push disabled r=baku

https://hg.mozilla.org/mozilla-central/rev/22a25d017920 https://hg.mozilla.org/mozilla-central/rev/3e55c9eebbbc

https://hg.mozilla.org/mozilla-central/rev/22a25d017920 https://hg.mozilla.org/mozilla-central/rev/3e55c9eebbbc

Comment on attachment 8984607 [details] Bug 1467919 - Don't initialize PushService unless dom.push.enabled is true Approval Request Comment [Feature/Bug causing the regression]: GeckoView [User impact if declined]: Gecko will hang when content uses a Service Worker [Is this code covered by automated tests?]: Partially [Has the fix been verified in Nightly?]: Yes, via GeckoViewExample [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: Both patches in this bug [Is the change risky?]: No [Why is the change risky/not risky?]: Mostly only affects GeckoView [String changes made/needed]: None

Comment on attachment 8984607 [details] Bug 1467919 - Don't initialize PushService unless dom.push.enabled is true Needed in order to disable web push in bug 1467921. This should land in beta 7.

https://hg.mozilla.org/releases/mozilla-beta/rev/2d9b44bd2d74 https://hg.mozilla.org/releases/mozilla-beta/rev/3c8ab5a011e3