1611035 - Web Notification is some times dropped from successfully received Web Push

Status: RESOLVED FIXED

(GeckoView :: General, defect, P1)

Description

[Jonathan Almeida [:jonalmeida]]

STR:

1. Receive a Web Push to show a Web Notification (for example, using https://gauntface.github.io/simple-push-demo/)
2. Observe the notification in the system tray.
3. Background the app or do something else. Swipe the app away.
4. Return to the app and send another WebPush.
5. Observe no notification is seen.

See the logs below of a push message received, but no logs of the notification showing up.

2020-01-21 19:17:03.675 5156-5349/? I/bt_stack: [INFO:device.cc(517)] 00:02:5b:15:55:84 : PlaybackPosNotificationResponse: Queue next play position update
2020-01-21 19:17:04.208 4526-4526/org.mozilla.reference.browser.debug W/Thread-45: type=1400 audit(0.0:234786): avc: granted { open } for path="/dev/ashmem" dev="tmpfs" ino=19560 scontext=u:r:untrusted_app_27:s0:c127,c257,c512,c768 tcontext=u:object_r:ashmem_device:s0 tclass=chr_file
2020-01-21 19:17:04.288 4693-4714/org.mozilla.reference.browser.debug D/GeckoViewAutofill[C]: handleEvent: focusin
2020-01-21 19:17:04.292 4693-4714/org.mozilla.reference.browser.debug D/GeckoViewAutofill[C]: handleEvent: focusout
2020-01-21 19:17:04.662 5156-5186/? D/AvrcpTargetJni: getCurrentPlayStatus
2020-01-21 19:17:04.662 5156-5186/? D/AvrcpNativeInterface: getPlayStatus
2020-01-21 19:17:04.664 5156-5349/? I/bt_stack: [INFO:device.cc(517)] 00:02:5b:15:55:84 : PlaybackPosNotificationResponse: Queue next play position update
2020-01-21 19:17:05.202 4526-4598/org.mozilla.reference.browser.debug I/AutoPushFeature: New push message decrypted.
2020-01-21 19:17:05.203 4526-4598/org.mozilla.reference.browser.debug D/push_ffi: push_dispatch_info_for_chid
2020-01-21 19:17:05.205 4526-4598/org.mozilla.reference.browser.debug D/push_ffi: push_decrypt
2020-01-21 19:17:05.206 1046-1046/? W/adbd: timeout expired while flushing socket, closing
2020-01-21 19:17:05.208 4526-4598/org.mozilla.reference.browser.debug I/AutoPushObserver: Received new push message for https://gauntface.github.io/simple-push-demo/
2020-01-21 19:17:05.209 4526-4564/org.mozilla.reference.browser.debug I/FxaDeviceConstellation: Executing: processing raw events
2020-01-21 19:17:05.209 4526-4564/org.mozilla.reference.browser.debug D/fxaclient_ffi: fxa_handle_push_message
2020-01-21 19:17:05.209 4526-4564/org.mozilla.reference.browser.debug W/fxa_client::ffi: Unexpected error: Error(
    
    JSON error: expected value at line 1 column 1)
2020-01-21 19:17:05.210 4526-4564/org.mozilla.reference.browser.debug E/FxaDeviceConstellation: Error while running: processing raw events
2020-01-21 19:17:05.211 4526-4559/org.mozilla.reference.browser.debug D/GeckoViewPushController: onEvent GeckoView:PushEvent {"data":"d2lyZWVk","scope":"https://gauntface.github.io/simple-push-demo/"}

Comment 1

[Jonathan Almeida [:jonalmeida]]

I'm going to close this for now since I can no longer reproduce it R-B any more. Will re-open if that changes.

Comment 2

[Jonathan Almeida [:jonalmeida]]

I was able to reproduce this again today. Updated STR provided above and here:

STR:

1. Receive a Web Push to show a Web Notification (for example, using https://gauntface.github.io/simple-push-demo/)
2. Observe the notification in the system tray.
3. Background the app or do something else. Swipe the app away.
4. Return to the app and send another WebPush.
5. Observe no notification is seen.

• You can still see the WebPush message delivered to the app in the logs.

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

If the message is delivered to a later instance than the one where the subscription was added, Gecko doesn't have any record of the subscription, so it drops it on the floor. I think the easiest thing here is to just call WebPushDelegate.onGetSubscription() before delivering the message if this is the case. Lina does this sound right to you?

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Hmm, I'm now not convinced I know what's going on here. Andrew, maybe you can help? What is responsible for populating registrations in ServiceWorkerManager? Web push messages aren't being delivered because we're missing one there on "fresh" instances of Gecko; that is, not the instance where the subscription was first acquired.

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Nevermind, my test was doing something wrong. Back to square one here.

Comment 6

[Lina Butler (ex-Mozilla)]

Hmm, so we get as far as here, but then the call to notifyPush{WithData} doesn't actually wake up the service worker? Or does Gecko not receive the GeckoView:PushEvent at all?

But also...

(In reply to James Willcox (:snorp) ([e-mail]) (he/him) from comment #3)

I think the easiest thing here is to just call WebPushDelegate.onGetSubscription() before delivering the message if this is the case. Lina does this sound right to you?

I thought it did already, but looks like we might have changed that in bug 1596588? Or, hmm, maybe not, I guess that didn't actually change the implementation, since we still have to get the subscription—also verifying that it still exists—to map the channel ID to the scope...is that right, Jon?

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

I believe we're getting GeckoView:PushEvent, but not sure where things go south after that. I thought I had reproduced it with a test, but it turns out I was just passing the wrong scope so it failed to find the SW registration.

Comment 8

[Jonathan Almeida [:jonalmeida]]

Sorry, I missed seeing this earlier.

(In reply to :Lina Cambridge from comment #6)

I thought it did already, but looks like we might have changed that in bug 1596588? Or, hmm, maybe not, I guess that didn't actually change the implementation, since we still have to get the subscription—also verifying that it still exists—to map the channel ID to the scope...is that right, Jon?

Yes, that's accurate. We've mapped getting a subscription to be the same as "get or create" if one doesn't exist.

Our channel ID is also derived from the scope so that mapping should be consistent.

In bug 1596588, we were only using the scope so we simplified the API around that.

Comment 9

[Jovan Gerodetti]

I think this might be an issue with the service worker notification API. See my report for fenix: https://github.com/mozilla-mobile/fenix/issues/10001

Comment 10

[James Willcox (:snorp) ([e-mail]) (he/him)]

Oof. This turns out to be pretty annoying. The push message is delivered successfully, but the notification is not shown because we don't have the content permission. These aren't persistently stored in GeckoView, because we prefer to rely on the prompting mechanisms to delegate this to the app. We then set an EXPIRE_SESSION value in nsIPermissionManager. This has mostly worked out alright, but there certainly are some rough edges. In this case, we don't (and can't, really) prompt because the Service Worker isn't a page and therefore really has no context in which to do such a thing.

I think we probably need to rethink how content permissions are exposed in GeckoView. We likely need to store them in Gecko and expose a mangement interface for apps.

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1611035 - Grant "desktop-notification" when delivering Web Push in GeckoView

Comment 12

[Jovan Gerodetti]

I might be naive here, but why can't GeckoView just ask the host app if the current site already has the permission or not? There does not have to be a user-facing permission request, right?

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Jovan Gerodetti from comment #12)

I might be naive here, but why can't GeckoView just ask the host app if the current site already has the permission or not? There does not have to be a user-facing permission request, right?

We don't have an API for this outside of the PermissionDelegate, which is only applicable to a GeckoSession. We don't have a GeckoSession here.

The patch I've attached just grants the permission before we deliver the Web Push message. You're required to have accepted the permission in order to have a subscription anyway, so it's probably relatively safe for now. For a real fix we probably need to go ahead and expose the full Gecko permission system to apps (uggghhh).

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a709f87941d
Grant "desktop-notification" when delivering Web Push in GeckoView r=geckoview-reviewers,agi

Comment 15

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/3a709f87941d