SimplePush: Delete Registrations on app update/uninstall

RESOLVED FIXED in Firefox 23

Status

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: nsm, Assigned: nsm)

Tracking

unspecified
B2G C4 (2jan on)
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:-, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
Posted patch unregister on uninstall (obsolete) — Splinter Review
Watches for app uninstall and unregisters all endpoints associated with the application.
Attachment #727005 - Flags: review?(doug.turner)
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 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.
Attachment #727005 - Flags: review?(doug.turner) → review+
removed whitespace
Attachment #727005 - Attachment is obsolete: true
Attachment #727208 - Flags: review?(doug.turner)
Attachment #727208 - Flags: review?(doug.turner) → review+
https://hg.mozilla.org/mozilla-central/rev/6af472638173
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → B2G C4 (2jan on)
Stale permissions ain't right, this gets blocking+.
blocking-b2g: --- → leo+
tracking-b2g18: ? → ---
Keywords: verifyme
(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.
blocking-b2g: leo+ → leo?
Keywords: verifyme
Note that this patch does not deal with app updates, only uninstalls.
Since bug 822712 isn't a blocker (it's disabled), neither is this. Approving instead.
blocking-b2g: leo? → -
Comment on attachment 727208 [details] [diff] [review]
unregister on uninstall

[Triage Comment]
Attachment #727208 - Flags: approval-mozilla-b2g18+
You need to log in before you can comment on or make changes to this bug.