Add ability to resubscribe an existing push channel

[data-sync-user]

As a result of the discussions in mozilla/application-services#3314, I believe it may be possible for us to get into a situation where:

• A client device has a push subscription for FxA which it believes to be valid.
• The push server believes the subscription to be valid.
• The FxA server believes the subscription to be invalid.

I'm not sure how we get into such a state, but it fits with observed behaviours.

Assuming that the client finds itself in such a state, the only way it can repair things is to explicitly unsubscribe and then crate a new subscription to send to the FxA server. I think it would be useful to have a resubscribe abstraction of some sort to make thing easier.

@jonalmeida what do you think?

[data-sync-user]

➤ Jonathan Almeida commented:

This sounds like a fair solution with two clarifications:

• Should we be calling resubscribe after forcing registration renewal is complete or as a replacement for that?
• We could also implement this today at the AC layer. If so, I wonder if we should be calling subscribe only if unsubscribe fails. In code, would this if-statement seem necessary?

fun resubscribe(scope: String) {
val result = appServicesPushManager.unsubscribe(scope)
if (result) {
subscribe(scope) { subscription -> accountManager.updatePushEndpoint(subscription) }
}
}A quick peek at communication.rs ( https://github.com/mozilla/application-services/blob/52a2c32ddb489f38fc0a4b6efa5ab463916c6844/components/push/src/communications.rs#L248 ) and it's usage ( https://github.com/mozilla/application-services/blob/52a2c32ddb489f38fc0a4b6efa5ab463916c6844/components/push/src/subscriber.rs#L100 ) shows that we only fail on communication exceptions, so we should be safe.

[data-sync-user]

➤ Ryan Kelly commented:

Should we be calling resubscribe after forcing registration renewal is complete or as a replacement for that?

I don't know to be honest. I wonder if "forcing registration renewal" should maybe even do an implicit resubscribe to all current push subscriptions, since the problem that's happening with FxA may also be happening elsewhere.

In code, would this if-statement seem necessary?

I think it should be fine to do without it if; indeed I can imagine circumstances under which we fail to unsubscribe because state has gotten out of sync between server and client, but re-subscribing will fix it.

[data-sync-user]

➤ Jonathan Almeida commented:

Merging the thread from a-c ( mozilla-mobile/android-components#8846 ) to here since the conversation there on a closed issue and may get lost. This issue is around the same area as well.

is this essentially the same as what's being discussed in mozilla/application-services#3696?

Yes, it's the same as this. For now, I've implemented the short-fix specifically for FxA only in order to get Send Tab working and unblock some other folks.

Actually, your comments in mozilla/application-services#3696 made me realize that an unsubscribe is likely to be necessary for this to work at all. The rust-component code for subscribe() will first check in its local store for an existing subscription before hitting the server, so unless something has occurred to clear that local state then it's always going to give back the same subscription URL.

I don't think we would need this outside of the scope of FxA afaict. For web content, we only notify the ServiceWorker ( https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/WebPushController.java#81 ) that the subscription is no longer valid and let them decide if they want to subscribe again.

I don't know to be honest. I wonder if "forcing registration renewal" should maybe even do an implicit resubscribe to all current push subscriptions, since the problem that's happening with FxA may also be happening elsewhere.

That would be an interesting solution!

• We only do this force registration renewal call when we see the subscriptionExpired flipped, so my immediate fear tells me that, if we run into FxA specific bugs (like the one Re-subscribe only FxA push subscription when subscriptionExpired true mozilla-mobile/android-components#8846 is trying to fix) where the subscriptionExpired is always true, we may be telling everyone to update their subscription when they do not have issues.
• Web developers do not have the ability to perform a registration renewal, so it would be nice if we could solve this by simply re-subscribing as they would.

Would it be prudent in that case to assume those web apps will perform their own checks to ensure push support still works for them? Surely, we would also be getting bug reports from web developers more often if messages no longer were delivered. I'm not aware of the Fennec ones at least. Ha, reading this article ( https://blog.pushpad.xyz/2018/09/web-push-subscription-age-affects-delivery-rates/ ) tells me that they expect browser push services to notify on invalid subscriptions!

I think if we can rely on verify_connection to tell subscribers which subscriptions have been changed ( https://github.com/mozilla-mobile/android-components/blob/b7e6ad09e8fb1791ff444a6ef3c5d4d577f317d5/components/feature/push/src/main/java/mozilla/components/feature/push/AutoPushFeature.kt#L284-L291 ) when endpoints are invalid, this should be covering the case of communication between the device and AutoPush servers and we only need to call "subscribe" then.

A bit outside scope, but one piece of knowledge is how desktop implements the onSubscriptionChanged event and if we on mobile are doing the right thing by expecting it from verify_connection alone.