851368 - SimplePush: Delete Registrations on app update/uninstall

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

When an app updates, what it uses push for (or whether it even needs push) may change. Delete all its registrations.

Similarly delete registrations when an app is uninstalled.

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: unregister on uninstall

Watches for app uninstall and unregisters all endpoints associated with the application.

Comment 2

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

There is currently no way to monitor app update, but the situation is mitigated by 

http://hg.mozilla.org/users/nsm.nikhil_gmail.com/push-ws1/rev/1dcc79a39380

which will prevent an app from being notified, and the lack of permissions will render navigator.pushNotification inaccessible to the application after it updates.
Unregistration due to inability to deliver message to application (which will happen in the case of the update) will require cooperation from system messages, which currently does not report the status of delivery.

Comment 3

[Doug Turner (:dougt)]

Comment on attachment 727005 [details] [diff] [review]
unregister on uninstall

Review of attachment 727005 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/push/src/PushService.js
@@ +359,5 @@
> +        if (!app) {
> +          debug("webapps-uninstall: No app found " + aData.origin);
> +          return;
> +        }
> +        

white space.

Comment 4

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: unregister on uninstall

removed whitespace

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6af472638173

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6af472638173

Comment 7

[Dietrich Ayala (:dietrich)]

Stale permissions ain't right, this gets blocking+.

Comment 8

[Jason Smith [:jsmith]]

(In reply to Dietrich Ayala (:dietrich) from comment #7)
> Stale permissions ain't right, this gets blocking+.

Push notifications is off by default on b2g18 and was already determined to not block for 1.1.

It's okay to ask for uplift on a per patch basis, but none of these should block per talking with Alex.

Comment 9

[[:fabrice] Fabrice Desré]

Note that this patch does not deal with app updates, only uninstalls.

Comment 10

[Alex Keybl [:akeybl]]

Since bug 822712 isn't a blocker (it's disabled), neither is this. Approving instead.

Comment 11

[Alex Keybl [:akeybl]]

Comment on attachment 727208 [details] [diff] [review]
unregister on uninstall

[Triage Comment]

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/b644771c03f5